diff --git a/cli/server.go b/cli/server.go index 097dd9460c57c..88bd6b2365efd 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 435ed2879c9a3..8f63ad2055b35 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -76,6 +76,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", @@ -87,6 +88,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) @@ -94,6 +96,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 2f45203b6b637..ce39ca02859a7 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": [ @@ -13313,6 +13334,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 b94b2307d3bad..6b84b50893888 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": [ @@ -11949,6 +11968,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 a76f6447dcabd..d5215b9964a14 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 a07f6d486c497..23ae7e9fe2654 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 24ebe13d03074..3d449e8e34e28 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`.. @@ -385,6 +394,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 @@ -611,10 +697,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, @@ -776,6 +866,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(/.*)?$`, @@ -801,6 +892,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, ".", `\.`)), } } @@ -942,6 +1034,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 8e46566ed2738..670d1cbf1123b 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 68244bf3a49c4..5219b54344320 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 + + 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()) + 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 18813bbbc6bc7..a89875cb75508 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 c86297307f922..b63ccaa55b366 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 c113d46cc50e4..bb90aaac1b2f7 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 475c55b91bed3..48c4781605d07 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 50c16fb30f63c..5be1c0f33a641 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 5d3ade987ee41..634a22b23c9bb 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 925e74a022d54..74eae2abcf4c6 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -256,6 +256,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "id": "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 fc36f184a7c97..05c572c77e880 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 f8e0ea267f8af..624bef797dd4d 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 @@ -2360,6 +2376,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "id": "string", "no_refresh": true, "regex": "string", + "revoke_url": "string", "scopes": [ "string" ], @@ -2861,6 +2878,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "id": "string", "no_refresh": true, "regex": "string", + "revoke_url": "string", "scopes": [ "string" ], @@ -3503,6 +3521,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "id": 0 } ], + "supports_revocation": true, "user": { "avatar_url": "string", "id": 0, @@ -3515,15 +3534,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 @@ -3564,6 +3584,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "id": "string", "no_refresh": true, "regex": "string", + "revoke_url": "string", "scopes": [ "string" ], @@ -3589,6 +3610,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.| @@ -12852,6 +12874,7 @@ None "id": "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 caf0f5c0944bb..e20dab247157a 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 9f10630ab915f..495ff60c47ab0 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; @@ -1054,6 +1062,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 5184219b38cca..e554bdc1c9435 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 7d7f49b3f5c07..976b923f1639c 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 617e3bde52b34..db229b28900f9 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 4ce66809e8267..0530c04ad3efa 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 = {