diff --git a/lib/AccessCheck/App/Controller.pm b/lib/AccessCheck/App/Controller.pm index 80b1a75b5e7ce66f1259bb199282f1c350e9fdd7..452d49137289c5454ccfb11d66e1e9168c6a62cf 100644 --- a/lib/AccessCheck/App/Controller.pm +++ b/lib/AccessCheck/App/Controller.pm @@ -119,6 +119,39 @@ sub check_token { return 1; } +=head2 check_csrf_token() + +Check if provided anti-CSRF token, as I<token> parameter, matches expected +one, abort otherwise. + +=cut + +sub check_csrf_token { + my ($self, %args) = @_; + + my $provided_token = $self->param('token'); + return $self->abort( + status => 403, + log_message => sprintf("missing anti-CSRF token for action %s", $self->current_route()), + user_message => "missing CSRF token" + ) if !$provided_token; + + my $expected_token = $self->csrf_token(); + + return $self->abort( + status => 403, + log_message => sprintf( + "invalid anti-CSRF token for action %s: %s instead of expected %s", + $self->current_route(), + $provided_token, + $expected_token, + ), + user_message => "invalid CSRF token" + ) if $provided_token ne $expected_token; + + return 1; +} + sub get_sp { my ($self, %args) = @_; diff --git a/lib/AccessCheck/App/Step3.pm b/lib/AccessCheck/App/Step3.pm index ad2ac24aeecefcff556a3d2a7e80ad34b78b5397..29be04261818fa635d68f383f3f503b68fd980e8 100644 --- a/lib/AccessCheck/App/Step3.pm +++ b/lib/AccessCheck/App/Step3.pm @@ -28,6 +28,8 @@ sub run { return if !$self->check_authentication(); } + return if !$self->check_csrf_token(); + my $entityid = $self->param('entityid'); my $email = $self->param('email'); diff --git a/t/app.t b/t/app.t index 7d901135192e9479080f502739c95bb8c973e90c..1985c77761e1615aba1153c47705acefeedddf05 100755 --- a/t/app.t +++ b/t/app.t @@ -20,7 +20,7 @@ plan(skip_all => 'live database required') unless $ENV{TEST_DB_NAME} && $ENV{TEST_DB_TYPE}; -plan tests => 10; +plan tests => 11; sub named_subtest { my ($name, $code, @args) = @_; @@ -224,10 +224,25 @@ named_subtest "email selection page, valid entityid" => sub { html_ok($res) or diag_file($res, $test_dir); }; -named_subtest "challenge page, missing entityid" => sub { +named_subtest "challenge page, missing CSRF token" => sub { my $t = get_test_object(test => $_[0]); $t->get_ok('/step3') + ->status_is(403) + ->text_is('html head title' => 'eduGAIN Access Check', 'expected title') + ->content_like(qr/Error:[\n\s]+missing CSRF token/, 'expected error message'); + + my $res = $t->tx()->res(); + html_ok($res) or diag_file($res, $test_dir); +}; + +named_subtest "challenge page, missing entityid" => sub { + my $t = get_test_object(test => $_[0]); + + # neutralize CSRF token check, as we short-circuit the form + $t->app()->hook(before_dispatch => sub { $_[0]->session(csrf_token => 'foo') }); + + $t->get_ok('/step3' => form => {token => 'foo'}) ->status_is(200) ->text_is('html head title' => 'eduGAIN Access Check', 'expected title') ->content_like(qr/Error:[\n\s]+missing parameter 'entityid'/, 'expected error message'); @@ -239,7 +254,10 @@ named_subtest "challenge page, missing entityid" => sub { named_subtest "challenge page, invalid entityid" => sub { my $t = get_test_object(test => $_[0]); - $t->get_ok('/step3' => form => {entityid => 'foo'}) + # neutralize CSRF token check, as we short-circuit the form + $t->app()->hook(before_dispatch => sub { $_[0]->session(csrf_token => 'foo') }); + + $t->get_ok('/step3' => form => {token => 'foo', entityid => 'foo'}) ->status_is(200) ->text_is('html head title' => 'eduGAIN Access Check', 'expected title') ->content_like(qr/Error:[\n\s]+invalid parameter 'entityid'/, 'expected error message'); @@ -251,7 +269,10 @@ named_subtest "challenge page, invalid entityid" => sub { named_subtest "challenge page, valid entityid, missing email" => sub { my $t = get_test_object(test => $_[0]); - $t->get_ok('/step3' => form => {entityid => 'https://sp.renater.fr/'}) + # neutralize CSRF token check, as we short-circuit the form + $t->app()->hook(before_dispatch => sub { $_[0]->session(csrf_token => 'foo') }); + + $t->get_ok('/step3' => form => {token => 'foo', entityid => 'https://sp.renater.fr/'}) ->status_is(200) ->text_is('html head title' => 'eduGAIN Access Check', 'expected title') ->content_like(qr/Error:[\n\s]+missing parameter 'email'/, 'expected error message'); @@ -263,7 +284,10 @@ named_subtest "challenge page, valid entityid, missing email" => sub { named_subtest "challenge page, valid entityid, invalid email" => sub { my $t = get_test_object(test => $_[0]); - $t->get_ok('/step3' => form => {entityid => 'https://sp.renater.fr/', email => 'foo'}) + # neutralize CSRF token check, as we short-circuit the form + $t->app()->hook(before_dispatch => sub { $_[0]->session(csrf_token => 'foo') }); + + $t->get_ok('/step3' => form => {token => 'foo', entityid => 'https://sp.renater.fr/', email => 'foo'}) ->status_is(200) ->text_is('html head title' => 'eduGAIN Access Check', 'expected title') ->content_like(qr/Error:[\n\s]+invalid parameter 'email'/, 'expected error message'); diff --git a/templates/web/edugain/step2.html.tt2 b/templates/web/edugain/step2.html.tt2 index 7c4eacc7f8a23264f8d96283374fa3697dbcd472..862d5e6bcc6be16e3cf52286971e752826fd4893 100644 --- a/templates/web/edugain/step2.html.tt2 +++ b/templates/web/edugain/step2.html.tt2 @@ -1,5 +1,6 @@ [% WRAPPER index.html.tt2 %] <form class="wizard clearfix" action="[% c.url_for('step3') %]" method="get"> + <input type="hidden" name="token" value="[% c.csrf_token() %]"> <div class="steps clearfix"> <ol> <li class="done">[% c.loc("Select your service provider") %]</li> diff --git a/templates/web/renater/step2.html.tt2 b/templates/web/renater/step2.html.tt2 index 60fc5cc23969f2e01e18d3c7f7f259e8e58a4494..12e6860587c1fb7e49d654e2785f1982f8c09bd9 100644 --- a/templates/web/renater/step2.html.tt2 +++ b/templates/web/renater/step2.html.tt2 @@ -1,5 +1,6 @@ [% WRAPPER index.html.tt2 %] <form class="wizard clearfix" action="[% c.url_for('step3') %]" method="get"> + <input type="hidden" name="token" value="[% c.csrf_token() %]"> <div class="steps clearfix"> <ol> <li class="done">[% c.loc("Select your service provider") %]</li>