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.
coder · Emyrk · Mar 29, 2024 · Mar 29, 2024 · Mar 29, 2024 · Mar 29, 2024
commit 3bb9e11c603331bac3459bcf3d48bfc998cc9104
diff --git a/coderd/authorize.go b/coderd/authorize.go
@@ -19,15 +19,15 @@ import (
 // This is faster than calling Authorize() on each object.
 func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
 	roles := httpmw.UserAuthorization(r)
-	objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.Actor, action, objects)
+	objects, err := rbac.Filter(r.Context(), h.Authorizer, roles, action, objects)
 	if err != nil {
 		// Log the error as Filter should not be erroring.
 		h.Logger.Error(r.Context(), "authorization filter failed",
 			slog.Error(err),
-			slog.F("user_id", roles.Actor.ID),
-			slog.F("username", roles.ActorName),
-			slog.F("roles", roles.Actor.SafeRoleNames()),
-			slog.F("scope", roles.Actor.SafeScopeName()),
+			slog.F("user_id", roles.ID),
+			slog.F("username", roles),
+			slog.F("roles", roles.SafeRoleNames()),
+			slog.F("scope", roles.SafeScopeName()),
 			slog.F("route", r.URL.Path),
 			slog.F("action", action),
 		)
@@ -65,7 +65,7 @@ func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objec
 //	}
 func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
 	roles := httpmw.UserAuthorization(r)
-	err := h.Authorizer.Authorize(r.Context(), roles.Actor, action, object.RBACObject())
+	err := h.Authorizer.Authorize(r.Context(), roles, action, object.RBACObject())
 	if err != nil {
 		// Log the errors for debugging
 		internalError := new(rbac.UnauthorizedError)
@@ -76,10 +76,10 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
 		// Log information for debugging. This will be very helpful
 		// in the early days
 		logger.Warn(r.Context(), "requester is not authorized to access the object",
-			slog.F("roles", roles.Actor.SafeRoleNames()),
-			slog.F("actor_id", roles.Actor.ID),
-			slog.F("actor_name", roles.ActorName),
-			slog.F("scope", roles.Actor.SafeScopeName()),
+			slog.F("roles", roles.SafeRoleNames()),
+			slog.F("actor_id", roles.ID),
+			slog.F("actor_name", roles),
+			slog.F("scope", roles.SafeScopeName()),
 			slog.F("route", r.URL.Path),
 			slog.F("action", action),
 			slog.F("object", object),
@@ -97,7 +97,7 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
 // Note the authorization is only for the given action and object type.
 func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action rbac.Action, objectType string) (rbac.PreparedAuthorized, error) {
 	roles := httpmw.UserAuthorization(r)
-	prepared, err := h.Authorizer.Prepare(r.Context(), roles.Actor, action, objectType)
+	prepared, err := h.Authorizer.Prepare(r.Context(), roles, action, objectType)
 	if err != nil {
 		return nil, xerrors.Errorf("prepare filter: %w", err)
 	}
@@ -128,10 +128,10 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
 
 	api.Logger.Debug(ctx, "check-auth",
 		slog.F("my_id", httpmw.APIKey(r).UserID),
-		slog.F("got_id", auth.Actor.ID),
-		slog.F("name", auth.ActorName),
-		slog.F("roles", auth.Actor.SafeRoleNames()),
-		slog.F("scope", auth.Actor.SafeScopeName()),
+		slog.F("got_id", auth.ID),
+		slog.F("name", auth),
+		slog.F("roles", auth.SafeRoleNames()),
+		slog.F("scope", auth.SafeScopeName()),
 	)
 
 	response := make(codersdk.AuthorizationResponse)
@@ -171,7 +171,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
 			Type:  v.Object.ResourceType.String(),
 		}
 		if obj.Owner == "me" {
-			obj.Owner = auth.Actor.ID
+			obj.Owner = auth.ID
 		}
 
 		// If a resource ID is specified, fetch that specific resource.
@@ -219,7 +219,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
 			obj = dbObj.RBACObject()
 		}
 
-		err := api.Authorizer.Authorize(ctx, auth.Actor, rbac.Action(v.Action), obj)
+		err := api.Authorizer.Authorize(ctx, auth, rbac.Action(v.Action), obj)
 		response[k] = err == nil
 	}
 

diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -133,7 +133,7 @@ type Options struct {
 	// after a successful authentication.
 	// This is somewhat janky, but seemingly the only reasonable way to add a header
 	// for all authenticated users under a condition, only in Enterprise.
-	PostAuthAdditionalHeadersFunc func(auth httpmw.Authorization, header http.Header)
+	PostAuthAdditionalHeadersFunc func(auth rbac.Subject, header http.Header)
 
 	// TLSCertificates is used to mesh DERP servers securely.
 	TLSCertificates    []tls.Certificate

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -155,7 +155,8 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) {
 
 var (
 	subjectProvisionerd = rbac.Subject{
-		ID: uuid.Nil.String(),
+		FriendlyName: "Provisioner Daemon",
+		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
 			{
 				Name:        "provisionerd",
@@ -182,7 +183,8 @@ var (
 	}.WithCachedASTValue()
 
 	subjectAutostart = rbac.Subject{
-		ID: uuid.Nil.String(),
+		FriendlyName: "Autostart",
+		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
 			{
 				Name:        "autostart",
@@ -203,7 +205,8 @@ var (
 
 	// See unhanger package.
 	subjectHangDetector = rbac.Subject{
-		ID: uuid.Nil.String(),
+		FriendlyName: "Hang Detector",
+		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
 			{
 				Name:        "hangdetector",
@@ -221,7 +224,8 @@ var (
 	}.WithCachedASTValue()
 
 	subjectSystemRestricted = rbac.Subject{
-		ID: uuid.Nil.String(),
+		FriendlyName: "System",
+		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
 			{
 				Name:        "system",

diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
@@ -44,27 +44,15 @@ func APIKey(r *http.Request) database.APIKey {
 	return key
 }
 
-// User roles are the 'subject' field of Authorize()
-type userAuthKey struct{}
-
-type Authorization struct {
-	Actor rbac.Subject
-	// ActorName is required for logging and human friendly related identification.
-	// It is usually the "username" of the user, but it can be the name of the
-	// external workspace proxy or other service type actor.
-	ActorName string
-}
-
 // UserAuthorizationOptional may return the roles and scope used for
 // authorization. Depends on the ExtractAPIKey handler.
-func UserAuthorizationOptional(r *http.Request) (Authorization, bool) {
-	auth, ok := r.Context().Value(userAuthKey{}).(Authorization)
-	return auth, ok
+func UserAuthorizationOptional(r *http.Request) (rbac.Subject, bool) {
+	return dbauthz.ActorFromContext(r.Context())
 }
 
 // UserAuthorization returns the roles and scope used for authorization. Depends
 // on the ExtractAPIKey handler.
-func UserAuthorization(r *http.Request) Authorization {
+func UserAuthorization(r *http.Request) rbac.Subject {
 	auth, ok := UserAuthorizationOptional(r)
 	if !ok {
 		panic("developer error: ExtractAPIKey middleware not provided")
@@ -119,7 +107,7 @@ type ExtractAPIKeyConfig struct {
 	//
 	// This is originally implemented to send entitlement warning headers after
 	// a user is authenticated to prevent additional CLI invocations.
-	PostAuthAdditionalHeadersFunc func(a Authorization, header http.Header)
+	PostAuthAdditionalHeadersFunc func(a rbac.Subject, header http.Header)
 }
 
 // ExtractAPIKeyMW calls ExtractAPIKey with the given config on each request,
@@ -142,9 +130,8 @@ func ExtractAPIKeyMW(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
 			// Actor is the user's authorization context.
 			ctx := r.Context()
 			ctx = context.WithValue(ctx, apiKeyContextKey{}, key)
-			ctx = context.WithValue(ctx, userAuthKey{}, authz)
-			// Set the auth context for the authzquerier as well.
-			ctx = dbauthz.As(ctx, authz.Actor)
+			// Set the auth context for the user.
+			ctx = dbauthz.As(ctx, authz)
 
 			next.ServeHTTP(rw, r.WithContext(ctx))
 		})
@@ -209,12 +196,12 @@ func APIKeyFromRequest(ctx context.Context, db database.Store, sessionTokenFunc
 // and authz object may be returned. False is returned if a response was written
 // to the request and the caller should give up.
 // nolint:revive
-func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyConfig) (*database.APIKey, *Authorization, bool) {
+func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyConfig) (*database.APIKey, *rbac.Subject, bool) {
 	ctx := r.Context()
 	// Write wraps writing a response to redirect if the handler
 	// specified it should. This redirect is used for user-facing pages
 	// like workspace applications.
-	write := func(code int, response codersdk.Response) (*database.APIKey, *Authorization, bool) {
+	write := func(code int, response codersdk.Response) (*database.APIKey, *rbac.Subject, bool) {
 		if cfg.RedirectToLogin {
 			RedirectToLogin(rw, r, nil, response.Message)
 			return nil, nil, false
@@ -229,7 +216,7 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
 	//
 	// It should be used when the API key is not provided or is invalid,
 	// but not when there are other errors.
-	optionalWrite := func(code int, response codersdk.Response) (*database.APIKey, *Authorization, bool) {
+	optionalWrite := func(code int, response codersdk.Response) (*database.APIKey, *rbac.Subject, bool) {
 		if cfg.Optional {
 			return nil, nil, true
 		}
@@ -451,21 +438,19 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
 	}
 
 	// Actor is the user's authorization context.
-	authz := Authorization{
-		ActorName: roles.Username,
-		Actor: rbac.Subject{
-			ID:     key.UserID.String(),
-			Roles:  rbac.RoleNames(roles.Roles),
-			Groups: roles.Groups,
-			Scope:  rbac.ScopeName(key.Scope),
-		}.WithCachedASTValue(),
-	}
+	actor := rbac.Subject{
+		FriendlyName: roles.Username,
+		ID:           key.UserID.String(),
+		Roles:        rbac.RoleNames(roles.Roles),
+		Groups:       roles.Groups,
+		Scope:        rbac.ScopeName(key.Scope),
+	}.WithCachedASTValue()
 
 	if cfg.PostAuthAdditionalHeadersFunc != nil {
-		cfg.PostAuthAdditionalHeadersFunc(authz, rw.Header())
+		cfg.PostAuthAdditionalHeadersFunc(actor, rw.Header())
 	}
 
-	return key, &authz, true
+	return key, &actor, true
 }
 
 // APITokenFromRequest returns the api token from the request.

diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go
@@ -127,8 +127,8 @@ func TestExtractUserRoles(t *testing.T) {
 			)
 			rtr.Get("/", func(_ http.ResponseWriter, r *http.Request) {
 				roles := httpmw.UserAuthorization(r)
-				require.Equal(t, user.ID.String(), roles.Actor.ID)
-				require.ElementsMatch(t, expRoles, roles.Actor.Roles.Names())
+				require.Equal(t, user.ID.String(), roles.ID)
+				require.ElementsMatch(t, expRoles, roles.Roles.Names())
 			})
 
 			req := httptest.NewRequest("GET", "/", nil)

diff --git a/coderd/httpmw/provisionerdaemon.go b/coderd/httpmw/provisionerdaemon.go
@@ -5,8 +5,6 @@ import (
 	"crypto/subtle"
 	"net/http"
 
-	"golang.org/x/xerrors"
-
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbauthz"
 	"github.com/coder/coder/v2/coderd/httpapi"
@@ -68,18 +66,6 @@ func ExtractProvisionerDaemonAuthenticated(opts ExtractProvisionerAuthConfig, ps
 			ctx = context.WithValue(ctx, provisionerDaemonContextKey{}, true)
 			// nolint:gocritic // Authenticating as a provisioner daemon.
 			ctx = dbauthz.AsProvisionerd(ctx)
-			subj, ok := dbauthz.ActorFromContext(ctx)
-			if !ok {
-				// This should never happen
-				httpapi.InternalServerError(w, xerrors.New("developer error: ExtractProvisionerDaemonAuth missing rbac actor"))
-			}
-
-			// Use the same subject for the userAuthKey
-			ctx = context.WithValue(ctx, userAuthKey{}, Authorization{
-				Actor:     subj,
-				ActorName: "provisioner_daemon",
-			})
-
 			next.ServeHTTP(w, r.WithContext(ctx))
 		})
 	}

diff --git a/coderd/httpmw/ratelimit.go b/coderd/httpmw/ratelimit.go
@@ -47,7 +47,7 @@ func RateLimit(count int, window time.Duration) func(http.Handler) http.Handler
 
 			// We avoid using rbac.Authorizer since rego is CPU-intensive
 			// and undermines the DoS-prevention goal of the rate limiter.
-			for _, role := range auth.Actor.SafeRoleNames() {
+			for _, role := range auth.SafeRoleNames() {
 				if role == rbac.RoleOwner() {
 					// HACK: use a random key each time to
 					// de facto disable rate limiting. The

diff --git a/coderd/httpmw/workspaceproxy.go b/coderd/httpmw/workspaceproxy.go
@@ -141,18 +141,6 @@ func ExtractWorkspaceProxy(opts ExtractWorkspaceProxyConfig) func(http.Handler)
 			// they can still only access the routes that the middleware is
 			// mounted to.
 			ctx = dbauthz.AsSystemRestricted(ctx)
-			subj, ok := dbauthz.ActorFromContext(ctx)
-			if !ok {
-				// This should never happen
-				httpapi.InternalServerError(w, xerrors.New("developer error: ExtractWorkspaceProxy missing rbac actor"))
-				return
-			}
-			// Use the same subject for the userAuthKey
-			ctx = context.WithValue(ctx, userAuthKey{}, Authorization{
-				Actor:     subj,
-				ActorName: "proxy_" + proxy.Name,
-			})
-
 			next.ServeHTTP(w, r.WithContext(ctx))
 		})
 	}

diff --git a/coderd/identityprovider/middleware.go b/coderd/identityprovider/middleware.go
@@ -142,7 +142,7 @@ func authorizeMW(accessURL *url.URL) func(next http.Handler) http.Handler {
 				AppName:     app.Name,
 				CancelURI:   cancel.String(),
 				RedirectURI: r.URL.String(),
-				Username:    ua.ActorName,
+				Username:    ua.FriendlyName,
 			})
 		})
 	}

diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go
@@ -74,6 +74,12 @@ func hashAuthorizeCall(actor Subject, action Action, object Object) [32]byte {
 // Subject is a struct that contains all the elements of a subject in an rbac
 // authorize.
 type Subject struct {
+	// FriendlyName is entirely optional and is used for logging and debugging
+	// It is not used in any functional way.
+	// It is usually the "username" of the user, but it can be the name of the
+	// external workspace proxy or other service type actor.
+	FriendlyName string
+
 	ID     string
 	Roles  ExpandableRoles
 	Groups []string

diff --git a/coderd/roles.go b/coderd/roles.go
@@ -28,7 +28,7 @@ func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	roles := rbac.SiteRoles()
-	httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Actor.Roles, roles))
+	httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles))
 }
 
 // assignableSiteRoles returns all org wide roles that can be assigned.
@@ -52,7 +52,7 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	roles := rbac.OrganizationRoles(organization.ID)
-	httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Actor.Roles, roles))
+	httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles))
 }
 
 func assignableRoles(actorRoles rbac.ExpandableRoles, roles []rbac.Role) []codersdk.AssignableRoles {

diff --git a/coderd/users.go b/coderd/users.go
@@ -518,7 +518,7 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) {
 	aReq.Old = user
 	defer commitAudit()
 
-	if auth.Actor.ID == user.ID.String() {
+	if auth.ID == user.ID.String() {
 		httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
 			Message: "You cannot delete yourself!",
 		})