-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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
Authn: Handle logout logic in auth broker #79635
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is truly amazing! Very nice and clean refactor.
I just want to give it a quick test before a final approval.
Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with AzureAD OIDC Logout, SAML logout, Generic OAuthLogout (with no logout url). Everything worked fine :)
What is this feature?
The logout handler previously had a bunch of provider specific logic when performing logout. This part was a bit messy and hard to follow.
I instead added logout functionality to
authn.Service
. The service it self perform all shared logic, revoking session token and generate default redirect url.Some clients (oauth and saml) can be configured with additional logic that should happen. To support this I added a extensions interface that clients can implement
authn.LogoutClient
. If a client implements this interface it will be called for users that has authenticated with it.For now it is only implemented for oauth clients. For these we remove access, refresh and id tokens and if configured create a new redirect url with support for oidc rp initiated logout.
Plan is to also fix enterprise saml client to also implement this interface and remove the TODO added here.
Which issue(s) does this PR fix?:
Fixes https://github.com/grafana/identity-access-team/issues/496
Special notes for your reviewer:
Please check that: