Problem/Motivation

The openid_connect_windows_aad module defines `openid_connect_windows_aad_openid_connect_redirect_logout_alter()` and fails to load the client from the passed context variable (which contains a plugin name, not a client id) , in order to do its thing, so tries to call method `getPlugin()` on NULL and errors out.

I don't know if this is a new problem, I just came across it whilst testing the fix for #3577049: Update.php fails with InvalidArgumentException : The configuration property settings.prompt.login doesn't exist

Steps to reproduce

Implement hook_openid_connect_redirect_logout_alter in a module and try to load a client:

  /** @var \Drupal\openid_connect\Entity\OpenIDConnectClientEntity $client */
  $client = OpenIDConnectClientEntity::load($context['client']);

Login. Logout. Oops.

Proposed resolution

Ensure the context passes the client id and not the plugin name.

Remaining tasks

Draw the rest of the owl.

User interface changes

None.

API changes

None, except implement the API as actually documented :-)

Data model changes

Nope.

Issue fork openid_connect-3577436

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

cafuego created an issue. See original summary.

cafuego’s picture

cafuego commented
Title: Incorrect context in hook_redirect_logout » Incorrect context in hook_redirect_logout_alter
cafuego’s picture

cafuego commented

A quick hacky work-around is to add a !empty($client) check top the conditional after trying to load the client in the plugin. Of course, that then means the logout redirect won't get changed if it should be.

cafuego’s picture

cafuego commented

Offhand, it looks like

$clientName = $provider?->id ?? 'unknown'; instead of
$clientName = $provider?->getPlugin()?->getPluginId() ?? 'unknown'; in src/LogoutService.php does the trick.

stefdewa made their first commit to this issue’s fork.

stefdewa opened merge request !191

stefdewa’s picture

stefdewa commented

Added MR, clientName is now the actual client ID and no longer the plugin ID.

stefdewa’s picture

stefdewa commented
Status: Active » Needs review
timmy_cos’s picture

timmy_cos commented

Patch seems to be working for me! FYI I'm fairly sure this issue was introduced in alpha7 with https://www.drupal.org/project/openid_connect/issues/3518252

cafuego’s picture

cafuego commented
Status: Needs review » Reviewed & tested by the community

Works as advertised for me as well.

szeidler’s picture

szeidler commented

I can confirm that the MR fixes the problem. Without the patch you will get a fatal error when the logout redirect is configured.

karimbou’s picture

karimbou commented

We're having an issue on version v1.5 since security patch fix, when trying to login, redirection has prompt=login%20%login in the URL instead of prompt=login
linked to 

public function defaultConfiguration() {
    return [
      'client_id' => '',
      'client_secret' => '',
      'prompt' => ['login' => 'login'],
    ];
  }
cafuego’s picture

cafuego commented

@karimbou That is because of #3577049: Update.php fails with InvalidArgumentException : The configuration property settings.prompt.login doesn't exist and it is fixed in the 3.x branch, at least.

You need to apply the patch from https://git.drupalcode.org/project/openid_connect/-/merge_requests/185/d...

pfrilling made their first commit to this issue’s fork.

pfrilling’s picture

pfrilling
Location Minster, OH
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +Chicago2026

Thanks all for the patches! Merge train has started

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • pfrilling committed aa194f23 on 3.x authored by stefdewa 
    fix: #3577436 Incorrect context in hook_redirect_logout_alter

By:...

Status: Fixed » Closed (fixed)

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