Skip to content

Commit 2321160

Browse files
EmyrkEmyrk
authored
feat: Dbauthz is now default, remove out of experimental (#6650)
* feat: dbauthz always on, out of experimental * Add ability to do rbac checks in unit tests * Remove AuthorizeAllEndpoints * Remove duplicate rbac checks
1 parent 8aae0b6 commit 2321160

37 files changed

+327
-1264
lines changed

‎cli/testdata/coder_list_--output_json.golden

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"updated_at": "[timestamp]",
66
"owner_id": "[first user ID]",
77
"owner_name": "testuser",
8+
"organization_id": "[first org ID]",
89
"template_id": "[template ID]",
910
"template_name": "test-template",
1011
"template_display_name": "",

‎coderd/apidoc/docs.go

+4-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+6-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apikey.go

-26
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,6 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
5555
aReq.Old = database.APIKey{}
5656
defer commitAudit()
5757

58-
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceAPIKey.WithOwner(user.ID.String())) {
59-
httpapi.ResourceNotFound(rw)
60-
return
61-
}
62-
6358
var createToken codersdk.CreateTokenRequest
6459
if !httpapi.Read(ctx, rw, r, &createToken) {
6560
return
@@ -134,11 +129,6 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
134129
ctx := r.Context()
135130
user := httpmw.UserParam(r)
136131

137-
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceAPIKey.WithOwner(user.ID.String())) {
138-
httpapi.ResourceNotFound(rw)
139-
return
140-
}
141-
142132
lifeTime := time.Hour * 24 * 7
143133
cookie, _, err := api.createAPIKey(ctx, createAPIKeyParams{
144134
UserID: user.ID,
@@ -190,11 +180,6 @@ func (api *API) apiKeyByID(rw http.ResponseWriter, r *http.Request) {
190180
return
191181
}
192182

193-
if !api.Authorize(r, rbac.ActionRead, key) {
194-
httpapi.ResourceNotFound(rw)
195-
return
196-
}
197-
198183
httpapi.Write(ctx, rw, http.StatusOK, convertAPIKey(key))
199184
}
200185

@@ -230,11 +215,6 @@ func (api *API) apiKeyByName(rw http.ResponseWriter, r *http.Request) {
230215
return
231216
}
232217

233-
if !api.Authorize(r, rbac.ActionRead, token) {
234-
httpapi.ResourceNotFound(rw)
235-
return
236-
}
237-
238218
httpapi.Write(ctx, rw, http.StatusOK, convertAPIKey(token))
239219
}
240220

@@ -327,7 +307,6 @@ func (api *API) tokens(rw http.ResponseWriter, r *http.Request) {
327307
func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
328308
var (
329309
ctx = r.Context()
330-
user = httpmw.UserParam(r)
331310
keyID = chi.URLParam(r, "keyid")
332311
auditor = api.Auditor.Load()
333312
aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{
@@ -344,11 +323,6 @@ func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
344323
aReq.Old = key
345324
defer commitAudit()
346325

347-
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceAPIKey.WithIDString(keyID).WithOwner(user.ID.String())) {
348-
httpapi.ResourceNotFound(rw)
349-
return
350-
}
351-
352326
err = api.Database.DeleteAPIKeyByID(ctx, keyID)
353327
if errors.Is(err, sql.ErrNoRows) {
354328
httpapi.ResourceNotFound(rw)

‎coderd/audit.go

-8
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,6 @@ import (
3737
// @Router /audit [get]
3838
func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) {
3939
ctx := r.Context()
40-
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceAuditLog) {
41-
httpapi.Forbidden(rw)
42-
return
43-
}
4440

4541
page, ok := parsePagination(rw, r)
4642
if !ok {
@@ -90,10 +86,6 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) {
9086
// @Router /audit/testgenerate [post]
9187
func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
9288
ctx := r.Context()
93-
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceAuditLog) {
94-
httpapi.Forbidden(rw)
95-
return
96-
}
9789

9890
key := httpmw.APIKey(r)
9991
user, err := api.Database.GetUserByID(ctx, key.UserID)

‎coderd/authorize.go

-22
Original file line numberDiff line numberDiff line change
@@ -51,28 +51,6 @@ type HTTPAuthorizer struct {
5151
// return
5252
// }
5353
func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
54-
// The experiment does not replace ALL rbac checks, but does replace most.
55-
// This statement aborts early on the checks that will be removed in the
56-
// future when this experiment is default.
57-
if api.Experiments.Enabled(codersdk.ExperimentAuthzQuerier) {
58-
// Some resource types do not interact with the persistent layer and
59-
// we need to keep these checks happening in the API layer.
60-
switch object.RBACObject().Type {
61-
case rbac.ResourceWorkspaceExecution.Type:
62-
// This is not a db resource, always in API layer
63-
case rbac.ResourceDeploymentValues.Type:
64-
// For metric cache items like DAU, we do not hit the DB.
65-
// Some db actions are in asserted in the authz layer.
66-
case rbac.ResourceReplicas.Type:
67-
// Replica rbac is checked for adding and removing replicas.
68-
case rbac.ResourceProvisionerDaemon.Type:
69-
// Provisioner rbac is checked for adding and removing provisioners.
70-
case rbac.ResourceDebugInfo.Type:
71-
// This is not a db resource, always in API layer.
72-
default:
73-
return true
74-
}
75-
}
7654
return api.HTTPAuth.Authorize(r, action, object)
7755
}
7856

‎coderd/coderd.go

+9-11
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,15 @@ func New(options *Options) *API {
166166
if options == nil {
167167
options = &Options{}
168168
}
169+
170+
if options.Authorizer == nil {
171+
options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
172+
}
173+
options.Database = dbauthz.New(
174+
options.Database,
175+
options.Authorizer,
176+
options.Logger.Named("authz_querier"),
177+
)
169178
experiments := initExperiments(
170179
options.Logger, options.DeploymentValues.Experiments.Value(),
171180
)
@@ -201,9 +210,6 @@ func New(options *Options) *API {
201210
if options.PrometheusRegistry == nil {
202211
options.PrometheusRegistry = prometheus.NewRegistry()
203212
}
204-
if options.Authorizer == nil {
205-
options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
206-
}
207213
if options.TailnetCoordinator == nil {
208214
options.TailnetCoordinator = tailnet.NewCoordinator()
209215
}
@@ -216,14 +222,6 @@ func New(options *Options) *API {
216222
if options.SSHConfig.HostnamePrefix == "" {
217223
options.SSHConfig.HostnamePrefix = "coder."
218224
}
219-
// TODO: remove this once we promote authz_querier out of experiments.
220-
if experiments.Enabled(codersdk.ExperimentAuthzQuerier) {
221-
options.Database = dbauthz.New(
222-
options.Database,
223-
options.Authorizer,
224-
options.Logger.Named("authz_querier"),
225-
)
226-
}
227225
if options.SetUserGroups == nil {
228226
options.SetUserGroups = func(context.Context, database.Store, uuid.UUID, []string) error { return nil }
229227
}

0 commit comments

Comments
 (0)