Skip Menu |
 

This queue is for tickets about the Plack-Middleware-Auth-Form CPAN distribution.

Report information
The Basics
Id: 75896
Status: resolved
Priority: 0/
Queue: Plack-Middleware-Auth-Form

People
Owner: Nobody in particular
Requestors: cpan [...] sourcentral.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.010
Fixed in: (no value)



Subject: Cookie Expiry Date not set for "remember" session
Download (untitled) / with headers
text/plain 563b
Only the first time the plack_session cookie is returned to the browser the expiry date is set. Every subsequent cookie returned is a session cookie (without expiry date set). Since the "remember" marker is removed from the session data there is no chance to set the expiry date again on the cookie if it is sent again. My proposed fix is attached: - remove deletion of remember attribute after setting expiry date on cookie - setting the remember attribute on every valid login to the actual state of the checkbox - remove remember attribute on logout
Subject: auth-form-expiry.patch
--- /usr/share/perl5/vendor_perl/Plack/Middleware/Auth/Form.pm 2011-08-04 21:59:06.000000000 +0200 +++ /usr/share/perl5/vendor_perl/Plack/Middleware/Auth/Form.pm.new 2012-03-20 06:50:23.974449572 +0100 @@ -31,7 +31,6 @@ if( $path ne '/logout' ){ $env->{'psgix.session.options'}{expires} = time + 60 * 60 * 24 * 30; } - delete $env->{'psgix.session'}{remember}; } if( $path eq '/login' ){ @@ -75,7 +74,7 @@ } if( !$login_error ){ $env->{'psgix.session'}{user_id} = $user_id; - $env->{'psgix.session'}{remember} = 1 if $params->get( 'remember' ); + $env->{'psgix.session'}{remember} = ($params->get( 'remember' ) ? 1 : 0); my $redir_to = delete $env->{'psgix.session'}{redir_to}; $redir_to = '/' if URI->new( $redir_to )->path eq $env->{PATH_INFO}; @@ -130,6 +129,7 @@ my($self, $env) = @_; if( $env->{REQUEST_METHOD} eq 'POST' ){ delete $env->{'psgix.session'}{user_id}; + delete $env->{'psgix.session'}{remember}; } return [ 303,
Download (untitled) / with headers
text/plain 261b
While fixing the tests I found out that I changed the remember attribute correctly within the module but not with the tests. For this I fixed the tests. This time the proposed patch includes the fixes for the changes with login on logged in account as well.
Subject: auth-form-test.patch
diff -Nur Plack-Middleware-Auth-Form-0.010.orig/t/unit.t Plack-Middleware-Auth-Form-0.010/t/unit.t --- Plack-Middleware-Auth-Form-0.010.orig/t/unit.t 2011-08-04 21:59:06.000000000 +0200 +++ Plack-Middleware-Auth-Form-0.010/t/unit.t 2012-03-20 08:29:01.140358155 +0100 @@ -34,13 +34,23 @@ is( $res->[1][1], '/landing_page', 'Redirection after login' ) or warn Dumper($res); is( $post_req->{'psgix.session'}{user_id}, 'joe', 'Username saved in the session' ); is( $post_req->{'psgix.session'}{redir_to}, undef, 'redir_to removed after usage' ); -ok( !exists $post_req->{'psgix.session'}{remember} ); +is( $post_req->{'psgix.session'}{remember}, 0, 'remember not set' ); +$post_req->{PATH_INFO} = '/logout'; +$res = $middleware->call( $post_req ); +ok( !exists( $post_req->{'psgix.session'}{user_id} ), 'User logged out' ); + +$post_req->{PATH_INFO} = '/login'; $post_req->{'psgix.session'}{redir_to} = '/new_landing_page'; $middleware = Plack::Middleware::Auth::Form->new( authenticator => sub { { user_id => 1 } } ); $res = $middleware->call( $post_req ); is( $post_req->{'psgix.session'}{user_id}, '1', 'User id saved in the session' ); +$post_req->{PATH_INFO} = '/logout'; +$res = $middleware->call( $post_req ); +ok( !exists( $post_req->{'psgix.session'}{user_id} ), 'User logged out' ); + +$post_req->{PATH_INFO} = '/login'; $middleware = Plack::Middleware::Auth::Form->new( authenticator => sub { 0 } ); $res = $middleware->call( $post_req ); like( join( '', @{ $res->[2] } ), qr/error.*form id="login_form"/, 'login form for login error' ); diff -Nur Plack-Middleware-Auth-Form-0.010.orig/t/wrap_body_override.t Plack-Middleware-Auth-Form-0.010/t/wrap_body_override.t --- Plack-Middleware-Auth-Form-0.010.orig/t/wrap_body_override.t 2011-08-04 21:59:06.000000000 +0200 +++ Plack-Middleware-Auth-Form-0.010/t/wrap_body_override.t 2012-03-20 08:30:16.960372929 +0100 @@ -48,9 +48,13 @@ is( $res->[1][1], '/landing_page', 'Redirection after login' ) or warn Dumper($res); is( $post_req->{'psgix.session'}{user_id}, 'joe', 'Username saved in the session' ); is( $post_req->{'psgix.session'}{redir_to}, undef, 'redir_to removed after usage' ); -ok( !exists $post_req->{'psgix.session'}{remember} ); +is( $post_req->{'psgix.session'}{remember}, 0, 'remember not set' ); +$post_req->{PATH_INFO} = '/logout'; +$res = $middleware->call( $post_req ); +ok( !exists( $post_req->{'psgix.session'}{user_id} ), 'User logged out' ); +$post_req->{PATH_INFO} = '/login'; $middleware = $class->new( authenticator => sub { 0 } ); $res = $middleware->call( $post_req ); $html = join '', @{ $res->[2] };
Download (untitled) / with headers
text/plain 222b
Thanks for the report! It got me thinking - somehow I expected the Session middleware not to send the cookie when there were no changes. I think this is how other systems work. Do you happen to know why it does that?
Download (untitled) / with headers
text/plain 937b
I see two aspects: 1) An expiring cookie should be updated from time to time to extend it's duration even if the token has not changed. Would be bad if an expiring cookie including the token is dropped during an active session. Even if something is changed in update algorithm of P::M::Session, this requires my fix, even if the hardcoded expiry date might not be hit. 2) An session cookie does not need do be updated until the session token changes. Maybe you are right and we should discuss this with Plack::Middleware::Session Maintainers. They have interesting undocumented features like $options->{no_store} and $options->{change_id}. Any maybe there is a better way to hand of handling of remember to the State::Cookie class itself, including a configurable expiry date. Maybe you can switch the remember logic: delete the expiry date if remember is not set. If remeber is set, the predefined State::Cookie expiry date is used.
Download (untitled) / with headers
text/plain 547b
I committed your changes in: https://github.com/zby/Plack-Middleware- Auth-Form/commit/650190fef5bea91851617b91cffb9cd02a3149cf and I added one test for this in: https://github.com/zby/Plack-Middleware-Auth- Form/commit/35191ba77013d639c0bf9c1a03ebebc9bb2eba47 I really think that the Session middleware should not reset Expire without request - but since they do we'll need to work around that. I think I'll make the release next week - there are already many changes there. I am thinking about changing the name to WebPrototypes::LoginForm.
The fix above is published in 0.011


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.