diff options
author | Edmund von der Burg <evdb@mysociety.org> | 2011-04-07 15:41:59 +0100 |
---|---|---|
committer | Edmund von der Burg <evdb@mysociety.org> | 2011-04-07 15:44:01 +0100 |
commit | d0059b5b46bf16d5adbeddffc412699a8c815725 (patch) | |
tree | ea82bb7a7652e2bc8ab37058b7dee95fba19f064 | |
parent | a932cf24e53109204e304ba68263d55326bee78f (diff) |
Add the 'remember_me' checkbox on login
-rw-r--r-- | notes/catalyst-master-merge-todos.txt | 10 | ||||
-rw-r--r-- | perllib/Catalyst/Plugin/Session/State/Cookie.pm | 357 | ||||
-rw-r--r-- | perllib/FixMyStreet/App.pm | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth.pm | 10 | ||||
-rw-r--r-- | perllib/FixMyStreet/TestMech.pm | 24 | ||||
-rw-r--r-- | t/app/controller/auth.t | 44 | ||||
-rw-r--r-- | templates/web/default/auth/general.html | 7 |
7 files changed, 421 insertions, 33 deletions
diff --git a/notes/catalyst-master-merge-todos.txt b/notes/catalyst-master-merge-todos.txt index e002321f4..a851d0681 100644 --- a/notes/catalyst-master-merge-todos.txt +++ b/notes/catalyst-master-merge-todos.txt @@ -1,13 +1,3 @@ -convert templates for new micro sites (or switch old code to use new headers and footers) - should we ditch flickr import? (does not seem to be getting huge usage and those using it would probably report using another method: http://www.flickr.com/search/?w=all&q=fixmystreet&m=tags) -add 'remember me on this computer' to auth login. What should default session lifetime be? Waiting for answer on http://lists.scsys.co.uk/pipermail/catalyst/2011-April/date.html - -merge in from master - -southhampton templates - Add newsletter signup to bottom of problem confirm page - -ON STAGING ONLY: toggle cobrands and languages using params, and persist the change using sessions
\ No newline at end of file diff --git a/perllib/Catalyst/Plugin/Session/State/Cookie.pm b/perllib/Catalyst/Plugin/Session/State/Cookie.pm new file mode 100644 index 000000000..7ffc77f1f --- /dev/null +++ b/perllib/Catalyst/Plugin/Session/State/Cookie.pm @@ -0,0 +1,357 @@ +package Catalyst::Plugin::Session::State::Cookie; +use Moose; +use namespace::autoclean; + +extends 'Catalyst::Plugin::Session::State'; + +use MRO::Compat; +use Catalyst::Utils (); + +our $VERSION = "0.17"; + +has _deleted_session_id => ( is => 'rw' ); + +sub setup_session { + my $c = shift; + + $c->maybe::next::method(@_); + + $c->_session_plugin_config->{cookie_name} + ||= Catalyst::Utils::appprefix($c) . '_session'; +} + +sub extend_session_id { + my ( $c, $sid, $expires ) = @_; + + if ( my $cookie = $c->get_session_cookie ) { + $c->update_session_cookie( $c->make_session_cookie( $sid ) ); + } + + $c->maybe::next::method( $sid, $expires ); +} + +sub set_session_id { + my ( $c, $sid ) = @_; + + $c->update_session_cookie( $c->make_session_cookie( $sid ) ); + + return $c->maybe::next::method($sid); +} + +sub update_session_cookie { + my ( $c, $updated ) = @_; + + unless ( $c->cookie_is_rejecting( $updated ) ) { + my $cookie_name = $c->_session_plugin_config->{cookie_name}; + $c->response->cookies->{$cookie_name} = $updated; + } +} + +sub cookie_is_rejecting { + my ( $c, $cookie ) = @_; + + if ( $cookie->{path} ) { + return 1 if index '/'.$c->request->path, $cookie->{path}; + } + + return 0; +} + +sub make_session_cookie { + my ( $c, $sid, %attrs ) = @_; + + my $cfg = $c->_session_plugin_config; + my $cookie = { + value => $sid, + ( $cfg->{cookie_domain} ? ( domain => $cfg->{cookie_domain} ) : () ), + ( $cfg->{cookie_path} ? ( path => $cfg->{cookie_path} ) : () ), + %attrs, + }; + + unless ( exists $cookie->{expires} ) { + $cookie->{expires} = $c->calculate_session_cookie_expires(); + } + + #beware: we have to accept also the old syntax "cookie_secure = true" + my $sec = $cfg->{cookie_secure} || 0; # default = 0 (not set) + $cookie->{secure} = 1 unless ( ($sec==0) || ($sec==2) ); + $cookie->{secure} = 1 if ( ($sec==2) && $c->req->secure ); + + $cookie->{httponly} = $cfg->{cookie_httponly}; + $cookie->{httponly} = 1 + unless defined $cookie->{httponly}; # default = 1 (set httponly) + + return $cookie; +} + +sub calc_expiry { # compat + my $c = shift; + $c->maybe::next::method( @_ ) || $c->calculate_session_cookie_expires( @_ ); +} + +sub calculate_session_cookie_expires { + my $c = shift; + my $cfg = $c->_session_plugin_config; + + my $value = $c->maybe::next::method(@_); + return $value if $value; + + if ( exists $c->session->{__cookie_expires} ) { + if ( $c->session->{__cookie_expires} > 0 ) { + return time() + $c->session->{__cookie_expires}; + } + else { + return undef; + } + } + elsif ( exists $cfg->{cookie_expires} ) { + if ( $cfg->{cookie_expires} > 0 ) { + return time() + $cfg->{cookie_expires}; + } + else { + return undef; + } + } + else { + return $c->session_expires; + } +} + +sub set_session_cookie_expire { + my $c = shift; + my $val = shift; + + if ( defined $val ) { + $c->session->{__cookie_expires} = $val; + } + else { + delete $c->session->{__cookie_expires}; + } + return 1; +} + +sub get_session_cookie { + my $c = shift; + + my $cookie_name = $c->_session_plugin_config->{cookie_name}; + + return $c->request->cookies->{$cookie_name}; +} + +sub get_session_id { + my $c = shift; + + if ( !$c->_deleted_session_id and my $cookie = $c->get_session_cookie ) { + my $sid = $cookie->value; + $c->log->debug(qq/Found sessionid "$sid" in cookie/) if $c->debug; + return $sid if $sid; + } + + $c->maybe::next::method(@_); +} + +sub delete_session_id { + my ( $c, $sid ) = @_; + + $c->_deleted_session_id(1); # to prevent get_session_id from returning it + + $c->update_session_cookie( $c->make_session_cookie( $sid, expires => 0 ) ); + + $c->maybe::next::method($sid); +} + +__PACKAGE__ + +__END__ + +=pod + +=head1 NAME + +Catalyst::Plugin::Session::State::Cookie - Maintain session IDs using cookies. + +=head1 SYNOPSIS + + use Catalyst qw/Session Session::State::Cookie Session::Store::Foo/; + +=head1 DESCRIPTION + +In order for L<Catalyst::Plugin::Session> to work the session ID needs to be +stored on the client, and the session data needs to be stored on the server. + +This plugin stores the session ID on the client using the cookie mechanism. + +=head1 PUBLIC METHODS + +=head2 set_session_cookie_expire $ttl_in_seconds + + $c->set_session_cookie_expire(3600); # set to 1 hour + $c->set_session_cookie_expire(0); # expire with browser session + $c->set_session_cookie_expire(undef); # fallback to default + +This lets you change the expiry for the current session's cookie. You can set a +number of seconds, 0 to expire the cookie when the browser quits or undef to +fallback to the configured defaults. The value you choose is persisted. + +Note this value has no effect on the exipry in the session store - it only +affects the cookie itself. + +=head1 METHODS + +=over 4 + +=item make_session_cookie + +Returns a hash reference with the default values for new cookies. + +=item update_session_cookie $hash_ref + +Sets the cookie based on C<cookie_name> in the response object. + +=item calc_expiry + +=item calculate_session_cookie_expires + +=item cookie_is_rejecting + +=item delete_session_id + +=item extend_session_id + +=item get_session_cookie + +=item get_session_id + +=item set_session_id + +=back + +=head1 EXTENDED METHODS + +=over 4 + +=item prepare_cookies + +Will restore if an appropriate cookie is found. + +=item finalize_cookies + +Will set a cookie called C<session> if it doesn't exist or if its value is not +the current session id. + +=item setup_session + +Will set the C<cookie_name> parameter to its default value if it isn't set. + +=back + +=head1 CONFIGURATION + +=over 4 + +=item cookie_name + +The name of the cookie to store (defaults to C<Catalyst::Utils::apprefix($c) . '_session'>). + +=item cookie_domain + +The name of the domain to store in the cookie (defaults to current host) + +=item cookie_expires + +Number of seconds from now you want to elapse before cookie will expire. +Set to 0 to create a session cookie, ie one which will die when the +user's browser is shut down. + +=item cookie_secure + +If this attribute B<set to 0> the cookie will not have the secure flag. + +If this attribute B<set to 1> (or true for backward compatibility) - the cookie +sent by the server to the client will get the secure flag that tells the browser +to send this cookie back to the server only via HTTPS. + +If this attribute B<set to 2> then the cookie will get the secure flag only if +the request that caused cookie generation was sent over https (this option is +not good if you are mixing https and http in your application). + +Default value is 0. + +=item cookie_httponly + +If this attribute B<set to 0>, the cookie will not have HTTPOnly flag. + +If this attribute B<set to 1>, the cookie will got HTTPOnly flag that should +prevent client side Javascript accessing the cookie value - this makes some +sort of session hijacking attacks significantly harder. Unfortunately not all +browsers support this flag (MSIE 6 SP1+, Firefox 3.0.0.6+, Opera 9.5+); if +a browser is not aware of HTTPOnly the flag will be ignored. + +Default value is 1. + +Note1: Many peole are confused by the name "HTTPOnly" - it B<does not mean> +that this cookie works only over HTTP and not over HTTPS. + +Note2: This parameter requires Catalyst::Runtime 5.80005 otherwise is skipped. + +=item cookie_path + +The path of the request url where cookie should be baked. + +=back + +For example, you could stick this in MyApp.pm: + + __PACKAGE__->config( 'Plugin::Session' => { + cookie_domain => '.mydomain.com', + }); + +=head1 CAVEATS + +Sessions have to be created before the first write to be saved. For example: + + sub action : Local { + my ( $self, $c ) = @_; + $c->res->write("foo"); + $c->session( ... ); + ... + } + +Will cause a session ID to not be set, because by the time a session is +actually created the headers have already been sent to the client. + +=head1 SEE ALSO + +L<Catalyst>, L<Catalyst::Plugin::Session>. + +=head1 AUTHORS + +Yuval Kogman E<lt>nothingmuch@woobling.orgE<gt> + +=head1 CONTRIBUTORS + +This module is derived from L<Catalyst::Plugin::Session::FastMmap> code, and +has been heavily modified since. + + Andrew Ford + Andy Grundman + Christian Hansen + Marcus Ramberg + Jonathan Rockway E<lt>jrockway@cpan.orgE<gt> + Sebastian Riedel + Florian Ragwitz + +=head1 COPYRIGHT + +Copyright (c) 2005 - 2009 +the Catalyst::Plugin::Session::State::Cookie L</AUTHORS> and L</CONTRIBUTORS> +as listed above. + +=head1 LICENSE + +This program is free software, you can redistribute it and/or modify it +under the same terms as Perl itself. + +=cut + +1; diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm index 6728ebef8..2ae90c2fa 100644 --- a/perllib/FixMyStreet/App.pm +++ b/perllib/FixMyStreet/App.pm @@ -15,7 +15,7 @@ use Catalyst ( 'Unicode', 'Session', 'Session::Store::DBIC', - 'Session::State::Cookie', + 'Session::State::Cookie', # FIXME - we're using our own override atm 'Authentication', ); diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index 16f0b994c..7526c2c25 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -48,8 +48,9 @@ Allow the user to legin with a username and a password. sub login : Private { my ( $self, $c ) = @_; - my $email = $c->req->param('email') || ''; - my $password = $c->req->param('password') || ''; + my $email = $c->req->param('email') || ''; + my $password = $c->req->param('password') || ''; + my $remember_me = $c->req->param('remember_me') || 0; # logout just in case $c->logout(); @@ -58,6 +59,11 @@ sub login : Private { && $password && $c->authenticate( { email => $email, password => $password } ) ) { + + # unless user asked to be remembered limit the session to browser + $c->set_session_cookie_expire(0) + unless $remember_me; + $c->res->redirect( $c->uri_for('/my') ); return; } diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm index 3d011d708..c16f288c8 100644 --- a/perllib/FixMyStreet/TestMech.pm +++ b/perllib/FixMyStreet/TestMech.pm @@ -280,4 +280,28 @@ sub visible_form_values { return \%params; } +=head2 session_cookie_expiry + + $expiry = $mech->session_cookie_expiry( ); + +Returns the current expiry time for the session cookie. Might be '0' which +indicates it expires at end of browser session. + +=cut + +sub session_cookie_expiry { + my $mech = shift; + + my $cookie_name = 'fixmystreet_app_session'; + my $expires = 'not found'; + + $mech # + ->cookie_jar # + ->scan( sub { $expires = $_[8] if $_[1] eq $cookie_name } ); + + croak "Could not find cookie '$cookie_name'" if $expires eq 'not found'; + + return $expires || 0; +} + 1; diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t index 6e1e8d58d..78d3a5abf 100644 --- a/t/app/controller/auth.t +++ b/t/app/controller/auth.t @@ -1,7 +1,7 @@ use strict; use warnings; -use Test::More tests => 97; +use Test::More tests => 94; use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; @@ -178,23 +178,33 @@ $mech->not_logged_in_ok; ok $user->password, "user now has a password"; } -# login using valid details -$mech->get_ok('/auth'); -$mech->submit_form_ok( - { - form_name => 'general_auth', - fields => { - email => $test_email, - password => $test_password, - }, - button => 'login', - }, - "login with '$test_email' & '$test_password" -); -is $mech->uri->path, '/my', "redirected to correct page"; +foreach my $remember_me ( '1', '0' ) { + subtest "login using valid details (remember_me => '$remember_me')" => sub { + $mech->get_ok('/auth'); + $mech->submit_form_ok( + { + form_name => 'general_auth', + fields => { + email => $test_email, + password => $test_password, + remember_me => ( $remember_me ? 1 : undef ), + }, + button => 'login', + }, + "login with '$test_email' & '$test_password" + ); + is $mech->uri->path, '/my', "redirected to correct page"; -# logout -$mech->log_out_ok; + # check that the cookie has no expiry set + my $expiry = $mech->session_cookie_expiry; + $remember_me + ? cmp_ok( $expiry, '>', 86400, "long expiry time" ) + : is( $expiry, 0, "no expiry time" ); + + # logout + $mech->log_out_ok; + }; +} # try to login with bad details $mech->get_ok('/auth'); diff --git a/templates/web/default/auth/general.html b/templates/web/default/auth/general.html index e0d516cbd..3d80a1049 100644 --- a/templates/web/default/auth/general.html +++ b/templates/web/default/auth/general.html @@ -33,11 +33,12 @@ <label for="password">[% loc('Password:') %]</label> <input type="password" name="password" value=""> <br> - <!-- FIXME - implement session length choosing + <label for="remember_me"> </label> - <input type="checkbox" name="remember_me"> + <input type="checkbox" name="remember_me" value='1' [% 'checked="checked"' IF remember_me %]> Remember me - do not use on a public computer - <br> --> + <br> + <label for="login"> </label> <input type="submit" name="login" value="[% loc('Log me in') %]"> |