chore: return 404, not 400 if missing or authz deny (#22069)
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user