Skip to content

Commit

Permalink
[SDP-1215] Fix the AnyRoleMiddleware so it returns 403 (instead of …
Browse files Browse the repository at this point in the history
…401) when the user role doesn't have the required role (#295)

### What

Fix the `AnyRoleMiddleware` so it returns 403 (instead of 401) when the user role doesn't have the required role.

How it's being handled:
![Screenshot 2024-05-20 at 5 16 28 PM](https://github.com/stellar/stellar-disbursement-platform-backend/assets/1952597/eee0af3e-6265-4cd2-9e39-dec712b82e7d)

### Why

Addresses https://stellarorg.atlassian.net/browse/SDP-1215.
  • Loading branch information
marcelosalloum authored May 20, 2024
1 parent c61d2ae commit 2e984e4
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
1 change: 1 addition & 0 deletions internal/serve/httphandler/wallets_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/go-chi/chi/v5"
"github.com/stellar/go/support/http/httpdecode"
"github.com/stellar/go/support/render/httpjson"

"github.com/stellar/stellar-disbursement-platform-backend/internal/data"
"github.com/stellar/stellar-disbursement-platform-backend/internal/serve/httperror"
"github.com/stellar/stellar-disbursement-platform-backend/internal/serve/validators"
Expand Down
14 changes: 9 additions & 5 deletions internal/serve/middleware/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,18 @@ func AnyRoleMiddleware(authManager auth.AuthManager, requiredRoles ...data.UserR
return
}

isValid, err := authManager.AnyRolesInTokenUser(ctx, token, data.FromUserRoleArrayToStringArray(requiredRoles))
if err != nil && !errors.Is(err, auth.ErrInvalidToken) && !errors.Is(err, auth.ErrUserNotFound) {
httperror.InternalError(ctx, "", err, nil).Render(rw)
hasAnyRoles, err := authManager.AnyRolesInTokenUser(ctx, token, data.FromUserRoleArrayToStringArray(requiredRoles))
if err != nil {
if errors.Is(err, auth.ErrInvalidToken) || errors.Is(err, auth.ErrUserNotFound) {
httperror.Unauthorized("", nil, nil).Render(rw)
} else {
httperror.InternalError(ctx, "", err, nil).Render(rw)
}
return
}

if !isValid {
httperror.Unauthorized("", nil, nil).Render(rw)
if !hasAnyRoles {
httperror.Forbidden("", nil, nil).Render(rw)
return
}

Expand Down
6 changes: 3 additions & 3 deletions internal/serve/middleware/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ func Test_AnyRoleMiddleware(t *testing.T) {
assert.JSONEq(t, `{"error":"An internal error occurred while processing this request."}`, string(respBody))
})

t.Run("returns Unauthorized error when the user does not have the required roles", func(t *testing.T) {
t.Run("returns Forbidden error when the user does not have the required roles", func(t *testing.T) {
token := "mytoken"
ctx := context.WithValue(context.Background(), TokenContextKey, token)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
Expand Down Expand Up @@ -523,8 +523,8 @@ func Test_AnyRoleMiddleware(t *testing.T) {
respBody, err := io.ReadAll(resp.Body)
require.NoError(t, err)

assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
assert.JSONEq(t, `{"error":"Not authorized."}`, string(respBody))
assert.Equal(t, http.StatusForbidden, resp.StatusCode)
assert.JSONEq(t, `{"error":"You don't have permission to perform this action."}`, string(respBody))
})

t.Run("returns Status Ok when user has the required roles", func(t *testing.T) {
Expand Down

0 comments on commit 2e984e4

Please sign in to comment.