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

Commit7fa5afa

Browse files
fix: Users that can update a template can also read the file (#6776)
* fix: Users that can update a template can also read the fileThis currently has a strange RBAC story. An issue will be filedto streamline this.This is a hotfix to resolve current functionality* Only showsource code tab if the user has permission to edit the template---------Co-authored-by: Bruno Quaresma <bruno_nonato_quaresma@hotmail.com>
1 parentfc21e15 commit7fa5afa

File tree

9 files changed

+262
-13
lines changed

9 files changed

+262
-13
lines changed

‎coderd/database/dbauthz/querier.go

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,57 @@ func (q *querier) GetAuditLogsOffset(ctx context.Context, arg database.GetAuditL
8888
}
8989

9090
func (q*querier)GetFileByHashAndCreator(ctx context.Context,arg database.GetFileByHashAndCreatorParams) (database.File,error) {
91-
returnfetch(q.log,q.auth,q.db.GetFileByHashAndCreator)(ctx,arg)
91+
file,err:=q.db.GetFileByHashAndCreator(ctx,arg)
92+
iferr!=nil {
93+
return database.File{},err
94+
}
95+
err=q.authorizeContext(ctx,rbac.ActionRead,file)
96+
iferr!=nil {
97+
// Check the user's access to the file's templates.
98+
ifq.authorizeUpdateFileTemplate(ctx,file)!=nil {
99+
return database.File{},err
100+
}
101+
}
102+
103+
returnfile,nil
92104
}
93105

94106
func (q*querier)GetFileByID(ctx context.Context,id uuid.UUID) (database.File,error) {
95-
returnfetch(q.log,q.auth,q.db.GetFileByID)(ctx,id)
107+
file,err:=q.db.GetFileByID(ctx,id)
108+
iferr!=nil {
109+
return database.File{},err
110+
}
111+
err=q.authorizeContext(ctx,rbac.ActionRead,file)
112+
iferr!=nil {
113+
// Check the user's access to the file's templates.
114+
ifq.authorizeUpdateFileTemplate(ctx,file)!=nil {
115+
return database.File{},err
116+
}
117+
}
118+
119+
returnfile,nil
120+
}
121+
122+
// authorizeReadFile is a hotfix for the fact that file permissions are
123+
// independent of template permissions. This function checks if the user has
124+
// update access to any of the file's templates.
125+
func (q*querier)authorizeUpdateFileTemplate(ctx context.Context,file database.File)error {
126+
tpls,err:=q.db.GetFileTemplates(ctx,file.ID)
127+
iferr!=nil {
128+
returnerr
129+
}
130+
// There __should__ only be 1 template per file, but there can be more than
131+
// 1, so check them all.
132+
for_,tpl:=rangetpls {
133+
// If the user has update access to any template, they have read access to the file.
134+
iferr:=q.authorizeContext(ctx,rbac.ActionUpdate,tpl);err==nil {
135+
returnnil
136+
}
137+
}
138+
139+
returnNotAuthorizedError{
140+
Err:xerrors.Errorf("not authorized to read file %s",file.ID),
141+
}
96142
}
97143

98144
func (q*querier)InsertFile(ctx context.Context,arg database.InsertFileParams) (database.File,error) {

‎coderd/database/dbauthz/system.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@ import (
1010
"github.com/coder/coder/coderd/rbac"
1111
)
1212

13+
func (q*querier)GetFileTemplates(ctx context.Context,fileID uuid.UUID) ([]database.GetFileTemplatesRow,error) {
14+
iferr:=q.authorizeContext(ctx,rbac.ActionRead,rbac.ResourceSystem);err!=nil {
15+
returnnil,err
16+
}
17+
returnq.db.GetFileTemplates(ctx,fileID)
18+
}
19+
1320
// GetWorkspaceAppsByAgentIDs
1421
// The workspace/job is already fetched.
1522
func (q*querier)GetWorkspaceAppsByAgentIDs(ctx context.Context,ids []uuid.UUID) ([]database.WorkspaceApp,error) {

‎coderd/database/dbfake/databasefake.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,47 @@ func (q *fakeQuerier) GetFileByID(_ context.Context, id uuid.UUID) (database.Fil
685685
return database.File{},sql.ErrNoRows
686686
}
687687

688+
func (q*fakeQuerier)GetFileTemplates(_ context.Context,id uuid.UUID) ([]database.GetFileTemplatesRow,error) {
689+
q.mutex.RLock()
690+
deferq.mutex.RUnlock()
691+
692+
rows:=make([]database.GetFileTemplatesRow,0)
693+
varfile database.File
694+
for_,f:=rangeq.files {
695+
iff.ID==id {
696+
file=f
697+
break
698+
}
699+
}
700+
iffile.Hash=="" {
701+
returnrows,nil
702+
}
703+
704+
for_,job:=rangeq.provisionerJobs {
705+
ifjob.FileID==id {
706+
for_,version:=rangeq.templateVersions {
707+
ifversion.JobID==job.ID {
708+
for_,template:=rangeq.templates {
709+
iftemplate.ID==version.TemplateID.UUID {
710+
rows=append(rows, database.GetFileTemplatesRow{
711+
FileID:file.ID,
712+
FileCreatedBy:file.CreatedBy,
713+
TemplateID:template.ID,
714+
TemplateOrganizationID:template.OrganizationID,
715+
TemplateCreatedBy:template.CreatedBy,
716+
UserACL:template.UserACL,
717+
GroupACL:template.GroupACL,
718+
})
719+
}
720+
}
721+
}
722+
}
723+
}
724+
}
725+
726+
returnrows,nil
727+
}
728+
688729
func (q*fakeQuerier)GetUserByEmailOrUsername(_ context.Context,arg database.GetUserByEmailOrUsernameParams) (database.User,error) {
689730
iferr:=validateDatabaseType(arg);err!=nil {
690731
return database.User{},err

‎coderd/database/modelmethods.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,13 @@ func (t Template) RBACObject() rbac.Object {
8888
WithGroupACL(t.GroupACL)
8989
}
9090

91+
func (tGetFileTemplatesRow)RBACObject() rbac.Object {
92+
returnrbac.ResourceTemplate.WithID(t.TemplateID).
93+
InOrg(t.TemplateOrganizationID).
94+
WithACLUserList(t.UserACL).
95+
WithGroupACL(t.GroupACL)
96+
}
97+
9198
func (tTemplate)DeepCopy()Template {
9299
cpy:=t
93100
cpy.UserACL=maps.Clone(t.UserACL)

‎coderd/database/querier.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 69 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries/files.sql

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,31 @@ INSERT INTO
2626
files (id, hash, created_at, created_by, mimetype,"data")
2727
VALUES
2828
($1, $2, $3, $4, $5, $6) RETURNING*;
29+
30+
-- name: GetFileTemplates :many
31+
-- Get all templates that use a file.
32+
SELECT
33+
files.idAS file_id,
34+
files.created_byAS file_created_by,
35+
templates.idAS template_id,
36+
templates.organization_idAS template_organization_id,
37+
templates.created_byAS template_created_by,
38+
templates.user_acl,
39+
templates.group_acl
40+
FROM
41+
templates
42+
INNER JOIN
43+
template_versions
44+
ONtemplates.id=template_versions.template_id
45+
INNER JOIN
46+
provisioner_jobs
47+
ON job_id=provisioner_jobs.id
48+
INNER JOIN
49+
files
50+
ONfiles.id=provisioner_jobs.file_id
51+
WHERE
52+
-- Only fetch template version associated files.
53+
storage_method='file'
54+
ANDprovisioner_jobs.type='template_version_import'
55+
AND file_id= @file_id
56+
;

‎enterprise/coderd/templates_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,53 @@ func TestUpdateTemplateACL(t *testing.T) {
976976
})
977977
}
978978

979+
funcTestReadFileWithTemplateUpdate(t*testing.T) {
980+
t.Parallel()
981+
t.Run("HasTemplateUpdate",func(t*testing.T) {
982+
t.Parallel()
983+
ctx:=testutil.Context(t,testutil.WaitMedium)
984+
985+
// Upload a file
986+
client:=coderdenttest.New(t,nil)
987+
first:=coderdtest.CreateFirstUser(t,client)
988+
_=coderdenttest.AddLicense(t,client, coderdenttest.LicenseOptions{
989+
Features: license.Features{
990+
codersdk.FeatureTemplateRBAC:1,
991+
},
992+
})
993+
994+
resp,err:=client.Upload(ctx,codersdk.ContentTypeTar,bytes.NewReader(make([]byte,1024)))
995+
require.NoError(t,err)
996+
997+
// Make a new user
998+
member,memberData:=coderdtest.CreateAnotherUser(t,client,first.OrganizationID)
999+
1000+
// Try to download file, this should fail
1001+
_,_,err=member.Download(ctx,resp.ID)
1002+
require.Error(t,err,"no template yet")
1003+
1004+
// Make a new template version with the file
1005+
version:=coderdtest.CreateTemplateVersion(t,client,first.OrganizationID,nil,func(request*codersdk.CreateTemplateVersionRequest) {
1006+
request.FileID=resp.ID
1007+
})
1008+
template:=coderdtest.CreateTemplate(t,client,first.OrganizationID,version.ID)
1009+
1010+
// Not in acl yet
1011+
_,_,err=member.Download(ctx,resp.ID)
1012+
require.Error(t,err,"not in acl yet")
1013+
1014+
err=client.UpdateTemplateACL(ctx,template.ID, codersdk.UpdateTemplateACL{
1015+
UserPerms:map[string]codersdk.TemplateRole{
1016+
memberData.ID.String():codersdk.TemplateRoleAdmin,
1017+
},
1018+
})
1019+
require.NoError(t,err)
1020+
1021+
_,_,err=member.Download(ctx,resp.ID)
1022+
require.NoError(t,err)
1023+
})
1024+
}
1025+
9791026
// TestTemplateAccess tests the rego -> sql conversion. We need to implement
9801027
// this test on at least 1 table type to ensure that the conversion is correct.
9811028
// The rbac tests only assert against static SQL queries.

‎site/src/components/TemplateLayout/TemplateLayout.tsx

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,17 +110,19 @@ export const TemplateLayout: FC<{ children?: JSX.Element }> = ({
110110
>
111111
Summary
112112
</NavLink>
113-
<NavLink
114-
to={`/templates/${templateName}/files`}
115-
className={({ isActive})=>
116-
combineClasses([
117-
styles.tabItem,
118-
isActive ?styles.tabItemActive :undefined,
119-
])
120-
}
121-
>
122-
Source Code
123-
</NavLink>
113+
{data.permissions.canUpdateTemplate&&(
114+
<NavLink
115+
to={`/templates/${templateName}/files`}
116+
className={({ isActive})=>
117+
combineClasses([
118+
styles.tabItem,
119+
isActive ?styles.tabItemActive :undefined,
120+
])
121+
}
122+
>
123+
Source Code
124+
</NavLink>
125+
)}
124126
</Stack>
125127
</Margins>
126128
</div>

0 commit comments

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