diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index b3f80cd4a5468..a5f9dcff54916 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -61,6 +61,7 @@ func logNotAuthorizedError(ctx context.Context, logger slog.Logger, err error) e slog.Error(err), ) } + return NotAuthorizedError{ Err: err, } diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index 4442619ef3850..89692504378fb 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -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 } diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index 6fe03e52d0ebe..0b5dad6cf2378 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -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 {