From 439b04178046d3c040a3034f64d977103d253881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Banaszewski?= Date: Fri, 19 Sep 2025 16:27:02 +0200 Subject: [PATCH] feat: add best effort attempt to revoke oauth access token in external auth provider (#19775) Solves #15575 Adds OAuth access token revocation when unlinking external auth provider. Due to revocation not being consistently implemented by providers this is only best effort attempt. Unsuccessful revocation won't influence link removal. --- cli/server.go | 2 + cli/server_test.go | 3 + coderd/apidoc/docs.go | 26 ++- coderd/apidoc/swagger.json | 24 +- coderd/coderdtest/oidctest/idp.go | 65 +++++- coderd/externalauth.go | 46 +++- coderd/externalauth/externalauth.go | 93 ++++++++ coderd/externalauth/externalauth_test.go | 211 +++++++++++++++++- coderd/externalauth_test.go | 67 +++++- coderd/promoauth/oauth2.go | 1 + codersdk/deployment.go | 1 + codersdk/deployment_test.go | 1 + codersdk/externalauth.go | 42 ++-- codersdk/testdata/githubcfg.yaml | 1 + docs/admin/external-auth/index.md | 4 + docs/reference/api/general.md | 1 + docs/reference/api/git.md | 19 +- docs/reference/api/schemas.md | 41 +++- site/src/api/api.ts | 5 +- site/src/api/typesGenerated.ts | 9 + .../ExternalAuthSettingsPageView.stories.tsx | 1 + .../ExternalAuthPage/ExternalAuthPage.tsx | 30 ++- .../ExternalAuthPage/ExternalAuthPageView.tsx | 4 +- site/src/testHelpers/entities.ts | 1 + 24 files changed, 629 insertions(+), 69 deletions(-) diff --git a/cli/server.go b/cli/server.go index 554ab5331b..f1106f5f10 100644 --- a/cli/server.go +++ b/cli/server.go @@ -2692,6 +2692,8 @@ func parseExternalAuthProvidersFromEnv(prefix string, environ []string) ([]coder provider.AuthURL = v.Value case "TOKEN_URL": provider.TokenURL = v.Value + case "REVOKE_URL": + provider.RevokeURL = v.Value case "VALIDATE_URL": provider.ValidateURL = v.Value case "REGEX": diff --git a/cli/server_test.go b/cli/server_test.go index 3f69443927..2eae0cde17 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -77,6 +77,7 @@ func TestReadExternalAuthProvidersFromEnv(t *testing.T) { "CODER_EXTERNAL_AUTH_1_CLIENT_SECRET=hunter12", "CODER_EXTERNAL_AUTH_1_TOKEN_URL=google.com", "CODER_EXTERNAL_AUTH_1_VALIDATE_URL=bing.com", + "CODER_EXTERNAL_AUTH_1_REVOKE_URL=revoke.url", "CODER_EXTERNAL_AUTH_1_SCOPES=repo:read repo:write", "CODER_EXTERNAL_AUTH_1_NO_REFRESH=true", "CODER_EXTERNAL_AUTH_1_DISPLAY_NAME=Google", @@ -88,6 +89,7 @@ func TestReadExternalAuthProvidersFromEnv(t *testing.T) { // Validate the first provider. assert.Equal(t, "1", providers[0].ID) assert.Equal(t, "gitlab", providers[0].Type) + assert.Equal(t, "", providers[0].RevokeURL) // Validate the second provider. assert.Equal(t, "2", providers[1].ID) @@ -95,6 +97,7 @@ func TestReadExternalAuthProvidersFromEnv(t *testing.T) { assert.Equal(t, "hunter12", providers[1].ClientSecret) assert.Equal(t, "google.com", providers[1].TokenURL) assert.Equal(t, "bing.com", providers[1].ValidateURL) + assert.Equal(t, "revoke.url", providers[1].RevokeURL) assert.Equal(t, []string{"repo:read", "repo:write"}, providers[1].Scopes) assert.Equal(t, true, providers[1].NoRefresh) assert.Equal(t, "Google", providers[1].DisplayName) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 58e5da2f9a..ecb2b4a7ea 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -960,6 +960,9 @@ const docTemplate = `{ "CoderSessionToken": [] } ], + "produces": [ + "application/json" + ], "tags": [ "Git" ], @@ -977,7 +980,10 @@ const docTemplate = `{ ], "responses": { "200": { - "description": "OK" + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.DeleteExternalAuthByIDResponse" + } } } } @@ -12770,6 +12776,18 @@ const docTemplate = `{ } } }, + "codersdk.DeleteExternalAuthByIDResponse": { + "type": "object", + "properties": { + "token_revocation_error": { + "type": "string" + }, + "token_revoked": { + "description": "TokenRevoked set to true if token revocation was attempted and was successful", + "type": "boolean" + } + } + }, "codersdk.DeleteWebpushSubscription": { "type": "object", "properties": { @@ -13249,6 +13267,9 @@ const docTemplate = `{ "$ref": "#/definitions/codersdk.ExternalAuthAppInstallation" } }, + "supports_revocation": { + "type": "boolean" + }, "user": { "description": "User is the user that authenticated with the provider.", "allOf": [ @@ -13322,6 +13343,9 @@ const docTemplate = `{ "description": "Regex allows API requesters to match an auth config by\na string (e.g. coder.com) instead of by it's type.\n\nGit clone makes use of this by parsing the URL from:\n'Username for \"https://github.com\":'\nAnd sending it to the Coder server to match against the Regex.", "type": "string" }, + "revoke_url": { + "type": "string" + }, "scopes": { "type": "array", "items": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index b44f2dfcce..0607160868 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -822,6 +822,7 @@ "CoderSessionToken": [] } ], + "produces": ["application/json"], "tags": ["Git"], "summary": "Delete external auth user link by ID", "operationId": "delete-external-auth-user-link-by-id", @@ -837,7 +838,10 @@ ], "responses": { "200": { - "description": "OK" + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.DeleteExternalAuthByIDResponse" + } } } } @@ -11413,6 +11417,18 @@ } } }, + "codersdk.DeleteExternalAuthByIDResponse": { + "type": "object", + "properties": { + "token_revocation_error": { + "type": "string" + }, + "token_revoked": { + "description": "TokenRevoked set to true if token revocation was attempted and was successful", + "type": "boolean" + } + } + }, "codersdk.DeleteWebpushSubscription": { "type": "object", "properties": { @@ -11885,6 +11901,9 @@ "$ref": "#/definitions/codersdk.ExternalAuthAppInstallation" } }, + "supports_revocation": { + "type": "boolean" + }, "user": { "description": "User is the user that authenticated with the provider.", "allOf": [ @@ -11958,6 +11977,9 @@ "description": "Regex allows API requesters to match an auth config by\na string (e.g. coder.com) instead of by it's type.\n\nGit clone makes use of this by parsing the URL from:\n'Username for \"https://github.com\":'\nAnd sending it to the Coder server to match against the Regex.", "type": "string" }, + "revoke_url": { + "type": "string" + }, "scopes": { "type": "array", "items": { diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index a76f6447dc..d5215b9964 100644 --- a/coderd/coderdtest/oidctest/idp.go +++ b/coderd/coderdtest/oidctest/idp.go @@ -46,6 +46,8 @@ import ( "github.com/coder/coder/v2/testutil" ) +type HookRevokeTokenFn func() (httpStatus int, err error) + type token struct { issued time.Time email string @@ -196,9 +198,11 @@ type FakeIDP struct { // hookValidRedirectURL can be used to reject a redirect url from the // IDP -> Application. Almost all IDPs have the concept of // "Authorized Redirect URLs". This can be used to emulate that. - hookValidRedirectURL func(redirectURL string) error - hookUserInfo func(email string) (jwt.MapClaims, error) - hookAccessTokenJWT func(email string, exp time.Time) jwt.MapClaims + hookValidRedirectURL func(redirectURL string) error + hookUserInfo func(email string) (jwt.MapClaims, error) + hookRevokeToken HookRevokeTokenFn + revokeTokenGitHubFormat bool // GitHub doesn't follow token revocation RFC spec + hookAccessTokenJWT func(email string, exp time.Time) jwt.MapClaims // defaultIDClaims is if a new client connects and we didn't preset // some claims. defaultIDClaims jwt.MapClaims @@ -327,6 +331,19 @@ func WithStaticUserInfo(info jwt.MapClaims) func(*FakeIDP) { } } +func WithRevokeTokenRFC(revokeFunc HookRevokeTokenFn) func(*FakeIDP) { + return func(f *FakeIDP) { + f.hookRevokeToken = revokeFunc + } +} + +func WithRevokeTokenGitHub(revokeFunc HookRevokeTokenFn) func(*FakeIDP) { + return func(f *FakeIDP) { + f.hookRevokeToken = revokeFunc + f.revokeTokenGitHubFormat = true + } +} + func WithDefaultIDClaims(claims jwt.MapClaims) func(*FakeIDP) { return func(f *FakeIDP) { f.defaultIDClaims = claims @@ -358,6 +375,7 @@ type With429Arguments struct { AuthorizePath bool KeysPath bool UserInfoPath bool + RevokePath bool DeviceAuth bool DeviceVerify bool } @@ -387,6 +405,10 @@ func With429(params With429Arguments) func(*FakeIDP) { http.Error(rw, "429, being manually blocked (userinfo)", http.StatusTooManyRequests) return } + if params.RevokePath && strings.Contains(r.URL.Path, revokeTokenPath) { + http.Error(rw, "429, being manually blocked (revoke)", http.StatusTooManyRequests) + return + } if params.DeviceAuth && strings.Contains(r.URL.Path, deviceAuth) { http.Error(rw, "429, being manually blocked (device-auth)", http.StatusTooManyRequests) return @@ -408,8 +430,10 @@ const ( authorizePath = "/oauth2/authorize" keysPath = "/oauth2/keys" userInfoPath = "/oauth2/userinfo" - deviceAuth = "/login/device/code" - deviceVerify = "/login/device" + // nolint:gosec // It also thinks this is a secret lol + revokeTokenPath = "/oauth2/revoke" + deviceAuth = "/login/device/code" + deviceVerify = "/login/device" ) func NewFakeIDP(t testing.TB, opts ...FakeIDPOpt) *FakeIDP { @@ -486,6 +510,7 @@ func (f *FakeIDP) updateIssuerURL(t testing.TB, issuer string) { TokenURL: u.ResolveReference(&url.URL{Path: tokenPath}).String(), JWKSURL: u.ResolveReference(&url.URL{Path: keysPath}).String(), UserInfoURL: u.ResolveReference(&url.URL{Path: userInfoPath}).String(), + RevokeURL: u.ResolveReference(&url.URL{Path: revokeTokenPath}).String(), DeviceCodeURL: u.ResolveReference(&url.URL{Path: deviceAuth}).String(), Algorithms: []string{ "RS256", @@ -756,6 +781,7 @@ type ProviderJSON struct { TokenURL string `json:"token_endpoint"` JWKSURL string `json:"jwks_uri"` UserInfoURL string `json:"userinfo_endpoint"` + RevokeURL string `json:"revocation_endpoint"` DeviceCodeURL string `json:"device_authorization_endpoint"` Algorithms []string `json:"id_token_signing_alg_values_supported"` // This is custom @@ -1146,6 +1172,29 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler { _ = json.NewEncoder(rw).Encode(claims) })) + mux.Handle(revokeTokenPath, http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if f.revokeTokenGitHubFormat { + u, p, ok := r.BasicAuth() + if !ok || !(u == f.clientID && p == f.clientSecret) { + httpError(rw, http.StatusForbidden, xerrors.Errorf("basic auth failed")) + return + } + } else { + _, ok := validateMW(rw, r) + if !ok { + httpError(rw, http.StatusForbidden, xerrors.Errorf("token validation failed")) + return + } + } + + code, err := f.hookRevokeToken() + if err != nil { + httpError(rw, code, xerrors.Errorf("hook err: %w", err)) + return + } + httpapi.Write(r.Context(), rw, code, "") + })) + // There is almost no difference between this and /userinfo. // The main tweak is that this route is "mounted" vs "handle" because "/userinfo" // should be strict, and this one needs to handle sub routes. @@ -1474,12 +1523,16 @@ func (f *FakeIDP) ExternalAuthConfig(t testing.TB, id string, custom *ExternalAu DisplayName: id, InstrumentedOAuth2Config: oauthCfg, ID: id, + ClientID: f.clientID, + ClientSecret: f.clientSecret, // No defaults for these fields by omitting the type Type: "", DisplayIcon: f.WellknownConfig().UserInfoURL, // Omit the /user for the validate so we can easily append to it when modifying // the cfg for advanced tests. - ValidateURL: f.locked.IssuerURL().ResolveReference(&url.URL{Path: "/external-auth-validate/"}).String(), + ValidateURL: f.locked.IssuerURL().ResolveReference(&url.URL{Path: "/external-auth-validate/"}).String(), + RevokeURL: f.locked.IssuerURL().ResolveReference(&url.URL{Path: revokeTokenPath}).String(), + RevokeTimeout: 1 * time.Second, DeviceAuth: &externalauth.DeviceAuth{ Config: oauthCfg, ClientID: f.clientID, diff --git a/coderd/externalauth.go b/coderd/externalauth.go index a07f6d486c..23ae7e9fe2 100644 --- a/coderd/externalauth.go +++ b/coderd/externalauth.go @@ -85,20 +85,37 @@ func (api *API) externalAuthByID(w http.ResponseWriter, r *http.Request) { // @ID delete-external-auth-user-link-by-id // @Security CoderSessionToken // @Tags Git -// @Success 200 +// @Produce json // @Param externalauth path string true "Git Provider ID" format(string) +// @Success 200 {object} codersdk.DeleteExternalAuthByIDResponse // @Router /external-auth/{externalauth} [delete] func (api *API) deleteExternalAuthByID(w http.ResponseWriter, r *http.Request) { config := httpmw.ExternalAuthParam(r) apiKey := httpmw.APIKey(r) ctx := r.Context() - err := api.Database.DeleteExternalAuthLink(ctx, database.DeleteExternalAuthLinkParams{ + link, err := api.Database.GetExternalAuthLink(ctx, database.GetExternalAuthLinkParams{ ProviderID: config.ID, UserID: apiKey.UserID, }) if err != nil { - if !errors.Is(err, sql.ErrNoRows) { + if errors.Is(err, sql.ErrNoRows) { + httpapi.ResourceNotFound(w) + return + } + httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to get external auth link during deletion.", + Detail: err.Error(), + }) + return + } + + err = api.Database.DeleteExternalAuthLink(ctx, database.DeleteExternalAuthLinkParams{ + ProviderID: config.ID, + UserID: apiKey.UserID, + }) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { httpapi.ResourceNotFound(w) return } @@ -109,7 +126,13 @@ func (api *API) deleteExternalAuthByID(w http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, w, http.StatusOK, "OK") + ok, err := config.RevokeToken(ctx, link) + resp := codersdk.DeleteExternalAuthByIDResponse{TokenRevoked: ok} + + if err != nil { + resp.TokenRevocationError = err.Error() + } + httpapi.Write(ctx, w, http.StatusOK, resp) } // @Summary Post external auth device by ID @@ -394,13 +417,14 @@ func ExternalAuthConfigs(auths []*externalauth.Config) []codersdk.ExternalAuthLi func ExternalAuthConfig(cfg *externalauth.Config) codersdk.ExternalAuthLinkProvider { return codersdk.ExternalAuthLinkProvider{ - ID: cfg.ID, - Type: cfg.Type, - Device: cfg.DeviceAuth != nil, - DisplayName: cfg.DisplayName, - DisplayIcon: cfg.DisplayIcon, - AllowRefresh: !cfg.NoRefresh, - AllowValidate: cfg.ValidateURL != "", + ID: cfg.ID, + Type: cfg.Type, + Device: cfg.DeviceAuth != nil, + DisplayName: cfg.DisplayName, + DisplayIcon: cfg.DisplayIcon, + AllowRefresh: !cfg.NoRefresh, + AllowValidate: cfg.ValidateURL != "", + SupportsRevocation: cfg.RevokeURL != "", } } diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index 55dba499e6..41797706ee 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -34,6 +34,9 @@ const ( // database for a failed refresh token. In rare cases, the error could be a large // HTML payload. failureReasonLimit = 400 + + // tokenRevocationTimeout timeout for requests to external oauth provider. + tokenRevocationTimeout = 10 * time.Second ) // Config is used for authentication for Git operations. @@ -43,6 +46,9 @@ type Config struct { ID string // Type is the type of provider. Type string + + ClientID string + ClientSecret string // DeviceAuth is set if the provider uses the device flow. DeviceAuth *DeviceAuth // DisplayName is the name of the provider to display to the user. @@ -69,6 +75,9 @@ type Config struct { // not be validated before being returned. ValidateURL string + RevokeURL string + RevokeTimeout time.Duration + // Regex is a Regexp matched against URLs for // a Git clone. e.g. "Username for 'https://github.com':" // The regex would be `github\.com`.. @@ -398,6 +407,83 @@ func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk return installs, true, nil } +func (c *Config) RevokeToken(ctx context.Context, link database.ExternalAuthLink) (bool, error) { + if c.RevokeURL == "" { + return false, nil + } + + reqCtx, cancel := context.WithTimeout(ctx, c.RevokeTimeout) + defer cancel() + req, err := c.TokenRevocationRequest(reqCtx, link) + if err != nil { + return false, err + } + + res, err := c.InstrumentedOAuth2Config.Do(ctx, promoauth.SourceRevoke, req) + if err != nil { + return false, err + } + defer res.Body.Close() + body, err := io.ReadAll(res.Body) + if err != nil { + return false, err + } + + if c.TokenRevocationResponseOk(res) { + return true, nil + } + return false, xerrors.Errorf("failed to revoke token: %d %s", res.StatusCode, string(body)) +} + +func (c *Config) TokenRevocationRequest(ctx context.Context, link database.ExternalAuthLink) (*http.Request, error) { + if c.Type == codersdk.EnhancedExternalAuthProviderGitHub.String() { + return c.TokenRevocationRequestGitHub(ctx, link) + } + return c.TokenRevocationRequestRFC7009(ctx, link) +} + +func (c *Config) TokenRevocationRequestRFC7009(ctx context.Context, link database.ExternalAuthLink) (*http.Request, error) { + p := url.Values{} + p.Add("client_id", c.ClientID) + p.Add("client_secret", c.ClientSecret) + if link.OAuthRefreshToken != "" { + p.Add("token_type_hint", "refresh_token") + p.Add("token", link.OAuthRefreshToken) + } else { + p.Add("token_type_hint", "access_token") + p.Add("token", link.OAuthAccessToken) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.RevokeURL, strings.NewReader(p.Encode())) + if err != nil { + return nil, err + } + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", link.OAuthAccessToken)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req, nil +} + +func (c *Config) TokenRevocationRequestGitHub(ctx context.Context, link database.ExternalAuthLink) (*http.Request, error) { + // GitHub doesn't follow RFC spec + // https://docs.github.com/en/rest/apps/oauth-applications?apiVersion=2022-11-28#delete-an-app-authorization + body := fmt.Sprintf("{\"access_token\":%q}", link.OAuthAccessToken) + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, c.RevokeURL, strings.NewReader(body)) + if err != nil { + return nil, err + } + req.Header.Add("Accept", "application/vnd.github+json") + req.Header.Add("X-GitHub-Api-Version", "2022-11-28") + req.SetBasicAuth(c.ClientID, c.ClientSecret) + return req, nil +} + +func (c *Config) TokenRevocationResponseOk(res *http.Response) bool { + // RFC spec on successful revocation returns 200, GitHub 204 + if c.Type == codersdk.EnhancedExternalAuthProviderGitHub.String() { + return res.StatusCode == http.StatusNoContent + } + return res.StatusCode == http.StatusOK +} + type DeviceAuth struct { // Config is provided for the http client method. Config promoauth.InstrumentedOAuth2Config @@ -639,10 +725,14 @@ func ConvertConfig(instrument *promoauth.Factory, entries []codersdk.ExternalAut cfg := &Config{ InstrumentedOAuth2Config: instrumented, ID: entry.ID, + ClientID: entry.ClientID, + ClientSecret: entry.ClientSecret, Regex: regex, Type: entry.Type, NoRefresh: entry.NoRefresh, ValidateURL: entry.ValidateURL, + RevokeURL: entry.RevokeURL, + RevokeTimeout: tokenRevocationTimeout, AppInstallationsURL: entry.AppInstallationsURL, AppInstallURL: entry.AppInstallURL, DisplayName: entry.DisplayName, @@ -807,6 +897,7 @@ func gitlabDefaults(config *codersdk.ExternalAuthConfig) codersdk.ExternalAuthCo AuthURL: "https://gitlab.com/oauth/authorize", TokenURL: "https://gitlab.com/oauth/token", ValidateURL: "https://gitlab.com/oauth/token/info", + RevokeURL: "https://gitlab.com/oauth/revoke", DisplayName: "GitLab", DisplayIcon: "/icon/gitlab.svg", Regex: `^(https?://)?gitlab\.com(/.*)?$`, @@ -832,6 +923,7 @@ func gitlabDefaults(config *codersdk.ExternalAuthConfig) codersdk.ExternalAuthCo AuthURL: au.ResolveReference(&url.URL{Path: "/oauth/authorize"}).String(), TokenURL: au.ResolveReference(&url.URL{Path: "/oauth/token"}).String(), ValidateURL: au.ResolveReference(&url.URL{Path: "/oauth/token/info"}).String(), + RevokeURL: au.ResolveReference(&url.URL{Path: "/oauth/revoke"}).String(), Regex: fmt.Sprintf(`^(https?://)?%s(/.*)?$`, strings.ReplaceAll(au.Host, ".", `\.`)), } } @@ -973,6 +1065,7 @@ var staticDefaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.External codersdk.EnhancedExternalAuthProviderSlack: { AuthURL: "https://slack.com/oauth/v2/authorize", TokenURL: "https://slack.com/api/oauth.v2.access", + RevokeURL: "https://slack.com/api/auth.revoke", DisplayName: "Slack", DisplayIcon: "/icon/slack.svg", // See: https://api.slack.com/authentication/oauth-v2#exchanging diff --git a/coderd/externalauth/externalauth_test.go b/coderd/externalauth/externalauth_test.go index 8e46566ed2..670d1cbf11 100644 --- a/coderd/externalauth/externalauth_test.go +++ b/coderd/externalauth/externalauth_test.go @@ -381,6 +381,150 @@ func TestRefreshToken(t *testing.T) { }) } +func TestRevokeToken(t *testing.T) { + t.Parallel() + + t.Run("RevokeTokenRFC_OK", func(t *testing.T) { + t.Parallel() + var link database.ExternalAuthLink + var config *externalauth.Config + fake, config, link := setupOauth2Test(t, testConfig{ + FakeIDPOpts: []oidctest.FakeIDPOpt{ + oidctest.WithRevokeTokenRFC(func() (int, error) { + return http.StatusOK, nil + }), + }, + }) + + ctx := oidc.ClientContext(testutil.Context(t, testutil.WaitLong), fake.HTTPClient(nil)) + revoked, err := config.RevokeToken(ctx, link) + require.NoError(t, err) + require.True(t, revoked) + }) + + t.Run("RevokeTokenRFC_WrongBearer", func(t *testing.T) { + t.Parallel() + fake, config, link := setupOauth2Test(t, testConfig{ + FakeIDPOpts: []oidctest.FakeIDPOpt{ + oidctest.WithRevokeTokenRFC(func() (int, error) { + return http.StatusOK, nil + }), + }, + }) + + link.OAuthAccessToken += "wrong_token" + ctx := oidc.ClientContext(testutil.Context(t, testutil.WaitLong), fake.HTTPClient(nil)) + revoked, err := config.RevokeToken(ctx, link) + require.Error(t, err) + require.Contains(t, err.Error(), "token validation failed") + require.False(t, revoked) + }) + + t.Run("RevokeTokenRFC_WrongURL", func(t *testing.T) { + t.Parallel() + fake, config, link := setupOauth2Test(t, testConfig{ + FakeIDPOpts: []oidctest.FakeIDPOpt{ + oidctest.WithRevokeTokenRFC(func() (int, error) { + return http.StatusOK, nil + }), + }, + }) + + config.RevokeURL = "%" + ctx := oidc.ClientContext(testutil.Context(t, testutil.WaitLong), fake.HTTPClient(nil)) + revoked, err := config.RevokeToken(ctx, link) + require.Error(t, err) + require.ErrorContains(t, err, "invalid URL escape") + require.False(t, revoked) + }) + + t.Run("RevokeTokenRFC_Timeout", func(t *testing.T) { + t.Parallel() + revokeExited := make(chan bool, 1) + testTimeout := make(chan bool, 1) + handlerDone := make(chan bool) + + go func() { + time.Sleep(5 * time.Second) + testTimeout <- true + }() + + fake, config, link := setupOauth2Test(t, testConfig{ + FakeIDPOpts: []oidctest.FakeIDPOpt{ + oidctest.WithRevokeTokenRFC(func() (int, error) { + defer func() { + handlerDone <- true + }() + + select { + case <-testTimeout: + t.Error("test timeout reached before context timeout") + return http.StatusOK, nil + case <-revokeExited: + return http.StatusOK, nil + } + }), + oidctest.WithServing(), + }, + }) + + ctx := oidc.ClientContext(testutil.Context(t, testutil.WaitLong), fake.HTTPClient(nil)) + config.RevokeTimeout = time.Millisecond * 10 + revoked, err := config.RevokeToken(ctx, link) + revokeExited <- true + require.ErrorIs(t, err, context.DeadlineExceeded) + require.False(t, revoked) + _ = testutil.RequireReceive(ctx, t, handlerDone) + }) + + t.Run("RevokeTokenGitHub_OK", func(t *testing.T) { + t.Parallel() + clientID := "clientID" + clientSecret := "clientSecret" + fake, config, link := setupOauth2Test(t, testConfig{ + FakeIDPOpts: []oidctest.FakeIDPOpt{ + oidctest.WithRevokeTokenGitHub(func() (int, error) { + return http.StatusNoContent, nil + }), + oidctest.WithStaticCredentials(clientID, clientSecret), + oidctest.WithServing(), + }, + }) + + config.Type = codersdk.EnhancedExternalAuthProviderGitHub.String() + config.ClientID = clientID + config.ClientSecret = clientSecret + ctx := oidc.ClientContext(testutil.Context(t, testutil.WaitLong), fake.HTTPClient(nil)) + revoked, err := config.RevokeToken(ctx, link) + require.NoError(t, err) + require.True(t, revoked) + }) + + t.Run("RevokeTokenGitHub_WrongAuth", func(t *testing.T) { + t.Parallel() + clientID := "clientID" + clientSecret := "clientSecret" + fake, config, link := setupOauth2Test(t, testConfig{ + FakeIDPOpts: []oidctest.FakeIDPOpt{ + oidctest.WithRevokeTokenGitHub(func() (int, error) { + return http.StatusNoContent, nil + }), + oidctest.WithStaticCredentials(clientID, clientSecret), + oidctest.WithServing(), + }, + }) + + config.Type = codersdk.EnhancedExternalAuthProviderGitHub.String() + config.ClientID = clientID + "bad" + config.ClientSecret = clientSecret + ctx := oidc.ClientContext(testutil.Context(t, testutil.WaitLong), fake.HTTPClient(nil)) + revoked, err := config.RevokeToken(ctx, link) + require.Error(t, err) + require.Contains(t, err.Error(), "basic auth failed") + require.False(t, revoked) + }) +} + func TestExchangeWithClientSecret(t *testing.T) { t.Parallel() instrument := promoauth.NewFactory(prometheus.NewRegistry()) @@ -416,6 +560,53 @@ func TestExchangeWithClientSecret(t *testing.T) { require.NoError(t, err) } +func TestTokenRevocationResponseOk(t *testing.T) { + t.Parallel() + + ghType := codersdk.EnhancedExternalAuthProviderGitHub.String() + rfcType := codersdk.EnhancedExternalAuthProviderAzureDevops.String() + tests := []struct { + name string + conf *externalauth.Config + resp http.Response + want bool + }{ + { + name: "GH_bad", + conf: &externalauth.Config{Type: ghType}, + resp: http.Response{StatusCode: http.StatusOK}, + want: false, + }, + { + name: "GH_ok", + conf: &externalauth.Config{Type: ghType}, + resp: http.Response{StatusCode: http.StatusNoContent}, + want: true, + }, + { + name: "RFC_ok", + conf: &externalauth.Config{Type: rfcType}, + resp: http.Response{StatusCode: http.StatusOK}, + want: true, + }, + { + name: "RFC_bad", + conf: &externalauth.Config{Type: rfcType}, + resp: http.Response{StatusCode: http.StatusNoContent}, + want: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := tc.conf.TokenRevocationResponseOk(&tc.resp) + if tc.want != got { + t.Errorf("unexpected response success, got: %v want: %v", got, tc.want) + } + }) + } +} + func TestConvertYAML(t *testing.T) { t.Parallel() @@ -494,6 +685,17 @@ func TestConvertYAML(t *testing.T) { require.NoError(t, err) require.Equal(t, "https://auth.com?client_id=id&redirect_uri=%2Fexternal-auth%2Fgitlab%2Fcallback&response_type=code&scope=read", config[0].AuthCodeURL("")) }) + + t.Run("RevokeTimeoutSet", func(t *testing.T) { + t.Parallel() + configs, err := externalauth.ConvertConfig(instrument, []codersdk.ExternalAuthConfig{{ + Type: string(codersdk.EnhancedExternalAuthProviderGitLab), + ClientID: "id", + ClientSecret: "secret", + }}, &url.URL{}) + require.NoError(t, err) + require.Equal(t, 10*time.Second, configs[0].RevokeTimeout) + }) } // TestConstantQueryParams verifies a constant query parameter can be set in the @@ -594,11 +796,16 @@ func setupOauth2Test(t *testing.T, settings testConfig) (*oidctest.FakeIDP, *ext ) f := promoauth.NewFactory(prometheus.NewRegistry()) + cid, cs := fake.AppCredentials() config := &externalauth.Config{ InstrumentedOAuth2Config: f.New("test-oauth2", fake.OIDCConfig(t, nil, settings.CoderOIDCConfigOpts...)), - ID: providerID, - ValidateURL: fake.WellknownConfig().UserInfoURL, + ID: providerID, + ClientID: cid, + ClientSecret: cs, + ValidateURL: fake.WellknownConfig().UserInfoURL, + RevokeURL: fake.WellknownConfig().RevokeURL, + RevokeTimeout: 1 * time.Second, } settings.ExternalAuthOpt(config) diff --git a/coderd/externalauth_test.go b/coderd/externalauth_test.go index 68244bf3a4..5219b54344 100644 --- a/coderd/externalauth_test.go +++ b/coderd/externalauth_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "net/url" "regexp" + "slices" "strings" "testing" "time" @@ -16,6 +17,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/coderdtest/oidctest" @@ -158,9 +160,29 @@ func TestExternalAuthManagement(t *testing.T) { t.Parallel() const githubID = "fake-github" const gitlabID = "fake-gitlab" + const slackID = "fake-slack" + const azureID = "fake-azure" + ghRevokeCalled := false + slRevokeCalled := false + azRevokeCalled := false - github := oidctest.NewFakeIDP(t, oidctest.WithServing()) + ghRevoke := func() (int, error) { + ghRevokeCalled = true + return http.StatusNoContent, nil + } + slRevoke := func() (int, error) { + slRevokeCalled = true + return http.StatusOK, nil + } + azRevoke := func() (int, error) { + azRevokeCalled = true + return http.StatusForbidden, xerrors.New("some error") + } + + github := oidctest.NewFakeIDP(t, oidctest.WithServing(), oidctest.WithRevokeTokenGitHub(ghRevoke)) gitlab := oidctest.NewFakeIDP(t, oidctest.WithServing()) + slack := oidctest.NewFakeIDP(t, oidctest.WithServing(), oidctest.WithRevokeTokenRFC(slRevoke)) + azure := oidctest.NewFakeIDP(t, oidctest.WithServing(), oidctest.WithRevokeTokenRFC(azRevoke)) owner := coderdtest.New(t, &coderdtest.Options{ ExternalAuthConfigs: []*externalauth.Config{ @@ -170,6 +192,13 @@ func TestExternalAuthManagement(t *testing.T) { gitlab.ExternalAuthConfig(t, gitlabID, nil, func(cfg *externalauth.Config) { cfg.Type = codersdk.EnhancedExternalAuthProviderGitLab.String() }), + slack.ExternalAuthConfig(t, slackID, nil, func(cfg *externalauth.Config) { + cfg.Type = codersdk.EnhancedExternalAuthProviderSlack.String() + cfg.RevokeURL = "" + }), + azure.ExternalAuthConfig(t, azureID, nil, func(cfg *externalauth.Config) { + cfg.Type = codersdk.EnhancedExternalAuthProviderAzureDevopsEntra.String() + }), }, }) ownerUser := coderdtest.CreateFirstUser(t, owner) @@ -180,25 +209,47 @@ func TestExternalAuthManagement(t *testing.T) { // List auths without any links. list, err := client.ListExternalAuths(ctx) require.NoError(t, err) - require.Len(t, list.Providers, 2) + require.Len(t, list.Providers, 4) require.Len(t, list.Links, 0) - // Log into github + // Log into github and slack github.ExternalLogin(t, client) + slack.ExternalLogin(t, client) + azure.ExternalLogin(t, client) list, err = client.ListExternalAuths(ctx) require.NoError(t, err) - require.Len(t, list.Providers, 2) - require.Len(t, list.Links, 1) - require.Equal(t, list.Links[0].ProviderID, githubID) + require.Len(t, list.Providers, 4) + require.Len(t, list.Links, 3) + require.True(t, slices.ContainsFunc(list.Links, func(l codersdk.ExternalAuthLink) bool { return l.ProviderID == githubID })) + require.True(t, slices.ContainsFunc(list.Links, func(l codersdk.ExternalAuthLink) bool { return l.ProviderID == slackID })) + require.True(t, slices.ContainsFunc(list.Links, func(l codersdk.ExternalAuthLink) bool { return l.ProviderID == azureID })) + require.False(t, ghRevokeCalled) + require.False(t, slRevokeCalled) + require.False(t, azRevokeCalled) // Unlink - err = client.UnlinkExternalAuthByID(ctx, githubID) + r, err := client.UnlinkExternalAuthByID(ctx, githubID) require.NoError(t, err) + require.True(t, r.TokenRevoked) + require.Empty(t, r.TokenRevocationError) + require.True(t, ghRevokeCalled) + + r, err = client.UnlinkExternalAuthByID(ctx, slackID) + require.NoError(t, err) + require.False(t, r.TokenRevoked) + require.Empty(t, r.TokenRevocationError) + require.False(t, slRevokeCalled) + + r, err = client.UnlinkExternalAuthByID(ctx, azureID) + require.NoError(t, err) + require.False(t, r.TokenRevoked) + require.Contains(t, r.TokenRevocationError, "some error") + require.True(t, azRevokeCalled) list, err = client.ListExternalAuths(ctx) require.NoError(t, err) - require.Len(t, list.Providers, 2) + require.Len(t, list.Providers, 4) require.Len(t, list.Links, 0) }) t.Run("RefreshAllProviders", func(t *testing.T) { diff --git a/coderd/promoauth/oauth2.go b/coderd/promoauth/oauth2.go index 18813bbbc6..a89875cb75 100644 --- a/coderd/promoauth/oauth2.go +++ b/coderd/promoauth/oauth2.go @@ -19,6 +19,7 @@ const ( SourceTokenSource Oauth2Source = "TokenSource" SourceAppInstallations Oauth2Source = "AppInstallations" SourceAuthorizeDevice Oauth2Source = "AuthorizeDevice" + SourceRevoke Oauth2Source = "Revoke" SourceGitAPIAuthUser Oauth2Source = "GitAPIAuthUser" SourceGitAPIListEmails Oauth2Source = "GitAPIListEmails" diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 47ae4f51c1..763da724c6 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -735,6 +735,7 @@ type ExternalAuthConfig struct { AuthURL string `json:"auth_url" yaml:"auth_url"` TokenURL string `json:"token_url" yaml:"token_url"` ValidateURL string `json:"validate_url" yaml:"validate_url"` + RevokeURL string `json:"revoke_url" yaml:"revoke_url"` AppInstallURL string `json:"app_install_url" yaml:"app_install_url"` AppInstallationsURL string `json:"app_installations_url" yaml:"app_installations_url"` NoRefresh bool `json:"no_refresh" yaml:"no_refresh"` diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index fc8f532d8f..ee3f7365d6 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -389,6 +389,7 @@ func TestExternalAuthYAMLConfig(t *testing.T) { AuthURL: "https://example.com/auth", TokenURL: "https://example.com/token", ValidateURL: "https://example.com/validate", + RevokeURL: "https://example.com/revoke", AppInstallURL: "https://example.com/install", AppInstallationsURL: "https://example.com/installations", NoRefresh: true, diff --git a/codersdk/externalauth.go b/codersdk/externalauth.go index 475c55b91b..48c4781605 100644 --- a/codersdk/externalauth.go +++ b/codersdk/externalauth.go @@ -49,9 +49,10 @@ const ( ) type ExternalAuth struct { - Authenticated bool `json:"authenticated"` - Device bool `json:"device"` - DisplayName string `json:"display_name"` + Authenticated bool `json:"authenticated"` + Device bool `json:"device"` + DisplayName string `json:"display_name"` + SupportsRevocation bool `json:"supports_revocation"` // User is the user that authenticated with the provider. User *ExternalAuthUser `json:"user"` @@ -72,6 +73,12 @@ type ListUserExternalAuthResponse struct { Links []ExternalAuthLink `json:"links"` } +type DeleteExternalAuthByIDResponse struct { + // TokenRevoked set to true if token revocation was attempted and was successful + TokenRevoked bool `json:"token_revoked"` + TokenRevocationError string `json:"token_revocation_error,omitempty"` +} + // ExternalAuthLink is a link between a user and an external auth provider. // It excludes information that requires a token to access, so can be statically // built from the database and configs. @@ -87,13 +94,14 @@ type ExternalAuthLink struct { // ExternalAuthLinkProvider are the static details of a provider. type ExternalAuthLinkProvider struct { - ID string `json:"id"` - Type string `json:"type"` - Device bool `json:"device"` - DisplayName string `json:"display_name"` - DisplayIcon string `json:"display_icon"` - AllowRefresh bool `json:"allow_refresh"` - AllowValidate bool `json:"allow_validate"` + ID string `json:"id"` + Type string `json:"type"` + Device bool `json:"device"` + DisplayName string `json:"display_name"` + DisplayIcon string `json:"display_icon"` + AllowRefresh bool `json:"allow_refresh"` + AllowValidate bool `json:"allow_validate"` + SupportsRevocation bool `json:"supports_revocation"` } type ExternalAuthAppInstallation struct { @@ -166,16 +174,22 @@ func (c *Client) ExternalAuthByID(ctx context.Context, provider string) (Externa // UnlinkExternalAuthByID deletes the external auth for the given provider by ID // for the user. This does not revoke the token from the IDP. -func (c *Client) UnlinkExternalAuthByID(ctx context.Context, provider string) error { +func (c *Client) UnlinkExternalAuthByID(ctx context.Context, provider string) (DeleteExternalAuthByIDResponse, error) { + noRevoke := DeleteExternalAuthByIDResponse{TokenRevoked: false} res, err := c.Request(ctx, http.MethodDelete, fmt.Sprintf("/api/v2/external-auth/%s", provider), nil) if err != nil { - return err + return noRevoke, err } defer res.Body.Close() if res.StatusCode != http.StatusOK { - return ReadBodyAsError(res) + return noRevoke, ReadBodyAsError(res) } - return nil + var resp DeleteExternalAuthByIDResponse + err = json.NewDecoder(res.Body).Decode(&resp) + if err != nil { + return noRevoke, err + } + return resp, nil } // ListExternalAuths returns the available external auth providers and the user's diff --git a/codersdk/testdata/githubcfg.yaml b/codersdk/testdata/githubcfg.yaml index c88cea947d..c5e61baa03 100644 --- a/codersdk/testdata/githubcfg.yaml +++ b/codersdk/testdata/githubcfg.yaml @@ -6,6 +6,7 @@ externalAuthProviders: auth_url: https://example.com/auth token_url: https://example.com/token validate_url: https://example.com/validate + revoke_url: https://example.com/revoke app_install_url: https://example.com/install app_installations_url: https://example.com/installations no_refresh: true diff --git a/docs/admin/external-auth/index.md b/docs/admin/external-auth/index.md index 5d3ade987e..634a22b23c 100644 --- a/docs/admin/external-auth/index.md +++ b/docs/admin/external-auth/index.md @@ -207,6 +207,7 @@ CODER_EXTERNAL_AUTH_0_ID="primary-github" CODER_EXTERNAL_AUTH_0_TYPE=github CODER_EXTERNAL_AUTH_0_CLIENT_ID=xxxxxx CODER_EXTERNAL_AUTH_0_CLIENT_SECRET=xxxxxxx +CODER_EXTERNAL_AUTH_0_REVOKE_URL=https://api.github.com/applications//grant ``` When configuring your GitHub OAuth application, set the @@ -246,6 +247,7 @@ CODER_EXTERNAL_AUTH_0_CLIENT_SECRET=xxxxxxx CODER_EXTERNAL_AUTH_0_VALIDATE_URL="https://gitlab.example.com/oauth/token/info" CODER_EXTERNAL_AUTH_0_AUTH_URL="https://gitlab.example.com/oauth/authorize" CODER_EXTERNAL_AUTH_0_TOKEN_URL="https://gitlab.example.com/oauth/token" +CODER_EXTERNAL_AUTH_0_REVOKE_URL="https://gitlab.example.com/oauth/revoke" CODER_EXTERNAL_AUTH_0_REGEX=gitlab\.example\.com ``` @@ -265,6 +267,7 @@ provider deployments. ```env CODER_EXTERNAL_AUTH_0_AUTH_URL="https://github.example.com/oauth/authorize" CODER_EXTERNAL_AUTH_0_TOKEN_URL="https://github.example.com/oauth/token" +CODER_EXTERNAL_AUTH_0_REVOKE_URL="https://github.example.com/oauth/revoke" CODER_EXTERNAL_AUTH_0_VALIDATE_URL="https://example.com/oauth/token/info" CODER_EXTERNAL_AUTH_0_REGEX=github\.company\.com ``` @@ -341,5 +344,6 @@ CODER_EXTERNAL_AUTH_1_CLIENT_SECRET=xxxxxxx CODER_EXTERNAL_AUTH_1_REGEX=github\.example\.com CODER_EXTERNAL_AUTH_1_AUTH_URL="https://github.example.com/login/oauth/authorize" CODER_EXTERNAL_AUTH_1_TOKEN_URL="https://github.example.com/login/oauth/access_token" +CODER_EXTERNAL_AUTH_1_REVOKE_URL="https://github.example.com/login/oauth/revoke" CODER_EXTERNAL_AUTH_1_VALIDATE_URL="https://github.example.com/api/v3/user" ``` diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index eab4e5e417..a89376db38 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -259,6 +259,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "mcp_url": "string", "no_refresh": true, "regex": "string", + "revoke_url": "string", "scopes": [ "string" ], diff --git a/docs/reference/api/git.md b/docs/reference/api/git.md index fc36f184a7..05c572c77e 100644 --- a/docs/reference/api/git.md +++ b/docs/reference/api/git.md @@ -80,6 +80,7 @@ curl -X GET http://coder-server:8080/api/v2/external-auth/{externalauth} \ "id": 0 } ], + "supports_revocation": true, "user": { "avatar_url": "string", "id": 0, @@ -105,6 +106,7 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl curl -X DELETE http://coder-server:8080/api/v2/external-auth/{externalauth} \ + -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` @@ -116,11 +118,22 @@ curl -X DELETE http://coder-server:8080/api/v2/external-auth/{externalauth} \ |----------------|------|----------------|----------|-----------------| | `externalauth` | path | string(string) | true | Git Provider ID | +### Example responses + +> 200 Response + +```json +{ + "token_revocation_error": "string", + "token_revoked": true +} +``` + ### Responses -| Status | Meaning | Description | Schema | -|--------|---------------------------------------------------------|-------------|--------| -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | | +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|-------------|----------------------------------------------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.DeleteExternalAuthByIDResponse](schemas.md#codersdkdeleteexternalauthbyidresponse) | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 6e41efd108..1f3f231f45 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -2196,6 +2196,22 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `allow_path_app_sharing` | boolean | false | | | | `allow_path_app_site_owner_access` | boolean | false | | | +## codersdk.DeleteExternalAuthByIDResponse + +```json +{ + "token_revocation_error": "string", + "token_revoked": true +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|--------------------------|---------|----------|--------------|--------------------------------------------------------------------------------| +| `token_revocation_error` | string | false | | | +| `token_revoked` | boolean | false | | Token revoked set to true if token revocation was attempted and was successful | + ## codersdk.DeleteWebpushSubscription ```json @@ -2363,6 +2379,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "mcp_url": "string", "no_refresh": true, "regex": "string", + "revoke_url": "string", "scopes": [ "string" ], @@ -2867,6 +2884,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "mcp_url": "string", "no_refresh": true, "regex": "string", + "revoke_url": "string", "scopes": [ "string" ], @@ -3509,6 +3527,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "id": 0 } ], + "supports_revocation": true, "user": { "avatar_url": "string", "id": 0, @@ -3521,15 +3540,16 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ### Properties -| Name | Type | Required | Restrictions | Description | -|-------------------|---------------------------------------------------------------------------------------|----------|--------------|-------------------------------------------------------------------------| -| `app_install_url` | string | false | | App install URL is the URL to install the app. | -| `app_installable` | boolean | false | | App installable is true if the request for app installs was successful. | -| `authenticated` | boolean | false | | | -| `device` | boolean | false | | | -| `display_name` | string | false | | | -| `installations` | array of [codersdk.ExternalAuthAppInstallation](#codersdkexternalauthappinstallation) | false | | Installations are the installations that the user has access to. | -| `user` | [codersdk.ExternalAuthUser](#codersdkexternalauthuser) | false | | User is the user that authenticated with the provider. | +| Name | Type | Required | Restrictions | Description | +|-----------------------|---------------------------------------------------------------------------------------|----------|--------------|-------------------------------------------------------------------------| +| `app_install_url` | string | false | | App install URL is the URL to install the app. | +| `app_installable` | boolean | false | | App installable is true if the request for app installs was successful. | +| `authenticated` | boolean | false | | | +| `device` | boolean | false | | | +| `display_name` | string | false | | | +| `installations` | array of [codersdk.ExternalAuthAppInstallation](#codersdkexternalauthappinstallation) | false | | Installations are the installations that the user has access to. | +| `supports_revocation` | boolean | false | | | +| `user` | [codersdk.ExternalAuthUser](#codersdkexternalauthuser) | false | | User is the user that authenticated with the provider. | ## codersdk.ExternalAuthAppInstallation @@ -3573,6 +3593,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "mcp_url": "string", "no_refresh": true, "regex": "string", + "revoke_url": "string", "scopes": [ "string" ], @@ -3601,6 +3622,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `no_refresh` | boolean | false | | | |`regex`|string|false||Regex allows API requesters to match an auth config by a string (e.g. coder.com) instead of by it's type. Git clone makes use of this by parsing the URL from: 'Username for "https://github.com":' And sending it to the Coder server to match against the Regex.| +|`revoke_url`|string|false||| |`scopes`|array of string|false||| |`token_url`|string|false||| |`type`|string|false||Type is the type of external auth config.| @@ -12868,6 +12890,7 @@ None "mcp_url": "string", "no_refresh": true, "regex": "string", + "revoke_url": "string", "scopes": [ "string" ], diff --git a/site/src/api/api.ts b/site/src/api/api.ts index caf0f5c094..e20dab2471 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -27,6 +27,7 @@ import { delay } from "../utils/delay"; import { OneWayWebSocket } from "../utils/OneWayWebSocket"; import { type FieldError, isApiError } from "./errors"; import type { + DeleteExternalAuthByIDResponse, DynamicParametersRequest, PostWorkspaceUsageRequest, } from "./typesGenerated"; @@ -1727,7 +1728,9 @@ class ApiMethods { return resp.data; }; - unlinkExternalAuthProvider = async (provider: string): Promise => { + unlinkExternalAuthProvider = async ( + provider: string, + ): Promise => { const resp = await this.axios.delete(`/api/v2/external-auth/${provider}`); return resp.data; }; diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 44b4bf7370..3a6449d5aa 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -775,6 +775,12 @@ export interface DatabaseReport extends BaseReport { readonly threshold_ms: number; } +// From codersdk/externalauth.go +export interface DeleteExternalAuthByIDResponse { + readonly token_revoked: boolean; + readonly token_revocation_error?: string; +} + // From codersdk/notifications.go export interface DeleteWebpushSubscription { readonly endpoint: string; @@ -988,6 +994,7 @@ export interface ExternalAuth { readonly authenticated: boolean; readonly device: boolean; readonly display_name: string; + readonly supports_revocation: boolean; readonly user: ExternalAuthUser | null; readonly app_installable: boolean; readonly installations: readonly ExternalAuthAppInstallation[]; @@ -1009,6 +1016,7 @@ export interface ExternalAuthConfig { readonly auth_url: string; readonly token_url: string; readonly validate_url: string; + readonly revoke_url: string; readonly app_install_url: string; readonly app_installations_url: string; readonly no_refresh: boolean; @@ -1057,6 +1065,7 @@ export interface ExternalAuthLinkProvider { readonly display_icon: string; readonly allow_refresh: boolean; readonly allow_validate: boolean; + readonly supports_revocation: boolean; } // From codersdk/externalauth.go diff --git a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx index f44ecd4144..a558918e71 100644 --- a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx +++ b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx @@ -15,6 +15,7 @@ const meta: Meta = { auth_url: "", token_url: "", validate_url: "", + revoke_url: "", app_install_url: "https://github.com/apps/coder/installations/new", app_installations_url: "", no_refresh: false, diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx index 7d7f49b3f5..976b923f16 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx @@ -4,6 +4,7 @@ import { unlinkExternalAuths, validateExternalAuth, } from "api/queries/externalAuth"; +import type { ExternalAuthLinkProvider } from "api/typesGenerated"; import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { type FC, useState } from "react"; @@ -18,7 +19,7 @@ const ExternalAuthPage: FC = () => { const [unlinked, setUnlinked] = useState(0); const externalAuthsQuery = useQuery(externalAuths()); - const [appToUnlink, setAppToUnlink] = useState(); + const [appToUnlink, setAppToUnlink] = useState(); const unlinkAppMutation = useMutation(unlinkExternalAuths(queryClient)); const validateAppMutation = useMutation(validateExternalAuth(queryClient)); @@ -29,8 +30,8 @@ const ExternalAuthPage: FC = () => { getAuthsError={externalAuthsQuery.error} auths={externalAuthsQuery.data} unlinked={unlinked} - onUnlinkExternalAuth={(providerID: string) => { - setAppToUnlink(providerID); + onUnlinkExternalAuth={(provider) => { + setAppToUnlink(provider); }} onValidateExternalAuth={async (providerID: string) => { try { @@ -50,21 +51,25 @@ const ExternalAuthPage: FC = () => { }} /> setAppToUnlink(undefined)} onConfirm={async () => { try { - await unlinkAppMutation.mutateAsync(appToUnlink!); + const unlinkResp = await unlinkAppMutation.mutateAsync( + appToUnlink?.id!, + ); // setAppToUnlink closes the modal setAppToUnlink(undefined); // refetch repopulates the external auth data @@ -72,8 +77,11 @@ const ExternalAuthPage: FC = () => { // this tells our child components to refetch their data // as at least 1 provider was unlinked. setUnlinked(unlinked + 1); - - displaySuccess("Successfully unlinked the oauth2 application."); + displaySuccess( + unlinkResp.token_revoked + ? "Successfully deleted external auth link and revoked token from the OAuth2 provider." + : "Successfully deleted external auth link. Token has NOT been revoked from the OAuth2 provider.", + ); } catch (e) { displayError(getErrorMessage(e, "Error unlinking application.")); } diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx index 617e3bde52..db229b2890 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx @@ -37,7 +37,7 @@ type ExternalAuthPageViewProps = { getAuthsError?: unknown; unlinked: number; auths?: ListUserExternalAuthResponse; - onUnlinkExternalAuth: (provider: string) => void; + onUnlinkExternalAuth: (provider: ExternalAuthLinkProvider) => void; onValidateExternalAuth: (provider: string) => void; }; @@ -83,7 +83,7 @@ export const ExternalAuthPageView: FC = ({ unlinked={unlinked} link={auths.links.find((l) => l.provider_id === app.id)} onUnlinkExternalAuth={() => { - onUnlinkExternalAuth(app.id); + onUnlinkExternalAuth(app); }} onValidateExternalAuth={() => { onValidateExternalAuth(app.id); diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 4ce66809e8..0530c04ad3 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -4437,6 +4437,7 @@ export const MockGithubExternalProvider: TypesGen.ExternalAuthLinkProvider = { display_name: "GitHub", allow_refresh: true, allow_validate: true, + supports_revocation: false, }; export const MockGithubAuthLink: TypesGen.ExternalAuthLink = {