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

Commitef9936f

Browse files
committed
Merge remote-tracking branch 'origin/main' into stevenmasley/rbac_views
2 parents4c0cd84 +418a8a7 commitef9936f

File tree

5 files changed

+65
-127
lines changed

5 files changed

+65
-127
lines changed

‎coderd/database/dbauthz/querier.go

Lines changed: 0 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,6 @@ func (q *querier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (data
251251
returnjob,nil
252252
}
253253

254-
func (q*querier)GetProvisionerJobsByIDs(ctx context.Context,ids []uuid.UUID) ([]database.ProvisionerJob,error) {
255-
// TODO: This is missing authorization and is incorrect. This call is used by telemetry, and by 1 http route.
256-
// That http handler should find a better way to fetch these jobs with easier rbac authz.
257-
returnq.db.GetProvisionerJobsByIDs(ctx,ids)
258-
}
259-
260254
func (q*querier)GetProvisionerLogsByIDBetween(ctx context.Context,arg database.GetProvisionerLogsByIDBetweenParams) ([]database.ProvisionerJobLog,error) {
261255
// Authorized read on job lets the actor also read the logs.
262256
_,err:=q.GetProvisionerJobByID(ctx,arg.JobID)
@@ -725,35 +719,6 @@ func (q *querier) GetTemplateVersionVariables(ctx context.Context, templateVersi
725719
returnq.db.GetTemplateVersionVariables(ctx,templateVersionID)
726720
}
727721

728-
func (q*querier)GetTemplateVersionsByIDs(ctx context.Context,ids []uuid.UUID) ([]database.TemplateVersion,error) {
729-
// TODO: This is so inefficient
730-
versions,err:=q.db.GetTemplateVersionsByIDs(ctx,ids)
731-
iferr!=nil {
732-
returnnil,err
733-
}
734-
checked:=make(map[uuid.UUID]bool)
735-
for_,v:=rangeversions {
736-
if_,ok:=checked[v.TemplateID.UUID];ok {
737-
continue
738-
}
739-
740-
obj:=v.RBACObjectNoTemplate()
741-
template,err:=q.db.GetTemplateByID(ctx,v.TemplateID.UUID)
742-
iferr==nil {
743-
obj=v.RBACObject(template)
744-
}
745-
iferr!=nil&&!xerrors.Is(err,sql.ErrNoRows) {
746-
returnnil,err
747-
}
748-
iferr:=q.authorizeContext(ctx,rbac.ActionRead,obj);err!=nil {
749-
returnnil,err
750-
}
751-
checked[v.TemplateID.UUID]=true
752-
}
753-
754-
returnversions,nil
755-
}
756-
757722
func (q*querier)GetTemplateVersionsByTemplateID(ctx context.Context,arg database.GetTemplateVersionsByTemplateIDParams) ([]database.TemplateVersion,error) {
758723
// An actor can read template versions if they can read the related template.
759724
template,err:=q.db.GetTemplateByID(ctx,arg.TemplateID)
@@ -1013,11 +978,6 @@ func (q *querier) GetUsersWithCount(ctx context.Context, arg database.GetUsersPa
1013978
returnusers,rowUsers[0].Count,nil
1014979
}
1015980

1016-
// TODO: Remove this and use a filter on GetUsers
1017-
func (q*querier)GetUsersByIDs(ctx context.Context,ids []uuid.UUID) ([]database.User,error) {
1018-
returnfetchWithPostFilter(q.auth,q.db.GetUsersByIDs)(ctx,ids)
1019-
}
1020-
1021981
func (q*querier)InsertUser(ctx context.Context,arg database.InsertUserParams) (database.User,error) {
1022982
// Always check if the assigned roles can actually be assigned by this actor.
1023983
impliedRoles:=append([]string{rbac.RoleMember()},arg.RBACRoles...)
@@ -1219,37 +1179,6 @@ func (q *querier) GetWorkspaceAgentByInstanceID(ctx context.Context, authInstanc
12191179
returnagent,nil
12201180
}
12211181

1222-
// GetWorkspaceAgentsByResourceIDs is an all or nothing call. If the user cannot read
1223-
// a single agent, the entire call will fail.
1224-
func (q*querier)GetWorkspaceAgentsByResourceIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceAgent,error) {
1225-
if_,ok:=ActorFromContext(ctx);!ok {
1226-
returnnil,NoActorError
1227-
}
1228-
// TODO: Make this more efficient. This is annoying because all these resources should be owned by the same workspace.
1229-
// So the authz check should just be 1 check, but we cannot do that easily here. We should see if all callers can
1230-
// instead do something like GetWorkspaceAgentsByWorkspaceID.
1231-
agents,err:=q.db.GetWorkspaceAgentsByResourceIDs(ctx,ids)
1232-
iferr!=nil {
1233-
returnnil,err
1234-
}
1235-
1236-
for_,a:=rangeagents {
1237-
// Check if we can fetch the workspace by the agent ID.
1238-
_,err:=q.GetWorkspaceByAgentID(ctx,a.ID)
1239-
iferr==nil {
1240-
continue
1241-
}
1242-
iferrors.Is(err,sql.ErrNoRows)&&!errors.As(err,&NotAuthorizedError{}) {
1243-
// The agent is not tied to a workspace, likely from an orphaned template version.
1244-
// Just return it.
1245-
continue
1246-
}
1247-
// Otherwise, we cannot read the workspace, so we cannot read the agent.
1248-
returnnil,err
1249-
}
1250-
returnagents,nil
1251-
}
1252-
12531182
func (q*querier)UpdateWorkspaceAgentLifecycleStateByID(ctx context.Context,arg database.UpdateWorkspaceAgentLifecycleStateByIDParams)error {
12541183
agent,err:=q.db.GetWorkspaceAgentByID(ctx,arg.ID)
12551184
iferr!=nil {
@@ -1302,20 +1231,6 @@ func (q *querier) GetWorkspaceAppsByAgentID(ctx context.Context, agentID uuid.UU
13021231
returnq.db.GetWorkspaceAppsByAgentID(ctx,agentID)
13031232
}
13041233

1305-
// GetWorkspaceAppsByAgentIDs is an all or nothing call. If the user cannot read a single app, the entire call will fail.
1306-
func (q*querier)GetWorkspaceAppsByAgentIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceApp,error) {
1307-
// TODO: This should be reworked. All these apps are likely owned by the same workspace, so we should be able to
1308-
// do 1 authz call. We should refactor this to be GetWorkspaceAppsByWorkspaceID.
1309-
for_,id:=rangeids {
1310-
_,err:=q.GetWorkspaceAgentByID(ctx,id)
1311-
iferr!=nil {
1312-
returnnil,err
1313-
}
1314-
}
1315-
1316-
returnq.db.GetWorkspaceAppsByAgentIDs(ctx,ids)
1317-
}
1318-
13191234
func (q*querier)GetWorkspaceBuildByID(ctx context.Context,buildID uuid.UUID) (database.WorkspaceBuild,error) {
13201235
returnfetch(q.log,q.auth,q.db.GetWorkspaceBuildByID)(ctx,buildID)
13211236
}
@@ -1373,21 +1288,6 @@ func (q *querier) GetWorkspaceResourceByID(ctx context.Context, id uuid.UUID) (d
13731288
returnresource,nil
13741289
}
13751290

1376-
// GetWorkspaceResourceMetadataByResourceIDs is an all or nothing call. If a single resource is not authorized, then
1377-
// an error is returned.
1378-
func (q*querier)GetWorkspaceResourceMetadataByResourceIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceResourceMetadatum,error) {
1379-
// TODO: This is very inefficient. Since all these resources are likely asscoiated with the same workspace.
1380-
for_,id:=rangeids {
1381-
// If we can read the resource, we can read the metadata.
1382-
_,err:=q.GetWorkspaceResourceByID(ctx,id)
1383-
iferr!=nil {
1384-
returnnil,err
1385-
}
1386-
}
1387-
1388-
returnq.db.GetWorkspaceResourceMetadataByResourceIDs(ctx,ids)
1389-
}
1390-
13911291
func (q*querier)GetWorkspaceResourcesByJobID(ctx context.Context,jobID uuid.UUID) ([]database.WorkspaceResource,error) {
13921292
job,err:=q.db.GetProvisionerJobByID(ctx,jobID)
13931293
iferr!=nil {
@@ -1433,21 +1333,6 @@ func (q *querier) GetWorkspaceResourcesByJobID(ctx context.Context, jobID uuid.U
14331333
returnq.db.GetWorkspaceResourcesByJobID(ctx,jobID)
14341334
}
14351335

1436-
// GetWorkspaceResourcesByJobIDs is an all or nothing call. If a single resource is not authorized, then
1437-
// an error is returned.
1438-
func (q*querier)GetWorkspaceResourcesByJobIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceResource,error) {
1439-
// TODO: This is very inefficient. Since all these resources are likely asscoiated with the same workspace.
1440-
for_,id:=rangeids {
1441-
// If we can read the resource, we can read the metadata.
1442-
_,err:=q.GetProvisionerJobByID(ctx,id)
1443-
iferr!=nil {
1444-
returnnil,err
1445-
}
1446-
}
1447-
1448-
returnq.db.GetWorkspaceResourcesByJobIDs(ctx,ids)
1449-
}
1450-
14511336
func (q*querier)InsertWorkspace(ctx context.Context,arg database.InsertWorkspaceParams) (database.Workspace,error) {
14521337
obj:=rbac.ResourceWorkspace.WithOwner(arg.OwnerID.String()).InOrg(arg.OrganizationID)
14531338
returninsert(q.log,q.auth,obj,q.db.InsertWorkspace)(ctx,arg)

‎coderd/database/dbauthz/querier_test.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ func (s *MethodTestSuite) TestTemplate() {
622622
TemplateID: uuid.NullUUID{UUID:t2.ID,Valid:true},
623623
})
624624
check.Args([]uuid.UUID{tv1.ID,tv2.ID,tv3.ID}).
625-
Asserts(t1,rbac.ActionRead,t2,rbac.ActionRead).
625+
Asserts(/*t1, rbac.ActionRead, t2, rbac.ActionRead*/).
626626
Returns(slice.New(tv1,tv2,tv3))
627627
}))
628628
s.Run("GetTemplateVersionsByTemplateID",s.Subtest(func(db database.Store,check*expects) {
@@ -797,7 +797,7 @@ func (s *MethodTestSuite) TestUser() {
797797
a:=dbgen.User(s.T(),db, database.User{CreatedAt:database.Now().Add(-time.Hour)})
798798
b:=dbgen.User(s.T(),db, database.User{CreatedAt:database.Now()})
799799
check.Args([]uuid.UUID{a.ID,b.ID}).
800-
Asserts(a,rbac.ActionRead,b,rbac.ActionRead).
800+
Asserts(/*a, rbac.ActionRead, b, rbac.ActionRead*/).
801801
Returns(slice.New(a,b))
802802
}))
803803
s.Run("InsertUser",s.Subtest(func(db database.Store,check*expects) {
@@ -972,7 +972,7 @@ func (s *MethodTestSuite) TestWorkspace() {
972972
build:=dbgen.WorkspaceBuild(s.T(),db, database.WorkspaceBuild{WorkspaceID:ws.ID,JobID:uuid.New()}).WithWorkspace(ws)
973973
res:=dbgen.WorkspaceResource(s.T(),db, database.WorkspaceResource{JobID:build.JobID})
974974
agt:=dbgen.WorkspaceAgent(s.T(),db, database.WorkspaceAgent{ResourceID:res.ID})
975-
check.Args([]uuid.UUID{res.ID}).Asserts(ws,rbac.ActionRead).
975+
check.Args([]uuid.UUID{res.ID}).Asserts(/*ws, rbac.ActionRead*/).
976976
Returns([]database.WorkspaceAgent{agt})
977977
}))
978978
s.Run("UpdateWorkspaceAgentLifecycleStateByID",s.Subtest(func(db database.Store,check*expects) {
@@ -1030,7 +1030,7 @@ func (s *MethodTestSuite) TestWorkspace() {
10301030
b:=dbgen.WorkspaceApp(s.T(),db, database.WorkspaceApp{AgentID:bAgt.ID})
10311031

10321032
check.Args([]uuid.UUID{a.AgentID,b.AgentID}).
1033-
Asserts(aWs,rbac.ActionRead,bWs,rbac.ActionRead).
1033+
Asserts(/*aWs, rbac.ActionRead, bWs, rbac.ActionRead*/).
10341034
Returns([]database.WorkspaceApp{a,b})
10351035
}))
10361036
s.Run("GetWorkspaceBuildByID",s.Subtest(func(db database.Store,check*expects) {
@@ -1093,7 +1093,7 @@ func (s *MethodTestSuite) TestWorkspace() {
10931093
a:=dbgen.WorkspaceResource(s.T(),db, database.WorkspaceResource{JobID:build.JobID})
10941094
b:=dbgen.WorkspaceResource(s.T(),db, database.WorkspaceResource{JobID:build.JobID})
10951095
check.Args([]uuid.UUID{a.ID,b.ID}).
1096-
Asserts(ws, []rbac.Action{rbac.ActionRead,rbac.ActionRead})
1096+
Asserts(/*ws, []rbac.Action{rbac.ActionRead, rbac.ActionRead}*/)
10971097
}))
10981098
s.Run("Build/GetWorkspaceResourcesByJobID",s.Subtest(func(db database.Store,check*expects) {
10991099
ws:=dbgen.Workspace(s.T(),db, database.Workspace{})
@@ -1115,7 +1115,9 @@ func (s *MethodTestSuite) TestWorkspace() {
11151115
ws:=dbgen.Workspace(s.T(),db, database.Workspace{})
11161116
build:=dbgen.WorkspaceBuild(s.T(),db, database.WorkspaceBuild{WorkspaceID:ws.ID,JobID:uuid.New()}).WithWorkspace(ws)
11171117
wJob:=dbgen.ProvisionerJob(s.T(),db, database.ProvisionerJob{ID:build.JobID,Type:database.ProvisionerJobTypeWorkspaceBuild})
1118-
check.Args([]uuid.UUID{tJob.ID,wJob.ID}).Asserts(v.RBACObject(tpl),rbac.ActionRead,ws,rbac.ActionRead).Returns([]database.WorkspaceResource{})
1118+
check.Args([]uuid.UUID{tJob.ID,wJob.ID}).
1119+
Asserts(/*v.RBACObject(tpl), rbac.ActionRead, ws, rbac.ActionRead*/ ).
1120+
Returns([]database.WorkspaceResource{})
11191121
}))
11201122
s.Run("InsertWorkspace",s.Subtest(func(db database.Store,check*expects) {
11211123
u:=dbgen.User(s.T(),db, database.User{})

‎coderd/database/dbauthz/system.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,56 @@ import (
1414
// to these objects. Might need a negative permission on the `Owner` role to
1515
// prevent owners.
1616

17+
// GetWorkspaceAppsByAgentIDs
18+
// The workspace/job is already fetched.
19+
// TODO: This function should be removed/replaced with something with proper auth.
20+
func (q*querier)GetWorkspaceAppsByAgentIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceApp,error) {
21+
returnq.db.GetWorkspaceAppsByAgentIDs(ctx,ids)
22+
}
23+
24+
// GetWorkspaceAgentsByResourceIDs
25+
// The workspace/job is already fetched.
26+
// TODO: This function should be removed/replaced with something with proper auth.
27+
func (q*querier)GetWorkspaceAgentsByResourceIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceAgent,error) {
28+
returnq.db.GetWorkspaceAgentsByResourceIDs(ctx,ids)
29+
}
30+
31+
// GetWorkspaceResourceMetadataByResourceIDs is only used for build data.
32+
// The workspace/job is already fetched.
33+
// TODO: This function should be removed/replaced with something with proper auth.
34+
func (q*querier)GetWorkspaceResourceMetadataByResourceIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceResourceMetadatum,error) {
35+
returnq.db.GetWorkspaceResourceMetadataByResourceIDs(ctx,ids)
36+
}
37+
38+
// GetUsersByIDs is only used for usernames on workspace return data.
39+
// This function should be replaced by joining this data to the workspace query
40+
// itself.
41+
// TODO: This function should be removed/replaced with something with proper auth.
42+
// A SQL compiled filter is an option.
43+
func (q*querier)GetUsersByIDs(ctx context.Context,ids []uuid.UUID) ([]database.User,error) {
44+
returnq.db.GetUsersByIDs(ctx,ids)
45+
}
46+
47+
func (q*querier)GetProvisionerJobsByIDs(ctx context.Context,ids []uuid.UUID) ([]database.ProvisionerJob,error) {
48+
// TODO: This is missing authorization and is incorrect. This call is used by telemetry, and by 1 http route.
49+
// That http handler should find a better way to fetch these jobs with easier rbac authz.
50+
returnq.db.GetProvisionerJobsByIDs(ctx,ids)
51+
}
52+
53+
// GetTemplateVersionsByIDs is only used for workspace build data.
54+
// The workspace is already fetched.
55+
// TODO: Find a way to replace this with proper authz.
56+
func (q*querier)GetTemplateVersionsByIDs(ctx context.Context,ids []uuid.UUID) ([]database.TemplateVersion,error) {
57+
returnq.db.GetTemplateVersionsByIDs(ctx,ids)
58+
}
59+
60+
// GetWorkspaceResourcesByJobIDs is only used for workspace build data.
61+
// The workspace is already fetched.
62+
// TODO: Find a way to replace this with proper authz.
63+
func (q*querier)GetWorkspaceResourcesByJobIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceResource,error) {
64+
returnq.db.GetWorkspaceResourcesByJobIDs(ctx,ids)
65+
}
66+
1767
func (q*querier)UpdateUserLinkedID(ctx context.Context,arg database.UpdateUserLinkedIDParams) (database.UserLink,error) {
1868
returnq.db.UpdateUserLinkedID(ctx,arg)
1969
}

‎coderd/templates.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"github.com/coder/coder/coderd/audit"
1717
"github.com/coder/coder/coderd/database"
18+
"github.com/coder/coder/coderd/database/dbauthz"
1819
"github.com/coder/coder/coderd/httpapi"
1920
"github.com/coder/coder/coderd/httpmw"
2021
"github.com/coder/coder/coderd/rbac"
@@ -82,11 +83,10 @@ func (api *API) deleteTemplate(rw http.ResponseWriter, r *http.Request) {
8283
return
8384
}
8485

85-
// TODO: This just returns the workspaces a user can view. We should use
86-
// a system function to get all workspaces that use this template.
87-
// This data should never be exposed to the user aside from a non-zero count.
88-
// Or we move this into a postgres constraint.
89-
workspaces,err:=api.Database.GetWorkspaces(ctx, database.GetWorkspacesParams{
86+
// This is just to get the workspace count, so we use a system context to
87+
// return ALL workspaces. Not just workspaces the user can view.
88+
// nolint:gocritic
89+
workspaces,err:=api.Database.GetWorkspaces(dbauthz.AsSystemRestricted(ctx), database.GetWorkspacesParams{
9090
TemplateIds: []uuid.UUID{template.ID},
9191
})
9292
iferr!=nil&&!errors.Is(err,sql.ErrNoRows) {

‎coderd/workspaces.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
571571
}
572572
aReq.New=workspace
573573

574-
users,err:=api.Database.GetUsersByIDs(ctx,[]uuid.UUID{user.ID,workspaceBuild.InitiatorID})
574+
initiator,err:=api.Database.GetUserByID(ctx,workspaceBuild.InitiatorID)
575575
iferr!=nil {
576576
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
577577
Message:"Internal error fetching user.",
@@ -585,6 +585,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
585585
WorkspaceBuilds: []telemetry.WorkspaceBuild{telemetry.ConvertWorkspaceBuild(workspaceBuild)},
586586
})
587587

588+
users:= []database.User{user,initiator}
588589
apiBuild,err:=api.convertWorkspaceBuild(
589590
workspaceBuild,
590591
workspace,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp