Support to logout from the OAuth2 Server provider

Is this also a problem in the 3.x branch?

Since September 12, 2022 there is an official spec for this.
See https://openid.net/specs/openid-connect-rpinitiated-1_0.html

re-rolled against current 7.x-1.x

Tested with Auth0 and worked.

@colan No, logout is supported in 3.x

@xaris.tsimpouris, thanks for starting this!
And to everyone in the thread that pushed it forward.

I have been trying the latest iteration at #31, and it seems to work OK 👍

I have iterated over it, and added some changes on top, but first, let me reply a bit to previous comments.

The comment at #16 is quite relevant:

By the way, this also means any hook_user_logout() implementations that are executed after this one get skipped entirely. So it's potentially a bigger problem than the session data simply being unavailable to them. @solideogloria is right, this could require adjusting the module weight.

⚠️ In other words, using this patch also means that the site may need to adjust the module weight for this module, so it runs at the end of the set.
There seems to not be a way around it, since a redirect is quite definitive.

Actually the watchdog line in user_logout_current_user() is still called (I see the message in the dblog), so I don't think we're preventing execution of other code...

Re #17: The watchdog message happens before invoking the hooks, hence it will appear at the start.
E.g. if module A and openid_connect are implementing hook_user_logout(), and the openid_connect module gets called first, the A_user_logout() hook will not be run.

Since September 12, 2022 there is an official spec for this.
See https://openid.net/specs/openid-connect-rpinitiated-1_0.html

Re #30: Yes.
Looking into the way this works, and the spec about Relying Party initiated logout linked, I think these changes actually implement it.

On masquerade module support, I added a check to verify if the module is there on the related code, since it may not be there.

I have also added the client identifier as parameter on the redirect, which is part of the optional parameters to pass on the spec, and will add an extra hint to the IdP that will produce an extra check.

Also, I have been trying to avoid the persistence of the token data into the session, but I could not arrive to a working solution.
I even tried to use the internal mechanism for redirect, but I could not make it work, basically the following hunk.
Ideas are welcome about how that may be possible.
In the mean time, I restricted this to only the token_id, since that is the only consumed piece.

    diff --git a/plugins/openid_connect_client/generic/OpenIDConnectClientGeneric.class.php b/plugins/openid_connect_client/generic/OpenIDConnectClientGeneric.class.php
    index 9db59a5..51fff9e 100644
    --- a/plugins/openid_connect_client/generic/OpenIDConnectClientGeneric.class.php
    +++ b/plugins/openid_connect_client/generic/OpenIDConnectClientGeneric.class.php
    @@ -66,7 +66,12 @@ class OpenIDConnectClientGeneric extends OpenIDConnectClientBase {
           'post_logout_redirect_uri' => htmlentities($post_logout_redirect_url),
           'client_id' => $this->getSetting('client_id'),
         ));
    -    drupal_goto($endpoints['end_session'], $url_options);
    +    $_SESSION['openid_connect_destination'] = array(
    +        $endpoints['end_session'],
    +        $url_options,
    +    );
    +    $_SESSION['openid_connect_op'] = 'logout';
    +    drupal_goto(OPENID_CONNECT_REDIRECT_PATH_BASE . '/' . $this->getName());
       }
     
     }

I opened a 2882270-logout-support topic branch and related new MR !107 with the following commits.

• 73288fd Add patch from #2882270-31 by solideogloria, xaris.tsimpouris, romanos, nrogers: Support to logout from the OAuth2 Server provider
• 17462c5 Formatting related changes
• 8bcd65e Link to relying party initiated logout, not to session management logout
• c8b3e9c Pass client identifier on logout request
• f453cce Check for masquerade module before checking its data
• c9edac6 Only store id_token for endSession
• 1f9b430 Trust openid_connect_get_connected_accounts() output
• 248beb3 Skip watchdog and show message on no IdP logout
• 09e48d8 Tweak logic on hook_user_logout() implementation

⚠️ In other words, using this patch also means that the site may need to adjust the module weight for this module, so it runs at the end of the set.

Rather than module weighting, hook_module_implements_alter() is a bit nicer to control hook ordering, in case some hooks need to fire early and others late.

Rather than module weighting, hook_module_implements_alter() is a bit nicer to control hook ordering, in case some hooks need to fire early and others late.

@drumm, that makes sense.

I have added an implementation of that hook moving the new hook_user_logout() to be executed at the end.

New commits on the 2882270-logout-support topic branch and related MR !107:

- 56dfedf Move hook_user_logout implementation to the end of the set

I've done a code only review of the latest MR. It looks in order. I hesitate to mark RTBC since I haven't manually tested it.

Looks good to me too. We will be running this on Drupal.org’s codebase.