Skip to content

Commit eeb3d63

Browse files
EmyrkEmyrk
authored
chore: merge authorization contexts (coder#12816)
* chore: merge authorization contexts Instead of 2 auth contexts from apikey and dbauthz, merge them to just use dbauthz. It is annoying to have two. * fixup authorization reference
1 parent 8e2d026 commit eeb3d63

File tree

16 files changed

+68
-99
lines changed

16 files changed

+68
-99
lines changed

‎coderd/authorize.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,15 @@ import (
1919
// This is faster than calling Authorize() on each object.
2020
func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
2121
roles := httpmw.UserAuthorization(r)
22-
objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.Actor, action, objects)
22+
objects, err := rbac.Filter(r.Context(), h.Authorizer, roles, action, objects)
2323
if err != nil {
2424
// Log the error as Filter should not be erroring.
2525
h.Logger.Error(r.Context(), "authorization filter failed",
2626
slog.Error(err),
27-
slog.F("user_id", roles.Actor.ID),
28-
slog.F("username", roles.ActorName),
29-
slog.F("roles", roles.Actor.SafeRoleNames()),
30-
slog.F("scope", roles.Actor.SafeScopeName()),
27+
slog.F("user_id", roles.ID),
28+
slog.F("username", roles),
29+
slog.F("roles", roles.SafeRoleNames()),
30+
slog.F("scope", roles.SafeScopeName()),
3131
slog.F("route", r.URL.Path),
3232
slog.F("action", action),
3333
)
@@ -65,7 +65,7 @@ func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objec
6565
// }
6666
func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
6767
roles := httpmw.UserAuthorization(r)
68-
err := h.Authorizer.Authorize(r.Context(), roles.Actor, action, object.RBACObject())
68+
err := h.Authorizer.Authorize(r.Context(), roles, action, object.RBACObject())
6969
if err != nil {
7070
// Log the errors for debugging
7171
internalError := new(rbac.UnauthorizedError)
@@ -76,10 +76,10 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
7676
// Log information for debugging. This will be very helpful
7777
// in the early days
7878
logger.Warn(r.Context(), "requester is not authorized to access the object",
79-
slog.F("roles", roles.Actor.SafeRoleNames()),
80-
slog.F("actor_id", roles.Actor.ID),
81-
slog.F("actor_name", roles.ActorName),
82-
slog.F("scope", roles.Actor.SafeScopeName()),
79+
slog.F("roles", roles.SafeRoleNames()),
80+
slog.F("actor_id", roles.ID),
81+
slog.F("actor_name", roles),
82+
slog.F("scope", roles.SafeScopeName()),
8383
slog.F("route", r.URL.Path),
8484
slog.F("action", action),
8585
slog.F("object", object),
@@ -97,7 +97,7 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
9797
// Note the authorization is only for the given action and object type.
9898
func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action rbac.Action, objectType string) (rbac.PreparedAuthorized, error) {
9999
roles := httpmw.UserAuthorization(r)
100-
prepared, err := h.Authorizer.Prepare(r.Context(), roles.Actor, action, objectType)
100+
prepared, err := h.Authorizer.Prepare(r.Context(), roles, action, objectType)
101101
if err != nil {
102102
return nil, xerrors.Errorf("prepare filter: %w", err)
103103
}
@@ -128,10 +128,10 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
128128

129129
api.Logger.Debug(ctx, "check-auth",
130130
slog.F("my_id", httpmw.APIKey(r).UserID),
131-
slog.F("got_id", auth.Actor.ID),
132-
slog.F("name", auth.ActorName),
133-
slog.F("roles", auth.Actor.SafeRoleNames()),
134-
slog.F("scope", auth.Actor.SafeScopeName()),
131+
slog.F("got_id", auth.ID),
132+
slog.F("name", auth),
133+
slog.F("roles", auth.SafeRoleNames()),
134+
slog.F("scope", auth.SafeScopeName()),
135135
)
136136

137137
response := make(codersdk.AuthorizationResponse)
@@ -171,7 +171,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
171171
Type: v.Object.ResourceType.String(),
172172
}
173173
if obj.Owner == "me" {
174-
obj.Owner = auth.Actor.ID
174+
obj.Owner = auth.ID
175175
}
176176

177177
// If a resource ID is specified, fetch that specific resource.
@@ -219,7 +219,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
219219
obj = dbObj.RBACObject()
220220
}
221221

222-
err := api.Authorizer.Authorize(ctx, auth.Actor, rbac.Action(v.Action), obj)
222+
err := api.Authorizer.Authorize(ctx, auth, rbac.Action(v.Action), obj)
223223
response[k] = err == nil
224224
}
225225

‎coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ type Options struct {
133133
// after a successful authentication.
134134
// This is somewhat janky, but seemingly the only reasonable way to add a header
135135
// for all authenticated users under a condition, only in Enterprise.
136-
PostAuthAdditionalHeadersFunc func(auth httpmw.Authorization, header http.Header)
136+
PostAuthAdditionalHeadersFunc func(auth rbac.Subject, header http.Header)
137137

138138
// TLSCertificates is used to mesh DERP servers securely.
139139
TLSCertificates []tls.Certificate

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) {
155155

156156
var (
157157
subjectProvisionerd = rbac.Subject{
158-
ID: uuid.Nil.String(),
158+
FriendlyName: "Provisioner Daemon",
159+
ID: uuid.Nil.String(),
159160
Roles: rbac.Roles([]rbac.Role{
160161
{
161162
Name: "provisionerd",
@@ -182,7 +183,8 @@ var (
182183
}.WithCachedASTValue()
183184

184185
subjectAutostart = rbac.Subject{
185-
ID: uuid.Nil.String(),
186+
FriendlyName: "Autostart",
187+
ID: uuid.Nil.String(),
186188
Roles: rbac.Roles([]rbac.Role{
187189
{
188190
Name: "autostart",
@@ -203,7 +205,8 @@ var (
203205

204206
// See unhanger package.
205207
subjectHangDetector = rbac.Subject{
206-
ID: uuid.Nil.String(),
208+
FriendlyName: "Hang Detector",
209+
ID: uuid.Nil.String(),
207210
Roles: rbac.Roles([]rbac.Role{
208211
{
209212
Name: "hangdetector",
@@ -221,7 +224,8 @@ var (
221224
}.WithCachedASTValue()
222225

223226
subjectSystemRestricted = rbac.Subject{
224-
ID: uuid.Nil.String(),
227+
FriendlyName: "System",
228+
ID: uuid.Nil.String(),
225229
Roles: rbac.Roles([]rbac.Role{
226230
{
227231
Name: "system",

‎coderd/httpmw/apikey.go

Lines changed: 18 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -44,27 +44,15 @@ func APIKey(r *http.Request) database.APIKey {
4444
return key
4545
}
4646

47-
// User roles are the 'subject' field of Authorize()
48-
type userAuthKey struct{}
49-
50-
type Authorization struct {
51-
Actor rbac.Subject
52-
// ActorName is required for logging and human friendly related identification.
53-
// It is usually the "username" of the user, but it can be the name of the
54-
// external workspace proxy or other service type actor.
55-
ActorName string
56-
}
57-
5847
// UserAuthorizationOptional may return the roles and scope used for
5948
// authorization. Depends on the ExtractAPIKey handler.
60-
func UserAuthorizationOptional(r *http.Request) (Authorization, bool) {
61-
auth, ok := r.Context().Value(userAuthKey{}).(Authorization)
62-
return auth, ok
49+
func UserAuthorizationOptional(r *http.Request) (rbac.Subject, bool) {
50+
return dbauthz.ActorFromContext(r.Context())
6351
}
6452

6553
// UserAuthorization returns the roles and scope used for authorization. Depends
6654
// on the ExtractAPIKey handler.
67-
func UserAuthorization(r *http.Request) Authorization {
55+
func UserAuthorization(r *http.Request) rbac.Subject {
6856
auth, ok := UserAuthorizationOptional(r)
6957
if !ok {
7058
panic("developer error: ExtractAPIKey middleware not provided")
@@ -119,7 +107,7 @@ type ExtractAPIKeyConfig struct {
119107
//
120108
// This is originally implemented to send entitlement warning headers after
121109
// a user is authenticated to prevent additional CLI invocations.
122-
PostAuthAdditionalHeadersFunc func(a Authorization, header http.Header)
110+
PostAuthAdditionalHeadersFunc func(a rbac.Subject, header http.Header)
123111
}
124112

125113
// ExtractAPIKeyMW calls ExtractAPIKey with the given config on each request,
@@ -142,9 +130,8 @@ func ExtractAPIKeyMW(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
142130
// Actor is the user's authorization context.
143131
ctx := r.Context()
144132
ctx = context.WithValue(ctx, apiKeyContextKey{}, key)
145-
ctx = context.WithValue(ctx, userAuthKey{}, authz)
146-
// Set the auth context for the authzquerier as well.
147-
ctx = dbauthz.As(ctx, authz.Actor)
133+
// Set the auth context for the user.
134+
ctx = dbauthz.As(ctx, authz)
148135

149136
next.ServeHTTP(rw, r.WithContext(ctx))
150137
})
@@ -209,12 +196,12 @@ func APIKeyFromRequest(ctx context.Context, db database.Store, sessionTokenFunc
209196
// and authz object may be returned. False is returned if a response was written
210197
// to the request and the caller should give up.
211198
// nolint:revive
212-
func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyConfig) (*database.APIKey, *Authorization, bool) {
199+
func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyConfig) (*database.APIKey, *rbac.Subject, bool) {
213200
ctx := r.Context()
214201
// Write wraps writing a response to redirect if the handler
215202
// specified it should. This redirect is used for user-facing pages
216203
// like workspace applications.
217-
write := func(code int, response codersdk.Response) (*database.APIKey, *Authorization, bool) {
204+
write := func(code int, response codersdk.Response) (*database.APIKey, *rbac.Subject, bool) {
218205
if cfg.RedirectToLogin {
219206
RedirectToLogin(rw, r, nil, response.Message)
220207
return nil, nil, false
@@ -229,7 +216,7 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
229216
//
230217
// It should be used when the API key is not provided or is invalid,
231218
// but not when there are other errors.
232-
optionalWrite := func(code int, response codersdk.Response) (*database.APIKey, *Authorization, bool) {
219+
optionalWrite := func(code int, response codersdk.Response) (*database.APIKey, *rbac.Subject, bool) {
233220
if cfg.Optional {
234221
return nil, nil, true
235222
}
@@ -451,21 +438,19 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
451438
}
452439

453440
// Actor is the user's authorization context.
454-
authz := Authorization{
455-
ActorName: roles.Username,
456-
Actor: rbac.Subject{
457-
ID: key.UserID.String(),
458-
Roles: rbac.RoleNames(roles.Roles),
459-
Groups: roles.Groups,
460-
Scope: rbac.ScopeName(key.Scope),
461-
}.WithCachedASTValue(),
462-
}
441+
actor := rbac.Subject{
442+
FriendlyName: roles.Username,
443+
ID: key.UserID.String(),
444+
Roles: rbac.RoleNames(roles.Roles),
445+
Groups: roles.Groups,
446+
Scope: rbac.ScopeName(key.Scope),
447+
}.WithCachedASTValue()
463448

464449
if cfg.PostAuthAdditionalHeadersFunc != nil {
465-
cfg.PostAuthAdditionalHeadersFunc(authz, rw.Header())
450+
cfg.PostAuthAdditionalHeadersFunc(actor, rw.Header())
466451
}
467452

468-
return key, &authz, true
453+
return key, &actor, true
469454
}
470455

471456
// APITokenFromRequest returns the api token from the request.

‎coderd/httpmw/authorize_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ func TestExtractUserRoles(t *testing.T) {
127127
)
128128
rtr.Get("/", func(_ http.ResponseWriter, r *http.Request) {
129129
roles := httpmw.UserAuthorization(r)
130-
require.Equal(t, user.ID.String(), roles.Actor.ID)
131-
require.ElementsMatch(t, expRoles, roles.Actor.Roles.Names())
130+
require.Equal(t, user.ID.String(), roles.ID)
131+
require.ElementsMatch(t, expRoles, roles.Roles.Names())
132132
})
133133

134134
req := httptest.NewRequest("GET", "/", nil)

‎coderd/httpmw/provisionerdaemon.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"crypto/subtle"
66
"net/http"
77

8-
"golang.org/x/xerrors"
9-
108
"github.com/coder/coder/v2/coderd/database"
119
"github.com/coder/coder/v2/coderd/database/dbauthz"
1210
"github.com/coder/coder/v2/coderd/httpapi"
@@ -68,18 +66,6 @@ func ExtractProvisionerDaemonAuthenticated(opts ExtractProvisionerAuthConfig, ps
6866
ctx = context.WithValue(ctx, provisionerDaemonContextKey{}, true)
6967
// nolint:gocritic // Authenticating as a provisioner daemon.
7068
ctx = dbauthz.AsProvisionerd(ctx)
71-
subj, ok := dbauthz.ActorFromContext(ctx)
72-
if !ok {
73-
// This should never happen
74-
httpapi.InternalServerError(w, xerrors.New("developer error: ExtractProvisionerDaemonAuth missing rbac actor"))
75-
}
76-
77-
// Use the same subject for the userAuthKey
78-
ctx = context.WithValue(ctx, userAuthKey{}, Authorization{
79-
Actor: subj,
80-
ActorName: "provisioner_daemon",
81-
})
82-
8369
next.ServeHTTP(w, r.WithContext(ctx))
8470
})
8571
}

‎coderd/httpmw/ratelimit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func RateLimit(count int, window time.Duration) func(http.Handler) http.Handler
4747

4848
// We avoid using rbac.Authorizer since rego is CPU-intensive
4949
// and undermines the DoS-prevention goal of the rate limiter.
50-
for _, role := range auth.Actor.SafeRoleNames() {
50+
for _, role := range auth.SafeRoleNames() {
5151
if role == rbac.RoleOwner() {
5252
// HACK: use a random key each time to
5353
// de facto disable rate limiting. The

‎coderd/httpmw/workspaceproxy.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -141,18 +141,6 @@ func ExtractWorkspaceProxy(opts ExtractWorkspaceProxyConfig) func(http.Handler)
141141
// they can still only access the routes that the middleware is
142142
// mounted to.
143143
ctx = dbauthz.AsSystemRestricted(ctx)
144-
subj, ok := dbauthz.ActorFromContext(ctx)
145-
if !ok {
146-
// This should never happen
147-
httpapi.InternalServerError(w, xerrors.New("developer error: ExtractWorkspaceProxy missing rbac actor"))
148-
return
149-
}
150-
// Use the same subject for the userAuthKey
151-
ctx = context.WithValue(ctx, userAuthKey{}, Authorization{
152-
Actor: subj,
153-
ActorName: "proxy_" + proxy.Name,
154-
})
155-
156144
next.ServeHTTP(w, r.WithContext(ctx))
157145
})
158146
}

‎coderd/identityprovider/middleware.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func authorizeMW(accessURL *url.URL) func(next http.Handler) http.Handler {
142142
AppName: app.Name,
143143
CancelURI: cancel.String(),
144144
RedirectURI: r.URL.String(),
145-
Username: ua.ActorName,
145+
Username: ua.FriendlyName,
146146
})
147147
})
148148
}

‎coderd/rbac/authz.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ func hashAuthorizeCall(actor Subject, action Action, object Object) [32]byte {
7474
// Subject is a struct that contains all the elements of a subject in an rbac
7575
// authorize.
7676
type Subject struct {
77+
// FriendlyName is entirely optional and is used for logging and debugging
78+
// It is not used in any functional way.
79+
// It is usually the "username" of the user, but it can be the name of the
80+
// external workspace proxy or other service type actor.
81+
FriendlyName string
82+
7783
ID string
7884
Roles ExpandableRoles
7985
Groups []string

‎coderd/roles.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) {
2828
}
2929

3030
roles := rbac.SiteRoles()
31-
httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Actor.Roles, roles))
31+
httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles))
3232
}
3333

3434
// assignableSiteRoles returns all org wide roles that can be assigned.
@@ -52,7 +52,7 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
5252
}
5353

5454
roles := rbac.OrganizationRoles(organization.ID)
55-
httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Actor.Roles, roles))
55+
httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles))
5656
}
5757

5858
func assignableRoles(actorRoles rbac.ExpandableRoles, roles []rbac.Role) []codersdk.AssignableRoles {

‎coderd/users.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) {
518518
aReq.Old = user
519519
defer commitAudit()
520520

521-
if auth.Actor.ID == user.ID.String() {
521+
if auth.ID == user.ID.String() {
522522
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
523523
Message: "You cannot delete yourself!",
524524
})

0 commit comments

Comments
 (0)