From 627839806052f60d3a615baa41825f1b2419ee0f Mon Sep 17 00:00:00 2001 From: Xavier Guimard Date: Wed, 5 Sep 2018 09:19:01 +0200 Subject: [PATCH] Move "afterData" entry point before "buildCookie" and add "endAuth" entrypoint (#1497) TODO: optimize notifications --- .../Lemonldap/NG/Portal/2F/Engines/Default.pm | 2 +- .../NG/Portal/Lib/Notifications/JSON.pm | 4 +- .../NG/Portal/Lib/Notifications/XML.pm | 4 +- .../lib/Lemonldap/NG/Portal/Lib/Wrapper.pm | 2 + .../lib/Lemonldap/NG/Portal/Main/Init.pm | 75 ++++++++----------- .../lib/Lemonldap/NG/Portal/Main/Issuer.pm | 2 + .../lib/Lemonldap/NG/Portal/Main/Plugin.pm | 2 + .../lib/Lemonldap/NG/Portal/Main/Run.pm | 16 ++-- .../Lemonldap/NG/Portal/Main/SecondFactor.pm | 2 +- .../lib/Lemonldap/NG/Portal/Plugins/CDA.pm | 2 +- .../Lemonldap/NG/Portal/Plugins/CheckState.pm | 6 +- .../Lemonldap/NG/Portal/Plugins/History.pm | 2 +- .../NG/Portal/Plugins/Notifications.pm | 2 +- .../Lemonldap/NG/Portal/Plugins/RESTServer.pm | 2 + .../Lemonldap/NG/Portal/Plugins/SOAPServer.pm | 2 + .../NG/Portal/Plugins/SingleSession.pm | 2 +- .../NG/Portal/Plugins/StayConnected.pm | 2 +- lemonldap-ng-portal/t/61-GrantSession.t | 4 + 18 files changed, 72 insertions(+), 61 deletions(-) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Engines/Default.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Engines/Default.pm index 32ff50387..ef56382ea 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Engines/Default.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Engines/Default.pm @@ -250,7 +250,7 @@ sub _choice { $req, [ sub { $res }, 'controlUrl', - 'buildCookie', @{ $self->p->afterData }, + 'buildCookie', @{ $self->p->endAuth }, ] ); } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Notifications/JSON.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Notifications/JSON.pm index 7497676f2..4001bbf18 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Notifications/JSON.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Notifications/JSON.pm @@ -173,10 +173,10 @@ sub getNotifBack { # One pending notification has been found and not accepted, # restart process to display pending notifications - # TODO: is it a good idea to launch all 'afterData' subs ? + # TODO: is it a good idea to launch all 'endAuth' subs ? $self->logger->debug( 'Pending notification has been found and not accepted'); - return $self->p->do( $req, $self->p->afterData ); + return $self->p->do( $req, $self->p->endAuth ); } # All pending notifications have been accepted, restore cookies and diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Notifications/XML.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Notifications/XML.pm index 9f3e24cf9..5144c1bcf 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Notifications/XML.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Notifications/XML.pm @@ -232,10 +232,10 @@ sub getNotifBack { # One pending notification has been found and not accepted, # restart process to display pending notifications - # TODO: is it a good idea to launch all 'afterData' subs ? + # TODO: is it a good idea to launch all 'endAuth' subs ? $self->logger->debug( 'Pending notification has been found and not accepted'); - return $self->p->do( $req, $self->p->afterData ); + return $self->p->do( $req, $self->p->endAuth ); } # All pending notifications have been accepted, restore cookies and diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Wrapper.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Wrapper.pm index 92a6aff2e..92048f3ea 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Wrapper.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/Wrapper.pm @@ -24,12 +24,14 @@ has availableModules => ( is => 'rw', default => sub { {} } ); # to each enabled underlying auth modules sub betweenAuthAndData { '_betweenAuthAndData' } sub afterData { '_afterData' } +sub endAuth { '_endAuth' } sub forAuthUser { '_forAuthUser' } sub beforeLogout { '_beforeLogout' } sub authCancel { '_authCancel' } sub _betweenAuthAndData { _wrapEntryPoint( @_, 'betweenAuthAndData' ); } sub _afterData { _wrapEntryPoint( @_, 'afterData' ); } +sub _endAuth { _wrapEntryPoint( @_, 'endAuth' ); } sub _forAuthUser { _wrapEntryPoint( @_, 'forAuthUser', 1 ); } sub _beforeLogout { _wrapEntryPoint( @_, 'beforeLogout', 1 ); } sub _authCancel { _wrapEntryPoint( @_, 'authCancel' ); } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Init.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Init.pm index 6999a15d7..eeba5ccfd 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Init.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Init.pm @@ -40,36 +40,32 @@ has _jsRedirect => ( is => 'rw' ); has trustedDomainsRe => ( is => 'rw' ); # Lists to store plugins entry-points -has beforeAuth => ( - is => 'rw', - isa => 'ArrayRef', - default => sub { [] } -); -has betweenAuthAndData => ( - is => 'rw', - isa => 'ArrayRef', - default => sub { [] } -); -has afterData => ( - is => 'rw', - isa => 'ArrayRef', - default => sub { [] } -); -has authCancel => ( - is => 'rw', - isa => 'ArrayRef', - default => sub { [] } -); -has forAuthUser => ( - is => 'rw', - isa => 'ArrayRef', - default => sub { [] } -); -has beforeLogout => ( - is => 'rw', - isa => 'ArrayRef', - default => sub { [] } -); +my @entryPoints; + +BEGIN { + @entryPoints = ( + + # Auth process entrypoints + qw(beforeAuth betweenAuthAndData afterData endAuth), + + # Authenticated users entrypoint + 'forAuthUser', + + # Logout entrypoint + 'beforeLogout', + + # Special endpoint + 'authCancel', # Clean pdata when user click on "cancel" + ); + + foreach (@entryPoints) { + has $_ => ( + is => 'rw', + isa => 'ArrayRef', + default => sub { [] } + ); + } +} has spRules => ( is => 'rw', @@ -155,10 +151,7 @@ sub reloadConf { %{ $self->{conf} } = %{ $self->localConfig }; # Reinitialize arrays - foreach ( - qw(_macros _groups beforeAuth betweenAuthAndData afterData authCancel forAuthUser beforeLogout) - ) - { + foreach ( qw(_macros _groups), @entryPoints) { $self->{$_} = []; } $self->spRules( {} ); @@ -222,7 +215,8 @@ sub reloadConf { $self->error("$type is not set"); return $self->fail; } - $mod = $self->conf->{$type} unless ( $self->conf->{$type} eq 'Same' ); + $mod = $self->conf->{$type} + unless ( $self->conf->{$type} eq 'Same' ); my $module = '::' . ucfirst($type) . '::' . $mod; $module =~ s/Authentication/Auth/; @@ -312,7 +306,7 @@ sub reloadConf { } # Clean $req->pdata after authentication - push @{ $self->afterData }, sub { + push @{ $self->endAuth }, sub { unless ( $_[0]->pdata->{keepPdata} ) { $self->logger->debug('Cleaning pdata'); $_[0]->pdata( {} ); @@ -348,10 +342,7 @@ sub findEP { my ( $self, $plugin, $obj ) = @_; # Standards entry points - foreach my $sub ( - qw(beforeAuth betweenAuthAndData afterData authCancel forAuthUser beforeLogout) - ) - { + foreach my $sub (@entryPoints) { if ( $obj->can($sub) ) { $self->logger->debug(" Found $sub entry point:"); if ( my $callback = $obj->$sub ) { @@ -412,8 +403,8 @@ sub fail { sub displayError { my ( $self, $req ) = @_; - return $self->sendError( $req, 'Portal error, contact your administrator', - 500 ); + return $self->sendError( $req, + 'Portal error, contact your administrator', 500 ); } 1; 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 df44a36ea..92f33e712 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Issuer.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Issuer.pm @@ -102,6 +102,8 @@ sub _redirect { @{ $self->p->betweenAuthAndData }, $self->p->sessionData, @{ $self->p->afterData }, + $self->p->validSession, + @{ $self->p->endAuth }, ( $restore ? sub { diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Plugin.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Plugin.pm index e94c03c67..9512940bc 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Plugin.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Plugin.pm @@ -161,6 +161,8 @@ setting C provisionning =item C: method called after C provisionning I<(macros, groups,...)> +=item C: method called when session is validated (after cookie build) + =item C: method called when user click on "cancel" during auth process diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm index a67b57882..dca072079 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm @@ -21,7 +21,11 @@ sub authProcess { qw(extractFormInfo getUser authenticate) } sub sessionData { qw(setAuthSessionInfo setSessionInfo setMacros setGroups setPersistentSessionInfo - setLocalGroups store secondFactor storeHistory buildCookie); + setLocalGroups store secondFactor); +} + +sub validSession { + qw(storeHistory buildCookie); } # RESPONSE HANDLER @@ -88,9 +92,10 @@ sub login { return $self->do( $req, [ - 'controlUrl', @{ $self->beforeAuth }, - $self->authProcess, @{ $self->betweenAuthAndData }, - $self->sessionData, @{ $self->afterData }, + 'controlUrl', @{ $self->beforeAuth }, + $self->authProcess, @{ $self->betweenAuthAndData }, + $self->sessionData, @{ $self->afterData }, + $self->validSession, @{ $self->endAuth }, ] ); } @@ -103,7 +108,8 @@ sub postLogin { 'restoreArgs', 'controlUrl', @{ $self->beforeAuth }, $self->authProcess, @{ $self->betweenAuthAndData }, $self->sessionData, - @{ $self->afterData }, + @{ $self->afterData }, $self->validSession, + @{ $self->endAuth }, ] ); } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/SecondFactor.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/SecondFactor.pm index d10ef08b8..2bb27375d 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/SecondFactor.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/SecondFactor.pm @@ -95,7 +95,7 @@ sub _verify { if ( my $l = $self->conf->{ $self->prefix . '2fAuthnLevel' } ) { $self->p->updateSession( $req, { authenticationLevel => $l } ); } - return $self->p->do( $req, [ @{ $self->p->afterData }, sub { PE_OK } ] ); + return $self->p->do( $req, [ @{ $self->p->endAuth }, sub { PE_OK } ] ); } 1; diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CDA.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CDA.pm index d0a441544..de634fff2 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CDA.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CDA.pm @@ -13,7 +13,7 @@ extends 'Lemonldap::NG::Common::Module'; # INTERFACE -use constant afterData => 'changeUrldc'; +use constant endAuth => 'changeUrldc'; use constant forAuthUser => 'changeUrldc'; sub init { 1 } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckState.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckState.pm index c6f49a80b..71c0b8697 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckState.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckState.pm @@ -53,9 +53,9 @@ sub check { 'authenticate', @{ $self->p->betweenAuthAndData }, qw( setAuthSessionInfo setSessionInfo setMacros setGroups - setPersistentSessionInfo setLocalGroups store secondFactor - storeHistory), - @{ $self->p->afterData } + setPersistentSessionInfo setLocalGroups store secondFactor), + @{ $self->p->afterData }, 'storeHistory', + @{ $self->p->endAuth } ] ); if ( $res = $self->p->process( $req, ) ) { diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/History.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/History.pm index e5e6364fc..6890ec60c 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/History.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/History.pm @@ -11,7 +11,7 @@ extends 'Lemonldap::NG::Portal::Main::Plugin', # INITIALIZATION -sub afterData { 'run' } +use constant endAuth => 'run'; sub init { 1 } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/Notifications.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/Notifications.pm index 9db7b8d53..37c2b4181 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/Notifications.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/Notifications.pm @@ -26,7 +26,7 @@ extends 'Lemonldap::NG::Portal::Main::Plugin'; # INTERFACE # Declare additional process steps -sub afterData { 'checkNotifDuringAuth' } +use constant endAuth => 'checkNotifDuringAuth'; # For now, notifications are done only during authentication process #sub forAuthUser { 'checkNotifForAuthUser' } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/RESTServer.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/RESTServer.pm index 6093673e1..bc484ba8a 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/RESTServer.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/RESTServer.pm @@ -238,6 +238,8 @@ sub newAuthSession { @{ $self->p->betweenAuthAndData }, $self->p->sessionData, @{ $self->p->afterData }, + $self->p->validSession, + @{ $self->p->endAuth }, ] ); $req->{error} = $self->p->process($req); diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/SOAPServer.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/SOAPServer.pm index 259a21a5b..7f0aab229 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/SOAPServer.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/SOAPServer.pm @@ -180,6 +180,8 @@ sub getCookies { @{ $self->p->betweenAuthAndData }, $self->p->sessionData, @{ $self->p->afterData }, + $self->p->validSession, + @{ $self->p->endAuth }, ] ); $req->{error} = $self->p->process($req); diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/SingleSession.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/SingleSession.pm index 8e154d09a..71fb40131 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/SingleSession.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/SingleSession.pm @@ -9,7 +9,7 @@ our $VERSION = '2.0.0'; extends 'Lemonldap::NG::Portal::Main::Plugin', 'Lemonldap::NG::Portal::Lib::OtherSessions'; -sub afterData { 'run' } +use constant endAuth => 'run'; sub init { 1 } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/StayConnected.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/StayConnected.pm index 69b597188..8796275f8 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/StayConnected.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/StayConnected.pm @@ -16,7 +16,7 @@ extends 'Lemonldap::NG::Portal::Main::Plugin'; # INTERFACE -use constant afterData => 'newDevice'; +use constant endAuth => 'newDevice'; use constant beforeAuth => 'check'; diff --git a/lemonldap-ng-portal/t/61-GrantSession.t b/lemonldap-ng-portal/t/61-GrantSession.t index 19401fad1..6f1db5bb7 100644 --- a/lemonldap-ng-portal/t/61-GrantSession.t +++ b/lemonldap-ng-portal/t/61-GrantSession.t @@ -31,6 +31,10 @@ ok( count(1); expectReject($res); +my $c = getCookies($res); +ok( not(%$c), 'No cookie' ); +count(1); + &Lemonldap::NG::Handler::Main::cfgNum( 0, 0 ); $client = LLNG::Manager::Test->new( {