From 603ec8e5d7bfd1d543faedbce1f67653760a38f3 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 15 Feb 2023 13:47:28 +0000 Subject: [PATCH 1/3] chore: break down dbauthz.System into smaller roles - rbac: export rbac.Permissions - dbauthz: move GetDeploymentDAUs, GetTemplateDAUs, GetTemplateAverageBuildTime from querier.go to system.go and removes auth checks - dbauthz: remove AsSystem(), add invididual roles for metrics cache, autostart, provisionerd, add restricted system role for everything else --- coderd/activitybump.go | 2 +- .../autobuild/executor/lifecycle_executor.go | 4 +- coderd/database/dbauthz/dbauthz.go | 107 ++++++++++++++---- coderd/database/dbauthz/querier.go | 26 ----- coderd/database/dbauthz/querier_test.go | 13 --- coderd/database/dbauthz/setup_test.go | 4 +- coderd/database/dbauthz/system.go | 15 +++ coderd/httpmw/apikey.go | 12 +- coderd/httpmw/authz.go | 2 +- coderd/httpmw/hsts_test.go | 2 +- coderd/httpmw/userparam.go | 6 +- coderd/httpmw/workspaceagent.go | 4 +- coderd/metricscache/metricscache.go | 2 +- .../provisionerdserver/provisionerdserver.go | 24 ++-- coderd/rbac/authz_internal_test.go | 4 +- coderd/rbac/builtin.go | 16 +-- coderd/rbac/scopes.go | 4 +- coderd/userauth.go | 22 ++-- coderd/users.go | 10 +- coderd/workspaceagents.go | 2 +- coderd/workspaceapps.go | 10 +- coderd/workspaceresourceauth.go | 10 +- enterprise/coderd/coderd_test.go | 31 ++++- enterprise/coderd/scim.go | 6 +- provisionerd/runner/runner.go | 4 +- 25 files changed, 201 insertions(+), 141 deletions(-) diff --git a/coderd/activitybump.go b/coderd/activitybump.go index 63cfacb528c2f..2c6c5e38bf0be 100644 --- a/coderd/activitybump.go +++ b/coderd/activitybump.go @@ -83,7 +83,7 @@ func activityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Sto }, nil) if err != nil { if !xerrors.Is(err, context.Canceled) { - // Bump will fail if the context is cancelled, but this is ok. + // Bump will fail if the context is canceled, but this is ok. log.Error(ctx, "bump failed", slog.Error(err), slog.F("workspace_id", workspaceID), ) diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 5af701de4b89d..f42894cd9682e 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -34,8 +34,8 @@ type Stats struct { // New returns a new autobuild executor. func New(ctx context.Context, db database.Store, log slog.Logger, tick <-chan time.Time) *Executor { le := &Executor{ - //nolint:gocritic // TODO: make an autostart role instead of using System - ctx: dbauthz.AsSystem(ctx), + //nolint:gocritic // Autostart has a limited set of permissions. + ctx: dbauthz.AsAutostart(ctx), db: db, tick: tick, log: log, diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index a5f9dcff54916..5ab903aea92de 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -46,12 +46,12 @@ func logNotAuthorizedError(ctx context.Context, logger slog.Logger, err error) e if err != nil && xerrors.As(err, &internalError) { e := new(topdown.Error) if xerrors.As(err, &e) || e.Code == topdown.CancelErr { - // For some reason rego changes a cancelled context to a topdown.CancelErr. We - // expect to check for cancelled context errors if the user cancels the request, + // For some reason rego changes a canceled context to a topdown.CancelErr. We + // expect to check for canceled context errors if the user cancels the request, // so we should change the error to a context.Canceled error. // // NotAuthorizedError is == to sql.ErrNoRows, which is not correct - // if it's actually a cancelled context. + // if it's actually a canceled context. internalError.SetInternal(context.Canceled) return internalError } @@ -117,29 +117,94 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) { return a, ok } -// AsSystem returns a context with a system actor. This is used for internal -// system operations that are not tied to any particular actor. -// When you use this function, be sure to add a //nolint comment -// explaining why it is necessary. -// -// We trust you have received the usual lecture from the local System -// Administrator. It usually boils down to these three things: -// #1) Respect the privacy of others. -// #2) Think before you type. -// #3) With great power comes great responsibility. -func AsSystem(ctx context.Context) context.Context { +// AsProvisionerd returns a context with an actor that has permissions required +// for provisionerd to function. +func AsProvisionerd(ctx context.Context) context.Context { + return context.WithValue(ctx, authContextKey{}, rbac.Subject{ + ID: uuid.Nil.String(), + Roles: rbac.Roles([]rbac.Role{ + { + Name: "provisionerd", + DisplayName: "Provisioner Daemon", + Site: rbac.Permissions(map[string][]rbac.Action{ + rbac.ResourceFile.Type: {rbac.ActionRead}, + rbac.ResourceTemplate.Type: {rbac.ActionRead, rbac.ActionUpdate}, + rbac.ResourceUser.Type: {rbac.ActionRead}, + rbac.ResourceWorkspace.Type: {rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, + }), + Org: map[string][]rbac.Permission{}, + User: []rbac.Permission{}, + }, + }), + Scope: rbac.ScopeAll, + }, + ) +} + +// AsAutostart returns a context with an actor that has permissions required +// for autostart to function. +func AsAutostart(ctx context.Context) context.Context { + return context.WithValue(ctx, authContextKey{}, rbac.Subject{ + ID: uuid.Nil.String(), + Roles: rbac.Roles([]rbac.Role{ + { + Name: "autostart", + DisplayName: "Autostart Daemon", + Site: rbac.Permissions(map[string][]rbac.Action{ + rbac.ResourceTemplate.Type: {rbac.ActionRead, rbac.ActionUpdate}, + rbac.ResourceWorkspace.Type: {rbac.ActionRead, rbac.ActionUpdate}, + }), + Org: map[string][]rbac.Permission{}, + User: []rbac.Permission{}, + }, + }), + Scope: rbac.ScopeAll, + }, + ) +} + +// AsMetricsCache returns a context with an actor that has permissions required +// for the metrics cache to function. +func AsMetricsCache(ctx context.Context) context.Context { + return context.WithValue(ctx, authContextKey{}, rbac.Subject{ + ID: uuid.Nil.String(), + Roles: rbac.Roles([]rbac.Role{ + { + Name: "metrics-cache", + DisplayName: "Metrics Cache", + Site: rbac.Permissions(map[string][]rbac.Action{ + rbac.ResourceTemplate.Type: {rbac.ActionRead}, + }), + Org: map[string][]rbac.Permission{}, + User: []rbac.Permission{}, + }, + }), + Scope: rbac.ScopeAll, + }, + ) +} + +// AsSystemRestricted returns a context with an actor that has permissions +// required for various system operations e.g. login, logout. +func AsSystemRestricted(ctx context.Context) context.Context { return context.WithValue(ctx, authContextKey{}, rbac.Subject{ ID: uuid.Nil.String(), Roles: rbac.Roles([]rbac.Role{ { Name: "system", - DisplayName: "System", - Site: []rbac.Permission{ - { - ResourceType: rbac.ResourceWildcard.Type, - Action: rbac.WildcardSymbol, - }, - }, + DisplayName: "Coder", + Site: rbac.Permissions(map[string][]rbac.Action{ + rbac.ResourceWildcard.Type: {rbac.ActionRead}, + rbac.ResourceAPIKey.Type: {rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, + rbac.ResourceGroup.Type: {rbac.ActionCreate, rbac.ActionUpdate}, + rbac.ResourceRoleAssignment.Type: {rbac.ActionCreate}, + rbac.ResourceOrganization.Type: {rbac.ActionCreate}, + rbac.ResourceOrganizationMember.Type: {rbac.ActionCreate}, + rbac.ResourceOrgRoleAssignment.Type: {rbac.ActionCreate}, + rbac.ResourceUser.Type: {rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, + rbac.ResourceUserData.Type: {rbac.ActionCreate, rbac.ActionUpdate}, + rbac.ResourceWorkspace.Type: {rbac.ActionUpdate}, + }), Org: map[string][]rbac.Permission{}, User: []rbac.Permission{}, }, diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index 89692504378fb..3f32b3360a04d 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -327,13 +327,6 @@ func (q *querier) GetProvisionerDaemons(ctx context.Context) ([]database.Provisi return fetchWithPostFilter(q.auth, fetch)(ctx, nil) } -func (q *querier) GetDeploymentDAUs(ctx context.Context) ([]database.GetDeploymentDAUsRow, error) { - if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.All()); err != nil { - return nil, err - } - return q.db.GetDeploymentDAUs(ctx) -} - func (q *querier) GetGroupsByOrganizationID(ctx context.Context, organizationID uuid.UUID) ([]database.Group, error) { return fetchWithPostFilter(q.auth, q.db.GetGroupsByOrganizationID)(ctx, organizationID) } @@ -622,16 +615,6 @@ func (q *querier) GetPreviousTemplateVersion(ctx context.Context, arg database.G return q.db.GetPreviousTemplateVersion(ctx, arg) } -func (q *querier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { - // An actor can read the average build time if they can read the related template. - // It doesn't make any sense to get the average build time for a template that doesn't - // exist, so omitting this check here. - if _, err := q.GetTemplateByID(ctx, arg.TemplateID.UUID); err != nil { - return database.GetTemplateAverageBuildTimeRow{}, err - } - return q.db.GetTemplateAverageBuildTime(ctx, arg) -} - func (q *querier) GetTemplateByID(ctx context.Context, id uuid.UUID) (database.Template, error) { return fetch(q.log, q.auth, q.db.GetTemplateByID)(ctx, id) } @@ -640,15 +623,6 @@ func (q *querier) GetTemplateByOrganizationAndName(ctx context.Context, arg data return fetch(q.log, q.auth, q.db.GetTemplateByOrganizationAndName)(ctx, arg) } -func (q *querier) GetTemplateDAUs(ctx context.Context, templateID uuid.UUID) ([]database.GetTemplateDAUsRow, error) { - // An actor can read the DAUs if they can read the related template. - // Again, it doesn't make sense to get DAUs for a template that doesn't exist. - if _, err := q.GetTemplateByID(ctx, templateID); err != nil { - return nil, err - } - return q.db.GetTemplateDAUs(ctx, templateID) -} - func (q *querier) GetTemplateVersionByID(ctx context.Context, tvid uuid.UUID) (database.TemplateVersion, error) { tv, err := q.db.GetTemplateVersionByID(ctx, tvid) if err != nil { diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go index 96290f57745ab..a22b70ac0861f 100644 --- a/coderd/database/dbauthz/querier_test.go +++ b/coderd/database/dbauthz/querier_test.go @@ -540,12 +540,6 @@ func (s *MethodTestSuite) TestTemplate() { TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}, }).Asserts(t1, rbac.ActionRead).Returns(b) })) - s.Run("GetTemplateAverageBuildTime", s.Subtest(func(db database.Store, check *expects) { - t1 := dbgen.Template(s.T(), db, database.Template{}) - check.Args(database.GetTemplateAverageBuildTimeParams{ - TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}, - }).Asserts(t1, rbac.ActionRead) - })) s.Run("GetTemplateByID", s.Subtest(func(db database.Store, check *expects) { t1 := dbgen.Template(s.T(), db, database.Template{}) check.Args(t1.ID).Asserts(t1, rbac.ActionRead).Returns(t1) @@ -560,10 +554,6 @@ func (s *MethodTestSuite) TestTemplate() { OrganizationID: o1.ID, }).Asserts(t1, rbac.ActionRead).Returns(t1) })) - s.Run("GetTemplateDAUs", s.Subtest(func(db database.Store, check *expects) { - t1 := dbgen.Template(s.T(), db, database.Template{}) - check.Args(t1.ID).Asserts(t1, rbac.ActionRead) - })) s.Run("GetTemplateVersionByJobID", s.Subtest(func(db database.Store, check *expects) { t1 := dbgen.Template(s.T(), db, database.Template{}) tv := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ @@ -1220,7 +1210,4 @@ func (s *MethodTestSuite) TestExtraMethods() { s.NoError(err, "insert provisioner daemon") check.Args().Asserts(d, rbac.ActionRead) })) - s.Run("GetDeploymentDAUs", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts(rbac.ResourceUser.All(), rbac.ActionRead) - })) } diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index 0b5dad6cf2378..d6e44406e7fa1 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -226,8 +226,8 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd } }) - s.Run("Cancelled", func() { - // Pass in a cancelled context + s.Run("Canceled", func() { + // Pass in a canceled context ctx, cancel := context.WithCancel(ctx) cancel() az.AlwaysReturn = rbac.ForbiddenWithInternal(&topdown.Error{Code: topdown.CancelErr}, diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go index bec4a6ae052e0..e624c49f65f7e 100644 --- a/coderd/database/dbauthz/system.go +++ b/coderd/database/dbauthz/system.go @@ -96,6 +96,21 @@ func (q *querier) GetTemplates(ctx context.Context) ([]database.Template, error) return q.db.GetTemplates(ctx) } +// Only used by metrics cache. +func (q *querier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { + return q.db.GetTemplateAverageBuildTime(ctx, arg) +} + +// Only used by metrics cache. +func (q *querier) GetTemplateDAUs(ctx context.Context, templateID uuid.UUID) ([]database.GetTemplateDAUsRow, error) { + return q.db.GetTemplateDAUs(ctx, templateID) +} + +// Only used by metrics cache. +func (q *querier) GetDeploymentDAUs(ctx context.Context) ([]database.GetDeploymentDAUsRow, error) { + return q.db.GetDeploymentDAUs(ctx) +} + // UpdateWorkspaceBuildCostByID is used by the provisioning system to update the cost of a workspace build. func (q *querier) UpdateWorkspaceBuildCostByID(ctx context.Context, arg database.UpdateWorkspaceBuildCostByIDParams) (database.WorkspaceBuild, error) { return q.db.UpdateWorkspaceBuildCostByID(ctx, arg) diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index 553fe43d89ef9..535466b264d7c 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -161,7 +161,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { } //nolint:gocritic // System needs to fetch API key to check if it's valid. - key, err := cfg.DB.GetAPIKeyByID(dbauthz.AsSystem(ctx), keyID) + key, err := cfg.DB.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), keyID) if err != nil { if errors.Is(err, sql.ErrNoRows) { optionalWrite(http.StatusUnauthorized, codersdk.Response{ @@ -195,7 +195,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { ) if key.LoginType == database.LoginTypeGithub || key.LoginType == database.LoginTypeOIDC { //nolint:gocritic // System needs to fetch UserLink to check if it's valid. - link, err = cfg.DB.GetUserLinkByUserIDLoginType(dbauthz.AsSystem(ctx), database.GetUserLinkByUserIDLoginTypeParams{ + link, err = cfg.DB.GetUserLinkByUserIDLoginType(dbauthz.AsSystemRestricted(ctx), database.GetUserLinkByUserIDLoginTypeParams{ UserID: key.UserID, LoginType: key.LoginType, }) @@ -279,7 +279,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { } if changed { //nolint:gocritic // System needs to update API Key LastUsed - err := cfg.DB.UpdateAPIKeyByID(dbauthz.AsSystem(ctx), database.UpdateAPIKeyByIDParams{ + err := cfg.DB.UpdateAPIKeyByID(dbauthz.AsSystemRestricted(ctx), database.UpdateAPIKeyByIDParams{ ID: key.ID, LastUsed: key.LastUsed, ExpiresAt: key.ExpiresAt, @@ -296,7 +296,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { // then we want to update the relevant oauth fields. if link.UserID != uuid.Nil { // nolint:gocritic - link, err = cfg.DB.UpdateUserLink(dbauthz.AsSystem(ctx), database.UpdateUserLinkParams{ + link, err = cfg.DB.UpdateUserLink(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLinkParams{ UserID: link.UserID, LoginType: link.LoginType, OAuthAccessToken: link.OAuthAccessToken, @@ -316,7 +316,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { // load. We update alongside the UserLink and APIKey since it's // easier on the DB to colocate writes. // nolint:gocritic - _, err = cfg.DB.UpdateUserLastSeenAt(dbauthz.AsSystem(ctx), database.UpdateUserLastSeenAtParams{ + _, err = cfg.DB.UpdateUserLastSeenAt(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLastSeenAtParams{ ID: key.UserID, LastSeenAt: database.Now(), UpdatedAt: database.Now(), @@ -334,7 +334,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { // The roles are used for RBAC authorize checks, and the status // is to block 'suspended' users from accessing the platform. // nolint:gocritic - roles, err := cfg.DB.GetAuthorizationUserRoles(dbauthz.AsSystem(ctx), key.UserID) + roles, err := cfg.DB.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), key.UserID) if err != nil { write(http.StatusUnauthorized, codersdk.Response{ Message: internalErrorMessage, diff --git a/coderd/httpmw/authz.go b/coderd/httpmw/authz.go index 5bfe69d47c956..8acd25b5e54f2 100644 --- a/coderd/httpmw/authz.go +++ b/coderd/httpmw/authz.go @@ -27,7 +27,7 @@ func AsAuthzSystem(mws ...func(http.Handler) http.Handler) func(http.Handler) ht } // nolint:gocritic // AsAuthzSystem needs to do this. - r = r.WithContext(dbauthz.AsSystem(ctx)) + r = r.WithContext(dbauthz.AsSystemRestricted(ctx)) chain.Handler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { r = r.WithContext(dbauthz.As(r.Context(), before)) next.ServeHTTP(rw, r) diff --git a/coderd/httpmw/hsts_test.go b/coderd/httpmw/hsts_test.go index 68f73e8adf506..16dcee78cbf61 100644 --- a/coderd/httpmw/hsts_test.go +++ b/coderd/httpmw/hsts_test.go @@ -96,7 +96,7 @@ func TestHSTS(t *testing.T) { req := httptest.NewRequest("GET", "/", nil) res := httptest.NewRecorder() got.ServeHTTP(res, req) - + require.Equal(t, tt.expectHeader, res.Header().Get("Strict-Transport-Security"), "expected header value") }) } diff --git a/coderd/httpmw/userparam.go b/coderd/httpmw/userparam.go index 4cbec80c695f6..bca52156c815a 100644 --- a/coderd/httpmw/userparam.go +++ b/coderd/httpmw/userparam.go @@ -70,7 +70,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han return } //nolint:gocritic // System needs to be able to get user from param. - user, err = db.GetUserByID(dbauthz.AsSystem(ctx), apiKey.UserID) + user, err = db.GetUserByID(dbauthz.AsSystemRestricted(ctx), apiKey.UserID) if xerrors.Is(err, sql.ErrNoRows) { httpapi.ResourceNotFound(rw) return @@ -84,7 +84,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han } } else if userID, err := uuid.Parse(userQuery); err == nil { //nolint:gocritic // If the userQuery is a valid uuid - user, err = db.GetUserByID(dbauthz.AsSystem(ctx), userID) + user, err = db.GetUserByID(dbauthz.AsSystemRestricted(ctx), userID) if err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: userErrorMessage, @@ -93,7 +93,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han } } else { // nolint:gocritic // Try as a username last - user, err = db.GetUserByEmailOrUsername(dbauthz.AsSystem(ctx), database.GetUserByEmailOrUsernameParams{ + user, err = db.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ Username: userQuery, }) if err != nil { diff --git a/coderd/httpmw/workspaceagent.go b/coderd/httpmw/workspaceagent.go index 980872434d114..b9905f7640394 100644 --- a/coderd/httpmw/workspaceagent.go +++ b/coderd/httpmw/workspaceagent.go @@ -48,7 +48,7 @@ func ExtractWorkspaceAgent(db database.Store) func(http.Handler) http.Handler { return } //nolint:gocritic // System needs to be able to get workspace agents. - agent, err := db.GetWorkspaceAgentByAuthToken(dbauthz.AsSystem(ctx), token) + agent, err := db.GetWorkspaceAgentByAuthToken(dbauthz.AsSystemRestricted(ctx), token) if err != nil { if errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{ @@ -66,7 +66,7 @@ func ExtractWorkspaceAgent(db database.Store) func(http.Handler) http.Handler { } //nolint:gocritic // System needs to be able to get workspace agents. - subject, err := getAgentSubject(dbauthz.AsSystem(ctx), db, agent) + subject, err := getAgentSubject(dbauthz.AsSystemRestricted(ctx), db, agent) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching workspace agent.", diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 7c073a7e8200b..82928e1f4e4bf 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -144,7 +144,7 @@ func countUniqueUsers(rows []database.GetTemplateDAUsRow) int { func (c *Cache) refresh(ctx context.Context) error { //nolint:gocritic // This is a system service. - ctx = dbauthz.AsSystem(ctx) + ctx = dbauthz.AsMetricsCache(ctx) err := c.database.DeleteOldAgentStats(ctx) if err != nil { return xerrors.Errorf("delete old stats: %w", err) diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index b97cc8594a573..1b5f470050483 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -57,8 +57,8 @@ type Server struct { // AcquireJob queries the database to lock a job. func (server *Server) AcquireJob(ctx context.Context, _ *proto.Empty) (*proto.AcquiredJob, error) { - //nolint:gocritic //TODO: make a provisionerd role - ctx = dbauthz.AsSystem(ctx) + //nolint:gocritic // Provisionerd has specific authz rules. + ctx = dbauthz.AsProvisionerd(ctx) // This prevents loads of provisioner daemons from consistently // querying the database when no jobs are available. // @@ -273,8 +273,8 @@ func (server *Server) AcquireJob(ctx context.Context, _ *proto.Empty) (*proto.Ac } func (server *Server) CommitQuota(ctx context.Context, request *proto.CommitQuotaRequest) (*proto.CommitQuotaResponse, error) { - //nolint:gocritic //TODO: make a provisionerd role - ctx = dbauthz.AsSystem(ctx) + //nolint:gocritic // Provisionerd has specific authz rules. + ctx = dbauthz.AsProvisionerd(ctx) jobID, err := uuid.Parse(request.JobId) if err != nil { return nil, xerrors.Errorf("parse job id: %w", err) @@ -304,8 +304,8 @@ func (server *Server) CommitQuota(ctx context.Context, request *proto.CommitQuot } func (server *Server) UpdateJob(ctx context.Context, request *proto.UpdateJobRequest) (*proto.UpdateJobResponse, error) { - //nolint:gocritic //TODO: make a provisionerd role - ctx = dbauthz.AsSystem(ctx) + //nolint:gocritic // Provisionerd has specific authz rules. + ctx = dbauthz.AsProvisionerd(ctx) parsedID, err := uuid.Parse(request.JobId) if err != nil { return nil, xerrors.Errorf("parse job id: %w", err) @@ -352,8 +352,8 @@ func (server *Server) UpdateJob(ctx context.Context, request *proto.UpdateJobReq slog.F("stage", log.Stage), slog.F("output", log.Output)) } - //nolint:gocritic //TODO: make a provisionerd role - logs, err := server.Database.InsertProvisionerJobLogs(dbauthz.AsSystem(context.Background()), insertParams) + //nolint:gocritic // Provisionerd has specific authz rules. + logs, err := server.Database.InsertProvisionerJobLogs(dbauthz.AsProvisionerd(context.Background()), insertParams) if err != nil { server.Logger.Error(ctx, "failed to insert job logs", slog.F("job_id", parsedID), slog.Error(err)) return nil, xerrors.Errorf("insert job logs: %w", err) @@ -478,8 +478,8 @@ func (server *Server) UpdateJob(ctx context.Context, request *proto.UpdateJobReq } func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*proto.Empty, error) { - //nolint:gocritic // TODO: make a provisionerd role - ctx = dbauthz.AsSystem(ctx) + //nolint:gocritic // Provisionerd has specific authz rules. + ctx = dbauthz.AsProvisionerd(ctx) jobID, err := uuid.Parse(failJob.JobId) if err != nil { return nil, xerrors.Errorf("parse job id: %w", err) @@ -606,8 +606,8 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p // CompleteJob is triggered by a provision daemon to mark a provisioner job as completed. func (server *Server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) (*proto.Empty, error) { - //nolint:gocritic // TODO: make a provisionerd role - ctx = dbauthz.AsSystem(ctx) + //nolint:gocritic // Provisionerd has specific authz rules. + ctx = dbauthz.AsProvisionerd(ctx) jobID, err := uuid.Parse(completed.JobId) if err != nil { return nil, xerrors.Errorf("parse job id: %w", err) diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index b9332f459c81a..041a8f7e704c7 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -811,7 +811,7 @@ func TestAuthorizeScope(t *testing.T) { Role: Role{ Name: "workspace_agent", DisplayName: "Workspace Agent", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ // Only read access for workspaces. ResourceWorkspace.Type: {ActionRead}, }), @@ -900,7 +900,7 @@ func TestAuthorizeScope(t *testing.T) { Role: Role{ Name: "create_workspace", DisplayName: "Create Workspace", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ // Only read access for workspaces. ResourceWorkspace.Type: {ActionCreate}, }), diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index b644a03e03695..dedac6a3fb7bb 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -77,7 +77,7 @@ var ( return Role{ Name: owner, DisplayName: "Owner", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ ResourceWildcard.Type: {WildcardSymbol}, }), Org: map[string][]Permission{}, @@ -90,7 +90,7 @@ var ( return Role{ Name: member, DisplayName: "", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ // All users can read all other users and know they exist. ResourceUser.Type: {ActionRead}, ResourceRoleAssignment.Type: {ActionRead}, @@ -98,7 +98,7 @@ var ( ResourceProvisionerDaemon.Type: {ActionRead}, }), Org: map[string][]Permission{}, - User: permissions(map[string][]Action{ + User: Permissions(map[string][]Action{ ResourceWildcard.Type: {WildcardSymbol}, }), } @@ -111,7 +111,7 @@ var ( return Role{ Name: auditor, DisplayName: "Auditor", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ // Should be able to read all template details, even in orgs they // are not in. ResourceTemplate.Type: {ActionRead}, @@ -126,7 +126,7 @@ var ( return Role{ Name: templateAdmin, DisplayName: "Template Admin", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ ResourceTemplate.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, // CRUD all files, even those they did not upload. ResourceFile.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, @@ -145,7 +145,7 @@ var ( return Role{ Name: userAdmin, DisplayName: "User Admin", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, // Full perms to manage org members @@ -430,9 +430,9 @@ func roleSplit(role string) (name string, orgID string, err error) { return arr[0], "", nil } -// permissions is just a helper function to make building roles that list out resources +// Permissions is just a helper function to make building roles that list out resources // and actions a bit easier. -func permissions(perms map[string][]Action) []Permission { +func Permissions(perms map[string][]Action) []Permission { list := make([]Permission, 0, len(perms)) for k, actions := range perms { for _, act := range actions { diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 82b64f7179135..6bec8b5533d78 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -79,7 +79,7 @@ var builtinScopes = map[ScopeName]Scope{ Role: Role{ Name: fmt.Sprintf("Scope_%s", ScopeAll), DisplayName: "All operations", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ ResourceWildcard.Type: {WildcardSymbol}, }), Org: map[string][]Permission{}, @@ -92,7 +92,7 @@ var builtinScopes = map[ScopeName]Scope{ Role: Role{ Name: fmt.Sprintf("Scope_%s", ScopeApplicationConnect), DisplayName: "Ability to connect to applications", - Site: permissions(map[string][]Action{ + Site: Permissions(map[string][]Action{ ResourceWorkspaceApplicationConnect.Type: {ActionCreate}, }), Org: map[string][]Permission{}, diff --git a/coderd/userauth.go b/coderd/userauth.go index 8585f0d3001f3..1a1ad6ad53b2e 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -58,7 +58,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { } //nolint:gocritic // In order to login, we need to get the user first! - user, err := api.Database.GetUserByEmailOrUsername(dbauthz.AsSystem(ctx), database.GetUserByEmailOrUsernameParams{ + user, err := api.Database.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ Email: loginWithPassword.Email, }) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { @@ -104,7 +104,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { } //nolint:gocritic // System needs to fetch user roles in order to login user. - roles, err := api.Database.GetAuthorizationUserRoles(dbauthz.AsSystem(ctx), user.ID) + roles, err := api.Database.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error.", @@ -775,7 +775,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook if user.ID == uuid.Nil { var organizationID uuid.UUID //nolint:gocritic - organizations, _ := tx.GetOrganizations(dbauthz.AsSystem(ctx)) + organizations, _ := tx.GetOrganizations(dbauthz.AsSystemRestricted(ctx)) if len(organizations) > 0 { // Add the user to the first organization. Once multi-organization // support is added, we should enable a configuration map of user @@ -784,7 +784,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook } //nolint:gocritic - _, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystem(ctx), database.GetUserByEmailOrUsernameParams{ + _, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ Username: params.Username, }) if err == nil { @@ -798,7 +798,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook params.Username = httpapi.UsernameFrom(alternate) //nolint:gocritic - _, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystem(ctx), database.GetUserByEmailOrUsernameParams{ + _, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ Username: params.Username, }) if xerrors.Is(err, sql.ErrNoRows) { @@ -818,7 +818,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook } //nolint:gocritic - user, _, err = api.CreateUser(dbauthz.AsSystem(ctx), tx, CreateUserRequest{ + user, _, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{ CreateUserRequest: codersdk.CreateUserRequest{ Email: params.Email, Username: params.Username, @@ -833,7 +833,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook if link.UserID == uuid.Nil { //nolint:gocritic - link, err = tx.InsertUserLink(dbauthz.AsSystem(ctx), database.InsertUserLinkParams{ + link, err = tx.InsertUserLink(dbauthz.AsSystemRestricted(ctx), database.InsertUserLinkParams{ UserID: user.ID, LoginType: params.LoginType, LinkedID: params.LinkedID, @@ -848,7 +848,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook if link.UserID != uuid.Nil { //nolint:gocritic - link, err = tx.UpdateUserLink(dbauthz.AsSystem(ctx), database.UpdateUserLinkParams{ + link, err = tx.UpdateUserLink(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLinkParams{ UserID: user.ID, LoginType: params.LoginType, OAuthAccessToken: params.State.Token.AccessToken, @@ -863,7 +863,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook // Ensure groups are correct. if len(params.Groups) > 0 { //nolint:gocritic - err := api.Options.SetUserGroups(dbauthz.AsSystem(ctx), tx, user.ID, params.Groups) + err := api.Options.SetUserGroups(dbauthz.AsSystemRestricted(ctx), tx, user.ID, params.Groups) if err != nil { return xerrors.Errorf("set user groups: %w", err) } @@ -897,7 +897,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook // longer sign in until an administrator finds the offending built-in // user and changes their username. //nolint:gocritic - user, err = tx.UpdateUserProfile(dbauthz.AsSystem(ctx), database.UpdateUserProfileParams{ + user, err = tx.UpdateUserProfile(dbauthz.AsSystemRestricted(ctx), database.UpdateUserProfileParams{ ID: user.ID, Email: user.Email, Username: user.Username, @@ -916,7 +916,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook } //nolint:gocritic - cookie, key, err := api.createAPIKey(dbauthz.AsSystem(ctx), createAPIKeyParams{ + cookie, key, err := api.createAPIKey(dbauthz.AsSystemRestricted(ctx), createAPIKeyParams{ UserID: user.ID, LoginType: params.LoginType, RemoteAddr: r.RemoteAddr, diff --git a/coderd/users.go b/coderd/users.go index 415920df2a30c..75696e1155e2a 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -38,8 +38,7 @@ import ( // @Router /users/first [get] func (api *API) firstUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - //nolint:gocritic // needed for first user check - userCount, err := api.Database.GetUserCount(dbauthz.AsSystem(ctx)) + userCount, err := api.Database.GetUserCount(ctx) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching user count.", @@ -80,8 +79,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { } // This should only function for the first user. - //nolint:gocritic // needed to create first user - userCount, err := api.Database.GetUserCount(dbauthz.AsSystem(ctx)) + userCount, err := api.Database.GetUserCount(ctx) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching user count.", @@ -122,7 +120,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { } //nolint:gocritic // needed to create first user - user, organizationID, err := api.CreateUser(dbauthz.AsSystem(ctx), api.Database, CreateUserRequest{ + user, organizationID, err := api.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, CreateUserRequest{ CreateUserRequest: codersdk.CreateUserRequest{ Email: createUser.Email, Username: createUser.Username, @@ -152,7 +150,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { // and add some rbac bypass when calling api functions this way?? // Add the admin role to this first user. //nolint:gocritic // needed to create first user - _, err = api.Database.UpdateUserRoles(dbauthz.AsSystem(ctx), database.UpdateUserRolesParams{ + _, err = api.Database.UpdateUserRoles(dbauthz.AsSystemRestricted(ctx), database.UpdateUserRolesParams{ GrantedRoles: []string{rbac.RoleOwner()}, ID: user.ID, }) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index e8ab2d4c771ab..51769a66a556b 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -632,7 +632,7 @@ func (api *API) workspaceAgentCoordinate(rw http.ResponseWriter, r *http.Request // Use a system context as the agent has disconnected and that token // may no longer be valid. //nolint:gocritic - ctx, cancel := context.WithTimeout(dbauthz.AsSystem(api.ctx), api.AgentInactiveDisconnectTimeout) + ctx, cancel := context.WithTimeout(dbauthz.AsSystemRestricted(api.ctx), api.AgentInactiveDisconnectTimeout) defer cancel() disconnectedAt = sql.NullTime{ diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 43714d089f9e8..5ec965db68df1 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -332,7 +332,7 @@ func (api *API) handleWorkspaceAppLogout(rw http.ResponseWriter, r *http.Request // unchecked API key, we validate that the secret matches the secret // we store in the database. //nolint:gocritic // needed for workspace app logout - apiKey, err := api.Database.GetAPIKeyByID(dbauthz.AsSystem(ctx), id) + apiKey, err := api.Database.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), id) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to lookup API key.", @@ -352,7 +352,7 @@ func (api *API) handleWorkspaceAppLogout(rw http.ResponseWriter, r *http.Request return } //nolint:gocritic // needed for workspace app logout - err = api.Database.DeleteAPIKeyByID(dbauthz.AsSystem(ctx), id) + err = api.Database.DeleteAPIKeyByID(dbauthz.AsSystemRestricted(ctx), id) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to delete API key.", @@ -412,10 +412,10 @@ func (api *API) handleWorkspaceAppLogout(rw http.ResponseWriter, r *http.Request // error while looking it up, an HTML error page is returned and false is // returned so the caller can return early. func (api *API) lookupWorkspaceApp(rw http.ResponseWriter, r *http.Request, agentID uuid.UUID, appSlug string) (database.WorkspaceApp, bool) { - // dbauthz.AsSystem is allowed here as the app authz is checked later. + // dbauthz.AsSystemRestricted is allowed here as the app authz is checked later. // The app authz is determined by the sharing level. //nolint:gocritic - app, err := api.Database.GetWorkspaceAppByAgentIDAndSlug(dbauthz.AsSystem(r.Context()), database.GetWorkspaceAppByAgentIDAndSlugParams{ + app, err := api.Database.GetWorkspaceAppByAgentIDAndSlug(dbauthz.AsSystemRestricted(r.Context()), database.GetWorkspaceAppByAgentIDAndSlugParams{ AgentID: agentID, Slug: appSlug, }) @@ -1026,7 +1026,7 @@ func decryptAPIKey(ctx context.Context, db database.Store, encryptedAPIKey strin // Lookup the API key so we can decrypt it. keyID := object.Header.KeyID //nolint:gocritic // needed to check API key - key, err := db.GetAPIKeyByID(dbauthz.AsSystem(ctx), keyID) + key, err := db.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), keyID) if err != nil { return database.APIKey{}, "", xerrors.Errorf("get API key by key ID: %w", err) } diff --git a/coderd/workspaceresourceauth.go b/coderd/workspaceresourceauth.go index 7fa8e8aa8907d..dfafe7ba1e853 100644 --- a/coderd/workspaceresourceauth.go +++ b/coderd/workspaceresourceauth.go @@ -128,7 +128,7 @@ func (api *API) postWorkspaceAuthGoogleInstanceIdentity(rw http.ResponseWriter, func (api *API) handleAuthInstanceID(rw http.ResponseWriter, r *http.Request, instanceID string) { ctx := r.Context() //nolint:gocritic // needed for auth instance id - agent, err := api.Database.GetWorkspaceAgentByInstanceID(dbauthz.AsSystem(ctx), instanceID) + agent, err := api.Database.GetWorkspaceAgentByInstanceID(dbauthz.AsSystemRestricted(ctx), instanceID) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ Message: fmt.Sprintf("Instance with id %q not found.", instanceID), @@ -143,7 +143,7 @@ func (api *API) handleAuthInstanceID(rw http.ResponseWriter, r *http.Request, in return } //nolint:gocritic // needed for auth instance id - resource, err := api.Database.GetWorkspaceResourceByID(dbauthz.AsSystem(ctx), agent.ResourceID) + resource, err := api.Database.GetWorkspaceResourceByID(dbauthz.AsSystemRestricted(ctx), agent.ResourceID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching provisioner job resource.", @@ -152,7 +152,7 @@ func (api *API) handleAuthInstanceID(rw http.ResponseWriter, r *http.Request, in return } //nolint:gocritic // needed for auth instance id - job, err := api.Database.GetProvisionerJobByID(dbauthz.AsSystem(ctx), resource.JobID) + job, err := api.Database.GetProvisionerJobByID(dbauthz.AsSystemRestricted(ctx), resource.JobID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching provisioner job.", @@ -176,7 +176,7 @@ func (api *API) handleAuthInstanceID(rw http.ResponseWriter, r *http.Request, in return } //nolint:gocritic // needed for auth instance id - resourceHistory, err := api.Database.GetWorkspaceBuildByID(dbauthz.AsSystem(ctx), jobData.WorkspaceBuildID) + resourceHistory, err := api.Database.GetWorkspaceBuildByID(dbauthz.AsSystemRestricted(ctx), jobData.WorkspaceBuildID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching workspace build.", @@ -188,7 +188,7 @@ func (api *API) handleAuthInstanceID(rw http.ResponseWriter, r *http.Request, in // for the latest history. If an instance ID is recycled by a cloud, // we'd hate to leak access to a user's workspace. //nolint:gocritic // needed for auth instance id - latestHistory, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(dbauthz.AsSystem(ctx), resourceHistory.WorkspaceID) + latestHistory, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(dbauthz.AsSystemRestricted(ctx), resourceHistory.WorkspaceID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching the latest workspace build.", diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index 1cba0a1f633c0..de3ca05f1b25e 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -6,7 +6,10 @@ import ( "testing" "time" + "github.com/google/uuid" + "github.com/coder/coder/coderd/database/dbauthz" + "github.com/coder/coder/coderd/rbac" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -103,7 +106,7 @@ func TestEntitlements(t *testing.T) { require.False(t, entitlements.HasLicense) coderdtest.CreateFirstUser(t, client) //nolint:gocritic // unit test - ctx := dbauthz.AsSystem(context.Background()) + ctx := testDBAuthzRole(context.Background()) _, err = api.Database.InsertLicense(ctx, database.InsertLicenseParams{ UploadedAt: database.Now(), Exp: database.Now().AddDate(1, 0, 0), @@ -134,7 +137,7 @@ func TestEntitlements(t *testing.T) { // Valid ctx := context.Background() //nolint:gocritic // unit test - _, err = api.Database.InsertLicense(dbauthz.AsSystem(ctx), database.InsertLicenseParams{ + _, err = api.Database.InsertLicense(testDBAuthzRole(ctx), database.InsertLicenseParams{ UploadedAt: database.Now(), Exp: database.Now().AddDate(1, 0, 0), JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{ @@ -146,7 +149,7 @@ func TestEntitlements(t *testing.T) { require.NoError(t, err) // Expired //nolint:gocritic // unit test - _, err = api.Database.InsertLicense(dbauthz.AsSystem(ctx), database.InsertLicenseParams{ + _, err = api.Database.InsertLicense(testDBAuthzRole(ctx), database.InsertLicenseParams{ UploadedAt: database.Now(), Exp: database.Now().AddDate(-1, 0, 0), JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{ @@ -156,7 +159,7 @@ func TestEntitlements(t *testing.T) { require.NoError(t, err) // Invalid //nolint:gocritic // unit test - _, err = api.Database.InsertLicense(dbauthz.AsSystem(ctx), database.InsertLicenseParams{ + _, err = api.Database.InsertLicense(testDBAuthzRole(ctx), database.InsertLicenseParams{ UploadedAt: database.Now(), Exp: database.Now().AddDate(1, 0, 0), JWT: "invalid", @@ -201,3 +204,23 @@ func TestAuditLogging(t *testing.T) { assert.Equal(t, reflect.ValueOf(ea).Type(), reflect.ValueOf(auditor).Type()) }) } + +// testDBAuthzRole returns a context with a subject that has a role +// with permissions required for test setup. +func testDBAuthzRole(ctx context.Context) context.Context { + return dbauthz.As(ctx, rbac.Subject{ + ID: uuid.Nil.String(), + Roles: rbac.Roles([]rbac.Role{ + { + Name: "testing", + DisplayName: "Unit Tests", + Site: rbac.Permissions(map[string][]rbac.Action{ + rbac.ResourceLicense.Type: {rbac.ActionCreate}, + }), + Org: map[string][]rbac.Permission{}, + User: []rbac.Permission{}, + }, + }), + Scope: rbac.ScopeAll, + }) +} diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/scim.go index b0ad00e72fd3a..9861c2217520d 100644 --- a/enterprise/coderd/scim.go +++ b/enterprise/coderd/scim.go @@ -157,7 +157,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { } //nolint:gocritic // needed for SCIM - user, _, err := api.AGPL.CreateUser(dbauthz.AsSystem(ctx), api.Database, agpl.CreateUserRequest{ + user, _, err := api.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, agpl.CreateUserRequest{ CreateUserRequest: codersdk.CreateUserRequest{ Username: sUser.UserName, Email: email, @@ -210,7 +210,7 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) { } //nolint:gocritic // needed for SCIM - dbUser, err := api.Database.GetUserByID(dbauthz.AsSystem(ctx), uid) + dbUser, err := api.Database.GetUserByID(dbauthz.AsSystemRestricted(ctx), uid) if err != nil { _ = handlerutil.WriteError(rw, err) return @@ -224,7 +224,7 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) { } //nolint:gocritic // needed for SCIM - _, err = api.Database.UpdateUserStatus(dbauthz.AsSystem(r.Context()), database.UpdateUserStatusParams{ + _, err = api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ ID: dbUser.ID, Status: status, UpdatedAt: database.Now(), diff --git a/provisionerd/runner/runner.go b/provisionerd/runner/runner.go index 4526da1fce58e..4947df09350cf 100644 --- a/provisionerd/runner/runner.go +++ b/provisionerd/runner/runner.go @@ -24,7 +24,6 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" - "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/tracing" "github.com/coder/coder/provisionerd/proto" sdkproto "github.com/coder/coder/provisionersdk/proto" @@ -887,8 +886,7 @@ func (r *Runner) commitQuota(ctx context.Context, resources []*sdkproto.Resource const stage = "Commit quota" - //nolint:gocritic // TODO: make a provisionerd role - resp, err := r.quotaCommitter.CommitQuota(dbauthz.AsSystem(ctx), &proto.CommitQuotaRequest{ + resp, err := r.quotaCommitter.CommitQuota(ctx, &proto.CommitQuotaRequest{ JobId: r.job.JobId, DailyCost: int32(cost), }) From 3f16df7e03f391b72532ca0255b6d16a30359f80 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 15 Feb 2023 15:10:35 +0000 Subject: [PATCH 2/3] fixup! chore: break down dbauthz.System into smaller roles --- enterprise/coderd/coderd_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index de3ca05f1b25e..f8141b620a29e 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -215,7 +215,7 @@ func testDBAuthzRole(ctx context.Context) context.Context { Name: "testing", DisplayName: "Unit Tests", Site: rbac.Permissions(map[string][]rbac.Action{ - rbac.ResourceLicense.Type: {rbac.ActionCreate}, + rbac.ResourceWildcard.Type: {rbac.WildcardSymbol}, }), Org: map[string][]rbac.Permission{}, User: []rbac.Permission{}, From c8bae4e08414155adca788228823b0765ab111e3 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 15 Feb 2023 15:52:53 +0000 Subject: [PATCH 3/3] remove AsMetricsCache --- coderd/database/dbauthz/dbauthz.go | 23 +---------------------- coderd/metricscache/metricscache.go | 2 +- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 5ab903aea92de..bdcc85ff618fe 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -163,29 +163,8 @@ func AsAutostart(ctx context.Context) context.Context { ) } -// AsMetricsCache returns a context with an actor that has permissions required -// for the metrics cache to function. -func AsMetricsCache(ctx context.Context) context.Context { - return context.WithValue(ctx, authContextKey{}, rbac.Subject{ - ID: uuid.Nil.String(), - Roles: rbac.Roles([]rbac.Role{ - { - Name: "metrics-cache", - DisplayName: "Metrics Cache", - Site: rbac.Permissions(map[string][]rbac.Action{ - rbac.ResourceTemplate.Type: {rbac.ActionRead}, - }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, - }, - }), - Scope: rbac.ScopeAll, - }, - ) -} - // AsSystemRestricted returns a context with an actor that has permissions -// required for various system operations e.g. login, logout. +// required for various system operations (login, logout, metrics cache). func AsSystemRestricted(ctx context.Context) context.Context { return context.WithValue(ctx, authContextKey{}, rbac.Subject{ ID: uuid.Nil.String(), diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 82928e1f4e4bf..826c1dad3e678 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -144,7 +144,7 @@ func countUniqueUsers(rows []database.GetTemplateDAUsRow) int { func (c *Cache) refresh(ctx context.Context) error { //nolint:gocritic // This is a system service. - ctx = dbauthz.AsMetricsCache(ctx) + ctx = dbauthz.AsSystemRestricted(ctx) err := c.database.DeleteOldAgentStats(ctx) if err != nil { return xerrors.Errorf("delete old stats: %w", err)