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

Issue #8216 - OpenID Connect RP-Initiated Logout #8286

Merged
merged 6 commits into from
Jul 20, 2022

Conversation

lachlan-roberts
Copy link
Contributor

Issue #8216

see https://openid.net/specs/openid-connect-rpinitiated-1_0.html

  • Redirect back to openid provider when HttpServletRequest.logout() is called.
  • Store the end_session_endpoint in the OpenIdConfiguration.

Comment on lines +184 to +188
if (logoutRedirectPath == null)
{
LOG.warn("redirect path must not be null, defaulting to /");
logoutRedirectPath = "/";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to allow null to be set here and have that mean "do not redirect on logout"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This redirect path is actually for the OpenId provider redirecting back to the webapp, not for the webapp to redirect to the OpenID Provider on logout.

post_logout_redirect_uri
OPTIONAL. URI to which the RP is requesting that the End-User's User Agent be redirected after a logout has been performed. This URI SHOULD use the https scheme and MAY contain port, path, and query parameter components; however, it MAY use the http scheme, provided that the Client Type is confidential, as defined in Section 2.1 of OAuth 2.0 [RFC6749], and provided the OP allows the use of http RP URIs. The URI MAY use an alternate scheme, such as one that is intended to identify a callback into a native application. The value MUST have been previously registered with the OP, either using the post_logout_redirect_uris Registration parameter or via another mechanism. An id_token_hint is also RECOMMENDED when this parameter is included.

So the "do not redirect on logout" setting is actually defined in OpenIdConfiguration as the endSessionEndpoint. But since this is optional we could not include this parameter if it is not explicitly set. Either way I think this does need some javadoc/documentation to explain this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand it is the redirect after the logout redirect. If there is no endSessionEndpoint, you just redirect to the logout redirect, which you are currently insisting is non null.
But you could allow null and define that to mean no redirecting at all on logout, thus giving the old behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I have changed to logic to not redirect back to application if LOGOUT_REDIRECT_PATH is null.

And updated the javadoc of attemptLogoutRedirect to explain how it works.

@lachlan-roberts lachlan-roberts requested a review from gregw July 18, 2022 04:00
@lachlan-roberts lachlan-roberts merged commit b74da7c into jetty-10.0.x Jul 20, 2022
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-8216-openid-logout branch July 20, 2022 04:39
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.

3 participants