From 3722cfd9072094fb914f482a26bb7aa9326e947c Mon Sep 17 00:00:00 2001 From: Daniel Perrett Date: Wed, 15 Jul 2015 13:44:10 +0100 Subject: [PATCH] Allow coderefs in token_per_request --- lib/Plack/Middleware/XSRFBlock.pm | 31 ++++++++++++++--- t/03.token_per_request.t | 55 +++++++++++++++++++++++++++++-- t/lib/Test/XSRFBlock/App.pm | 38 +++++++++++++++++++++ 3 files changed, 117 insertions(+), 7 deletions(-) diff --git a/lib/Plack/Middleware/XSRFBlock.pm b/lib/Plack/Middleware/XSRFBlock.pm index 8064607..bed6abc 100644 --- a/lib/Plack/Middleware/XSRFBlock.pm +++ b/lib/Plack/Middleware/XSRFBlock.pm @@ -46,7 +46,12 @@ sub prepare_app { $self->cookie_options( $self->cookie_options || {} ); # default to one token per session, not one per request - $self->token_per_request( $self->token_per_request || 0 ); + my $token_per_request = $self->token_per_request ? 1 : 0; + $self->token_per_request( + ref $self->token_per_request eq 'CODE' + ? $self->token_per_request + : sub { $token_per_request } + ); # default to a cookie life of three hours $self->cookie_expiry_seconds( $self->cookie_expiry_seconds || (3 * 60 * 60) ); @@ -123,9 +128,9 @@ sub call { return Plack::Util::response_cb($self->app->($env), sub { my $res = shift; - # if we asked for token_per_request then we *always* create a new token + # Determine whether to create a new token, based on the request $cookie_value = $self->_token_generator->() - if $self->token_per_request; + if $self->token_per_request->( $self, $request, $env ); # make it easier to work with the headers my $headers = Plack::Util::headers($res->[1]); @@ -388,11 +393,27 @@ and C on the cookie to force the cookie to only be sent on SSL requests. =item token_per_request (default: 0) -If this is true a new token is assigned for each request made. +If this is true a new token is assigned for each request made (but see below). -This may make your application more secure, or less susceptible to +This may make your application more secure, but more susceptible to double-submit issues. +If this is a coderef, the coderef will be evaluated with the following arguments: + +=over + +=item * The middleware object itself, + +=item * The request, + +=item * The environment + +=back + +If the result of the evaluation is a true value, a new token will be assigned. +This allows fine-grained control, for example to avoid assigning new tokens when +incidental requests are made (e.g. on-page ajax requests). + =item meta_tag (default: undef) If this is set, use the value as the name of the meta tag to add to the head diff --git a/t/03.token_per_request.t b/t/03.token_per_request.t index 4c0e664..1e17292 100644 --- a/t/03.token_per_request.t +++ b/t/03.token_per_request.t @@ -65,15 +65,65 @@ for my $appname ( }; } +# test buffered and non-buffered apps for token_per_request_sub behaviour +for my $appname ( + 'psgix.input.non-buffered.token_per_request_sub', + 'psgix.input.buffered.token_per_request_sub', +) { + subtest $appname => sub { + my $ua = LWP::UserAgent->new; + $ua->cookie_jar( HTTP::Cookies->new ); + + test_psgi ua => $ua, app => $app{$appname}, client => sub { + my $cb = shift; + my ($res, $h_cookie, $jar, $token); + + my %token = %{ _two_requests($cb, $ua) }; + + is( + $token{1}, + $token{2}, + 'cookie tokens are the same for two requests for /form/html using token_per_request_sub' + ); + }; + }; +} + +for my $appname ( + 'psgix.input.non-buffered.token_per_request_sub', + 'psgix.input.buffered.token_per_request_sub', +) { + subtest "$appname-post" => sub { + my $ua = LWP::UserAgent->new; + $ua->cookie_jar( HTTP::Cookies->new ); + + test_psgi ua => $ua, app => $app{$appname}, client => sub { + my $cb = shift; + my ($res, $h_cookie, $jar, $token); + + my %token = %{ _two_requests($cb, $ua, '/form/xhtml') }; + + isnt( + $token{1}, + $token{2}, + 'cookie tokens are different for two requests for /form/xhtml using token_per_request_sub' + ); + }; + }; +} + + sub _two_requests { - my ($cb, $ua) = @_; + my ($cb, $ua, $url) = @_; + + $url ||= "/form/html"; my $jar = $ua->cookie_jar; my %token; # making two requests should result in different tokens for (1..2) { - my $res = $cb->(GET "/form/html"); + my $res = $cb->(GET $url); is ( $res->code, HTTP_OK, @@ -92,4 +142,5 @@ sub _two_requests { } + done_testing; diff --git a/t/lib/Test/XSRFBlock/App.pm b/t/lib/Test/XSRFBlock/App.pm index 497edcf..6431632 100644 --- a/t/lib/Test/XSRFBlock/App.pm +++ b/t/lib/Test/XSRFBlock/App.pm @@ -208,6 +208,44 @@ sub setup_test_apps { $mapped; }; + # create a new token only if the request is to a path which contains the string xhtml + $app{'psgix.input.non-buffered.token_per_request_sub'} = builder { + if ($ENV{PLACK_DEBUG}) { + use Log::Dispatch; + my $logger = Log::Dispatch->new( + outputs => [ + [ + 'Screen', + min_level => 'debug', + stderr => 1, + newline => 1 + ] + ], + ); + enable "LogDispatch", logger => $logger; + } + enable 'XSRFBlock', + token_per_request => sub { $_[1]->path =~ /xhtml/i }; + $mapped; + }; + + # psgix.input.buffered + # create a new token only if the request is to a path which contains the string xhtml + $app{'psgix.input.buffered.token_per_request_sub'} = builder { + enable sub { + my $app = shift; + sub { + my $env = shift; + my $req = Plack::Request->new($env); + my $content = $req->content; # <<< force psgix.input.buffered true. + $app->($env); + }; + }; + enable 'XSRFBlock', + token_per_request => sub { $_[1]->path =~ /xhtml/i }; + $mapped; + }; + $app{'psgix.input.non-buffered.token_per_session'} = builder { if ($ENV{PLACK_DEBUG}) { use Log::Dispatch;