Update OIDC logout flow

If an error occurs in AtoM after successful authentication with the OIDC endpoint, end the AtoM session and redirect to the standard AtoM 'Unauthorized' message template. Previously, errors that occurred in AtoM after successful authentication with the OIDC endpoint would fail silently redirecting the user to the AtoM home page in an unauthenticated state.
artefactual · Jan 24, 2025 · a3d7628 · a3d7628
1 parent 9a278dd
commit a3d7628
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 9 deletions.
diff --git a/plugins/arOidcPlugin/lib/oidcUser.class.php b/plugins/arOidcPlugin/lib/oidcUser.class.php
@@ -111,7 +111,6 @@ public function authenticate($username = null, $password = null): bool
             $userMatchingSource = $this->getProviderConfigValue('user_matching_source', '');
             if (!arOidc::validateUserMatchingSource($userMatchingSource)) {
                 $this->logger->err('OIDC user matching source is configured but is not set properly. Unable to match OIDC users to AtoM users.');
-                $this->logout();
 
                 return $authenticated;
             }
@@ -133,7 +132,6 @@ public function authenticate($username = null, $password = null): bool
             $autoCreateUser = $this->getProviderConfigValue('auto_create_atom_user', true);
             if (!is_bool($autoCreateUser)) {
                 $this->logger->err('OIDC auto_create_atom_user is configured but is not set properly - value should be of type bool. Unable to match OIDC users to AtoM users.');
-                $this->logout();
 
                 return $authenticated;
             }
@@ -149,15 +147,13 @@ public function authenticate($username = null, $password = null): bool
             // If user is null and $autoCreateUser is true, then something failed.
             if (null === $user && $autoCreateUser) {
                 $this->logger->err('OIDC authentication succeeded but unable to find or create user in AtoM.');
-                $this->logout();
 
                 return $authenticated;
             }
 
             // If user is null and $autoCreateUser is false, then user has not been previously created or matching failed.
             if (null === $user && !$autoCreateUser) {
                 $this->logger->err('OIDC authentication succeeded but user not found and auto_create_atom_user is set to false.');
-                $this->logout();
 
                 return $authenticated;
             }
@@ -168,7 +164,6 @@ public function authenticate($username = null, $password = null): bool
             $setGroupsFromClaims = $this->getProviderConfigValue('set_groups_from_attributes', false);
             if (!is_bool($setGroupsFromClaims)) {
                 $this->logger->err('OIDC set_groups_from_attributes is configured but is not set properly - value should be of type bool. Unable to complete authentication.');
-                $this->logout();
 
                 return $authenticated;
             }
@@ -282,13 +277,15 @@ public function isAuthenticated(): bool
     /**
      * Logout from AtoM and the OIDC server.
      */
-    public function logout(): void
+    public function logout($sendOidcLogout = false): void
     {
+        // Clean up AtoM session.
         $idToken = $this->getAttribute('oidc-token', null);
         $this->unsetAttributes();
         $this->signOut();
 
-        if (true == $this->getProviderConfigValue('send_oidc_logout', false) && !empty($idToken)) {
+        // Clean up OIDC session.
+        if (!empty($idToken) && $sendOidcLogout) {
             $logoutRedirectUrl = sfConfig::get('app_oidc_logout_redirect_url', '');
             if (empty($logoutRedirectUrl)) {
                 $logoutRedirectUrl = null;

diff --git a/plugins/arOidcPlugin/modules/oidc/actions/loginAction.class.php b/plugins/arOidcPlugin/modules/oidc/actions/loginAction.class.php
@@ -39,7 +39,12 @@ public function execute($request)
                 $this->context->user->validateProviderId($providerId, true);
             }
 
-            $this->context->user->authenticate();
+            $result = $this->context->user->authenticate();
+            if ($result == false) {
+                $this->context->user->logout(false);
+
+                $this->redirect('admin/secure');
+            }
         }
 
         // Redirect to module/action the user was trying to reach before being redirected

diff --git a/plugins/arOidcPlugin/modules/oidc/actions/logoutAction.class.php b/plugins/arOidcPlugin/modules/oidc/actions/logoutAction.class.php
@@ -21,7 +21,7 @@ class OidcLogoutAction extends sfAction
 {
     public function execute($request)
     {
-        $this->getUser()->logout();
+        $this->getUser()->logout($this->getUser()->getProviderConfigValue('send_oidc_logout', false));
         $this->redirect('@homepage');
     }
 }