chore: merge authorization contexts (#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
This commit is contained in:
Steven Masley 2024-03-29 10:14:27 -05:00 committed by GitHub
parent 8e2d026d99
commit eeb3d63be6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 68 additions and 99 deletions

View File

@ -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
}

View File

@ -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

View File

@ -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",

View File

@ -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.

View File

@ -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)

View File

@ -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))
})
}

View File

@ -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

View File

@ -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))
})
}

View File

@ -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,
})
})
}

View File

@ -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

View File

@ -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 {

View File

@ -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!",
})

View File

@ -224,7 +224,7 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
// are warnings that aid in debugging. These messages do not prevent authorization,
// but may indicate that the request is not configured correctly.
// If an error is returned, the request should be aborted with a 500 error.
func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, []string, error) {
func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *rbac.Subject, dbReq *databaseRequest) (bool, []string, error) {
var warnings []string
accessMethod := dbReq.AccessMethod
if accessMethod == "" {
@ -267,12 +267,12 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
// workspaces owned by different users.
if isPathApp &&
sharingLevel == database.AppSharingLevelOwner &&
dbReq.Workspace.OwnerID.String() != roles.Actor.ID &&
dbReq.Workspace.OwnerID.String() != roles.ID &&
!p.DeploymentValues.Dangerous.AllowPathAppSiteOwnerAccess.Value() {
// This is not ideal to check for the 'owner' role, but we are only checking
// to determine whether to show a warning for debugging reasons. This does
// not do any authz checks, so it is ok.
if roles != nil && slices.Contains(roles.Actor.Roles.Names(), rbac.RoleOwner()) {
if roles != nil && slices.Contains(roles.Roles.Names(), rbac.RoleOwner()) {
warnings = append(warnings, "path-based apps with \"owner\" share level are only accessible by the workspace owner (see --dangerous-allow-path-app-site-owner-access)")
}
return false, warnings, nil
@ -286,11 +286,11 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
// rbacResourceOwned is for the level "authenticated". We still need to
// make sure the API key has permissions to connect to the actor's own
// workspace. Scopes would prevent this.
rbacResourceOwned rbac.Object = rbac.ResourceWorkspaceApplicationConnect.WithOwner(roles.Actor.ID)
rbacResourceOwned rbac.Object = rbac.ResourceWorkspaceApplicationConnect.WithOwner(roles.ID)
)
if dbReq.AccessMethod == AccessMethodTerminal {
rbacResource = dbReq.Workspace.ExecutionRBAC()
rbacResourceOwned = rbac.ResourceWorkspaceExecution.WithOwner(roles.Actor.ID)
rbacResourceOwned = rbac.ResourceWorkspaceExecution.WithOwner(roles.ID)
}
// Do a standard RBAC check. This accounts for share level "owner" and any
@ -299,7 +299,7 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
// Regardless of share level or whether it's enabled or not, the owner of
// the workspace can always access applications (as long as their API key's
// scope allows it).
err := p.Authorizer.Authorize(ctx, roles.Actor, rbacAction, rbacResource)
err := p.Authorizer.Authorize(ctx, *roles, rbacAction, rbacResource)
if err == nil {
return true, []string{}, nil
}
@ -312,7 +312,7 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
case database.AppSharingLevelAuthenticated:
// Check with the owned resource to ensure the API key has permissions
// to connect to the actor's own workspace. This enforces scopes.
err := p.Authorizer.Authorize(ctx, roles.Actor, rbacAction, rbacResourceOwned)
err := p.Authorizer.Authorize(ctx, *roles, rbacAction, rbacResourceOwned)
if err == nil {
return true, []string{}, nil
}

View File

@ -476,8 +476,8 @@ type API struct {
//
// This header is used by the CLI to display warnings to the user without having
// to make additional requests!
func (api *API) writeEntitlementWarningsHeader(a httpmw.Authorization, header http.Header) {
roles, err := a.Actor.Roles.Expand()
func (api *API) writeEntitlementWarningsHeader(a rbac.Subject, header http.Header) {
roles, err := a.Roles.Expand()
if err != nil {
return
}

View File

@ -107,7 +107,7 @@ func (p *provisionerDaemonAuth) authorize(r *http.Request, tags map[string]strin
return tags, true
}
ua := httpmw.UserAuthorization(r)
if err := p.authorizer.Authorize(ctx, ua.Actor, rbac.ActionCreate, rbac.ResourceProvisionerDaemon); err == nil {
if err := p.authorizer.Authorize(ctx, ua, rbac.ActionCreate, rbac.ResourceProvisionerDaemon); err == nil {
// User is allowed to create provisioner daemons
return tags, true
}

View File

@ -351,7 +351,7 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
return execTmpl(tmpl, state)
}
ctx := dbauthz.As(r.Context(), actor.Actor)
ctx := dbauthz.As(r.Context(), *actor)
var eg errgroup.Group
var user database.User