diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index b926506bd5..f845f1a26b 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -88,11 +88,57 @@ func (q *querier) GetAuditLogsOffset(ctx context.Context, arg database.GetAuditL } func (q *querier) GetFileByHashAndCreator(ctx context.Context, arg database.GetFileByHashAndCreatorParams) (database.File, error) { - return fetch(q.log, q.auth, q.db.GetFileByHashAndCreator)(ctx, arg) + file, err := q.db.GetFileByHashAndCreator(ctx, arg) + if err != nil { + return database.File{}, err + } + err = q.authorizeContext(ctx, rbac.ActionRead, file) + if err != nil { + // Check the user's access to the file's templates. + if q.authorizeUpdateFileTemplate(ctx, file) != nil { + return database.File{}, err + } + } + + return file, nil } func (q *querier) GetFileByID(ctx context.Context, id uuid.UUID) (database.File, error) { - return fetch(q.log, q.auth, q.db.GetFileByID)(ctx, id) + file, err := q.db.GetFileByID(ctx, id) + if err != nil { + return database.File{}, err + } + err = q.authorizeContext(ctx, rbac.ActionRead, file) + if err != nil { + // Check the user's access to the file's templates. + if q.authorizeUpdateFileTemplate(ctx, file) != nil { + return database.File{}, err + } + } + + return file, nil +} + +// authorizeReadFile is a hotfix for the fact that file permissions are +// independent of template permissions. This function checks if the user has +// update access to any of the file's templates. +func (q *querier) authorizeUpdateFileTemplate(ctx context.Context, file database.File) error { + tpls, err := q.db.GetFileTemplates(ctx, file.ID) + if err != nil { + return err + } + // There __should__ only be 1 template per file, but there can be more than + // 1, so check them all. + for _, tpl := range tpls { + // If the user has update access to any template, they have read access to the file. + if err := q.authorizeContext(ctx, rbac.ActionUpdate, tpl); err == nil { + return nil + } + } + + return NotAuthorizedError{ + Err: xerrors.Errorf("not authorized to read file %s", file.ID), + } } func (q *querier) InsertFile(ctx context.Context, arg database.InsertFileParams) (database.File, error) { diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go index d27ff55ea0..28ca1628e0 100644 --- a/coderd/database/dbauthz/system.go +++ b/coderd/database/dbauthz/system.go @@ -10,6 +10,13 @@ import ( "github.com/coder/coder/coderd/rbac" ) +func (q *querier) GetFileTemplates(ctx context.Context, fileID uuid.UUID) ([]database.GetFileTemplatesRow, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.GetFileTemplates(ctx, fileID) +} + // GetWorkspaceAppsByAgentIDs // The workspace/job is already fetched. func (q *querier) GetWorkspaceAppsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceApp, error) { diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index a38b9baeac..7efd098957 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -685,6 +685,47 @@ func (q *fakeQuerier) GetFileByID(_ context.Context, id uuid.UUID) (database.Fil return database.File{}, sql.ErrNoRows } +func (q *fakeQuerier) GetFileTemplates(_ context.Context, id uuid.UUID) ([]database.GetFileTemplatesRow, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + rows := make([]database.GetFileTemplatesRow, 0) + var file database.File + for _, f := range q.files { + if f.ID == id { + file = f + break + } + } + if file.Hash == "" { + return rows, nil + } + + for _, job := range q.provisionerJobs { + if job.FileID == id { + for _, version := range q.templateVersions { + if version.JobID == job.ID { + for _, template := range q.templates { + if template.ID == version.TemplateID.UUID { + rows = append(rows, database.GetFileTemplatesRow{ + FileID: file.ID, + FileCreatedBy: file.CreatedBy, + TemplateID: template.ID, + TemplateOrganizationID: template.OrganizationID, + TemplateCreatedBy: template.CreatedBy, + UserACL: template.UserACL, + GroupACL: template.GroupACL, + }) + } + } + } + } + } + } + + return rows, nil +} + func (q *fakeQuerier) GetUserByEmailOrUsername(_ context.Context, arg database.GetUserByEmailOrUsernameParams) (database.User, error) { if err := validateDatabaseType(arg); err != nil { return database.User{}, err diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 870052afc8..0d037b7c50 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -88,6 +88,13 @@ func (t Template) RBACObject() rbac.Object { WithGroupACL(t.GroupACL) } +func (t GetFileTemplatesRow) RBACObject() rbac.Object { + return rbac.ResourceTemplate.WithID(t.TemplateID). + InOrg(t.TemplateOrganizationID). + WithACLUserList(t.UserACL). + WithGroupACL(t.GroupACL) +} + func (t Template) DeepCopy() Template { cpy := t cpy.UserACL = maps.Clone(t.UserACL) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 99cb9f7624..793d3909c9 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -60,6 +60,8 @@ type sqlcQuerier interface { GetDeploymentWorkspaceStats(ctx context.Context) (GetDeploymentWorkspaceStatsRow, error) GetFileByHashAndCreator(ctx context.Context, arg GetFileByHashAndCreatorParams) (File, error) GetFileByID(ctx context.Context, id uuid.UUID) (File, error) + // Get all templates that use a file. + GetFileTemplates(ctx context.Context, fileID uuid.UUID) ([]GetFileTemplatesRow, error) // This will never count deleted users. GetFilteredUserCount(ctx context.Context, arg GetFilteredUserCountParams) (int64, error) GetGitAuthLink(ctx context.Context, arg GetGitAuthLinkParams) (GitAuthLink, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index dd9fe69dd2..42912f8968 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -674,6 +674,75 @@ func (q *sqlQuerier) GetFileByID(ctx context.Context, id uuid.UUID) (File, error return i, err } +const getFileTemplates = `-- name: GetFileTemplates :many +SELECT + files.id AS file_id, + files.created_by AS file_created_by, + templates.id AS template_id, + templates.organization_id AS template_organization_id, + templates.created_by AS template_created_by, + templates.user_acl, + templates.group_acl +FROM + templates +INNER JOIN + template_versions + ON templates.id = template_versions.template_id +INNER JOIN + provisioner_jobs + ON job_id = provisioner_jobs.id +INNER JOIN + files + ON files.id = provisioner_jobs.file_id +WHERE + -- Only fetch template version associated files. + storage_method = 'file' + AND provisioner_jobs.type = 'template_version_import' + AND file_id = $1 +` + +type GetFileTemplatesRow struct { + FileID uuid.UUID `db:"file_id" json:"file_id"` + FileCreatedBy uuid.UUID `db:"file_created_by" json:"file_created_by"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + TemplateOrganizationID uuid.UUID `db:"template_organization_id" json:"template_organization_id"` + TemplateCreatedBy uuid.UUID `db:"template_created_by" json:"template_created_by"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` +} + +// Get all templates that use a file. +func (q *sqlQuerier) GetFileTemplates(ctx context.Context, fileID uuid.UUID) ([]GetFileTemplatesRow, error) { + rows, err := q.db.QueryContext(ctx, getFileTemplates, fileID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetFileTemplatesRow + for rows.Next() { + var i GetFileTemplatesRow + if err := rows.Scan( + &i.FileID, + &i.FileCreatedBy, + &i.TemplateID, + &i.TemplateOrganizationID, + &i.TemplateCreatedBy, + &i.UserACL, + &i.GroupACL, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const insertFile = `-- name: InsertFile :one INSERT INTO files (id, hash, created_at, created_by, mimetype, "data") diff --git a/coderd/database/queries/files.sql b/coderd/database/queries/files.sql index 1f54386bb3..97fded9a63 100644 --- a/coderd/database/queries/files.sql +++ b/coderd/database/queries/files.sql @@ -26,3 +26,31 @@ INSERT INTO files (id, hash, created_at, created_by, mimetype, "data") VALUES ($1, $2, $3, $4, $5, $6) RETURNING *; + +-- name: GetFileTemplates :many +-- Get all templates that use a file. +SELECT + files.id AS file_id, + files.created_by AS file_created_by, + templates.id AS template_id, + templates.organization_id AS template_organization_id, + templates.created_by AS template_created_by, + templates.user_acl, + templates.group_acl +FROM + templates +INNER JOIN + template_versions + ON templates.id = template_versions.template_id +INNER JOIN + provisioner_jobs + ON job_id = provisioner_jobs.id +INNER JOIN + files + ON files.id = provisioner_jobs.file_id +WHERE + -- Only fetch template version associated files. + storage_method = 'file' + AND provisioner_jobs.type = 'template_version_import' + AND file_id = @file_id +; diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index f983e05a87..413662cbde 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -976,6 +976,53 @@ func TestUpdateTemplateACL(t *testing.T) { }) } +func TestReadFileWithTemplateUpdate(t *testing.T) { + t.Parallel() + t.Run("HasTemplateUpdate", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + + // Upload a file + client := coderdenttest.New(t, nil) + first := coderdtest.CreateFirstUser(t, client) + _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }) + + resp, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(make([]byte, 1024))) + require.NoError(t, err) + + // Make a new user + member, memberData := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + + // Try to download file, this should fail + _, _, err = member.Download(ctx, resp.ID) + require.Error(t, err, "no template yet") + + // Make a new template version with the file + version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil, func(request *codersdk.CreateTemplateVersionRequest) { + request.FileID = resp.ID + }) + template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) + + // Not in acl yet + _, _, err = member.Download(ctx, resp.ID) + require.Error(t, err, "not in acl yet") + + err = client.UpdateTemplateACL(ctx, template.ID, codersdk.UpdateTemplateACL{ + UserPerms: map[string]codersdk.TemplateRole{ + memberData.ID.String(): codersdk.TemplateRoleAdmin, + }, + }) + require.NoError(t, err) + + _, _, err = member.Download(ctx, resp.ID) + require.NoError(t, err) + }) +} + // TestTemplateAccess tests the rego -> sql conversion. We need to implement // this test on at least 1 table type to ensure that the conversion is correct. // The rbac tests only assert against static SQL queries. diff --git a/site/src/components/TemplateLayout/TemplateLayout.tsx b/site/src/components/TemplateLayout/TemplateLayout.tsx index ed1ce35093..c44d3ac5a9 100644 --- a/site/src/components/TemplateLayout/TemplateLayout.tsx +++ b/site/src/components/TemplateLayout/TemplateLayout.tsx @@ -110,17 +110,19 @@ export const TemplateLayout: FC<{ children?: JSX.Element }> = ({ > Summary - - combineClasses([ - styles.tabItem, - isActive ? styles.tabItemActive : undefined, - ]) - } - > - Source Code - + {data.permissions.canUpdateTemplate && ( + + combineClasses([ + styles.tabItem, + isActive ? styles.tabItemActive : undefined, + ]) + } + > + Source Code + + )}