Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: break down dbauthz.System into smaller roles#6218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
johnstcn merged 4 commits intomainfromcj/authz-system-breakup
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletioncoderd/activitybump.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -83,7 +83,7 @@ func activityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Sto
},nil)
iferr!=nil {
if!xerrors.Is(err,context.Canceled) {
// Bump will fail if the context iscancelled, but this is ok.
// Bump will fail if the context iscanceled, but this is ok.
log.Error(ctx,"bump failed",slog.Error(err),
slog.F("workspace_id",workspaceID),
)
Expand Down
4 changes: 2 additions & 2 deletionscoderd/autobuild/executor/lifecycle_executor.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,8 +34,8 @@ type Stats struct {
// New returns a new autobuild executor.
funcNew(ctx context.Context,db database.Store,log slog.Logger,tick<-chan time.Time)*Executor {
le:=&Executor{
//nolint:gocritic //TODO: make an autostart role insteadofusing System
ctx:dbauthz.AsSystem(ctx),
//nolint:gocritic //Autostart has a limited setofpermissions.
ctx:dbauthz.AsAutostart(ctx),
db:db,
tick:tick,
log:log,
Expand Down
86 changes: 65 additions & 21 deletionscoderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,12 +46,12 @@ func logNotAuthorizedError(ctx context.Context, logger slog.Logger, err error) e
iferr!=nil&&xerrors.As(err,&internalError) {
e:=new(topdown.Error)
ifxerrors.As(err,&e)||e.Code==topdown.CancelErr {
// For some reason rego changes acancelled context to a topdown.CancelErr. We
// expect to check forcancelled context errors if the user cancels the request,
// For some reason rego changes acanceled context to a topdown.CancelErr. We
// expect to check forcanceled 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 acancelled context.
// if it's actually acanceled context.
internalError.SetInternal(context.Canceled)
returninternalError
}
Expand DownExpand Up@@ -117,29 +117,73 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) {
returna,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.
funcAsSystem(ctx context.Context) context.Context {
// AsProvisionerd returns a context with an actor that has permissions required
// for provisionerd to function.
funcAsProvisionerd(ctx context.Context) context.Context {
returncontext.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.
funcAsAutostart(ctx context.Context) context.Context {
returncontext.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,
},
)
}

// AsSystemRestricted returns a context with an actor that has permissions
// required for various system operations (login, logout, metrics cache).
funcAsSystemRestricted(ctx context.Context) context.Context {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Do we plan on keeping this around? Or is it a catch all for the remaining stuff for now?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is a catch-all for the remaining stuff. It's mostly used for HTTP middleware.
I've pared down all the perms except read, which can still be pared down to the bare minimum if need be.

If we need to break it down further in future, we can do so. I think this is fine for now though.

returncontext.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{},
},
Expand Down
26 changes: 0 additions & 26 deletionscoderd/database/dbauthz/querier.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -327,13 +327,6 @@ func (q *querier) GetProvisionerDaemons(ctx context.Context) ([]database.Provisi
returnfetchWithPostFilter(q.auth,fetch)(ctx,nil)
}

func (q*querier)GetDeploymentDAUs(ctx context.Context) ([]database.GetDeploymentDAUsRow,error) {
iferr:=q.authorizeContext(ctx,rbac.ActionRead,rbac.ResourceUser.All());err!=nil {
returnnil,err
}
returnq.db.GetDeploymentDAUs(ctx)
}

func (q*querier)GetGroupsByOrganizationID(ctx context.Context,organizationID uuid.UUID) ([]database.Group,error) {
returnfetchWithPostFilter(q.auth,q.db.GetGroupsByOrganizationID)(ctx,organizationID)
}
Expand DownExpand Up@@ -622,16 +615,6 @@ func (q *querier) GetPreviousTemplateVersion(ctx context.Context, arg database.G
returnq.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
}
returnq.db.GetTemplateAverageBuildTime(ctx,arg)
}

func (q*querier)GetTemplateByID(ctx context.Context,id uuid.UUID) (database.Template,error) {
returnfetch(q.log,q.auth,q.db.GetTemplateByID)(ctx,id)
}
Expand All@@ -640,15 +623,6 @@ func (q *querier) GetTemplateByOrganizationAndName(ctx context.Context, arg data
returnfetch(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 {
returnnil,err
}
returnq.db.GetTemplateDAUs(ctx,templateID)
}

func (q*querier)GetTemplateVersionByID(ctx context.Context,tvid uuid.UUID) (database.TemplateVersion,error) {
tv,err:=q.db.GetTemplateVersionByID(ctx,tvid)
iferr!=nil {
Expand Down
13 changes: 0 additions & 13 deletionscoderd/database/dbauthz/querier_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
Expand All@@ -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{
Expand DownExpand Up@@ -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)
}))
}
4 changes: 2 additions & 2 deletionscoderd/database/dbauthz/setup_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -226,8 +226,8 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd
}
})

s.Run("Cancelled",func() {
// Pass in acancelled context
s.Run("Canceled",func() {
// Pass in acanceled context
ctx,cancel:=context.WithCancel(ctx)
cancel()
az.AlwaysReturn=rbac.ForbiddenWithInternal(&topdown.Error{Code:topdown.CancelErr},
Expand Down
15 changes: 15 additions & 0 deletionscoderd/database/dbauthz/system.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -96,6 +96,21 @@ func (q *querier) GetTemplates(ctx context.Context) ([]database.Template, error)
returnq.db.GetTemplates(ctx)
}

// Only used by metrics cache.
func (q*querier)GetTemplateAverageBuildTime(ctx context.Context,arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow,error) {
returnq.db.GetTemplateAverageBuildTime(ctx,arg)
}

// Only used by metrics cache.
func (q*querier)GetTemplateDAUs(ctx context.Context,templateID uuid.UUID) ([]database.GetTemplateDAUsRow,error) {
returnq.db.GetTemplateDAUs(ctx,templateID)
}

// Only used by metrics cache.
func (q*querier)GetDeploymentDAUs(ctx context.Context) ([]database.GetDeploymentDAUsRow,error) {
returnq.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) {
returnq.db.UpdateWorkspaceBuildCostByID(ctx,arg)
Expand Down
12 changes: 6 additions & 6 deletionscoderd/httpmw/apikey.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
iferr!=nil {
iferrors.Is(err,sql.ErrNoRows) {
optionalWrite(http.StatusUnauthorized, codersdk.Response{
Expand DownExpand Up@@ -195,7 +195,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
)
ifkey.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,
})
Expand DownExpand Up@@ -279,7 +279,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
}
ifchanged {
//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,
Expand All@@ -296,7 +296,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
// then we want to update the relevant oauth fields.
iflink.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,
Expand All@@ -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(),
Expand All@@ -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)
iferr!=nil {
write(http.StatusUnauthorized, codersdk.Response{
Message:internalErrorMessage,
Expand Down
2 changes: 1 addition & 1 deletioncoderd/httpmw/authz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,7 +28,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)
Expand Down
2 changes: 1 addition & 1 deletioncoderd/httpmw/hsts_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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")
})
}
Expand Down
6 changes: 3 additions & 3 deletionscoderd/httpmw/userparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand All@@ -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,
Expand All@@ -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 {
Expand Down
4 changes: 2 additions & 2 deletionscoderd/httpmw/workspaceagent.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
iferr!=nil {
iferrors.Is(err,sql.ErrNoRows) {
httpapi.Write(ctx,rw,http.StatusUnauthorized, codersdk.Response{
Expand All@@ -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)
iferr!=nil {
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
Message:"Internal error fetching workspace agent.",
Expand Down
2 changes: 1 addition & 1 deletioncoderd/metricscache/metricscache.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.AsSystemRestricted(ctx)
err:=c.database.DeleteOldAgentStats(ctx)
iferr!=nil {
returnxerrors.Errorf("delete old stats: %w",err)
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp