Problem/Motivation

Error occured on logout in the cases below:

  • Case 1: If other modules that use externalauth (like simplesamlphp_auth) are enabled.
  • Case 2: If failed to load connect client.

Error: Call to a member function label() on bool in Drupal\openid_connect\Controller\OpenIDConnectRedirectController->redirectLogout() (line 324 of /var/www/d9/web/modules/contrib/openid_connect/src/Controller/OpenIDConnectRedirectController.php)

Notice: Undefined variable: end_session_enabled in Drupal\openid_connect\Controller\OpenIDConnectRedirectController->redirectLogout() (line 327 of modules/contrib/openid_connect/src/Controller/OpenIDConnectRedirectController.php).

Steps to reproduce

For case 1:

  1. Install Drupal (9.2.0) with standard profile
  2. Login as admin user
  3. Install OpenID Connect and SimpleSAMLphp Authentication (8.x-3.2)
  4. Enable 'Enable this user to leverage SAML authentication' on user/1/edit (add record to authmap table)
  5. Logout

For case 2:

  1. Install Drupal and OpenID Connect then configure client and user to login
  2. Login as configured user
  3. Remove client entity
  4. Logout

Proposed resolution

  • Exclude mapped users except associated by openid_connect from logout process.
  • Do not use $entity in warning message.
  • Declear $end_session_enabled outside of foreach.

Remaining tasks

Review patch.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#2 3221625-error-occured-on-logout.patch2.42 KBhayashi

Comments

hayashi created an issue. See original summary.

hayashi’s picture

hayashi commented
StatusFileSize
new 3221625-error-occured-on-logout.patch2.42 KB

I created a patch. Please review.

jcnventura’s picture

jcnventura
Primary language English
commented
Status: Active » Needs work 
  1. +++ b/src/Controller/OpenIDConnectRedirectController.php
@@ -289,7 +289,14 @@ class OpenIDConnectRedirectController implements ContainerInjectionInterface, Ac
+      $config = $this->configFactory->get('openid_connect.settings');
+      $end_session_enabled = $config->get('end_session_enabled') ?? FALSE;
+

    There's no need for this change. So why do it? 

  2. +++ b/src/Controller/OpenIDConnectRedirectController.php
@@ -300,13 +307,10 @@ class OpenIDConnectRedirectController implements ContainerInjectionInterface, Ac
-            $config = $this->configFactory->get('openid_connect.settings');
-

    Please revert. 

  3. +++ b/src/Controller/OpenIDConnectRedirectController.php
@@ -300,13 +307,10 @@ class OpenIDConnectRedirectController implements ContainerInjectionInterface, Ac
-            $end_session_enabled = $config->get('end_session_enabled') ?? FALSE;

    Please revert.

hayashi’s picture

hayashi commented
Status: Needs work » Needs review

Those changes are needed to fix below issues caused by undefined $end_session_enabled if $entity is empty.

  1. Notice: Undefined variable: end_session_enabled in Drupal\openid_connect\Controller\OpenIDConnectRedirectController->redirectLogout() (line 323 of modules/contrib/openid_connect/src/Controller/OpenIDConnectRedirectController.php).
  2. Warning message will be shown regardless of configuration:
    if (!$end_session_enabled)(original OpenIDConnectRedirectController.php L323) will be FALSE if `end_session_enabled` configuration is TRUE.)

  • jcnventura committed 35f6f8a on 2.x 
    Issue #3221625 by hayashi, jcnventura: Error occurred on logout
jcnventura’s picture

jcnventura
Primary language English
commented
Status: Needs review » Fixed

Thanks for providing that information. It seems that that entire else block was misplaced at some point. I've now moved it to the correct place.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.