mirror of
https://github.com/coder/coder.git
synced 2025-07-18 14:17:22 +00:00
chore: Ensure cancelled errors return proper (#6200)
The authz library returns a 404 if the authorization fails. If the context is cancelled, then a 404 message is inaccurate. Add a unit test to ensure context cancelled errors are raised properly
This commit is contained in:
@ -61,6 +61,7 @@ func logNotAuthorizedError(ctx context.Context, logger slog.Logger, err error) e
|
||||
slog.Error(err),
|
||||
)
|
||||
}
|
||||
|
||||
return NotAuthorizedError{
|
||||
Err: err,
|
||||
}
|
||||
|
@ -438,12 +438,16 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
|
||||
}
|
||||
}
|
||||
|
||||
if len(added) > 0 && q.authorizeContext(ctx, rbac.ActionCreate, roleAssign) != nil {
|
||||
return logNotAuthorizedError(ctx, q.log, xerrors.Errorf("not authorized to assign roles"))
|
||||
if len(added) > 0 {
|
||||
if err := q.authorizeContext(ctx, rbac.ActionCreate, roleAssign); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if len(removed) > 0 && q.authorizeContext(ctx, rbac.ActionDelete, roleAssign) != nil {
|
||||
return logNotAuthorizedError(ctx, q.log, xerrors.Errorf("not authorized to delete roles"))
|
||||
if len(removed) > 0 {
|
||||
if err := q.authorizeContext(ctx, rbac.ActionDelete, roleAssign); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
for _, roleName := range grantedRoles {
|
||||
@ -1221,7 +1225,7 @@ func (q *querier) GetWorkspaceAgentsByResourceIDs(ctx context.Context, ids []uui
|
||||
continue
|
||||
}
|
||||
// Otherwise, we cannot read the workspace, so we cannot read the agent.
|
||||
return nil, logNotAuthorizedError(ctx, q.log, err)
|
||||
return nil, err
|
||||
}
|
||||
return agents, nil
|
||||
}
|
||||
|
@ -9,11 +9,11 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/open-policy-agent/opa/topdown"
|
||||
"github.com/stretchr/testify/require"
|
||||
"github.com/stretchr/testify/suite"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog"
|
||||
"github.com/coder/coder/coderd/coderdtest"
|
||||
@ -225,6 +225,26 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd
|
||||
s.ErrorAs(err, &dbauthz.NotAuthorizedError{}, "error should be NotAuthorizedError")
|
||||
}
|
||||
})
|
||||
|
||||
s.Run("Cancelled", func() {
|
||||
// Pass in a cancelled context
|
||||
ctx, cancel := context.WithCancel(ctx)
|
||||
cancel()
|
||||
az.AlwaysReturn = rbac.ForbiddenWithInternal(&topdown.Error{Code: topdown.CancelErr},
|
||||
rbac.Subject{}, "", rbac.Object{}, nil)
|
||||
|
||||
// If we have assertions, that means the method should FAIL
|
||||
// if RBAC will disallow the request. The returned error should
|
||||
// be expected to be a NotAuthorizedError.
|
||||
resp, err := callMethod(ctx)
|
||||
|
||||
// This is unfortunate, but if we are using `Filter` the error returned will be nil. So filter out
|
||||
// any case where the error is nil and the response is an empty slice.
|
||||
if err != nil || !hasEmptySliceResponse(resp) {
|
||||
s.Errorf(err, "method should an error with cancellation")
|
||||
s.ErrorIsf(err, context.Canceled, "error should match context.Cancelled")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func hasEmptySliceResponse(values []reflect.Value) bool {
|
||||
|
Reference in New Issue
Block a user