From f3603170797447251c8ad57175514e73f0b472c7 Mon Sep 17 00:00:00 2001 From: Maxime Besson Date: Thu, 25 Jan 2024 16:00:23 +0100 Subject: [PATCH 1/2] Display explicit token timeout error when issuer context is lost (#3092) --- .../lib/Lemonldap/NG/Portal/Main/Issuer.pm | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Issuer.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Issuer.pm index 3969b3669a..17934e0c35 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Issuer.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Issuer.pm @@ -18,6 +18,7 @@ use Lemonldap::NG::Portal::Main::Constants qw( PE_OK PE_RENEWSESSION PE_UPGRADESESSION + PE_TOKENEXPIRED ); extends 'Lemonldap::NG::Portal::Main::Plugin'; @@ -139,21 +140,35 @@ sub _redirect { sub _forAuthUser { my ( $self, $req, @path ) = @_; + my $restore_failed; if ( $req->pdata->{issuerTs} and $req->pdata->{issuerTs} + $self->conf->{issuersTimeout} < time ) { + $restore_failed = 1; $self->cleanAllPdata($req); } + $self->logger->debug('Processing _forAuthUser'); if ( my $r = $req->pdata->{ $self->ipath } ) { $self->logger->debug("Restoring request to $self->{path} issuer"); - $self->restoreRequest( $req, $r ); - @path = @{ $req->pdata->{ $self->ipath . 'Path' } } - if ( $req->pdata->{ $self->ipath . 'Path' } ); + if ( $self->restoreRequest( $req, $r ) ) { + @path = @{ $req->pdata->{ $self->ipath . 'Path' } } + if ( $req->pdata->{ $self->ipath . 'Path' } ); + + # In case a confirm form is shown, we need it to POST on the + # current Path + $req->data->{confirmFormAction} = URI->new( $req->uri )->path; + } + else { + $restore_failed = 1; + $self->cleanAllPdata($req); + } + } - # In case a confirm form is shown, we need it to POST on the - # current Path - $req->data->{confirmFormAction} = URI->new( $req->uri )->path; + if ( $restore_failed and !@path ) { + $self->logger->error("Unable to restore issuer context"); + $req->mustRedirect(1); + return $self->p->do( $req, [ sub { PE_TOKENEXPIRED } ] ); } # Clean pdata: keepPdata has been set, so pdata must be cleaned here @@ -221,8 +236,11 @@ sub storeRequest { sub restoreRequest { my ( $self, $req, $token ) = @_; - my $env = $self->_ott->getToken($token); + + my $result = 0; + my $env = $self->_ott->getToken($token); if ($env) { + $result = 1; $self->logger->debug("Restoring request from $token"); if ( my $c = delete $env->{content} ) { $env->{'psgix.input.buffered'} = 0; @@ -237,7 +255,7 @@ sub restoreRequest { } $req->{uri} = uri_unescape( $req->env->{REQUEST_URI} ); $req->{uri} =~ s|^//+|/|g; - return $req; + return $result; } sub reAuth { -- GitLab From bf0e31c4037e6f01ccb6b0e70c4344660aee9770 Mon Sep 17 00:00:00 2001 From: Maxime Besson Date: Thu, 25 Jan 2024 16:00:47 +0100 Subject: [PATCH 2/2] Unit test for #3092 --- lemonldap-ng-portal/t/37-Issuer-Timeout.t | 312 ++++++++++++++-------- 1 file changed, 197 insertions(+), 115 deletions(-) diff --git a/lemonldap-ng-portal/t/37-Issuer-Timeout.t b/lemonldap-ng-portal/t/37-Issuer-Timeout.t index 80cc84c054..196ea7defc 100644 --- a/lemonldap-ng-portal/t/37-Issuer-Timeout.t +++ b/lemonldap-ng-portal/t/37-Issuer-Timeout.t @@ -11,137 +11,219 @@ BEGIN { require 't/oidc-lib.pm'; } -my $debug = 'error'; +my $debug = 'debug'; my $access_token; my $op; my $res; # Initialization ok( $op = op(), 'OP portal' ); -count(1); - -# Request for RP1 -my $authrequest1 = buildForm( - { - scope => "openid", - response_type => "code", - client_id => "rpid", - redirect_uri => "http://rp1.example.com/", - } -); -ok( - $res = $op->_get( - '/oauth2/authorize', - accept => 'text/html', - query => $authrequest1 - ), - 'Authorization request to RP1' -); -count(1); - -my $pdata = expectCookie( $res, 'lemonldappdata' ); - -# Uncomment this to make the unit test pass -#my $pdata = ""; + +subtest "Request RP1, wait for timeout, request RP2" => sub { + + # Request for RP1 + my $authrequest1 = buildForm( { + scope => "openid", + response_type => "code", + client_id => "rpid", + redirect_uri => "http://rp1.example.com/", + } + ); + ok( + $res = $op->_get( + '/oauth2/authorize', + accept => 'text/html', + query => $authrequest1 + ), + 'Authorization request to RP1' + ); + + my $pdata = expectCookie( $res, 'lemonldappdata' ); # Uncomment this to wait until issuersTimeout has expired, leading to a different error -Time::Fake->offset("+10m"); - -# Request for RP2 with previous pdata still around -my $authrequest2 = buildForm( - { - scope => "openid", - response_type => "code", - client_id => "rp2id", - redirect_uri => "http://rp2.example.com/", - } -); -ok( - $res = $op->_get( - '/oauth2/authorize', - accept => 'text/html', - query => $authrequest2, - cookie => "lemonldappdata=$pdata", - - ), - 'Authorization request to RP2' -); -count(1); - -$pdata = expectCookie( $res, 'lemonldappdata' ); -my ( $host, $url, $query ) = expectForm( $res, '#', undef, 'user', 'password' ); - -$query =~ s/user=/user=dwho/; -$query =~ s/password=/password=dwho/; - -# Login to OP -ok( - $res = $op->_post( - '/oauth2/authorize', - IO::String->new($query), - query => $authrequest2, - accept => 'text/html', - length => length($query), - cookie => "lemonldappdata=$pdata", - - ), - 'Authorization request to RP2' -); -count(1); - -$pdata = expectCookie( $res, 'lemonldappdata' ); - -# Process second factor -( $host, $url, $query ) = - expectForm( $res, undef, '/ext2fcheck?skin=bootstrap', 'token', 'code' ); - -ok( - $res->[2]->[0] =~ + Time::Fake->offset("+10m"); + + # Request for RP2 with previous pdata still around + my $authrequest2 = buildForm( { + scope => "openid", + response_type => "code", + client_id => "rp2id", + redirect_uri => "http://rp2.example.com/", + } + ); + ok( + $res = $op->_get( + '/oauth2/authorize', + accept => 'text/html', + query => $authrequest2, + cookie => "lemonldappdata=$pdata", + + ), + 'Authorization request to RP2' + ); + + $pdata = expectCookie( $res, 'lemonldappdata' ); + my ( $host, $url, $query ) = + expectForm( $res, '#', undef, 'user', 'password' ); + + $query =~ s/user=/user=dwho/; + $query =~ s/password=/password=dwho/; + + # Login to OP + ok( + $res = $op->_post( + '/oauth2/authorize', + IO::String->new($query), + query => $authrequest2, + accept => 'text/html', + length => length($query), + cookie => "lemonldappdata=$pdata", + + ), + 'Authorization request to RP2' + ); + + $pdata = expectCookie( $res, 'lemonldappdata' ); + + # Process second factor + ( $host, $url, $query ) = + expectForm( $res, undef, '/ext2fcheck?skin=bootstrap', 'token', 'code' ); + + ok( + $res->[2]->[0] =~ qr%%, - 'Found EXTCODE input' -) or print STDERR Dumper( $res->[2]->[0] ); -count(1); - -$query =~ s/code=/code=A1b2C0/; -ok( - $res = $op->_post( - '/ext2fcheck', - IO::String->new($query), - length => length($query), - accept => 'text/html', - cookie => "lemonldappdata=$pdata", - - ), - 'Post code' -); -count(1); - -# We now should be logged in, but lost the original URL -expectRedirection( $res, "http://auth.op.com/oauth2" ); -my $id = expectCookie($res); - -ok( - $res = $op->_get( - '/oauth2', - accept => 'text/html', - cookie => "lemonldap=$id; lemonldappdata=$pdata", - ), - 'Authorization request to RP1' -); -count(1); - -# We should be redirected to RP2 -expectRedirection( $res, qr#^http://rp2.example.com/# ); + 'Found EXTCODE input' + ) or print STDERR Dumper( $res->[2]->[0] ); + + $query =~ s/code=/code=A1b2C0/; + ok( + $res = $op->_post( + '/ext2fcheck', + IO::String->new($query), + length => length($query), + accept => 'text/html', + cookie => "lemonldappdata=$pdata", + + ), + 'Post code' + ); + + # We now should be logged in, but lost the original URL + expectRedirection( $res, "http://auth.op.com/oauth2" ); + my $id = expectCookie($res); + + ok( + $res = $op->_get( + '/oauth2', + accept => 'text/html', + cookie => "lemonldap=$id; lemonldappdata=$pdata", + ), + 'Authorization request to RP1' + ); + + # We should be redirected to RP2 + expectRedirection( $res, qr#^http://rp2.example.com/# ); +}; + +subtest "Request RP1, wait for timeout, complete login" => sub { + + Time::Fake->reset; + + # Request for RP1 + my $authrequest1 = buildForm( { + scope => "openid", + response_type => "code", + client_id => "rpid", + redirect_uri => "http://rp1.example.com/", + } + ); + ok( + $res = $op->_get( + '/oauth2/authorize', + accept => 'text/html', + query => $authrequest1 + ), + 'Authorization request to RP1' + ); + + my $pdata = expectCookie( $res, 'lemonldappdata' ); + my ( $host, $url, $query ) = + expectForm( $res, '#', undef, 'user', 'password' ); + $query =~ s/user=/user=dwho/; + $query =~ s/password=/password=dwho/; + + # Login to OP + ok( + $res = $op->_post( + '/oauth2/authorize', + IO::String->new($query), + query => $authrequest1, + accept => 'text/html', + length => length($query), + cookie => "lemonldappdata=$pdata", + + ), + 'Authorization request to RP2' + ); + + $pdata = expectCookie( $res, 'lemonldappdata' ); + + # Process second factor + ( $host, $url, $query ) = + expectForm( $res, undef, '/ext2fcheck?skin=bootstrap', 'token', 'code' ); + + ok( + $res->[2]->[0] =~ +qr%%, + 'Found EXTCODE input' + ) or print STDERR Dumper( $res->[2]->[0] ); + + $query =~ s/code=/code=A1b2C0/; + + # Wait long enough for the issuersTimeout to expire + Time::Fake->offset("+10m"); + + ok( + $res = $op->_post( + '/ext2fcheck', + IO::String->new($query), + length => length($query), + accept => 'text/html', + cookie => "lemonldappdata=$pdata", + + ), + 'Post code' + ); + $pdata = expectCookie( $res, 'lemonldappdata' ); + + # We now should be logged in, but lost the original URL + expectRedirection( $res, "http://auth.op.com/oauth2" ); + my $id = expectCookie($res); + + ok( + $res = $op->_get( + '/oauth2', + accept => 'text/html', + cookie => "lemonldap=$id; lemonldappdata=$pdata", + ), + 'Authorization request to RP1' + ); + expectPortalError( $res, 82 ); + $pdata = expectCookie( $res, 'lemonldappdata' ); + ok( !$pdata, "pdata was cleared" ); + +}; clean_sessions(); -done_testing( count() ); +done_testing(); sub op { - return LLNG::Manager::Test->new( - { + return LLNG::Manager::Test->new( { ini => { logLevel => $debug, domain => 'idp.com', + sfLoginTimeout => "1800", + issuersTimeout => "300", portal => 'http://auth.op.com/', authentication => 'Demo', userDB => 'Same', -- GitLab