fix: Users that can update a template can also read the file (#6776)

* fix: Users that can update a template can also read the file

This currently has a strange RBAC story. An issue will be filed
to 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>

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) {

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) {

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

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)

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)

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")

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
+;

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.

site/src/components/TemplateLayout/TemplateLayout.tsx

@@ -110,17 +110,19 @@ export const TemplateLayout: FC<{ children?: JSX.Element }> = ({
             >
               Summary
             </NavLink>
-            <NavLink
-              to={`/templates/${templateName}/files`}
-              className={({ isActive }) =>
-                combineClasses([
-                  styles.tabItem,
-                  isActive ? styles.tabItemActive : undefined,
-                ])
-              }
-            >
-              Source Code
-            </NavLink>
+            {data.permissions.canUpdateTemplate && (
+              <NavLink
+                to={`/templates/${templateName}/files`}
+                className={({ isActive }) =>
+                  combineClasses([
+                    styles.tabItem,
+                    isActive ? styles.tabItemActive : undefined,
+                  ])
+                }
+              >
+                Source Code
+              </NavLink>
+            )}
           </Stack>
         </Margins>
       </div>