Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do an idp logout even when oidc.isAuthenticated is false #640

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tusharpandey13
Copy link
Contributor

@tusharpandey13 tusharpandey13 commented Jan 2, 2025

This PR solves the following issue (this was reported in an ESD):

express-openid-connect does not do IdP logout if no local session is found

The logout() code checks for an existing app session and, if not found, returns:

async logout(params = {})
  
  [...]
  
  if (!req.oidc.isAuthenticated()) {
    debug('end-user already logged out, redirecting to %s', returnURL);
    return res.redirect(returnURL);
  }
  
  [...]
  
  // idp logout comes later
  returnURL = client.endSessionUrl(...)
  res.redirect(returnURL);
}

(code)

The problem with this logic is that there are cases where the application session might be shorter than the Auth0 session. In those cases the IdP logout won’t be requested, but our customers (and their users) might still expect the logout action to terminate the Auth0 session (assuming idpLogout is true).

While the behavior is documented in this sequence diagram, it’s like to go unnoticed, and probably unexpected.

I think we should do the IdP logout if idpLogout is set to true even if isAuthenticated() returns false. We won’t have the id_token_hint to include in the request, but it’s not a mandatory parameter.

@tusharpandey13 tusharpandey13 requested a review from a team as a code owner January 2, 2025 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants