From 73353417fb079a9719a30924d0722639dd8b0d75 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 be558797389b9..4b18669ba3c22 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1106,6 +1106,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 c5e95e44998b2..a118eb2c3da4e 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/mail" + "net/url" "sort" "strconv" "strings" @@ -734,6 +735,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 e1c0b977c00d2..19ec6bc1c53de 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -543,6 +543,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 { @@ -1917,6 +1919,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 26491efb10565..241b66f2e3f7a 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -430,7 +430,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 de879ee23daa5..eb6769a652c5f 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1407,6 +1407,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 638727e0c1d986d27bc583ce3932171d07260fe6 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 4dbdc0d4e4f91..c68b49b61dfcd 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -300,6 +300,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 b1c1ee39c0ad60ef56cea7455d44c71a395cc655 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 a118eb2c3da4e..33293bd2a23b9 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -742,7 +742,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() @@ -757,6 +757,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), @@ -765,16 +770,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 } @@ -787,19 +793,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 } @@ -820,7 +825,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 239ae317ba6d1e270a4b2b34fddd344f4320946c 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 241b66f2e3f7a..3fef19ebefdd9 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -430,26 +430,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 723bff8615ca4db1ce19cde9a65d798014a35ddb 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 | 85 ++++++++++++------- docs/reference/api/users.md | 32 +++++++ docs/reference/cli/server.md | 20 +++++ .../cli/testdata/coder_server_--help.golden | 6 ++ site/src/api/typesGenerated.ts | 5 ++ 10 files changed, 205 insertions(+), 31 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 93d9d69517ec9..e96c670faada1 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -586,6 +586,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 96a03c5b1f05e..eeee0103edb62 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -389,6 +389,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 329951003007b..09d382db824d0 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6057,6 +6057,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": [ @@ -12480,6 +12505,12 @@ const docTemplate = `{ "issuer_url": { "type": "string" }, + "logout_endpoint": { + "type": "string" + }, + "logout_redirect_uri": { + "type": "string" + }, "name_field": { "type": "string" }, @@ -12524,6 +12555,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 63b7146365d9f..2894b16a77429 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5345,6 +5345,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": [ @@ -11230,6 +11251,12 @@ "issuer_url": { "type": "string" }, + "logout_endpoint": { + "type": "string" + }, + "logout_redirect_uri": { + "type": "string" + }, "name_field": { "type": "string" }, @@ -11274,6 +11301,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 66e85f3f6978a..368a7ac50325d 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -365,6 +365,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 20ed37f81f7f7..52f854e58fbab 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -2008,6 +2008,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", @@ -2478,6 +2480,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", @@ -3973,6 +3977,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", @@ -3994,37 +4000,54 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith ### Properties -| Name | Type | Required | Restrictions | Description | -|-------------------------------|----------------------------------|----------|--------------|----------------------------------------------------------------------------------| -| `allow_signups` | boolean | false | | | -| `auth_url_params` | object | false | | | -| `client_cert_file` | string | false | | | -| `client_id` | string | false | | | -| `client_key_file` | string | false | | Client key file & ClientCertFile are used in place of ClientSecret for PKI auth. | -| `client_secret` | string | false | | | -| `email_domain` | array of string | false | | | -| `email_field` | string | false | | | -| `group_allow_list` | array of string | false | | | -| `group_auto_create` | boolean | false | | | -| `group_mapping` | object | false | | | -| `group_regex_filter` | [serpent.Regexp](#serpentregexp) | false | | | -| `groups_field` | string | false | | | -| `icon_url` | [serpent.URL](#serpenturl) | false | | | -| `ignore_email_verified` | boolean | false | | | -| `ignore_user_info` | boolean | false | | | -| `issuer_url` | string | false | | | -| `name_field` | string | false | | | -| `organization_assign_default` | boolean | false | | | -| `organization_field` | string | false | | | -| `organization_mapping` | object | false | | | -| `scopes` | array of string | false | | | -| `sign_in_text` | string | false | | | -| `signups_disabled_text` | string | false | | | -| `skip_issuer_checks` | boolean | false | | | -| `user_role_field` | string | false | | | -| `user_role_mapping` | object | false | | | -| `user_roles_default` | array of string | false | | | -| `username_field` | string | false | | | +| Name | Type | Required | Restrictions | Description | +|--------------------------------------|----------------------------------|----------|--------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `allow_signups` | boolean | false | | | +| `auth_url_params` | object | false | | | +| `client_cert_file` | string | false | | | +| `client_id` | string | false | | | +| `client_key_file` | string | false | | Client key file & ClientCertFile are used in place of ClientSecret for PKI auth. | +| `client_secret` | string | false | | | +| `email_domain` | array of string | false | | | +| `email_field` | string | false | | | +| `group_allow_list` | array of string | false | | | +| `group_auto_create` | boolean | false | | | +| `group_mapping` | object | false | | | +| `group_regex_filter` | [serpent.Regexp](#serpentregexp) | false | | | +| `groups_field` | string | false | | | +| `icon_url` | [serpent.URL](#serpenturl) | false | | | +| `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 | | | +| `organization_mapping` | object | false | | | +| `scopes` | array of string | false | | | +| `sign_in_text` | string | false | | | +| `signups_disabled_text` | string | false | | | +| `skip_issuer_checks` | boolean | false | | | +| `source_user_info_from_access_token` | boolean | false | | Source user info from access token as mentioned above is an edge case. This allows sourcing the user_info from the access token itself instead of a user_info endpoint. This assumes the access token is a valid JWT with a set of claims to be merged with the id_token. | +| `user_role_field` | string | false | | | +| `user_role_mapping` | object | false | | | +| `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 diff --git a/docs/reference/api/users.md b/docs/reference/api/users.md index d8aac77cfa83b..96a260d36d659 100644 --- a/docs/reference/api/users.md +++ b/docs/reference/api/users.md @@ -337,6 +337,38 @@ curl -X GET http://coder-server:8080/api/v2/users/oauth2/github/callback \ 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). + +>>>>>>> c0bbf7105 (chore: add auto generated files) ## OpenID Connect Callback ### Code samples diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 98cb2a90c20da..3a5a183097d42 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -683,6 +683,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 ebaf1a5ac0bbd..18d76c617ebb9 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -587,6 +587,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 eb6769a652c5f..edb4b78eb6914 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1411,6 +1411,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 33320a6047feba1c8ebe88b25aed7cfa784fbb1b 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 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 09d382db824d0..4befa051f6f20 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6070,8 +6070,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 2894b16a77429..2569abb935395 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5354,8 +5354,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 33293bd2a23b9..54dfa631b704f 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -735,10 +735,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 From 91c274a15b20dbaed0817c21606fd271808eb04d 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 19ec6bc1c53de..ba23572764a51 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1932,7 +1932,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 a6eaf19d3d9004009d00a08592d0f30f9106f860 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 54dfa631b704f..4003fd72c1be5 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" @@ -735,7 +737,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 @@ -745,8 +818,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) @@ -757,8 +840,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: ""} @@ -784,22 +865,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{ @@ -812,12 +910,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 ba23572764a51..f507c8d477f09 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1919,16 +1919,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 2c507c5776f996cbac369cb6f7c8fea2990c861c 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 e96c670faada1..621d57812b242 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -586,10 +586,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 eeee0103edb62..38dd2085c0a80 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -389,9 +389,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 3a5a183097d42..1eb27d03e6e27 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -683,17 +683,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 18d76c617ebb9..4d78d09278b3a 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -587,10 +587,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)