From 01f06671a1312cfa09c2af6e8d0cc87d96c0febf Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 13 Feb 2026 08:19:07 -0600 Subject: [PATCH] chore: return 404, not 400 if missing or authz deny (#22069) --- coderd/apikey_test.go | 2 +- coderd/httpmw/userparam.go | 8 ++++ coderd/httpmw/userparam_test.go | 48 +++++++++++++++++++++- coderd/notifications_test.go | 2 +- coderd/users_test.go | 6 +-- enterprise/cli/organizationmembers_test.go | 2 +- enterprise/coderd/users_test.go | 4 +- enterprise/members_test.go | 2 +- 8 files changed, 64 insertions(+), 10 deletions(-) diff --git a/coderd/apikey_test.go b/coderd/apikey_test.go index 65feb1c9cb..76d4f9bf11 100644 --- a/coderd/apikey_test.go +++ b/coderd/apikey_test.go @@ -400,7 +400,7 @@ func TestAPIKey_Deleted(t *testing.T) { require.Error(t, err) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) - require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) } func TestAPIKey_SetDefault(t *testing.T) { diff --git a/coderd/httpmw/userparam.go b/coderd/httpmw/userparam.go index 2fbcc45848..141f30e535 100644 --- a/coderd/httpmw/userparam.go +++ b/coderd/httpmw/userparam.go @@ -106,6 +106,10 @@ func ExtractUserContext(ctx context.Context, db database.Store, rw http.Response if userID, err := uuid.Parse(userQuery); err == nil { user, err = db.GetUserByID(ctx, userID) if err != nil { + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return database.User{}, false + } httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: userErrorMessage, Detail: fmt.Sprintf("queried user=%q", userQuery), @@ -120,6 +124,10 @@ func ExtractUserContext(ctx context.Context, db database.Store, rw http.Response Username: userQuery, }) if err != nil { + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return database.User{}, false + } httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: userErrorMessage, Detail: fmt.Sprintf("queried user=%q", userQuery), diff --git a/coderd/httpmw/userparam_test.go b/coderd/httpmw/userparam_test.go index 4c1fdd3458..22eb72de1f 100644 --- a/coderd/httpmw/userparam_test.go +++ b/coderd/httpmw/userparam_test.go @@ -71,7 +71,53 @@ func TestUserParam(t *testing.T) { })).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() - require.Equal(t, http.StatusBadRequest, res.StatusCode) + // User "ben" doesn't exist, so expect 404. + require.Equal(t, http.StatusNotFound, res.StatusCode) + }) + + t.Run("NotFoundByUsername", func(t *testing.T) { + t.Parallel() + db, rw, r := setup(t) + + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { + r = returnedRequest + })).ServeHTTP(rw, r) + + routeContext := chi.NewRouteContext() + routeContext.URLParams.Add("user", "nonexistent-user") + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeContext)) + httpmw.ExtractUserParam(db)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.WriteHeader(http.StatusOK) + })).ServeHTTP(rw, r) + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusNotFound, res.StatusCode) + }) + + t.Run("NotFoundByUUID", func(t *testing.T) { + t.Parallel() + db, rw, r := setup(t) + + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { + r = returnedRequest + })).ServeHTTP(rw, r) + + routeContext := chi.NewRouteContext() + // Use a valid UUID that doesn't exist in the database. + routeContext.URLParams.Add("user", "88888888-4444-4444-4444-121212121212") + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeContext)) + httpmw.ExtractUserParam(db)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.WriteHeader(http.StatusOK) + })).ServeHTTP(rw, r) + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusNotFound, res.StatusCode) }) t.Run("me", func(t *testing.T) { diff --git a/coderd/notifications_test.go b/coderd/notifications_test.go index f49ec8e0ad..f9260f1598 100644 --- a/coderd/notifications_test.go +++ b/coderd/notifications_test.go @@ -150,7 +150,7 @@ func TestNotificationPreferences(t *testing.T) { require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error") // NOTE: ExtractUserParam gets in the way here, and returns a 400 Bad Request instead of a 403 Forbidden. // This is not ideal, and we should probably change this behavior. - require.Equal(t, http.StatusBadRequest, sdkError.StatusCode()) + require.Equal(t, http.StatusNotFound, sdkError.StatusCode()) }) t.Run("Admin may read any users' preferences", func(t *testing.T) { diff --git a/coderd/users_test.go b/coderd/users_test.go index b00aead7d6..3a6ec32cde 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -349,7 +349,7 @@ func TestDeleteUser(t *testing.T) { err := client.DeleteUser(context.Background(), firstUser.UserID) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) - require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) }) t.Run("HasWorkspaces", func(t *testing.T) { t.Parallel() @@ -1010,7 +1010,7 @@ func TestUpdateUserProfile(t *testing.T) { require.ErrorAs(t, err, &apiErr) // Right now, we are raising a BAD request error because we don't support a // user accessing other users info - require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) }) t.Run("ConflictingUsername", func(t *testing.T) { @@ -2602,7 +2602,7 @@ func TestUserAutofillParameters(t *testing.T) { var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) - require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) // u1 should be able to read u2's parameters as u1 is site admin. _, err = client1.UserAutofillParameters( diff --git a/enterprise/cli/organizationmembers_test.go b/enterprise/cli/organizationmembers_test.go index 0569929548..5efef1c158 100644 --- a/enterprise/cli/organizationmembers_test.go +++ b/enterprise/cli/organizationmembers_test.go @@ -64,7 +64,7 @@ func TestRemoveOrganizationMembers(t *testing.T) { buf := new(bytes.Buffer) inv.Stdout = buf err := inv.WithContext(ctx).Run() - require.ErrorContains(t, err, "must be an existing uuid or username") + require.ErrorContains(t, err, "Resource not found or you do not have access to this resource") }) } diff --git a/enterprise/coderd/users_test.go b/enterprise/coderd/users_test.go index 7cfef59fa9..6876ca72b4 100644 --- a/enterprise/coderd/users_test.go +++ b/enterprise/coderd/users_test.go @@ -356,7 +356,7 @@ func TestGrantSiteRoles(t *testing.T) { AssignToUser: uuid.NewString(), Roles: []string{codersdk.RoleOwner}, Error: true, - StatusCode: http.StatusBadRequest, + StatusCode: http.StatusNotFound, }, { Name: "MemberCannotUpdateRoles", @@ -364,7 +364,7 @@ func TestGrantSiteRoles(t *testing.T) { AssignToUser: first.UserID.String(), Roles: []string{}, Error: true, - StatusCode: http.StatusBadRequest, + StatusCode: http.StatusNotFound, }, { // Cannot update your own roles diff --git a/enterprise/members_test.go b/enterprise/members_test.go index 5fc072874a..89e2929cdd 100644 --- a/enterprise/members_test.go +++ b/enterprise/members_test.go @@ -152,7 +152,7 @@ func TestEnterpriseMembers(t *testing.T) { require.Error(t, err) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) - require.Contains(t, apiErr.Message, "must be an existing") + require.Contains(t, apiErr.Message, "Resource not found or you do not have access to this resource") }) // Calling it from a user without the org access.