Skip to content

Commit 951d74f

Browse files
EmyrkEmyrk
committed
Do not export the authzQuerier
1 parent ef97e4b commit 951d74f

File tree

6 files changed

+223
-221
lines changed

6 files changed

+223
-221
lines changed

‎coderd/coderd.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -159,13 +159,11 @@ func New(options *Options) *API {
159159
experiments := initExperiments(options.Logger, options.DeploymentConfig.Experiments.Value, options.DeploymentConfig.Experimental.Value)
160160
// TODO: remove this once we promote authz_querier out of experiments.
161161
if experiments.Enabled(codersdk.ExperimentAuthzQuerier) {
162-
if _, ok := (options.Database).(*dbauthz.AuthzQuerier); !ok {
163-
options.Database = dbauthz.New(
164-
options.Database,
165-
options.Authorizer,
166-
options.Logger.Named("authz_query"),
167-
)
168-
}
162+
options.Database = dbauthz.New(
163+
options.Database,
164+
options.Authorizer,
165+
options.Logger.Named("authz_query"),
166+
)
169167
}
170168
if options.AppHostname != "" && options.AppHostnameRegex == nil || options.AppHostname == "" && options.AppHostnameRegex != nil {
171169
panic("coderd: both AppHostname and AppHostnameRegex must be set or unset")
@@ -209,9 +207,7 @@ func New(options *Options) *API {
209207
}
210208
// TODO: remove this once we promote authz_querier out of experiments.
211209
if experiments.Enabled(codersdk.ExperimentAuthzQuerier) {
212-
if _, ok := (options.Database).(*dbauthz.AuthzQuerier); !ok {
213-
options.Database = dbauthz.New(options.Database, options.Authorizer, options.Logger.Named("authz_querier"))
214-
}
210+
options.Database = dbauthz.New(options.Database, options.Authorizer, options.Logger.Named("authz_querier"))
215211
}
216212
if options.SetUserGroups == nil {
217213
options.SetUserGroups = func(context.Context, database.Store, uuid.UUID, []string) error { return nil }

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
"github.com/coder/coder/coderd/rbac"
1616
)
1717

18-
var _ database.Store = (*AuthzQuerier)(nil)
18+
var _ database.Store = (*authzQuerier)(nil)
1919

2020
var (
2121
// NoActorError wraps ErrNoRows for the api to return a 404. This is the correct
@@ -40,7 +40,7 @@ func (NotAuthorizedError) Unwrap() error {
4040
return sql.ErrNoRows
4141
}
4242

43-
func LogNotAuthorizedError(ctx context.Context, logger slog.Logger, err error) error {
43+
func logNotAuthorizedError(ctx context.Context, logger slog.Logger, err error) error {
4444
// Only log the errors if it is an UnauthorizedError error.
4545
internalError := new(rbac.UnauthorizedError)
4646
if err != nil && xerrors.As(err, internalError) {
@@ -55,37 +55,42 @@ func LogNotAuthorizedError(ctx context.Context, logger slog.Logger, err error) e
5555
}
5656
}
5757

58-
// AuthzQuerier is a wrapper around the database store that performs authorization
59-
// checks before returning data. All AuthzQuerier methods expect an authorization
58+
// authzQuerier is a wrapper around the database store that performs authorization
59+
// checks before returning data. All authzQuerier methods expect an authorization
6060
// subject present in the context. If no subject is present, most methods will
6161
// fail.
6262
//
6363
// Use WithAuthorizeContext to set the authorization subject in the context for
6464
// the common user case.
65-
type AuthzQuerier struct {
65+
type authzQuerier struct {
6666
db database.Store
6767
auth rbac.Authorizer
6868
log slog.Logger
6969
}
7070

71-
func New(db database.Store, authorizer rbac.Authorizer, logger slog.Logger) *AuthzQuerier {
72-
return &AuthzQuerier{
71+
func New(db database.Store, authorizer rbac.Authorizer, logger slog.Logger) database.Store {
72+
// If the underlying db store is already an authzquerier, return it.
73+
// Do not double wrap.
74+
if _, ok := db.(*authzQuerier); ok {
75+
return db
76+
}
77+
return &authzQuerier{
7378
db: db,
7479
auth: authorizer,
7580
log: logger,
7681
}
7782
}
7883

7984
// authorizeContext is a helper function to authorize an action on an object.
80-
func (q *AuthzQuerier) authorizeContext(ctx context.Context, action rbac.Action, object rbac.Objecter) error {
85+
func (q *authzQuerier) authorizeContext(ctx context.Context, action rbac.Action, object rbac.Objecter) error {
8186
act, ok := ActorFromContext(ctx)
8287
if !ok {
8388
return NoActorError
8489
}
8590

8691
err := q.auth.Authorize(ctx, act, action, object.RBACObject())
8792
if err != nil {
88-
return LogNotAuthorizedError(ctx, q.log, err)
93+
return logNotAuthorizedError(ctx, q.log, err)
8994
}
9095
return nil
9196
}
@@ -143,7 +148,7 @@ func insert[
143148
// Authorize the action
144149
err = authorizer.Authorize(ctx, act, rbac.ActionCreate, object.RBACObject())
145150
if err != nil {
146-
return empty, LogNotAuthorizedError(ctx, logger, err)
151+
return empty, logNotAuthorizedError(ctx, logger, err)
147152
}
148153

149154
// Insert the database object
@@ -226,7 +231,7 @@ func fetch[
226231
// Authorize the action
227232
err = authorizer.Authorize(ctx, act, rbac.ActionRead, object.RBACObject())
228233
if err != nil {
229-
return empty, LogNotAuthorizedError(ctx, logger, err)
234+
return empty, logNotAuthorizedError(ctx, logger, err)
230235
}
231236

232237
return object, nil
@@ -290,7 +295,7 @@ func fetchAndQuery[
290295
// Authorize the action
291296
err = authorizer.Authorize(ctx, act, action, object.RBACObject())
292297
if err != nil {
293-
return empty, LogNotAuthorizedError(ctx, logger, err)
298+
return empty, logNotAuthorizedError(ctx, logger, err)
294299
}
295300

296301
return queryFunc(ctx, arg)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestNotAuthorizedError(t *testing.T) {
6363

6464
testErr := xerrors.New("custom error")
6565

66-
err := dbauthz.LogNotAuthorizedError(context.Background(), slogtest.Make(t, nil), testErr)
66+
err := dbauthz.logNotAuthorizedError(context.Background(), slogtest.Make(t, nil), testErr)
6767
require.ErrorIs(t, err, sql.ErrNoRows, "must be a sql.ErrNoRows")
6868

6969
var authErr dbauthz.NotAuthorizedError

0 commit comments

Comments
 (0)