-
Notifications
You must be signed in to change notification settings - Fork 887
feat: add API endpoint for retrieving OIDC logout URL #17015
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
Conversation
All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
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.
Pull Request Overview
This PR introduces a new API endpoint for retrieving the full OIDC logout URL so that the frontend can redirect users to end their OIDC session in addition to the server-side logout. Key changes include:
- Updates to type definitions and API schemas to include logout endpoint and redirect URI information.
- Implementation of a new logout flow in the API client that calls the new endpoint and redirects the browser.
- Updates to CLI, API documentation, and deployment configuration to support the new OIDC logout settings.
Reviewed Changes
Copilot reviewed 11 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
site/src/api/typesGenerated.ts | Added logout_endpoint and logout_redirect_uri to OIDCConfig and a new OIDCLogoutResponse interface. |
site/src/api/api.ts | Updated logout method to fetch the OIDC logout URL and handle a two-step logout. |
docs/reference/cli/server.md | Documented new CLI flags for logout endpoint configuration. |
docs/reference/api/users.md | Added API documentation and code samples for the new OIDC logout URL endpoint. |
docs/reference/api/schemas.md | Added schema for codersdk.OIDCLogoutResponse and updated OIDC-related properties. |
docs/reference/api/general.md | Updated general API reference documentation with OIDC logout properties. |
codersdk/users.go | Introduced a new OIDCLogoutResponse struct for user logout responses. |
codersdk/deployment.go | Added new deployment options for OIDC logout endpoint and logout redirect URI. |
coderd/userauth.go | Implemented the HTTP handler for constructing and returning the OIDC logout URL. |
coderd/coderd.go | Added a new route to wire the OIDC logout endpoint in the router. |
coderd/apidoc/docs.go | Updated API docs to include the new OIDC logout URL endpoint. |
Files not reviewed (4)
- cli/testdata/coder_server_--help.golden: Language not supported
- cli/testdata/server-config.yaml.golden: Language not supported
- coderd/apidoc/swagger.json: Language not supported
- enterprise/cli/testdata/coder_server_--help.golden: Language not supported
Comments suppressed due to low confidence (1)
coderd/userauth.go:830
- [nitpick] Consider using a parameter name that aligns with the documented 'logout_redirect_uri' (or a standard like 'post_logout_redirect_uri') instead of 'logout_uri', to avoid potential confusion and ensure consistency with OIDC provider expectations.
if logoutURI != "" {
I agree. Generally OIDC urls are "public" |
Thank you for your opinion! |
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 we can solve this via the OIDC discovery url, and not use a hard coded url. I also see there might be different ways to logout.
We should probably implement this via the discovery endpoint.
end_session_endpoint
w/ OIDC spec
Spec)
An RP requests that the OP log out the End-User by redirecting the End-User's User Agent to the OP's Logout Endpoint.
I think this is what was being implemented in this PR.
- User hits
Logout
on Coder - Redirected to OIDC page's
/logout
- User redirected to Coder, presumably now unauthenticated
revocation_endpoint
Oauth2
OIDC is a superset of OIDC. Some providers (Google Workspaces) use this in the OIDC discovery endpoint.
- User hits
Logout
on Coder - Coder sends the revocation request to OIDC
- Coder logs out the user, and their OIDC session is now invalid as well
So there is more than 1 way to revoke an oauth/OIDC token. And the OIDC provider should provide this via discovery. I do not think we have to configure this in the deployment config.
We can pull out these claims manually outside the OIDC package we use today:
Ignore, but extra notes.
For completeness I also found a logout mechanism that allows the IdP to log out the user? Coder should logout a user with an invalid token, so I think we do not need this.
https://openid.net/specs/openid-connect-backchannel-1_0.html#LogoutToken
logout = async (): Promise<void> => { | ||
return this.axios.post("/api/v2/users/logout"); | ||
try { | ||
// Fetch the stored ID token from the backend | ||
const response = await this.axios.get("/api/v2/users/oidc-logout"); | ||
|
||
// Redirect to OIDC logout after Coder logout | ||
if (response.data.oidc_logout_url) { | ||
// Coder session logout | ||
await this.axios.post("/api/v2/users/logout"); | ||
|
||
// OIDC logout | ||
window.location.href = response.data.oidc_logout_url; | ||
} else { | ||
// Redirect normally if no token is available | ||
console.warn( | ||
"No ID token found, continuing logout without OIDC logout.", | ||
); | ||
return this.axios.post("/api/v2/users/logout"); | ||
} | ||
} catch (error) { | ||
console.error("Logout failed", error); | ||
return this.axios.post("/api/v2/users/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.
Is this logout used by both OIDC, github, and password auth?
We should not even try this if this logout method is not configured or using password based auth.
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.
Is this logout used by both OIDC, github, and password auth?
We should not even try this if this logout method is not configured or using password based auth.
The code is intended to perform an OIDC logout only if the appropriate OIDC configuration is present.
The backend checks the user's login type and returns an OIDC logout URL only if the user is authenticated via OIDC.
If the login type is not OIDC (such as for GitHub or password-based auth), the backend will not attempt an OIDC logout, and a standard logout (/api/v2/users/logout
) flow will be used instead.
I wonder that you think it's better checking login type on frontend.
Thank you for your suggestion. |
Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
@Emyrk
All OAuth (OIDC) related processes are executed only if the appropriate values are properly configured. |
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 am testing this with auth0 and having some issues. I need to spend some more time on this.
PR description
Introduces a new API endpoint
/api/v2/users/oidc-logout
that constructs and returns the full OIDC logout URL.The endpoint retrieves necessary OIDC configuration (e.g., logout endpoint, client ID, logout redirect URI) from environment variables.
This URL is enabling the frontend to redirect the user for complete session logout on the OIDC provider side.
Motivation
Currently, when a user signs out, only the Coder's session is cleared, but the OIDC provider's session remains active.
This means that a user can immediately sign in again with their OIDC account without truly logging out of the provider. This behavior can be problematic in scenarios where a complete logout is required such as changing user.
With this new endpoint (
/api/v2/users/oidc-logout
), if the coder server is run with specific arguments to enable OIDC session logout, the full logout URL will support the termination of the OIDC session.Concerns
This PR intentionally does not modify existing CSP or CORS policies.
Instead, it returns the full URL so that the frontend can perform the redirect.
While this avoids cross-origin issues, it means that the complete URL is exposed in the response.
However, because the endpoint is protected and the URL is intended solely for redirection, this design is acceptable for the intended use case (in my opinion).