Skip to content

Commit b01814c

Browse files
mafredrimafredri
authored
Merge branch 'main' into mafredri/feat-agent-recreate-devcontainer-refactor
2 parents 31c7bff + 46d4b28 commit b01814c

File tree

9 files changed

+162
-11
lines changed

9 files changed

+162
-11
lines changed

‎coderd/coderd.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,9 @@ func New(options *Options) *API {
314314

315315
if options.Authorizer == nil {
316316
options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
317+
if buildinfo.IsDev() {
318+
options.Authorizer = rbac.Recorder(options.Authorizer)
319+
}
317320
}
318321

319322
if options.AccessControlStore == nil {
@@ -456,8 +459,14 @@ func New(options *Options) *API {
456459
options.NotificationsEnqueuer = notifications.NewNoopEnqueuer()
457460
}
458461

459-
ctx, cancel := context.WithCancel(context.Background())
460462
r := chi.NewRouter()
463+
// We add this middleware early, to make sure that authorization checks made
464+
// by other middleware get recorded.
465+
if buildinfo.IsDev() {
466+
r.Use(httpmw.RecordAuthzChecks)
467+
}
468+
469+
ctx, cancel := context.WithCancel(context.Background())
461470

462471
// nolint:gocritic // Load deployment ID. This never changes
463472
depID, err := options.Database.GetDeploymentID(dbauthz.AsSystemRestricted(ctx))

‎coderd/httpapi/httpapi.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/coder/websocket/wsjson"
2121

2222
"github.com/coder/coder/v2/coderd/httpapi/httpapiconstraints"
23+
"github.com/coder/coder/v2/coderd/rbac"
2324
"github.com/coder/coder/v2/coderd/tracing"
2425
"github.com/coder/coder/v2/codersdk"
2526
)
@@ -198,6 +199,20 @@ func Write(ctx context.Context, rw http.ResponseWriter, status int, response int
198199
_, span := tracing.StartSpan(ctx)
199200
defer span.End()
200201

202+
if rec, ok := rbac.GetAuthzCheckRecorder(ctx); ok {
203+
// If you're here because you saw this header in a response, and you're
204+
// trying to investigate the code, here are a couple of notable things
205+
// for you to know:
206+
// - If any of the checks are `false`, they might not represent the whole
207+
// picture. There could be additional checks that weren't performed,
208+
// because processing stopped after the failure.
209+
// - The checks are recorded by the `authzRecorder` type, which is
210+
// configured on server startup for development and testing builds.
211+
// - If this header is missing from a response, make sure the response is
212+
// being written by calling `httpapi.Write`!
213+
rw.Header().Set("x-authz-checks", rec.String())
214+
}
215+
201216
rw.Header().Set("Content-Type", "application/json; charset=utf-8")
202217
rw.WriteHeader(status)
203218

@@ -213,6 +228,10 @@ func WriteIndent(ctx context.Context, rw http.ResponseWriter, status int, respon
213228
_, span := tracing.StartSpan(ctx)
214229
defer span.End()
215230

231+
if rec, ok := rbac.GetAuthzCheckRecorder(ctx); ok {
232+
rw.Header().Set("x-authz-checks", rec.String())
233+
}
234+
216235
rw.Header().Set("Content-Type", "application/json; charset=utf-8")
217236
rw.WriteHeader(status)
218237

‎coderd/httpmw/authz.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/go-chi/chi/v5"
77

88
"github.com/coder/coder/v2/coderd/database/dbauthz"
9+
"github.com/coder/coder/v2/coderd/rbac"
910
)
1011

1112
// AsAuthzSystem is a chained handler that temporarily sets the dbauthz context
@@ -35,3 +36,15 @@ func AsAuthzSystem(mws ...func(http.Handler) http.Handler) func(http.Handler) ht
3536
})
3637
}
3738
}
39+
40+
// RecordAuthzChecks enables recording all of the authorization checks that
41+
// occurred in the processing of a request. This is mostly helpful for debugging
42+
// and understanding what permissions are required for a given action.
43+
//
44+
// Requires using a Recorder Authorizer.
45+
func RecordAuthzChecks(next http.Handler) http.Handler {
46+
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
47+
r = r.WithContext(rbac.WithAuthzCheckRecorder(r.Context()))
48+
next.ServeHTTP(rw, r)
49+
})
50+
}

‎coderd/rbac/authz.go

Lines changed: 113 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
_ "embed"
77
"encoding/json"
88
"errors"
9+
"fmt"
910
"strings"
1011
"sync"
1112
"time"
@@ -362,11 +363,11 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subject Subject, action p
362363
defer span.End()
363364

364365
err := a.authorize(ctx, subject, action, object)
365-
366-
span.SetAttributes(attribute.Bool("authorized", err == nil))
366+
authorized := err == nil
367+
span.SetAttributes(attribute.Bool("authorized", authorized))
367368

368369
dur := time.Since(start)
369-
if err != nil {
370+
if !authorized {
370371
a.authorizeHist.WithLabelValues("false").Observe(dur.Seconds())
371372
return err
372373
}
@@ -741,3 +742,112 @@ func rbacTraceAttributes(actor Subject, action policy.Action, objectType string,
741742
attribute.String("object_type", objectType),
742743
)...)
743744
}
745+
746+
type authRecorder struct {
747+
authz Authorizer
748+
}
749+
750+
// Recorder returns an Authorizer that records any authorization checks made
751+
// on the Context provided for the authorization check.
752+
//
753+
// Requires using the RecordAuthzChecks middleware.
754+
func Recorder(authz Authorizer) Authorizer {
755+
return &authRecorder{authz: authz}
756+
}
757+
758+
func (c *authRecorder) Authorize(ctx context.Context, subject Subject, action policy.Action, object Object) error {
759+
err := c.authz.Authorize(ctx, subject, action, object)
760+
authorized := err == nil
761+
recordAuthzCheck(ctx, action, object, authorized)
762+
return err
763+
}
764+
765+
func (c *authRecorder) Prepare(ctx context.Context, subject Subject, action policy.Action, objectType string) (PreparedAuthorized, error) {
766+
return c.authz.Prepare(ctx, subject, action, objectType)
767+
}
768+
769+
type authzCheckRecorderKey struct{}
770+
771+
type AuthzCheckRecorder struct {
772+
// lock guards checks
773+
lock sync.Mutex
774+
// checks is a list preformatted authz check IDs and their result
775+
checks []recordedCheck
776+
}
777+
778+
type recordedCheck struct {
779+
name string
780+
// true => authorized, false => not authorized
781+
result bool
782+
}
783+
784+
func WithAuthzCheckRecorder(ctx context.Context) context.Context {
785+
return context.WithValue(ctx, authzCheckRecorderKey{}, &AuthzCheckRecorder{})
786+
}
787+
788+
func recordAuthzCheck(ctx context.Context, action policy.Action, object Object, authorized bool) {
789+
r, ok := ctx.Value(authzCheckRecorderKey{}).(*AuthzCheckRecorder)
790+
if !ok {
791+
return
792+
}
793+
794+
// We serialize the check using the following syntax
795+
var b strings.Builder
796+
if object.OrgID != "" {
797+
_, err := fmt.Fprintf(&b, "organization:%v::", object.OrgID)
798+
if err != nil {
799+
return
800+
}
801+
}
802+
if object.AnyOrgOwner {
803+
_, err := fmt.Fprint(&b, "organization:any::")
804+
if err != nil {
805+
return
806+
}
807+
}
808+
if object.Owner != "" {
809+
_, err := fmt.Fprintf(&b, "owner:%v::", object.Owner)
810+
if err != nil {
811+
return
812+
}
813+
}
814+
if object.ID != "" {
815+
_, err := fmt.Fprintf(&b, "id:%v::", object.ID)
816+
if err != nil {
817+
return
818+
}
819+
}
820+
_, err := fmt.Fprintf(&b, "%v.%v", object.RBACObject().Type, action)
821+
if err != nil {
822+
return
823+
}
824+
825+
r.lock.Lock()
826+
defer r.lock.Unlock()
827+
r.checks = append(r.checks, recordedCheck{name: b.String(), result: authorized})
828+
}
829+
830+
func GetAuthzCheckRecorder(ctx context.Context) (*AuthzCheckRecorder, bool) {
831+
checks, ok := ctx.Value(authzCheckRecorderKey{}).(*AuthzCheckRecorder)
832+
if !ok {
833+
return nil, false
834+
}
835+
836+
return checks, true
837+
}
838+
839+
// String serializes all of the checks recorded, using the following syntax:
840+
func (r *AuthzCheckRecorder) String() string {
841+
r.lock.Lock()
842+
defer r.lock.Unlock()
843+
844+
if len(r.checks) == 0 {
845+
return "nil"
846+
}
847+
848+
checks := make([]string, 0, len(r.checks))
849+
for _, check := range r.checks {
850+
checks = append(checks, fmt.Sprintf("%v=%v", check.name, check.result))
851+
}
852+
return strings.Join(checks, "; ")
853+
}

‎coderd/users.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"slices"
1010

1111
"github.com/go-chi/chi/v5"
12-
"github.com/go-chi/render"
1312
"github.com/google/uuid"
1413
"golang.org/x/xerrors"
1514

@@ -273,8 +272,7 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) {
273272
organizationIDsByUserID[organizationIDsByMemberIDsRow.UserID] = organizationIDsByMemberIDsRow.OrganizationIDs
274273
}
275274

276-
render.Status(r, http.StatusOK)
277-
render.JSON(rw, r, codersdk.GetUsersResponse{
275+
httpapi.Write(ctx, rw, http.StatusOK, codersdk.GetUsersResponse{
278276
Users: convertUsers(users, organizationIDsByUserID),
279277
Count: int(userCount),
280278
})

‎coderd/util/syncmap/map.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package syncmap
22

3-
import "sync"
3+
import (
4+
"sync"
5+
)
46

57
// Map is a type safe sync.Map
68
type Map[K, V any] struct {

‎enterprise/coderd/coderd.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
7171
}
7272
if options.Options.Authorizer == nil {
7373
options.Options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
74+
if buildinfo.IsDev() {
75+
options.Authorizer = rbac.Recorder(options.Authorizer)
76+
}
7477
}
7578
if options.ReplicaErrorGracePeriod == 0 {
7679
// This will prevent the error from being shown for a minute

‎go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,6 @@ require (
116116
github.com/go-chi/chi/v5 v5.1.0
117117
github.com/go-chi/cors v1.2.1
118118
github.com/go-chi/httprate v0.15.0
119-
github.com/go-chi/render v1.0.1
120119
github.com/go-jose/go-jose/v4 v4.0.5
121120
github.com/go-logr/logr v1.4.2
122121
github.com/go-playground/validator/v10 v10.26.0

‎go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,6 @@ github.com/go-chi/hostrouter v0.2.0 h1:GwC7TZz8+SlJN/tV/aeJgx4F+mI5+sp+5H1PelQUj
367367
github.com/go-chi/hostrouter v0.2.0/go.mod h1:pJ49vWVmtsKRKZivQx0YMYv4h0aX+Gcn6V23Np9Wf1s=
368368
github.com/go-chi/httprate v0.15.0 h1:j54xcWV9KGmPf/X4H32/aTH+wBlrvxL7P+SdnRqxh5g=
369369
github.com/go-chi/httprate v0.15.0/go.mod h1:rzGHhVrsBn3IMLYDOZQsSU4fJNWcjui4fWKJcCId1R4=
370-
github.com/go-chi/render v1.0.1 h1:4/5tis2cKaNdnv9zFLfXzcquC9HbeZgCnxGnKrltBS8=
371-
github.com/go-chi/render v1.0.1/go.mod h1:pq4Rr7HbnsdaeHagklXub+p6Wd16Af5l9koip1OvJns=
372370
github.com/go-ini/ini v1.67.0 h1:z6ZrTEZqSWOTyH2FlglNbNgARyHG8oLW9gMELqKr06A=
373371
github.com/go-ini/ini v1.67.0/go.mod h1:ByCAeIL28uOIIG0E3PJtZPDL8WnHpFKFOtgjp+3Ies8=
374372
github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE=

0 commit comments

Comments
 (0)