Skip to content

Commit ace67cb

Browse files
committed
remove 2 ways to customizing a fake authorizer
1 parent 5d83571 commit ace67cb

File tree

3 files changed

+24
-15
lines changed

3 files changed

+24
-15
lines changed

coderd/coderdtest/authorize.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -353,21 +353,29 @@ func (s *PreparedRecorder) CompileToSQL(ctx context.Context, cfg regosql.Convert
353353
return s.prepped.CompileToSQL(ctx, cfg)
354354
}
355355

356-
// FakeAuthorizer is an Authorizer that always returns the same error.
356+
// FakeAuthorizer is an Authorizer that will return an error based on the
357+
// "ConditionalReturn" function. By default, **no error** is returned.
358+
// Meaning 'FakeAuthorizer' by default will never return "unauthorized".
357359
type FakeAuthorizer struct {
358-
// AlwaysReturn is the error that will be returned by Authorize.
359-
AlwaysReturn error
360360
ConditionalReturn func(context.Context, rbac.Subject, policy.Action, rbac.Object) error
361361
}
362362

363363
var _ rbac.Authorizer = (*FakeAuthorizer)(nil)
364364

365+
// AlwaysReturn is the error that will be returned by Authorize.
366+
func (d *FakeAuthorizer) AlwaysReturn(err error) *FakeAuthorizer {
367+
d.ConditionalReturn = func(_ context.Context, _ rbac.Subject, _ policy.Action, _ rbac.Object) error {
368+
return err
369+
}
370+
return d
371+
}
372+
365373
func (d *FakeAuthorizer) Authorize(ctx context.Context, subject rbac.Subject, action policy.Action, object rbac.Object) error {
366374
if d.ConditionalReturn != nil {
367375
return d.ConditionalReturn(ctx, subject, action, object)
368376

369377
}
370-
return d.AlwaysReturn
378+
return nil
371379
}
372380

373381
func (d *FakeAuthorizer) Prepare(_ context.Context, subject rbac.Subject, action policy.Action, _ string) (rbac.PreparedAuthorized, error) {

coderd/database/dbauthz/dbauthz_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func TestInTX(t *testing.T) {
8181

8282
db := dbmem.New()
8383
q := dbauthz.New(db, &coderdtest.RecordingAuthorizer{
84-
Wrapped: &coderdtest.FakeAuthorizer{AlwaysReturn: xerrors.New("custom error")},
84+
Wrapped: (&coderdtest.FakeAuthorizer{}).AlwaysReturn(xerrors.New("custom error")),
8585
}, slog.Make(), coderdtest.AccessControlStorePointer())
8686
actor := rbac.Subject{
8787
ID: uuid.NewString(),
@@ -110,7 +110,7 @@ func TestNew(t *testing.T) {
110110
db = dbmem.New()
111111
exp = dbgen.Workspace(t, db, database.Workspace{})
112112
rec = &coderdtest.RecordingAuthorizer{
113-
Wrapped: &coderdtest.FakeAuthorizer{AlwaysReturn: nil},
113+
Wrapped: &coderdtest.FakeAuthorizer{},
114114
}
115115
subj = rbac.Subject{}
116116
ctx = dbauthz.As(context.Background(), rbac.Subject{})
@@ -135,7 +135,7 @@ func TestNew(t *testing.T) {
135135
func TestDBAuthzRecursive(t *testing.T) {
136136
t.Parallel()
137137
q := dbauthz.New(dbmem.New(), &coderdtest.RecordingAuthorizer{
138-
Wrapped: &coderdtest.FakeAuthorizer{AlwaysReturn: nil},
138+
Wrapped: &coderdtest.FakeAuthorizer{},
139139
}, slog.Make(), coderdtest.AccessControlStorePointer())
140140
actor := rbac.Subject{
141141
ID: uuid.NewString(),

coderd/database/dbauthz/setup_test.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
114114
s.methodAccounting[methodName]++
115115

116116
db := dbmem.New()
117-
fakeAuthorizer := &coderdtest.FakeAuthorizer{
118-
AlwaysReturn: nil,
119-
}
117+
fakeAuthorizer := &coderdtest.FakeAuthorizer{}
120118
rec := &coderdtest.RecordingAuthorizer{
121119
Wrapped: fakeAuthorizer,
122120
}
@@ -174,8 +172,11 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
174172
// Always run
175173
s.Run("Success", func() {
176174
rec.Reset()
177-
fakeAuthorizer.AlwaysReturn = nil
178-
fakeAuthorizer.ConditionalReturn = testCase.successAuthorizer
175+
if testCase.successAuthorizer != nil {
176+
fakeAuthorizer.ConditionalReturn = testCase.successAuthorizer
177+
} else {
178+
fakeAuthorizer.AlwaysReturn(nil)
179+
}
179180

180181
outputs, err := callMethod(ctx)
181182
if testCase.err == nil {
@@ -233,7 +234,7 @@ func (s *MethodTestSuite) NoActorErrorTest(callMethod func(ctx context.Context)
233234
// Asserts that the error returned is a NotAuthorizedError.
234235
func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderdtest.FakeAuthorizer, testCase expects, callMethod func(ctx context.Context) ([]reflect.Value, error)) {
235236
s.Run("NotAuthorized", func() {
236-
az.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("Always fail authz"), rbac.Subject{}, "", rbac.Object{}, nil)
237+
az.AlwaysReturn(rbac.ForbiddenWithInternal(xerrors.New("Always fail authz"), rbac.Subject{}, "", rbac.Object{}, nil))
237238

238239
// If we have assertions, that means the method should FAIL
239240
// if RBAC will disallow the request. The returned error should
@@ -258,8 +259,8 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd
258259
// Pass in a canceled context
259260
ctx, cancel := context.WithCancel(ctx)
260261
cancel()
261-
az.AlwaysReturn = rbac.ForbiddenWithInternal(&topdown.Error{Code: topdown.CancelErr},
262-
rbac.Subject{}, "", rbac.Object{}, nil)
262+
az.AlwaysReturn(rbac.ForbiddenWithInternal(&topdown.Error{Code: topdown.CancelErr},
263+
rbac.Subject{}, "", rbac.Object{}, nil))
263264

264265
// If we have assertions, that means the method should FAIL
265266
// if RBAC will disallow the request. The returned error should

0 commit comments

Comments
 (0)