From 549658fa72f46128afb2bbfc12541dd451d48925 Mon Sep 17 00:00:00 2001 From: Christophe Maudoux Date: Tue, 25 Feb 2020 10:04:30 +0100 Subject: [PATCH] Compute session with real and spoofed attributes & Improve unit test (#2104) --- .../Lemonldap/NG/Portal/Plugins/CheckUser.pm | 37 ++++++++++++------- .../NG/Portal/Plugins/Impersonation.pm | 17 ++------- ...CheckUser-with-Impersonation-and-Macros.t} | 15 +++++++- 3 files changed, 40 insertions(+), 29 deletions(-) rename lemonldap-ng-portal/t/{67-CheckUser-with-Impersonation-and-whatToTrace.t => 67-CheckUser-with-Impersonation-and-Macros.t} (88%) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckUser.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckUser.pm index 1447f1013..cf1a390c9 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckUser.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckUser.pm @@ -46,7 +46,7 @@ sub persistentAttrs { sub init { my ($self) = @_; my $hd = $self->p->HANDLER; - $self->addAuthRoute( checkuser => 'check', ['POST'] ); + $self->addAuthRoute( checkuser => 'check', ['POST'] ); $self->addAuthRouteWithRedirect( checkuser => 'display', ['GET'] ); # Parse identity rule @@ -69,7 +69,7 @@ sub init { # RUNNING METHOD sub display { - my ( $self, $req ) = @_; + my ( $self, $req ) = @_; my ( $attrs, $array_attrs ) = ( {}, [] ); $self->logger->debug("Display current session data..."); @@ -129,9 +129,7 @@ sub display { sub check { my ( $self, $req ) = @_; my ( $attrs, $array_attrs, $array_hdrs ) = ( {}, [], [] ); - my $msg = my $auth = my $compute = ''; - my $authLevel = $req->userData->{authenticationLevel}; - my $authMode = $req->userData->{_auth}; + my $msg = my $auth = my $compute = ''; # Check token if ( $self->ottRule->( $req, {} ) ) { @@ -252,16 +250,24 @@ sub check { unless $self->conf->{checkUserDisplayPersistentInfo}; if ($compute) { - $msg = 'checkUserComputeSession'; - $attrs->{authenticationLevel} = $authLevel; - $attrs->{_auth} = $authMode; - + $msg = 'checkUserComputeSession'; if ( $self->conf->{impersonationRule} ) { $self->logger->debug("Map real attributes..."); my %realAttrs = map { ( "$self->{conf}->{impersonationPrefix}$_" => $attrs->{$_} ) } keys %$attrs; $attrs = { %$attrs, %realAttrs }; + + # Compute groups and macros with real and spoofed attributes + $self->logger->debug( + "Compute groups and macros with real and spoofed attributes" + ); + $req->sessionInfo($attrs); + $req->steps( [ $self->p->groupsAndMacros, 'setLocalGroups' ] ); + if ( my $error = $self->p->process($req) ) { + $self->logger->debug("Process returned error: $error"); + return $req->error($error); + } } } @@ -365,7 +371,10 @@ sub _userData { my ( $self, $req ) = @_; # Compute session - my $steps = [ 'getUser', 'setSessionInfo', $self->p->groupsAndMacros, ]; + my $steps = [ + 'getUser', 'setAuthSessionInfo', + 'setSessionInfo', $self->p->groupsAndMacros, + ]; $self->conf->{checkUserDisplayPersistentInfo} ? push @$steps, 'setPersistentSessionInfo', 'setLocalGroups' : push @$steps, 'setLocalGroups'; @@ -378,7 +387,7 @@ sub _userData { . ")" ); } $self->logger->debug("Process returned error: $error"); - return $req->error($error); + return $req->error(PE_BADCREDENTIALS); } unless ( defined $req->sessionInfo->{uid} ) { @@ -448,7 +457,7 @@ sub _splitAttributes { if ( $element->{key} eq 'groups' ) { $self->logger->debug('Key "groups" found'); my $separator = $self->{conf}->{multiValuesSeparator}; - my @tmp = split /\Q$separator/, $element->{value}; + my @tmp = split /\Q$separator/, $element->{value}; $grps = [ map { { value => $_ } } sort @tmp ]; next; } @@ -491,8 +500,8 @@ sub _splitAttributes { sub _removePersistentAttributes { my ( $self, $attrs ) = @_; - my $regex = join '|', split /\s+/, $self->persistentAttrs; - my @keys = grep /$regex/, keys %$attrs; + my $regex = join '|', split /\s+/, $self->persistentAttrs; + my @keys = grep /$regex/, keys %$attrs; $self->logger->debug("Remove persistent session attributes"); delete @$attrs{@keys}; diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/Impersonation.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/Impersonation.pm index f6a7090af..40a9782dc 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/Impersonation.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/Impersonation.pm @@ -115,14 +115,6 @@ sub run { } } - # Update spoof session - $self->logger->debug("Populating spoof session..."); - foreach (qw (_auth _userDB authenticationLevel)) { - $self->logger->debug("Processing $_..."); - $spoofSession->{$_} = - $realSession->{"$self->{conf}->{impersonationPrefix}$_"}; - } - # Merging SSO Groups and hGroups & dedup $spoofSession->{groups} ||= ''; $spoofSession->{hGroups} ||= {}; @@ -189,12 +181,11 @@ sub _userData { my $raz = 0; # Compute Macros and Groups with real and spoof sessions - $req->{sessionInfo} = {%$realSession}; - - # Search user in database + $req->sessionInfo($realSession); $req->steps( [ - 'getUser', 'setSessionInfo', - $self->p->groupsAndMacros, 'setLocalGroups' + 'getUser', 'setAuthSessionInfo', + 'setSessionInfo', $self->p->groupsAndMacros, + 'setLocalGroups' ] ); if ( my $error = $self->p->process($req) ) { diff --git a/lemonldap-ng-portal/t/67-CheckUser-with-Impersonation-and-whatToTrace.t b/lemonldap-ng-portal/t/67-CheckUser-with-Impersonation-and-Macros.t similarity index 88% rename from lemonldap-ng-portal/t/67-CheckUser-with-Impersonation-and-whatToTrace.t rename to lemonldap-ng-portal/t/67-CheckUser-with-Impersonation-and-Macros.t index a34e5f6e0..ea1545865 100644 --- a/lemonldap-ng-portal/t/67-CheckUser-with-Impersonation-and-whatToTrace.t +++ b/lemonldap-ng-portal/t/67-CheckUser-with-Impersonation-and-Macros.t @@ -26,6 +26,7 @@ my $client = LLNG::Manager::Test->new( { userControl => '^[\w\.\-/\s]+$', whatToTrace => '_whatToTrace', macros => { + authLevel => '"Macro_$authenticationLevel"', _whatToTrace => '$real__user ? "$_user / $real__user" : "$_user / $_user"', }, @@ -130,7 +131,12 @@ ok( $res->[2]->[0] =~ m%_whatToTrace%, or explain( $res->[2]->[0], 'Macro Key _whatToTrace' ); ok( $res->[2]->[0] =~ m%uid%, 'Found uid' ) or explain( $res->[2]->[0], 'Attribute Value uid' ); -count(8); +ok( $res->[2]->[0] =~ m%Macro_1%, 'Found uid' ) + or explain( $res->[2]->[0], 'Attribute Value uid' ); +ok( my $nbr = ( $res->[2]->[0] =~ s%Macro_1%%g ), + 'Found two macros' ) + or explain( $res->[2]->[0], 'Macros not well computed' ); +count(10); ok( $res = $client->_get( @@ -161,7 +167,12 @@ ok( m%
%, 'Found trspan="allowed"' ) or explain( $res->[2]->[0], 'trspan="allowed"' ); -count(3); +ok( $res->[2]->[0] =~ m%Macro_1%, 'Found uid' ) + or explain( $res->[2]->[0], 'Attribute Value uid' ); +ok( my $nbr = ( $res->[2]->[0] =~ s%Macro_1%%g ), + 'Found two well computed macros' ) + or explain( $res->[2]->[0], 'Macros not well computed' ); +count(5); $client->logout($id); clean_sessions();