diff options
-rw-r--r-- | perllib/DBIx/Class/EncodedColumn.pm | 262 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth.pm | 10 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 12 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/Update.pm | 10 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Tokens.pm | 18 | ||||
-rw-r--r-- | t/app/controller/report_new.t | 18 | ||||
-rw-r--r-- | t/app/controller/report_updates.t | 64 |
7 files changed, 380 insertions, 14 deletions
diff --git a/perllib/DBIx/Class/EncodedColumn.pm b/perllib/DBIx/Class/EncodedColumn.pm new file mode 100644 index 000000000..b4a08b35c --- /dev/null +++ b/perllib/DBIx/Class/EncodedColumn.pm @@ -0,0 +1,262 @@ +package DBIx::Class::EncodedColumn; + +use strict; +use warnings; + +use base qw/DBIx::Class/; +use Sub::Name; + +__PACKAGE__->mk_classdata( '_column_encoders' ); + +our $VERSION = '0.00011'; +$VERSION = eval $VERSION; + +sub register_column { + my $self = shift; + my ($column, $info) = @_; + $self->next::method(@_); + + return unless exists $info->{encode_column} && $info->{encode_column} == 1; + $self->throw_exception("'encode_class' is a required argument.") + unless exists $info->{encode_class} && defined $info->{encode_class}; + my $class = $info->{encode_class}; + + my $args = exists $info->{encode_args} ? $info->{encode_args} : {}; + $self->throw_exception("'encode_args' must be a hashref") + unless ref $args eq 'HASH'; + + $class = join("::", 'DBIx::Class::EncodedColumn', $class); + eval "require ${class};"; + $self->throw_exception("Failed to use encode_class '${class}': $@") if $@; + + defined( my $encode_sub = eval{ $class->make_encode_sub($column, $args) }) || + $self->throw_exception("Failed to create encoder with class '$class': $@"); + $self->_column_encoders({$column => $encode_sub, %{$self->_column_encoders || {}}}); + + if ( exists $info->{encode_check_method} && $info->{encode_check_method} ){ + no strict 'refs'; + defined( my $check_sub = eval{ $class->make_check_sub($column, $args) }) || + $self->throw_exception("Failed to create checker with class '$class': $@"); + my $name = join '::', $self->result_class, $info->{encode_check_method}; + *$name = subname $name, $check_sub; + } +} + +# mySociety override to allow direct setting without double encryption +sub set_column { + my $self = shift; + return $self->next::method(@_) unless defined $_[1] and not defined $_[2]; + my $encs = $self->_column_encoders; + if(exists $encs->{$_[0]} && defined(my $encoder = $encs->{$_[0]})){ + return $self->next::method($_[0], $encoder->($_[1])); + } + $self->next::method(@_); +} + +sub new { + my($self, $attr, @rest) = @_; + my $encoders = $self->_column_encoders; + for my $col (grep { defined $encoders->{$_} } keys %$encoders ) { + next unless exists $attr->{$col} && defined $attr->{$col}; + $attr->{$col} = $encoders->{$col}->( $attr->{$col} ); + } + return $self->next::method($attr, @rest); +} + +1; + +__END__; + +=head1 NAME + +DBIx::Class::EncodedColumn - Automatically encode columns + +=head1 SYNOPSIS + +In your L<DBIx::Class> Result class +(sometimes erroneously referred to as the 'table' class): + + __PACKAGE__->load_components(qw/EncodedColumn ... Core/); + + #Digest encoder with hex format and SHA-1 algorithm + __PACKAGE__->add_columns( + 'password' => { + data_type => 'CHAR', + size => 40, + encode_column => 1, + encode_class => 'Digest', + encode_args => {algorithm => 'SHA-1', format => 'hex'}, + } + + #SHA-1 / hex encoding / generate check method + __PACKAGE__->add_columns( + 'password' => { + data_type => 'CHAR', + size => 40 + 10, + encode_column => 1, + encode_class => 'Digest', + encode_args => {algorithm => 'SHA-1', format => 'hex', salt_length => 10}, + encode_check_method => 'check_password', + } + + #MD5 / base64 encoding / generate check method + __PACKAGE__->add_columns( + 'password' => { + data_type => 'CHAR', + size => 22, + encode_column => 1, + encode_class => 'Digest', + encode_args => {algorithm => 'MD5', format => 'base64'}, + encode_check_method => 'check_password', + } + + #Eksblowfish bcrypt / cost of 8/ no key_nul / generate check method + __PACKAGE__->add_columns( + 'password' => { + data_type => 'CHAR', + size => 59, + encode_column => 1, + encode_class => 'Crypt::Eksblowfish::Bcrypt', + encode_args => { key_nul => 0, cost => 8 }, + encode_check_method => 'check_password', + } + +In your application code: + + #updating the value. + $row->password('plaintext'); + my $digest = $row->password; + + #checking against an existing value with a check_method + $row->check_password('old_password'); #true + $row->password('new_password'); + $row->check_password('new_password'); #returns true + $row->check_password('old_password'); #returns false + + +B<Note:> The component needs to be loaded I<before> Core. + +=head1 DESCRIPTION + +This L<DBIx::Class> component can be used to automatically encode a column's +contents whenever the value of that column is set. + +This module is similar to the existing L<DBIx::Class::DigestColumns>, but there +is some key differences: + +=over 4 + +=item C<DigestColumns> performs the encode operation on C<insert> and C<update>, +and C<EncodedColumn> performs the operation when the value is set, or on C<new>. + +=item C<DigestColumns> supports only algorithms of the L<Digest> family. +C<EncodedColumn> employs a set of thin wrappers around different cipher modules +to provide support for any cipher you wish to use and wrappers are very simple +to write (typically less than 30 lines). + +=item C<EncodedColumn> supports having more than one encoded column per table +and each column can use a different cipher. + +=item C<Encode> adds only one item to the namespace of the object utilizing +it (C<_column_encoders>). + +=back + +There is, unfortunately, some features that C<EncodedColumn> doesn't support. +C<DigestColumns> supports changing certain options at runtime, as well as +the option to not automatically encode values on set. The author of this module +found these options to be non-essential and omitted them by design. + +=head1 Options added to add_column + +If any one of these options is present the column will be treated as a digest +column and all of the defaults will be applied to the rest of the options. + +=head2 encode_enable => 1 + +Enable automatic encoding of column values. If this option is not set to true +any other options will become no-ops. + +=head2 encode_check_method => $method_name + +By using the encode_check_method attribute when you declare a column you +can create a check method for that column. The check method accepts a plain +text string, and returns a boolean that indicates whether the digest of the +provided value matches the current value. + +=head2 encode_class + +The class to use for encoding. Available classes are: + +=over 4 + +=item C<Crypt::Eksblowfish::Bcrypt> - uses +L<DBIx::Class::EncodedColumn::Crypt::Eksblowfish::Bcrypt> and +requires L<Crypt::Eksblowfish::Bcrypt> to be installed + +=item C<Digest> - uses L<DBIx::Class::EncodedColumn::Digest> +requires L<Digest> to be installed as well as the algorithm required +(L<Digest::SHA>, L<Digest::Whirlpool>, etc) + +=item C<Crypt::OpenPGP> - L<DBIx::Class::EncodedColumn::Crypt::OpenPGP> +and requires L<Crypt::OpenPGP> to be installed + +=back + +Please see the relevant class's documentation for information about the +specific arguments accepted by each and make sure you include the encoding +algorithm (e.g. L<Crypt::OpenPGP>) in your application's requirements. + +=head1 EXTENDED METHODS + +The following L<DBIx::Class::ResultSource> method is extended: + +=over 4 + +=item B<register_column> - Handle the options described above. + +=back + +The following L<DBIx::Class::Row> methods are extended by this module: + +=over 4 + +=item B<new> - Encode the columns on new() so that copy and create DWIM. + +=item B<set_column> - Encode values whenever column is set. + +=back + +=head1 SEE ALSO + +L<DBIx::Class::DigestColumns>, L<DBIx::Class>, L<Digest> + +=head1 AUTHOR + +Guillermo Roditi (groditi) <groditi@cpan.org> + +Inspired by the original module written by Tom Kirkpatrick (tkp) <tkp@cpan.org> +featuring contributions from Guillermo Roditi (groditi) <groditi@cpan.org> +and Marc Mims <marc@questright.com> + +=head1 CONTRIBUTORS + +jshirley - J. Shirley <cpan@coldhardcode.com> + +kentnl - Kent Fredric <kentnl@cpan.org> + +mst - Matt S Trout <mst@shadowcat.co.uk> + +wreis - Wallace reis <wreis@cpan.org> + +=head1 COPYRIGHT + +Copyright (c) 2008 - 2009 the DBIx::Class::EncodedColumn L</AUTHOR> and +L</CONTRIBUTORS> as listed above. + +=head1 LICENSE + +This library is free software and may be distributed under the same terms +as perl itself. + +=cut diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index a5afff9c5..c67de692a 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -108,6 +108,11 @@ sub email_sign_in : Private { return; } + my $user_params = {}; + $user_params->{password} = $c->req->param('password_register') + if $c->req->param('password_register'); + my $user = $c->model('DB::User')->new( $user_params ); + my $token_obj = $c->model('DB::Token') # ->create( { @@ -116,7 +121,7 @@ sub email_sign_in : Private { email => $good_email, r => $c->req->param('r'), name => $c->req->param('name'), - password => $c->req->param('password_register'), + password => $user->password, } } ); @@ -158,9 +163,8 @@ sub token : Path('/M') : Args(1) { # find or create the user related to the token. my $user = $c->model('DB::User')->find_or_create( { email => $data->{email} } ); $user->name( $data->{name} ) if $data->{name}; - $user->password( $data->{password} ) if $data->{password}; + $user->password( $data->{password}, 1 ) if $data->{password}; $user->update; - $c->authenticate( { email => $user->email }, 'no_password' ); # send the user to their page diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index 78c9b5ae0..a9ec2f935 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -845,9 +845,13 @@ sub save_user_and_report : Private { $report->confirm; } else { - - # user exists and we are not logged in as them. Throw away changes to - # the name and phone. TODO - propagate changes using tokens. + # User exists and we are not logged in as them. + # Store changes in token for when token is validated. + $c->stash->{token_data} = { + name => $report->user->name, + phone => $report->user->phone, + password => $report->user->password, + }; $report->user->discard_changes(); } @@ -932,9 +936,11 @@ sub redirect_or_confirm_creation : Private { } # otherwise create a confirm token and email it to them. + my $data = $c->stash->{token_data} || {}; my $token = $c->model("DB::Token")->create( { scope => 'problem', data => { + %$data, id => $report->id } } ); diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index 2abe65b1f..501dd2b41 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -231,6 +231,14 @@ sub save_update : Private { # Logged in and same user, so can confirm update straight away $update->user->update; $update->confirm; + } else { + # User exists and we are not logged in as them. + # Store changes in token for when token is validated. + $c->stash->{token_data} = { + name => $update->user->name, + password => $update->user->password, + }; + $update->user->discard_changes(); } # If there was a photo add that too @@ -272,10 +280,12 @@ sub redirect_or_confirm_creation : Private { } # otherwise create a confirm token and email it to them. + my $data = $c->stash->{token_data} || {}; my $token = $c->model("DB::Token")->create( { scope => 'comment', data => { + %$data, id => $update->id, add_alert => ( $c->req->param('add_alert') ? 1 : 0 ), } diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm index 1fef0f07e..9abef591d 100644 --- a/perllib/FixMyStreet/App/Controller/Tokens.pm +++ b/perllib/FixMyStreet/App/Controller/Tokens.pm @@ -32,7 +32,8 @@ sub confirm_problem : Path('/P') { $c->forward( 'load_auth_token', [ $token_code, 'problem' ] ); # Load the problem - my $problem_id = $auth_token->data->{id}; + my $data = $auth_token->data; + my $problem_id = $data->{id}; my $problem = $c->cobrand->problems->find( { id => $problem_id } ) || $c->detach('token_error'); $c->stash->{problem} = $problem; @@ -59,6 +60,11 @@ sub confirm_problem : Path('/P') { $c->forward( '/report/new/create_reporter_alert' ); # log the problem creation user in to the site + if ( $data->{name} || $data->{password} ) { + $problem->user->name( $data->{name} ) if $data->{name}; + $problem->user->password( $data->{password}, 1 ) if $data->{password}; + $problem->user->update; + } $c->authenticate( { email => $problem->user->email }, 'no_password' ); $c->set_session_cookie_expire(0); @@ -133,8 +139,9 @@ sub confirm_update : Path('/C') { $c->forward( 'load_auth_token', [ $token_code, 'comment' ] ); # Load the problem - my $comment_id = $auth_token->data->{id}; - $c->stash->{add_alert} = $auth_token->data->{add_alert}; + my $data = $auth_token->data; + my $comment_id = $data->{id}; + $c->stash->{add_alert} = $data->{add_alert}; my $comment = $c->model('DB::Comment')->find( { id => $comment_id } ) || $c->detach('token_error'); @@ -146,6 +153,11 @@ sub confirm_update : Path('/C') { return; } + if ( $data->{name} || $data->{password} ) { + $comment->user->name( $data->{name} ) if $data->{name}; + $comment->user->password( $data->{password}, 1 ) if $data->{password}; + $comment->user->update; + } $c->authenticate( { email => $comment->user->email }, 'no_password' ); $c->set_session_cookie_expire(0); diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index 537786ebc..f06c23501 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -331,6 +331,9 @@ foreach my $test ( my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); ok $user, "test user does exist"; $user->problems->delete; + $user->name( 'Old Name' ); + $user->password( 'old_password' ); + $user->update; } else { ok !FixMyStreet::App->model('DB::User')->find( { email => $test_email } ), "test user does not exist"; @@ -366,11 +369,15 @@ foreach my $test ( # check that we got the errors expected is_deeply $mech->form_errors, [], "check there were no errors"; - # check that the user has been created + # check that the user has been created/ not changed my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); - ok $user, "created new user"; - ok $user->check_password('secret'), 'password set correctly'; + ok $user, "user found"; + if ($test->{user}) { + ok $user->check_password('old_password'), 'password unchanged'; + } else { + ok $user->check_password('secret'), 'password set correctly'; + } # find the report my $report = $user->problems->first; @@ -398,6 +405,11 @@ foreach my $test ( $mech->get_ok( '/report/' . $report->id ); + if ($test->{user}) { + is $report->name, 'Joe Bloggs', 'name updated correctly'; + ok $report->user->check_password('secret'), 'password updated correctly'; + } + # check that the reporter has an alert my $alert = FixMyStreet::App->model('DB::Alert')->find( { user => $report->user, diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t index 3b53e6c46..3076a4564 100644 --- a/t/app/controller/report_updates.t +++ b/t/app/controller/report_updates.t @@ -406,7 +406,7 @@ for my $test ( for my $test ( { - desc => 'submit an update for a non registered user, signing in with wrong password', + desc => 'submit an update for a registered user, signing in with wrong password', form_values => { submit_update => 1, rznvy => 'registered@example.com', @@ -420,7 +420,7 @@ for my $test ( ], }, { - desc => 'submit an update for a non registered user and sign in', + desc => 'submit an update for a registered user and sign in', form_values => { submit_update => 1, rznvy => 'registered@example.com', @@ -479,6 +479,66 @@ for my $test ( }; } +subtest 'submit an update for a registered user, creating update by email' => sub { + my $user = $mech->create_user_ok( 'registered@example.com' ); + $user->update( { name => 'Mr Reg', password => 'secret2' } ); + $report->comments->delete; + $mech->log_out_ok(); + $mech->clear_emails_ok(); + $mech->get_ok("/report/$report_id"); + $mech->submit_form_ok( { + with_fields => { + submit_update => 1, + rznvy => 'registered@example.com', + update => 'Update from a user', + add_alert => undef, + name => 'New Name', + password_register => 'new_secret', + }, + }, 'submit update' ); + + $mech->content_contains('Nearly Done! Now check your email'); + + # No change to user yet. + $user->discard_changes; + ok $user->check_password( 'secret2' ), 'password unchanged'; + is $user->name, 'Mr Reg', 'name unchanged'; + + my $email = $mech->get_email; + ok $email, "got an email"; + like $email->body, qr/confirm the update you/i, "Correct email text"; + + my ( $url, $url_token ) = $email->body =~ m{(http://\S+/C/)(\S+)}; + ok $url, "extracted confirm url '$url'"; + + my $token = FixMyStreet::App->model('DB::Token')->find( { + token => $url_token, + scope => 'comment' + } ); + ok $token, 'Token found in database'; + + my $update_id = $token->data->{id}; + my $add_alerts = $token->data->{add_alert}; + my $update = FixMyStreet::App->model('DB::Comment')->find( { id => $update_id } ); + + ok $update, 'found update in database'; + is $update->state, 'unconfirmed', 'update unconfirmed'; + is $update->user->email, 'registered@example.com', 'update email'; + is $update->text, 'Update from a user', 'update text'; + + $mech->get_ok( $url . $url_token ); + $mech->content_contains("/report/$report_id#update_$update_id"); + + # User should have new name and password + $user->discard_changes; + ok $user->check_password( 'new_secret' ), 'password changed'; + is $user->name, 'New Name', 'name changed'; + + $update->discard_changes; + is $update->state, 'confirmed', 'update confirmed'; + $mech->delete_user( $user ); +}; + for my $test ( { desc => 'submit update for registered user', |