-
-
Notifications
You must be signed in to change notification settings - Fork 25
feat: front-channel logout #216
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
base: main
Are you sure you want to change the base?
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.
Hi @cdanis
thx for this PR 👍
I think you're still working on it, but I thought I'd start by adding my two cents 😂
| //Scopes: []string{"openid", "profile", "email"}, | ||
| CallbackUri: "/oidc/callback", | ||
| LogoutUri: "/logout", | ||
| FrontChannelLogoutUri: "/frontchannel-logout", |
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.
Should we support environment variable expansion?
| toa.handleLogout(rw, req, session) | ||
| return | ||
| } | ||
| if toa.Config.FrontChannelLogoutUri != "" && strings.HasPrefix(req.RequestURI, toa.Config.FrontChannelLogoutUri) { |
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.
I think I would prefer to move this before the if err == nil && session != nil { and then pass in the "optional" session and claims.
Then we can handle both cases (with and without session) within the handleFrontChannelLogout function and we have everything in one place.
Eg:
session, updateSession, claims, err := toa.getSessionForRequest(req)
// Handle front-channel logout
if err == nil && strings.HasPrefix(req.RequestURI, toa.Config.FrontChannelLogoutUri) {
toa.handleFrontChannelLogout(rw, req, session, claims)
return
}What do you think?
|
I'll get back to this soon I hope 😅 but yes, not done yet. Aside from review comments, I also wanted to generate formatted error responses, and do some more testing. |
Closes #197