From 6bffc22127a09e3b8126c8ce892c87cb4586a42a Mon Sep 17 00:00:00 2001 From: shpark Date: Thu, 20 Mar 2025 08:28:24 +0000 Subject: [PATCH 1/9] feat: support OIDC logout with Coder logout --- coderd/coderd.go | 1 + coderd/userauth.go | 90 ++++++++++++++++++++++++++++++++++ codersdk/deployment.go | 22 +++++++++ site/src/api/api.ts | 21 +++++++- site/src/api/typesGenerated.ts | 2 + 5 files changed, 135 insertions(+), 1 deletion(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 190a043a92ac9..e0c4d5b5e918c 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1136,6 +1136,7 @@ func New(options *Options) *API { r.Post("/", api.postUser) r.Get("/", api.users) r.Post("/logout", api.postLogout) + r.Get("/oidc-logout", api.userOIDCLogoutURL) // These routes query information about site wide roles. r.Route("/roles", func(r chi.Router) { r.Get("/", api.AssignableSiteRoles) diff --git a/coderd/userauth.go b/coderd/userauth.go index 63f54f6d157ff..0406a3b613450 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/mail" + "net/url" "sort" "strconv" "strings" @@ -743,6 +744,95 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { }) } +// Returns the OIDC logout URL. +// +// @Summary Returns URL for the OIDC logout +// @ID user-oidc-logout +// @Security CoderSessionToken +// @Produce json +// @Tags Users +// @Success 200 {object} map[string]string "Returns a map containing the OIDC logout URL" +// @Router /users/oidc-logout [get] +func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Get logged-in user + apiKey := httpmw.APIKey(r) + user, err := api.Database.GetUserByID(ctx, apiKey.UserID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{ + Message: "Failed to retrieve user information.", + }) + return + } + + // Retrieve the user's OAuthAccessToken for logout + // nolint:gocritic // We only can get user link by user ID and login type with the system auth. + link, err := api.Database.GetUserLinkByUserIDLoginType(dbauthz.AsSystemRestricted(ctx), + database.GetUserLinkByUserIDLoginTypeParams{ + UserID: user.ID, + LoginType: user.LoginType, + }) + if err != nil { + api.Logger.Error(ctx, "failed to retrieve OIDC user link", "error", err) + if xerrors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "No OIDC link found for this user.", + }) + } else { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve user authentication data.", + }) + } + return + } + + rawIDToken := link.OAuthAccessToken + + // Retrieve OIDC environment variables + dvOIDC := api.DeploymentValues.OIDC + oidcEndpoint := dvOIDC.LogoutEndpoint.Value() + oidcClientID := dvOIDC.ClientID.Value() + logoutURI := dvOIDC.LogoutRedirectURI.Value() + + if oidcEndpoint == "" { + api.Logger.Error(ctx, "missing OIDC logout endpoint") + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "OIDC configuration is missing.", + }) + return + } + + // Construct OIDC Logout URL + logoutURL, err := url.Parse(oidcEndpoint) + if err != nil { + api.Logger.Error(ctx, "failed to parse OIDC endpoint", "error", err) + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Invalid OIDC endpoint.", + }) + return + } + + // Build parameters + q := url.Values{} + + if oidcClientID != "" { + q.Set("client_id", oidcClientID) + } + if rawIDToken != "" { + q.Set("id_token_hint", rawIDToken) + } + if logoutURI != "" { + q.Set("logout_uri", logoutURI) + } + + logoutURL.RawQuery = q.Encode() + + // Return full logout URL + response := map[string]string{"oidc_logout_url": logoutURL.String()} + httpapi.Write(ctx, rw, http.StatusOK, response) +} + // GithubOAuth2Team represents a team scoped to an organization. type GithubOAuth2Team struct { Organization string diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 428ebac4944f5..945fd49d6b852 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -555,6 +555,8 @@ type OIDCConfig struct { IconURL serpent.URL `json:"icon_url" typescript:",notnull"` SignupsDisabledText serpent.String `json:"signups_disabled_text" typescript:",notnull"` SkipIssuerChecks serpent.Bool `json:"skip_issuer_checks" typescript:",notnull"` + LogoutEndpoint serpent.String `json:"logout_endpoint" typescript:",notnull"` + LogoutRedirectURI serpent.String `json:"logout_redirect_uri" typescript:",notnull"` } type TelemetryConfig struct { @@ -1966,6 +1968,26 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Group: &deploymentGroupOIDC, YAML: "dangerousSkipIssuerChecks", }, + { + Name: "OIDC logout endpoint", + Description: "OIDC endpoint for logout.", + Flag: "logout-endpoint", + Env: "CODER_OIDC_LOGOUT_ENDPOINT", + Default: "", + Value: &c.OIDC.LogoutEndpoint, + Group: &deploymentGroupOIDC, + YAML: "logoutEndpoint", + }, + { + Name: "OIDC logout redirect URI", + Description: "OIDC redirect URI after logout.", + Flag: "logout-redirect-uri", + Env: "CODER_OIDC_LOGOUT_URI", + Default: "", + Value: &c.OIDC.LogoutRedirectURI, + Group: &deploymentGroupOIDC, + YAML: "logoutRedirectURI", + }, // Telemetry settings telemetryEnable, { diff --git a/site/src/api/api.ts b/site/src/api/api.ts index b042735357ab0..81beed2e3ee1b 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -457,7 +457,26 @@ class ApiMethods { }; logout = async (): Promise => { - 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"); + } }; getAuthenticatedUser = async () => { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 1e9b471ad46f4..a6e5cad603b08 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1474,6 +1474,8 @@ export interface OIDCConfig { readonly icon_url: string; readonly signups_disabled_text: string; readonly skip_issuer_checks: boolean; + readonly logout_endpoint: string; + readonly logout_redirect_uri: string; } // From codersdk/organizations.go From 39a35ef0c6a001cc13829dc7d24d7e467d519633 Mon Sep 17 00:00:00 2001 From: shpark Date: Thu, 20 Mar 2025 14:40:35 +0000 Subject: [PATCH 2/9] feat(coderd): add OIDCLogoutResponse --- codersdk/users.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codersdk/users.go b/codersdk/users.go index 31854731a0ae1..ac7977b6c7d74 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -311,6 +311,11 @@ type UserParameter struct { Value string `json:"value"` } +// OIDCLogoutResponse represents the response for an OIDC logout request +type OIDCLogoutResponse struct { + URL string `json:"oidc_logout_url"` +} + // UserAutofillParameters returns all recently used parameters for the given user. func (c *Client) UserAutofillParameters(ctx context.Context, user string, templateID uuid.UUID) ([]UserParameter, error) { res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/autofill-parameters?template_id=%s", user, templateID), nil) From c2c3ddfd8559220c3e3c72bf9dce46cb7c7d2924 Mon Sep 17 00:00:00 2001 From: shpark Date: Thu, 20 Mar 2025 14:43:39 +0000 Subject: [PATCH 3/9] refactor(coderd): apply codersdk.OIDCLogoutResponse at success --- coderd/userauth.go | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index 0406a3b613450..4dd35495ca4a8 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -751,7 +751,7 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { // @Security CoderSessionToken // @Produce json // @Tags Users -// @Success 200 {object} map[string]string "Returns a map containing the OIDC logout URL" +// @Success 200 {object} codersdk.OIDCLogoutResponse "Returns a map containing the OIDC logout URL" // @Router /users/oidc-logout [get] func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -766,6 +766,11 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { return } + logger := api.Logger.Named(userAuthLoggerName) + + // Default response: empty URL if OIDC logout is not supported + response := codersdk.OIDCLogoutResponse{URL: ""} + // Retrieve the user's OAuthAccessToken for logout // nolint:gocritic // We only can get user link by user ID and login type with the system auth. link, err := api.Database.GetUserLinkByUserIDLoginType(dbauthz.AsSystemRestricted(ctx), @@ -774,16 +779,17 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { LoginType: user.LoginType, }) if err != nil { - api.Logger.Error(ctx, "failed to retrieve OIDC user link", "error", err) if xerrors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: "No OIDC link found for this user.", - }) - } else { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to retrieve user authentication data.", - }) + logger.Warn(ctx, "no OIDC link found for this user") + httpapi.Write(ctx, rw, http.StatusOK, response) + return } + + logger.Error(ctx, "failed to retrieve OIDC user link", "error", err) + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve user authentication data.", + Detail: err.Error(), + }) return } @@ -796,19 +802,18 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { logoutURI := dvOIDC.LogoutRedirectURI.Value() if oidcEndpoint == "" { - api.Logger.Error(ctx, "missing OIDC logout endpoint") - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "OIDC configuration is missing.", - }) + logger.Warn(ctx, "missing OIDC logout endpoint") + httpapi.Write(ctx, rw, http.StatusOK, response) return } // Construct OIDC Logout URL logoutURL, err := url.Parse(oidcEndpoint) if err != nil { - api.Logger.Error(ctx, "failed to parse OIDC endpoint", "error", err) + logger.Error(ctx, "failed to parse OIDC endpoint", "error", err) httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Invalid OIDC endpoint.", + Detail: err.Error(), }) return } @@ -829,7 +834,7 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { logoutURL.RawQuery = q.Encode() // Return full logout URL - response := map[string]string{"oidc_logout_url": logoutURL.String()} + response.URL = logoutURL.String() httpapi.Write(ctx, rw, http.StatusOK, response) } From c11ee844781f164490da324408201abf3d29f5cd Mon Sep 17 00:00:00 2001 From: shpark Date: Thu, 20 Mar 2025 14:53:39 +0000 Subject: [PATCH 4/9] chore(site): apply fmt --- site/src/api/api.ts | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 81beed2e3ee1b..ffdb78909e55b 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -457,26 +457,28 @@ class ApiMethods { }; logout = async (): Promise => { - 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"); - } + 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"); + } }; getAuthenticatedUser = async () => { From c0bbf71054a6d93c75f34ff4189c996929424665 Mon Sep 17 00:00:00 2001 From: shpark Date: Thu, 20 Mar 2025 14:57:14 +0000 Subject: [PATCH 5/9] chore: add auto generated files --- cli/testdata/coder_server_--help.golden | 6 +++ cli/testdata/server-config.yaml.golden | 6 +++ coderd/apidoc/docs.go | 39 +++++++++++++++++++ coderd/apidoc/swagger.json | 35 +++++++++++++++++ docs/reference/api/general.md | 2 + docs/reference/api/schemas.md | 22 +++++++++++ docs/reference/api/users.md | 31 +++++++++++++++ docs/reference/cli/server.md | 20 ++++++++++ .../cli/testdata/coder_server_--help.golden | 6 +++ site/src/api/typesGenerated.ts | 5 +++ 10 files changed, 172 insertions(+) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index df1f982bc52fe..6d6b25deb3f8e 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -592,6 +592,12 @@ OIDC OPTIONS: --oidc-username-field string, $CODER_OIDC_USERNAME_FIELD (default: preferred_username) OIDC claim field to use as the username. + --logout-endpoint string, $CODER_OIDC_LOGOUT_ENDPOINT + OIDC endpoint for logout. + + --logout-redirect-uri string, $CODER_OIDC_LOGOUT_URI + OIDC redirect URI after logout. + --oidc-sign-in-text string, $CODER_OIDC_SIGN_IN_TEXT (default: OpenID Connect) The text to show on the OpenID Connect sign in button. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index cffaf65cd3cef..f41cfc86123ba 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -401,6 +401,12 @@ oidc: # an insecure OIDC configuration. It is not recommended to use this flag. # (default: , type: bool) dangerousSkipIssuerChecks: false + # OIDC endpoint for logout. + # (default: , type: string) + logoutEndpoint: "" + # OIDC redirect URI after logout. + # (default: , type: string) + logoutRedirectURI: "" # Telemetry is critical to our ability to improve Coder. We strip all personal # information before sending data to our servers. Please only disable telemetry # when required by your organization's security policy. diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index fe6aacf84d5dd..53497212e8934 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6402,6 +6402,31 @@ const docTemplate = `{ } } }, + "/users/oidc-logout": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Users" + ], + "summary": "Returns URL for the OIDC logout", + "operationId": "user-oidc-logout", + "responses": { + "200": { + "description": "Returns a map containing the OIDC logout URL", + "schema": { + "$ref": "#/definitions/codersdk.OIDCLogoutResponse" + } + } + } + } + }, "/users/oidc/callback": { "get": { "security": [ @@ -13013,6 +13038,12 @@ const docTemplate = `{ "issuer_url": { "type": "string" }, + "logout_endpoint": { + "type": "string" + }, + "logout_redirect_uri": { + "type": "string" + }, "name_field": { "type": "string" }, @@ -13061,6 +13092,14 @@ const docTemplate = `{ } } }, + "codersdk.OIDCLogoutResponse": { + "type": "object", + "properties": { + "oidc_logout_url": { + "type": "string" + } + } + }, "codersdk.Organization": { "type": "object", "required": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 7a399a0e044b4..eab879f8555db 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5659,6 +5659,27 @@ } } }, + "/users/oidc-logout": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Users"], + "summary": "Returns URL for the OIDC logout", + "operationId": "user-oidc-logout", + "responses": { + "200": { + "description": "Returns a map containing the OIDC logout URL", + "schema": { + "$ref": "#/definitions/codersdk.OIDCLogoutResponse" + } + } + } + } + }, "/users/oidc/callback": { "get": { "security": [ @@ -11724,6 +11745,12 @@ "issuer_url": { "type": "string" }, + "logout_endpoint": { + "type": "string" + }, + "logout_redirect_uri": { + "type": "string" + }, "name_field": { "type": "string" }, @@ -11772,6 +11799,14 @@ } } }, + "codersdk.OIDCLogoutResponse": { + "type": "object", + "properties": { + "oidc_logout_url": { + "type": "string" + } + } + }, "codersdk.Organization": { "type": "object", "required": ["created_at", "id", "is_default", "updated_at"], diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index 2b4a1e36c22cc..a1f515b258c4f 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -367,6 +367,8 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "ignore_email_verified": true, "ignore_user_info": true, "issuer_url": "string", + "logout_endpoint": "string", + "logout_redirect_uri": "string", "name_field": "string", "organization_assign_default": true, "organization_field": "string", diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index a7e5e1421e06e..89823ae289050 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -2017,6 +2017,8 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "ignore_email_verified": true, "ignore_user_info": true, "issuer_url": "string", + "logout_endpoint": "string", + "logout_redirect_uri": "string", "name_field": "string", "organization_assign_default": true, "organization_field": "string", @@ -2490,6 +2492,8 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "ignore_email_verified": true, "ignore_user_info": true, "issuer_url": "string", + "logout_endpoint": "string", + "logout_redirect_uri": "string", "name_field": "string", "organization_assign_default": true, "organization_field": "string", @@ -4133,6 +4137,8 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith "ignore_email_verified": true, "ignore_user_info": true, "issuer_url": "string", + "logout_endpoint": "string", + "logout_redirect_uri": "string", "name_field": "string", "organization_assign_default": true, "organization_field": "string", @@ -4174,6 +4180,8 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith | `ignore_email_verified` | boolean | false | | | | `ignore_user_info` | boolean | false | | Ignore user info & UserInfoFromAccessToken are mutually exclusive. Only 1 can be set to true. Ideally this would be an enum with 3 states, ['none', 'userinfo', 'access_token']. However, for backward compatibility, `ignore_user_info` must remain. And `access_token` is a niche, non-spec compliant edge case. So it's use is rare, and should not be advised. | | `issuer_url` | string | false | | | +| `logout_endpoint` | string | false | | | +| `logout_redirect_uri` | string | false | | | | `name_field` | string | false | | | | `organization_assign_default` | boolean | false | | | | `organization_field` | string | false | | | @@ -4188,6 +4196,20 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith | `user_roles_default` | array of string | false | | | | `username_field` | string | false | | | +## codersdk.OIDCLogoutResponse + +```json +{ + "oidc_logout_url": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|-------------------|--------|----------|--------------|-------------| +| `oidc_logout_url` | string | false | | | + ## codersdk.Organization ```json diff --git a/docs/reference/api/users.md b/docs/reference/api/users.md index 3f0c38571f7c4..7d085d57b7ae6 100644 --- a/docs/reference/api/users.md +++ b/docs/reference/api/users.md @@ -373,6 +373,37 @@ curl -X GET http://coder-server:8080/api/v2/users/oauth2/github/device \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Returns URL for the OIDC logout + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/users/oidc-logout \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /users/oidc-logout` + +### Example responses + +> 200 Response + +```json +{ + "oidc_logout_url": "string" +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|----------------------------------------------|----------------------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | Returns a map containing the OIDC logout URL | [codersdk.OIDCLogoutResponse](schemas.md#codersdkoidclogoutresponse) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## OpenID Connect Callback ### Code samples diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 91d565952d943..40499ac222664 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -705,6 +705,26 @@ The custom text to show on the error page informing about disabled OIDC signups. OIDC issuer urls must match in the request, the id_token 'iss' claim, and in the well-known configuration. This flag disables that requirement, and can lead to an insecure OIDC configuration. It is not recommended to use this flag. +### --logout-endpoint + +| | | +|-------------|------------------------------------------| +| Type | string | +| Environment | $CODER_OIDC_LOGOUT_ENDPOINT | +| YAML | oidc.logoutEndpoint | + +OIDC endpoint for logout. + +### --logout-redirect-uri + +| | | +|-------------|-------------------------------------| +| Type | string | +| Environment | $CODER_OIDC_LOGOUT_URI | +| YAML | oidc.logoutRedirectURI | + +OIDC redirect URI after logout. + ### --telemetry | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index f0b3e4b0aaac7..eac6692475e6f 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -593,6 +593,12 @@ OIDC OPTIONS: --oidc-username-field string, $CODER_OIDC_USERNAME_FIELD (default: preferred_username) OIDC claim field to use as the username. + --logout-endpoint string, $CODER_OIDC_LOGOUT_ENDPOINT + OIDC endpoint for logout. + + --logout-redirect-uri string, $CODER_OIDC_LOGOUT_URI + OIDC redirect URI after logout. + --oidc-sign-in-text string, $CODER_OIDC_SIGN_IN_TEXT (default: OpenID Connect) The text to show on the OpenID Connect sign in button. diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index a6e5cad603b08..74f2c052f3e6a 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1478,6 +1478,11 @@ export interface OIDCConfig { readonly logout_redirect_uri: string; } +// From codersdk/users.go +export interface OIDCLogoutResponse { + readonly oidc_logout_url: string; +} + // From codersdk/organizations.go export interface Organization extends MinimalOrganization { readonly description: string; From 2063e6e968b7a505236739edad9bceb361185299 Mon Sep 17 00:00:00 2001 From: shpark Date: Tue, 25 Mar 2025 09:35:18 +0000 Subject: [PATCH 6/9] chore: fix docs --- coderd/apidoc/docs.go | 4 ++-- coderd/apidoc/swagger.json | 4 ++-- coderd/userauth.go | 6 +++--- docs/reference/api/users.md | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 53497212e8934..3b0e968db96e6 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6415,8 +6415,8 @@ const docTemplate = `{ "tags": [ "Users" ], - "summary": "Returns URL for the OIDC logout", - "operationId": "user-oidc-logout", + "summary": "Get user OIDC logout URL", + "operationId": "get-user-oidc-logout-url", "responses": { "200": { "description": "Returns a map containing the OIDC logout URL", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index eab879f8555db..acffaabba7e1c 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5668,8 +5668,8 @@ ], "produces": ["application/json"], "tags": ["Users"], - "summary": "Returns URL for the OIDC logout", - "operationId": "user-oidc-logout", + "summary": "Get user OIDC logout URL", + "operationId": "get-user-oidc-logout-url", "responses": { "200": { "description": "Returns a map containing the OIDC logout URL", diff --git a/coderd/userauth.go b/coderd/userauth.go index 4dd35495ca4a8..859c95a2d168c 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -744,10 +744,10 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { }) } -// Returns the OIDC logout URL. +// Returns URL for the OIDC logout. // -// @Summary Returns URL for the OIDC logout -// @ID user-oidc-logout +// @Summary Get user OIDC logout URL +// @ID get-user-oidc-logout-url // @Security CoderSessionToken // @Produce json // @Tags Users diff --git a/docs/reference/api/users.md b/docs/reference/api/users.md index 7d085d57b7ae6..5e33d15bd9c86 100644 --- a/docs/reference/api/users.md +++ b/docs/reference/api/users.md @@ -373,7 +373,7 @@ curl -X GET http://coder-server:8080/api/v2/users/oauth2/github/device \ To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Returns URL for the OIDC logout +## Get user OIDC logout URL ### Code samples From f2f487c9778a50369633485896edbf42a0f570a8 Mon Sep 17 00:00:00 2001 From: Sanghoon Park Date: Thu, 3 Apr 2025 13:11:44 +0900 Subject: [PATCH 7/9] Update codersdk/deployment.go Co-authored-by: Steven Masley --- codersdk/deployment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 945fd49d6b852..6048b5d7487f4 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1981,7 +1981,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { { Name: "OIDC logout redirect URI", Description: "OIDC redirect URI after logout.", - Flag: "logout-redirect-uri", + Flag: "oidc-logout-redirect-uri", Env: "CODER_OIDC_LOGOUT_URI", Default: "", Value: &c.OIDC.LogoutRedirectURI, From 23158173f706b4a32a8b46666c348300b85877a4 Mon Sep 17 00:00:00 2001 From: shpark Date: Thu, 3 Apr 2025 08:54:31 +0000 Subject: [PATCH 8/9] feat(coderd): implement OIDC revocation with RP initiated logout --- coderd/userauth.go | 118 +++++++++++++++++++++++++++++++++++++---- codersdk/deployment.go | 10 ---- 2 files changed, 108 insertions(+), 20 deletions(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index 859c95a2d168c..06d51c11f5406 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -3,8 +3,10 @@ package coderd import ( "context" "database/sql" + "encoding/base64" "errors" "fmt" + "io" "net/http" "net/mail" "net/url" @@ -744,7 +746,78 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { }) } -// Returns URL for the OIDC logout. +// getDiscoveryEndpoints will return endpoints for end session and revocation +func (api *API) getDiscoveryEndpoints() (endSessionEndpoint string, revocationEndpoint string, err error) { + oidcProvider := api.OIDCConfig.Provider + + var discoveryConfig struct { + EndSessionEndpoint string `json:"end_session_endpoint"` + RevocationEndpoint string `json:"revocation_endpoint"` + } + + // Extract endpoints + if err := oidcProvider.Claims(&discoveryConfig); err != nil { + return "", "", xerrors.Errorf("failed to extract endpoints from OIDC provider discovery claims: %w", err) + } + + return discoveryConfig.EndSessionEndpoint, discoveryConfig.RevocationEndpoint, nil +} + +// revokeOAuthToken will revoke a particular token +func (api *API) revokeOAuthToken(ctx context.Context, token string, revocationEndpoint string) error { + logger := api.Logger.Named(userAuthLoggerName) + + if token == "" || revocationEndpoint == "" { + logger.Warn(ctx, "skip OAuth token revocation") + return nil + } + + dvOIDC := api.DeploymentValues.OIDC + oidcClientID := dvOIDC.ClientID.Value() + oidcClientSecret := dvOIDC.ClientSecret.Value() + + if oidcClientID == "" || oidcClientSecret == "" { + return xerrors.New("missing required configs for revocation (endpoint, client ID, or secret)") + } + + data := url.Values{} + data.Set("token", token) + + revokeReq, err := http.NewRequestWithContext(ctx, http.MethodPost, revocationEndpoint, strings.NewReader(data.Encode())) + if err != nil { + return xerrors.Errorf("failed to create revoke request object: %w", err) + } + + revokeReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") + auth := base64.StdEncoding.EncodeToString([]byte(oidcClientID + ":" + oidcClientSecret)) + revokeReq.Header.Set("Authorization", "Basic "+auth) + + httpClient := &http.Client{} + resp, err := httpClient.Do(revokeReq) + if err != nil { + return xerrors.Errorf("failed to send revoke request to %s: %w", revocationEndpoint, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + respBodyBytes, _ := io.ReadAll(resp.Body) + respBodyStr := string(respBodyBytes) + + logger.Warn(ctx, "failed to request OAuth token revocation", + slog.F("status_code", resp.StatusCode), + slog.F("response_body", respBodyStr), + slog.F("endpoint", revocationEndpoint), + slog.F("client_id", oidcClientID), + ) + + return xerrors.Errorf("failed to revoke with status %d: %s", resp.StatusCode, respBodyStr) + } + + logger.Info(ctx, "success to revoke OAuth token", slog.F("status_code", resp.StatusCode)) + return nil // Success +} + +// Returns URL for the OIDC logout after token revocation. // // @Summary Get user OIDC logout URL // @ID get-user-oidc-logout-url @@ -754,8 +827,18 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { // @Success 200 {object} codersdk.OIDCLogoutResponse "Returns a map containing the OIDC logout URL" // @Router /users/oidc-logout [get] func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { + logger := api.Logger.Named(userAuthLoggerName) ctx := r.Context() + // Check if OIDC is configured + if api.OIDCConfig == nil || api.OIDCConfig.Provider == nil { + logger.Warn(ctx, "unable to support OIDC logout with current configuration") + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve OIDC configuration.", + }) + return + } + // Get logged-in user apiKey := httpmw.APIKey(r) user, err := api.Database.GetUserByID(ctx, apiKey.UserID) @@ -766,8 +849,6 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { return } - logger := api.Logger.Named(userAuthLoggerName) - // Default response: empty URL if OIDC logout is not supported response := codersdk.OIDCLogoutResponse{URL: ""} @@ -793,22 +874,39 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { return } - rawIDToken := link.OAuthAccessToken + accessToken := link.OAuthAccessToken + refreshToken := link.OAuthRefreshToken // Retrieve OIDC environment variables dvOIDC := api.DeploymentValues.OIDC - oidcEndpoint := dvOIDC.LogoutEndpoint.Value() oidcClientID := dvOIDC.ClientID.Value() logoutURI := dvOIDC.LogoutRedirectURI.Value() - if oidcEndpoint == "" { + endSessionEndpoint, revocationEndpoint, err := api.getDiscoveryEndpoints() + if err != nil { + logger.Error(ctx, "failed to get OIDC discovery endpoints", slog.Error(err)) + + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to process OIDC configuration.", + }) + return + } + + // Perform token revocation first + err = api.revokeOAuthToken(ctx, refreshToken, revocationEndpoint) + if err != nil { + // Do not return since this step is optional + logger.Warn(ctx, "failed to revoke OAuth token during logout", slog.Error(err)) + } + + if endSessionEndpoint == "" { logger.Warn(ctx, "missing OIDC logout endpoint") httpapi.Write(ctx, rw, http.StatusOK, response) return } // Construct OIDC Logout URL - logoutURL, err := url.Parse(oidcEndpoint) + logoutURL, err := url.Parse(endSessionEndpoint) if err != nil { logger.Error(ctx, "failed to parse OIDC endpoint", "error", err) httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -821,12 +919,12 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) { // Build parameters q := url.Values{} + if accessToken != "" { + q.Set("id_token_hint", accessToken) + } if oidcClientID != "" { q.Set("client_id", oidcClientID) } - if rawIDToken != "" { - q.Set("id_token_hint", rawIDToken) - } if logoutURI != "" { q.Set("logout_uri", logoutURI) } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 6048b5d7487f4..826507541cf32 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1968,16 +1968,6 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Group: &deploymentGroupOIDC, YAML: "dangerousSkipIssuerChecks", }, - { - Name: "OIDC logout endpoint", - Description: "OIDC endpoint for logout.", - Flag: "logout-endpoint", - Env: "CODER_OIDC_LOGOUT_ENDPOINT", - Default: "", - Value: &c.OIDC.LogoutEndpoint, - Group: &deploymentGroupOIDC, - YAML: "logoutEndpoint", - }, { Name: "OIDC logout redirect URI", Description: "OIDC redirect URI after logout.", From 99c1e59e015c8f5a59e2195f11f3589737a01734 Mon Sep 17 00:00:00 2001 From: shpark Date: Thu, 3 Apr 2025 10:13:19 +0000 Subject: [PATCH 9/9] chore: add auto generated files --- cli/testdata/coder_server_--help.golden | 5 +---- cli/testdata/server-config.yaml.golden | 3 --- docs/reference/cli/server.md | 12 +----------- enterprise/cli/testdata/coder_server_--help.golden | 5 +---- 4 files changed, 3 insertions(+), 22 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 6d6b25deb3f8e..47eee357f2109 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -592,10 +592,7 @@ OIDC OPTIONS: --oidc-username-field string, $CODER_OIDC_USERNAME_FIELD (default: preferred_username) OIDC claim field to use as the username. - --logout-endpoint string, $CODER_OIDC_LOGOUT_ENDPOINT - OIDC endpoint for logout. - - --logout-redirect-uri string, $CODER_OIDC_LOGOUT_URI + --oidc-logout-redirect-uri string, $CODER_OIDC_LOGOUT_URI OIDC redirect URI after logout. --oidc-sign-in-text string, $CODER_OIDC_SIGN_IN_TEXT (default: OpenID Connect) diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index f41cfc86123ba..bf8bacb877a36 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -401,9 +401,6 @@ oidc: # an insecure OIDC configuration. It is not recommended to use this flag. # (default: , type: bool) dangerousSkipIssuerChecks: false - # OIDC endpoint for logout. - # (default: , type: string) - logoutEndpoint: "" # OIDC redirect URI after logout. # (default: , type: string) logoutRedirectURI: "" diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 40499ac222664..91b310b9bd82a 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -705,17 +705,7 @@ The custom text to show on the error page informing about disabled OIDC signups. OIDC issuer urls must match in the request, the id_token 'iss' claim, and in the well-known configuration. This flag disables that requirement, and can lead to an insecure OIDC configuration. It is not recommended to use this flag. -### --logout-endpoint - -| | | -|-------------|------------------------------------------| -| Type | string | -| Environment | $CODER_OIDC_LOGOUT_ENDPOINT | -| YAML | oidc.logoutEndpoint | - -OIDC endpoint for logout. - -### --logout-redirect-uri +### --oidc-logout-redirect-uri | | | |-------------|-------------------------------------| diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index eac6692475e6f..50ac3a140ef96 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -593,10 +593,7 @@ OIDC OPTIONS: --oidc-username-field string, $CODER_OIDC_USERNAME_FIELD (default: preferred_username) OIDC claim field to use as the username. - --logout-endpoint string, $CODER_OIDC_LOGOUT_ENDPOINT - OIDC endpoint for logout. - - --logout-redirect-uri string, $CODER_OIDC_LOGOUT_URI + --oidc-logout-redirect-uri string, $CODER_OIDC_LOGOUT_URI OIDC redirect URI after logout. --oidc-sign-in-text string, $CODER_OIDC_SIGN_IN_TEXT (default: OpenID Connect)