diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index 26b8258206f33..44472bce3d734 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -296,7 +296,7 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon }).Token() if err != nil { return write(http.StatusUnauthorized, codersdk.Response{ - Message: "Could not refresh expired Oauth token.", + Message: "Could not refresh expired Oauth token. Try re-authenticating to resolve this issue.", Detail: err.Error(), }) } diff --git a/coderd/workspaceapps/apptest/apptest.go b/coderd/workspaceapps/apptest/apptest.go index 8851cf5113b55..09e734fa92e8c 100644 --- a/coderd/workspaceapps/apptest/apptest.go +++ b/coderd/workspaceapps/apptest/apptest.go @@ -125,7 +125,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) { resp, err := requestWithRetries(ctx, t, appDetails.AppClient(t), http.MethodGet, appDetails.PathAppURL(appDetails.Apps.Owner).String(), nil) require.NoError(t, err) defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) + require.Equal(t, http.StatusForbidden, resp.StatusCode) body, err := io.ReadAll(resp.Body) require.NoError(t, err) require.Contains(t, string(body), "Path-based applications are disabled") diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go index d252f2c15f023..492d8455479f0 100644 --- a/coderd/workspaceapps/proxy.go +++ b/coderd/workspaceapps/proxy.go @@ -256,8 +256,8 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request, func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) { if s.DisablePathApps { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusUnauthorized, - Title: "Unauthorized", + Status: http.StatusForbidden, + Title: "Forbidden", Description: "Path-based applications are disabled on this Coder deployment by the administrator.", RetryEnabled: false, DashboardURL: s.DashboardURL.String(), diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 3a44f4a3caa83..53e8308b5d65e 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -359,7 +359,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req } if organization.ID != template.OrganizationID { - httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{ + httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ Message: fmt.Sprintf("Template is not in organization %q.", organization.Name), }) return diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 6fba7c9fba099..e3749d72953fe 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -447,7 +447,7 @@ func TestPostWorkspacesByOrganization(t *testing.T) { require.Error(t, err) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) - require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) + require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) }) t.Run("AlreadyExists", func(t *testing.T) { diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 8e76a36b1df14..2927ea88b9d1e 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -1,12 +1,14 @@ package coderd_test import ( + "context" "net/http" "regexp" "testing" "github.com/golang-jwt/jwt/v4" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" @@ -357,6 +359,40 @@ func TestUserOIDC(t *testing.T) { runner.ForceRefresh(t, client, claims) } }) + + t.Run("FailedRefresh", func(t *testing.T) { + t.Parallel() + + runner := setupOIDCTest(t, oidcTestConfig{ + FakeOpts: []oidctest.FakeIDPOpt{ + oidctest.WithRefreshHook(func(_ string) error { + // Always "expired" refresh token. + return xerrors.New("refresh token is expired") + }), + }, + Config: func(cfg *coderd.OIDCConfig) { + cfg.AllowSignups = true + }, + }) + + claims := jwt.MapClaims{ + "email": "alice@coder.com", + } + // Login a new client that signs up + client, resp := runner.Login(t, claims) + require.Equal(t, http.StatusOK, resp.StatusCode) + + // Expire the token, cause a refresh + runner.ExpireOauthToken(t, client) + + // This should fail because the oauth token refresh should fail. + _, err := client.User(context.Background(), codersdk.Me) + require.Error(t, err) + var apiError *codersdk.Error + require.ErrorAs(t, err, &apiError) + require.Equal(t, http.StatusUnauthorized, apiError.StatusCode()) + require.ErrorContains(t, apiError, "refresh") + }) }) } @@ -576,14 +612,16 @@ type oidcTestRunner struct { // ForceRefresh will use an authenticated codersdk.Client, and force their // OIDC token to be expired and require a refresh. The refresh will use the claims provided. // It just calls the /users/me endpoint to trigger the refresh. - ForceRefresh func(t *testing.T, client *codersdk.Client, idToken jwt.MapClaims) + ForceRefresh func(t *testing.T, client *codersdk.Client, idToken jwt.MapClaims) + ExpireOauthToken func(t *testing.T, client *codersdk.Client) } type oidcTestConfig struct { Userinfo jwt.MapClaims // Config allows modifying the Coderd OIDC configuration. - Config func(cfg *coderd.OIDCConfig) + Config func(cfg *coderd.OIDCConfig) + FakeOpts []oidctest.FakeIDPOpt } func (r *oidcTestRunner) AssertRoles(t *testing.T, userIdent string, roles []string) { @@ -633,10 +671,12 @@ func setupOIDCTest(t *testing.T, settings oidcTestConfig) *oidcTestRunner { t.Helper() fake := oidctest.NewFakeIDP(t, - oidctest.WithStaticUserInfo(settings.Userinfo), - oidctest.WithLogging(t, nil), - // Run fake IDP on a real webserver - oidctest.WithServing(), + append([]oidctest.FakeIDPOpt{ + oidctest.WithStaticUserInfo(settings.Userinfo), + oidctest.WithLogging(t, nil), + // Run fake IDP on a real webserver + oidctest.WithServing(), + }, settings.FakeOpts...)..., ) ctx := testutil.Context(t, testutil.WaitMedium) @@ -665,5 +705,8 @@ func setupOIDCTest(t *testing.T, settings oidcTestConfig) *oidcTestRunner { ForceRefresh: func(t *testing.T, client *codersdk.Client, idToken jwt.MapClaims) { helper.ForceRefresh(t, api.Database, client, idToken) }, + ExpireOauthToken: func(t *testing.T, client *codersdk.Client) { + helper.ExpireOauthToken(t, api.Database, client) + }, } } diff --git a/site/src/components/RequireAuth/RequireAuth.tsx b/site/src/components/RequireAuth/RequireAuth.tsx index 4ece776e282a3..5d237be195638 100644 --- a/site/src/components/RequireAuth/RequireAuth.tsx +++ b/site/src/components/RequireAuth/RequireAuth.tsx @@ -6,7 +6,6 @@ import { embedRedirect } from "../../utils/redirect" import { FullScreenLoader } from "../Loader/FullScreenLoader" import { DashboardProvider } from "components/Dashboard/DashboardProvider" import { ProxyProvider } from "contexts/ProxyContext" -import { getErrorDetail } from "api/errors" export const RequireAuth: FC = () => { const [authState, authSend] = useAuth() @@ -23,10 +22,7 @@ export const RequireAuth: FC = () => { // 401 Unauthorized // If we encountered an authentication error, then our token is probably // invalid and we should update the auth state to reflect that. - if ( - error.response.status === 401 && - getErrorDetail(error)?.startsWith("API key expired") - ) { + if (error.response.status === 401) { authSend("SIGN_OUT") } diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index 16ca542a5d916..ddc9f1ee880b4 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -447,7 +447,7 @@ describe("UsersPage", () => { server.use( rest.put(`/api/v2/users/${MockUser.id}/roles`, (req, res, ctx) => { return res( - ctx.status(401), + ctx.status(400), ctx.json({ message: "message from the backend" }), ) }), diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index caf0399e13fce..f3362edec7881 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -192,7 +192,7 @@ export type AuthEvent = | { type: "UPDATE_PROFILE"; data: TypesGen.UpdateUserProfileRequest } export const authMachine = - /** @xstate-layout N4IgpgJg5mDOIC5QEMCuAXAFgZXc9YAdADYD2yEAlgHZQCS1l6lyxAghpgCL7IDEEUtSI0AbqQDWRNFlz4iZCjXqNmrDlh54EY0gGN8lIQG0ADAF0z5xKAAOpWEyPUbIAB6IAjAFZThACwATAAcAGwAzOGewZ7hoYH+ADQgAJ6Igaam3oSeGf7BcbnBAJyBAL5lyTI4eAQk5FS0DE7qnFr8gsKEulKE1XJ1io0qLextvDrU4gbMJhbGntZIIPaOsy7LHgg+fkFhkdGx8Ump6bEA7ITn56HnkaFFpRVVnAMKDcrNamOavAJCIimkmkr1q7yUTVULB+3AmuhmzisxkCSzsDicQlcWx2ARCESiMTiCWSaQQgUC5124UCxQKDxCT0qIH6YPqEJG3w0sLwfDAACc+aQ+YRbMR8AAzIUAWz6oPkbOGX2hXPak2mhjmlgsrlWGI2oGxvlx+wJR2JpzJ-lM4UIphKgXuj3KTJZ8scUGEEAA8hg+Ng6ABxAByAH06EGrDr0essV5TOdslloqZPKZ-Od-NESV4Hjb-OESqFvAyEudgs9mXK6u7GJD-l0ekQawxI8tdTHNl5ybtPKnPKFin3ivns2TU3mi1FrmFSuEK67q5QPZ9qLyBUKRWL0JK+TLm9RW2i1s5Y9tu4Q8ZksiFgmnR93PLbad5h6FMgm5y6q02l56GH7A1DL0AFUABVDxWaMT07BAogHQhgmCfwBxvbwiwKe87WyYIM1ibxPHzG5PxeWRWRrSAGBFQVxUoYgRAgOi+GAgAFLg2FAgBRENmIAJS9AAxOgABkOIg9toINdIi0fcJzn7dMshfXtR2ibx-EIUJkOKbwiVw4p52-QhyIgSjbGo2iiFQWwIEMWhmPMxjOkBcRegXH8PQo6gqNIGi6MIKybOYOyHLANV9A1A95m1NsoMxGDPGfHIiPCfxvDSwcCJUwsci0nT4j0gzSLdX9PO83zLOs2yoHsnyLLXQVhVFCVpVlIrFw8kyvLM2q-ICqqavKsKEU1MTYv1dwvESzxktS9LexOUlonyHKBzyilM30r82vc2soB9dB62c4EjN-fbRuPOLJNghK-HJckX2KFLimHFTSkCQhZIKUwByQotnRImpiuXWh9sO7ogV6GszsWKMLvGrY4n8dSbn8bTfFyXx01e8JsLU580unG5CsB9rdtB-kGs3ZrdxOj0zuio89VPWTTHenHTEegpwm+nDwkwzSPuKIjEPOckkOJt5CD0IQaKgVA+WUUDMDAfjKD5WB0GA2B+QA4MwwjBnILh09b1CHIOdTN8Uoe+9pve0WhZKHHNJRiomWoUgIDgVw3NhpmYIAWm8YIAhxule1zfNilCFTi2yTno8pfICwpCXWSGFdRhVXg-Y7K6A9TUPg8KftYijmOLUOdTQhiDJShuVMEjToHPX23OJIm2CExDuT7QiLJEKy76L3wpDrmteJEOb0mV3by7O+jvwUodEI7hw-MK9JQIjgvGlSkQyI01Caeds8uf4a8fDiiuSk7VmpTMsr24bQIlDAluc5HruE-ab-LqQvPqeHwtwb6ZCQmlB+C0vA3D8C7FMCYdJxG8D-YypkQrdAYmAQBMFt7pg+nJOBc1PBZTUhpBS000r3GQVtEmp9OplQshgvyHsOLrj5Ngq629tL4PkpSIhr0ggrW0rpTMOEUElXod1cqTCiAUyFBwzuXDsiyV4YpDKmFC6v2EflUR5xxEdTQT1CqgVlADQsgo7EDxsjjzvhAjKUDtjZS0WtAqNDJY1mUG3GKxsYLLxtHcGkxwCIpnyPzNmelpoDnyHotxrJpbUFlvLRWytVbq01trdh3j-ZXTSn4BKyE7Q3hrsEbwttRZXBpGWR6aF0yaTdmUIAA */ + /** @xstate-layout N4IgpgJg5mDOIC5QEMCuAXAFgZXc9YAdADYD2yEAlgHZQCS1l6lyxAghpgCL7IDEEUtSI0AbqQDWRNFlz4iZCjXqNmrDlh54EY0gGN8lIQG0ADAF0z5xKAAOpWEyPUbIAB6IAjAFZThACwATAAcAGwAzOGewZ7hoYH+ADQgAJ6Igaam3oSeGf7BcbnBAJyBAL5lyTI4eAQk5FS0DE7qnFr8gsKEulKE1XJ1io0qLextvDrU4gbMJhbGntZIIPaOsy7LHgg+fkFhkdGx8Ump6bEA7ITn56HnkaFFpRVVnAMKDcrNamOavAJCIimkmkr1q7yUTVULB+3AmuhmzisxkCSzsDicQlcWx2ARCESiMTiCWSaQQgUC5124UCxQKDxCT0qIH6YPqEJG3w0sLwfDAACc+aQ+YRbMR8AAzIUAWz6oPkbOGX2hXPak2mhjmlgsrlWGI2oGxvlx+wJR2JpzJ-lM4UIphKgXuj3KTJZ8scUGEEAA8hg+Ng6ABxAByAH06EGrDr0essV5TOdslloqZPKZ-Od-NESV4Hjb-OESqFvAyEudgs9mXK6u7GJD-l0ekQawxI8tdTHNl5ybtPKnPKFin3ivns2TU3mi1FrmFSuEK67q5QPZ9qLyBUKRWL0JK+TLm9RW2i1s5Y9tu4Q8ZksiFgmnR93PLbad5h6FMgm5y6q02l56GH7A1DL0AFUABVDxWaMT07BAogHQhgmCfwBxvbwiwKe87WyYIM1ibxPHzG5PxeWRWRrSAGBFQVxUoYgRAgOi+GAgAFLg2FAgBRENmIAJS9AAxOgABkOIg9toINdIi0fcJzn7dMshfXtR2ibx-EIUJkOKbwiVw4p52-QhyIgSjbGo2iiFQWwIEMWhmPMxjOkBcRegXH8PQo6gqNIGi6MIKybOYOyHLANV9A1A95m1NsoMxGDPGfHIiPCfxvDSwcCJUwsci0nT4j0gzSLdX9PO83zLOs2yoHsnyLLXQVhVFCVpVlIrFw8kyvLM2q-ICqqavKsKEU1MTYv1dwvESzxktS9LexOUlonyHKBzyilM30r82vc2soB9dB62c4EjN-fbRuPOLJNghK-HJckX2KFLimHFTSkCQhZIKUwByQotnRImpiuXWh9sO7ogV6GszsWKMLvGrY4n8dSbn8bTfFyXx01e8JsLU580unG5CsB9rdtB-kGs3ZrdxOj0zuio89VPWTTHenHTEegpwm+nDwkwzSPuKIjEPOckkOJt5CD0IQaKgVA+WUUDMDAfjKD5WB0GA2B+QA4MwwjBnILh09b1CHIOdTN8Uoe+9pve0WhZKHHNJRiomWoUgIDgVw3NhpmYIAWlCFS3w04cCwyDmKWpYjK22hUV1GFVeD9jsroD1MAhxule1zfNimDi1DnU0IYgyUoblTBIJbIkrvQwVOJIm2CE2CK5olKCIskQrLvovfCkOua14kQmugd2hhG8u5uC78FKHRCO4cPzQvSUCI4LxpUpEMiNNQjH0nPKn+GvDiM3KW52dcO+vmLQSNuEvzRC0uQtDZIPnbSu68rj9PAiCKuNaKOslMw31HJEbIA5874SvOEbSH9aZ-i6iFboDEwC-3igXM28RfB2kCH9I4o4gg2kflaeSalyShH3ltEmn9OplQsqgvyHsOLrj5Bgq669tIfTki7RSGVXpBBWtpXSmYcIIOMqZFBlA0GEApkKDhzcuHZFkvJSkc1PCYUzgRVaojojnAkXXKRPUKqBWUANCyijsQPGyEPO0s0lKZSLtlHRIj8obUMcDPaDcYrGxgvPG0dwaTHAIimfI-M2Z6WmlA8RNDJbS2oLLeWitlaq3VprbW7DfH+yumlPwj83zBBvKXYI3hbaiyuDSMsj00Lpk0m7MoQA */ createMachine( { id: "authState",