Skip to content

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

Closed
wants to merge 9 commits into from

Conversation

esifea
Copy link

@esifea esifea commented Mar 20, 2025

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).

@cdr-bot cdr-bot bot added the community Pull Requests and issues created by the community. label Mar 20, 2025
Copy link

github-actions bot commented Mar 20, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@esifea
Copy link
Author

esifea commented Mar 20, 2025

I have read the CLA Document and I hereby sign the CLA

cdrci2 added a commit to coder/cla that referenced this pull request Mar 20, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 != "" {

@matifali
Copy link
Member

@Emyrk can you please also check the #13904 and #15575 issues to decide if it's related to a solution to those.
Thanks.

@matifali matifali added the api Area: HTTP API label Mar 28, 2025
@Emyrk
Copy link
Member

Emyrk commented Apr 2, 2025

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).

I agree. Generally OIDC urls are "public"

@esifea
Copy link
Author

esifea commented Apr 2, 2025

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).

I agree. Generally OIDC urls are "public"

Thank you for your opinion!
Feel free to share any additional suggestions if further modifications are needed.

Copy link
Member

@Emyrk Emyrk left a 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.

  1. User hits Logout on Coder
  2. Redirected to OIDC page's /logout
  3. User redirected to Coder, presumably now unauthenticated

revocation_endpoint Oauth2

Spec

OIDC is a superset of OIDC. Some providers (Google Workspaces) use this in the OIDC discovery endpoint.

  1. User hits Logout on Coder
  2. Coder sends the revocation request to OIDC
  3. 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:

coreos/go-oidc#246 (comment)

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

Comment on lines 459 to 482
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");
}
};
Copy link
Member

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.

Copy link
Author

@esifea esifea Apr 3, 2025

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.

@esifea
Copy link
Author

esifea commented Apr 3, 2025

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:

coreos/go-oidc#246 (comment)

Thank you for your suggestion.
If we can retrieve the logout endpoint dynamically, we won't need to configure a hard-coded endpoint in the deployment settings.
I'll investigate these options further.

@esifea esifea requested a review from Emyrk April 3, 2025 11:23
@esifea
Copy link
Author

esifea commented Apr 3, 2025

@Emyrk
I’ve applied the reviews.

  • Mainly, the end_session_endpoint and revocation_endpoint are now fetched from the OIDC discovery document.

    • As a result, oidc-logout-endpoint has been removed from the deployment values.
  • Additionally, OAuth token revocation is now implemented just before the RP-Initiated Logout.

All OAuth (OIDC) related processes are executed only if the appropriate values are properly configured.

Copy link
Member

@Emyrk Emyrk left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Area: HTTP API community Pull Requests and issues created by the community. stale This issue is like stale bread.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants