From d7365d5b40e45b26a323c6cef11e252704b69a39 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 12 Oct 2022 01:13:07 +0000 Subject: [PATCH 1/5] fix: allow regular users to push files - As part of merging support for Template RBAC and user groups a permission check on reading files was relaxed. With the addition of admin roles on individual templates, regular users are now able to push template versions if they have inherited the 'admin' role for a template. In order to do so they need to be able to create and read their own files. Since collisions on hash in the past were ignored, this means that a regular user who pushes a template version with a file hash that collides with an existing hash will not be able to read the file (since it belongs to another user). This commit fixes the underlying problem which was that the files table had a primary key on the 'hash' column. This was not a problem at the time because only template admins and other users with similar elevated roles were able to read all files regardless of ownership. To fix this a new column and primary key 'id' has been introduced to the files table. The unique constraint has been updated to be hash+created_by. Tables (provisioner_jobs) that referenced files.hash have been updated to reference files.id. Relevant API endpoints have also been updated. --- cli/templatecreate.go | 7 +- cli/templatepull.go | 2 +- cli/templatepush.go | 2 +- .../autobuild/executor/lifecycle_executor.go | 2 +- coderd/coderd.go | 2 +- coderd/coderdtest/authorize.go | 2 +- coderd/coderdtest/coderdtest.go | 4 +- coderd/database/databasefake/databasefake.go | 19 ++++- coderd/database/dump.sql | 12 ++- .../migrations/000059_file_id.down.sql | 41 ++++++++++ .../database/migrations/000059_file_id.up.sql | 42 +++++++++++ coderd/database/models.go | 3 +- coderd/database/querier.go | 3 +- coderd/database/queries.sql.go | 74 ++++++++++++++----- coderd/database/queries/files.sql | 19 ++++- coderd/database/queries/provisionerjobs.sql | 2 +- coderd/database/unique_constraint.go | 1 + coderd/files.go | 32 ++++++-- coderd/files_test.go | 5 +- coderd/provisionerdaemons.go | 2 +- coderd/provisionerdaemons_test.go | 4 +- coderd/provisionerjobs.go | 8 +- coderd/templates.go | 3 +- coderd/templateversions.go | 16 ++-- coderd/templateversions_test.go | 8 +- coderd/workspacebuilds.go | 2 +- coderd/workspaces.go | 2 +- codersdk/files.go | 8 +- codersdk/organizations.go | 2 +- codersdk/provisionerdaemons.go | 18 ++--- enterprise/coderd/templates_test.go | 4 +- 31 files changed, 261 insertions(+), 90 deletions(-) create mode 100644 coderd/database/migrations/000059_file_id.down.sql create mode 100644 coderd/database/migrations/000059_file_id.up.sql diff --git a/cli/templatecreate.go b/cli/templatecreate.go index a0f4014f712f0..de0b8eab8f27e 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -10,6 +10,7 @@ import ( "unicode/utf8" "github.com/briandowns/spinner" + "github.com/google/uuid" "github.com/spf13/cobra" "golang.org/x/xerrors" @@ -91,7 +92,7 @@ func templateCreate() *cobra.Command { Client: client, Organization: organization, Provisioner: database.ProvisionerType(provisioner), - FileHash: resp.Hash, + FileID: resp.ID, ParameterFile: parameterFile, }) if err != nil { @@ -148,7 +149,7 @@ type createValidTemplateVersionArgs struct { Client *codersdk.Client Organization codersdk.Organization Provisioner database.ProvisionerType - FileHash string + FileID uuid.UUID ParameterFile string // Template is only required if updating a template's active version. Template *codersdk.Template @@ -165,7 +166,7 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers req := codersdk.CreateTemplateVersionRequest{ Name: args.Name, StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: args.FileHash, + FileID: args.FileID, Provisioner: codersdk.ProvisionerType(args.Provisioner), ParameterValues: parameters, } diff --git a/cli/templatepull.go b/cli/templatepull.go index 5660261c5158d..09f70c91b8f9e 100644 --- a/cli/templatepull.go +++ b/cli/templatepull.go @@ -66,7 +66,7 @@ func templatePull() *cobra.Command { latest := versions[0] // Download the tar archive. - raw, ctype, err := client.Download(ctx, latest.Job.StorageSource) + raw, ctype, err := client.Download(ctx, latest.Job.FileID) if err != nil { return xerrors.Errorf("download template: %w", err) } diff --git a/cli/templatepush.go b/cli/templatepush.go index f858c6daecb0d..40bafed0ef00c 100644 --- a/cli/templatepush.go +++ b/cli/templatepush.go @@ -80,7 +80,7 @@ func templatePush() *cobra.Command { Client: client, Organization: organization, Provisioner: database.ProvisionerType(provisioner), - FileHash: resp.Hash, + FileID: resp.ID, ParameterFile: parameterFile, Template: &template, ReuseParameters: !alwaysPrompt, diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 22536583b6d3f..f21fde9a8af7b 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -276,7 +276,7 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa Provisioner: template.Provisioner, Type: database.ProvisionerJobTypeWorkspaceBuild, StorageMethod: priorJob.StorageMethod, - StorageSource: priorJob.StorageSource, + FileID: priorJob.FileID, Input: input, }) if err != nil { diff --git a/coderd/coderd.go b/coderd/coderd.go index 29439a2001a99..0b4c231c2ac87 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -280,7 +280,7 @@ func New(options *Options) *API { // file content is expensive so it should be small. httpmw.RateLimitPerMinute(12), ) - r.Get("/{hash}", api.fileByHash) + r.Get("/{hashID}", api.fileByID) r.Post("/", api.postFile) }) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index fa778988641f1..d23a1ebd748d7 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -369,7 +369,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a "{workspaceagent}": workspace.LatestBuild.Resources[0].Agents[0].ID.String(), "{buildnumber}": strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10), "{template}": template.ID.String(), - "{hash}": file.Hash, + "{hashID}": file.ID.String(), "{workspaceresource}": workspace.LatestBuild.Resources[0].ID.String(), "{workspaceapp}": workspace.LatestBuild.Resources[0].Agents[0].Apps[0].Name, "{templateversion}": version.ID.String(), diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index d7ac4eb14be97..7ec28668cea91 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -383,7 +383,7 @@ func CreateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID file, err := client.Upload(context.Background(), codersdk.ContentTypeTar, data) require.NoError(t, err) templateVersion, err := client.CreateTemplateVersion(context.Background(), organizationID, codersdk.CreateTemplateVersionRequest{ - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) @@ -431,7 +431,7 @@ func UpdateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID require.NoError(t, err) templateVersion, err := client.CreateTemplateVersion(context.Background(), organizationID, codersdk.CreateTemplateVersionRequest{ TemplateID: templateID, - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 5a652164d3a6f..02156f8ff0624 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -316,12 +316,24 @@ func (q *fakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error { return sql.ErrNoRows } -func (q *fakeQuerier) GetFileByHash(_ context.Context, hash string) (database.File, error) { +func (q *fakeQuerier) GetFileByHashAndCreator(_ context.Context, arg database.GetFileByHashAndCreatorParams) (database.File, error) { q.mutex.RLock() defer q.mutex.RUnlock() for _, file := range q.files { - if file.Hash == hash { + if file.Hash == arg.Hash && file.CreatedBy == arg.CreatedBy { + return file, nil + } + } + return database.File{}, sql.ErrNoRows +} + +func (q *fakeQuerier) GetFileByID(_ context.Context, id uuid.UUID) (database.File, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + for _, file := range q.files { + if file.ID == id { return file, nil } } @@ -1888,6 +1900,7 @@ func (q *fakeQuerier) InsertFile(_ context.Context, arg database.InsertFileParam //nolint:gosimple file := database.File{ + ID: arg.ID, Hash: arg.Hash, CreatedAt: arg.CreatedAt, CreatedBy: arg.CreatedBy, @@ -2072,7 +2085,7 @@ func (q *fakeQuerier) InsertProvisionerJob(_ context.Context, arg database.Inser InitiatorID: arg.InitiatorID, Provisioner: arg.Provisioner, StorageMethod: arg.StorageMethod, - StorageSource: arg.StorageSource, + FileID: arg.FileID, Type: arg.Type, Input: arg.Input, } diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index eb16074e90525..9345907d044c4 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -151,7 +151,8 @@ CREATE TABLE files ( created_at timestamp with time zone NOT NULL, created_by uuid NOT NULL, mimetype character varying(64) NOT NULL, - data bytea NOT NULL + data bytea NOT NULL, + id uuid DEFAULT gen_random_uuid() NOT NULL ); CREATE TABLE gitsshkeys ( @@ -270,10 +271,10 @@ CREATE TABLE provisioner_jobs ( initiator_id uuid NOT NULL, provisioner provisioner_type NOT NULL, storage_method provisioner_storage_method NOT NULL, - storage_source text NOT NULL, type provisioner_job_type NOT NULL, input jsonb NOT NULL, - worker_id uuid + worker_id uuid, + file_id uuid NOT NULL ); CREATE TABLE site_configs ( @@ -432,7 +433,10 @@ ALTER TABLE ONLY audit_logs ADD CONSTRAINT audit_logs_pkey PRIMARY KEY (id); ALTER TABLE ONLY files - ADD CONSTRAINT files_pkey PRIMARY KEY (hash); + ADD CONSTRAINT files_hash_created_by_key UNIQUE (hash, created_by); + +ALTER TABLE ONLY files + ADD CONSTRAINT files_pkey PRIMARY KEY (id); ALTER TABLE ONLY gitsshkeys ADD CONSTRAINT gitsshkeys_pkey PRIMARY KEY (user_id); diff --git a/coderd/database/migrations/000059_file_id.down.sql b/coderd/database/migrations/000059_file_id.down.sql new file mode 100644 index 0000000000000..34187d078d78e --- /dev/null +++ b/coderd/database/migrations/000059_file_id.down.sql @@ -0,0 +1,41 @@ +BEGIN; + +-- Add back the storage_source column. This must be nullable temporarily. +ALTER TABLE provisioner_jobs ADD COLUMN storage_source text; + +-- Set the storage_source to the hash of the files.id reference. +UPDATE + provisioner_jobs +SET + storage_source=files.hash +FROM + files +WHERE + provisioner_jobs.file_id = files.id; + +-- Now that we've populated storage_source drop the file_id column. +ALTER TABLE provisioner_jobs DROP COLUMN file_id; +-- We can set the storage_source column as NOT NULL now. +ALTER TABLE provisioner_jobs ALTER COLUMN storage_source SET NOT NULL; + +-- Delete all the duplicate rows where hashes collide. +-- We filter on 'id' to ensure only 1 unique row. +DELETE FROM + files a +USING + files b +WHERE + a.created_by < b.created_by +AND + a.hash = b.hash; + +-- Drop the primary key on files.id. +ALTER TABLE files DROP CONSTRAINT files_pkey; +-- Drdop the id column. +ALTER TABLE files DROP COLUMN id; +-- Drop the unique constraint on hash + owner. +ALTER TABLE files DROP CONSTRAINT files_hash_owner_id_key; +-- Set the primary key back to hash. +ALTER TABLE files ADD PRIMARY KEY (hash); + +COMMIT; diff --git a/coderd/database/migrations/000059_file_id.up.sql b/coderd/database/migrations/000059_file_id.up.sql new file mode 100644 index 0000000000000..10a0558cf99df --- /dev/null +++ b/coderd/database/migrations/000059_file_id.up.sql @@ -0,0 +1,42 @@ +-- This migration updates the files table to move the unique +-- constraint to be hash + created_by. This is necessary to +-- allow regular users who have been granted admin to a specific +-- template to be able to push and read files used for template +-- versions they create. +-- Prior to this collisions on file.hash were not an issue +-- since users who could push files could also read all files. +-- +-- This migration also adds a 'files.id' column as the primary +-- key. As a side effect the provisioner_jobs must now reference +-- the files.id column since the 'hash' column is now ambiguous. +BEGIN; + +-- Drop the primary key on hash. +ALTER TABLE files DROP CONSTRAINT files_pkey; + +-- Add an 'id' colum and designate it the primary key. +ALTER TABLE files ADD COLUMN + id uuid NOT NULL PRIMARY KEY DEFAULT gen_random_uuid (); + +-- Update the constraint to include the user who created it. +ALTER TABLE files ADD UNIQUE(hash, created_by); + +-- Update provisioner_jobs to include a file_id column. +-- This must be temporarily nullable. +ALTER TABLE provisioner_jobs ADD COLUMN file_id uuid; + +-- Update all the rows to point to key in the files table. +UPDATE provisioner_jobs +SET + file_id = files.id +FROM + files +WHERE + provisioner_jobs.storage_source = files.hash; + +-- Enforce NOT NULL on file_id now. +ALTER TABLE provisioner_jobs ALTER COLUMN file_id SET NOT NULL; +-- Drop storage_source since it is no longer useful for anything. +ALTER TABLE provisioner_jobs DROP COLUMN storage_source; + +COMMIT; diff --git a/coderd/database/models.go b/coderd/database/models.go index f669b5e618138..2e50d2b647bd7 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -404,6 +404,7 @@ type File struct { CreatedBy uuid.UUID `db:"created_by" json:"created_by"` Mimetype string `db:"mimetype" json:"mimetype"` Data []byte `db:"data" json:"data"` + ID uuid.UUID `db:"id" json:"id"` } type GitSSHKey struct { @@ -501,10 +502,10 @@ type ProvisionerJob struct { InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` StorageMethod ProvisionerStorageMethod `db:"storage_method" json:"storage_method"` - StorageSource string `db:"storage_source" json:"storage_source"` Type ProvisionerJobType `db:"type" json:"type"` Input json.RawMessage `db:"input" json:"input"` WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"` + FileID uuid.UUID `db:"file_id" json:"file_id"` } type ProvisionerJobLog struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index b58f6abbccfb8..e9c577bcb11a7 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -39,7 +39,8 @@ type sqlcQuerier interface { // are included. GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUID) (GetAuthorizationUserRolesRow, error) GetDeploymentID(ctx context.Context) (string, error) - GetFileByHash(ctx context.Context, hash string) (File, error) + GetFileByHashAndCreator(ctx context.Context, arg GetFileByHashAndCreatorParams) (File, error) + GetFileByID(ctx context.Context, id uuid.UUID) (File, error) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (GitSSHKey, error) GetGroupByID(ctx context.Context, id uuid.UUID) (Group, error) GetGroupByOrgAndName(ctx context.Context, arg GetGroupByOrgAndNameParams) (Group, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 15a5080e309ae..051fae12ce350 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -647,19 +647,51 @@ func (q *sqlQuerier) InsertAuditLog(ctx context.Context, arg InsertAuditLogParam return i, err } -const getFileByHash = `-- name: GetFileByHash :one +const getFileByHashAndCreator = `-- name: GetFileByHashAndCreator :one SELECT - hash, created_at, created_by, mimetype, data + hash, created_at, created_by, mimetype, data, id FROM files WHERE hash = $1 +AND + created_by = $2 +LIMIT + 1 +` + +type GetFileByHashAndCreatorParams struct { + Hash string `db:"hash" json:"hash"` + CreatedBy uuid.UUID `db:"created_by" json:"created_by"` +} + +func (q *sqlQuerier) GetFileByHashAndCreator(ctx context.Context, arg GetFileByHashAndCreatorParams) (File, error) { + row := q.db.QueryRowContext(ctx, getFileByHashAndCreator, arg.Hash, arg.CreatedBy) + var i File + err := row.Scan( + &i.Hash, + &i.CreatedAt, + &i.CreatedBy, + &i.Mimetype, + &i.Data, + &i.ID, + ) + return i, err +} + +const getFileByID = `-- name: GetFileByID :one +SELECT + hash, created_at, created_by, mimetype, data, id +FROM + files +WHERE + id = $1 LIMIT 1 ` -func (q *sqlQuerier) GetFileByHash(ctx context.Context, hash string) (File, error) { - row := q.db.QueryRowContext(ctx, getFileByHash, hash) +func (q *sqlQuerier) GetFileByID(ctx context.Context, id uuid.UUID) (File, error) { + row := q.db.QueryRowContext(ctx, getFileByID, id) var i File err := row.Scan( &i.Hash, @@ -667,18 +699,20 @@ func (q *sqlQuerier) GetFileByHash(ctx context.Context, hash string) (File, erro &i.CreatedBy, &i.Mimetype, &i.Data, + &i.ID, ) return i, err } const insertFile = `-- name: InsertFile :one INSERT INTO - files (hash, created_at, created_by, mimetype, "data") + files (id, hash, created_at, created_by, mimetype, "data") VALUES - ($1, $2, $3, $4, $5) RETURNING hash, created_at, created_by, mimetype, data + ($1, $2, $3, $4, $5, $6) RETURNING hash, created_at, created_by, mimetype, data, id ` type InsertFileParams struct { + ID uuid.UUID `db:"id" json:"id"` Hash string `db:"hash" json:"hash"` CreatedAt time.Time `db:"created_at" json:"created_at"` CreatedBy uuid.UUID `db:"created_by" json:"created_by"` @@ -688,6 +722,7 @@ type InsertFileParams struct { func (q *sqlQuerier) InsertFile(ctx context.Context, arg InsertFileParams) (File, error) { row := q.db.QueryRowContext(ctx, insertFile, + arg.ID, arg.Hash, arg.CreatedAt, arg.CreatedBy, @@ -701,6 +736,7 @@ func (q *sqlQuerier) InsertFile(ctx context.Context, arg InsertFileParams) (File &i.CreatedBy, &i.Mimetype, &i.Data, + &i.ID, ) return i, err } @@ -2237,7 +2273,7 @@ WHERE SKIP LOCKED LIMIT 1 - ) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id + ) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id ` type AcquireProvisionerJobParams struct { @@ -2267,17 +2303,17 @@ func (q *sqlQuerier) AcquireProvisionerJob(ctx context.Context, arg AcquireProvi &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ) return i, err } const getProvisionerJobByID = `-- name: GetProvisionerJobByID :one SELECT - id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id + id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id FROM provisioner_jobs WHERE @@ -2299,17 +2335,17 @@ func (q *sqlQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (P &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ) return i, err } const getProvisionerJobsByIDs = `-- name: GetProvisionerJobsByIDs :many SELECT - id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id + id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id FROM provisioner_jobs WHERE @@ -2337,10 +2373,10 @@ func (q *sqlQuerier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUI &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ); err != nil { return nil, err } @@ -2356,7 +2392,7 @@ func (q *sqlQuerier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUI } const getProvisionerJobsCreatedAfter = `-- name: GetProvisionerJobsCreatedAfter :many -SELECT id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id FROM provisioner_jobs WHERE created_at > $1 +SELECT id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id FROM provisioner_jobs WHERE created_at > $1 ` func (q *sqlQuerier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]ProvisionerJob, error) { @@ -2380,10 +2416,10 @@ func (q *sqlQuerier) GetProvisionerJobsCreatedAfter(ctx context.Context, created &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ); err != nil { return nil, err } @@ -2408,12 +2444,12 @@ INSERT INTO initiator_id, provisioner, storage_method, - storage_source, + file_id, "type", "input" ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id ` type InsertProvisionerJobParams struct { @@ -2424,7 +2460,7 @@ type InsertProvisionerJobParams struct { InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` StorageMethod ProvisionerStorageMethod `db:"storage_method" json:"storage_method"` - StorageSource string `db:"storage_source" json:"storage_source"` + FileID uuid.UUID `db:"file_id" json:"file_id"` Type ProvisionerJobType `db:"type" json:"type"` Input json.RawMessage `db:"input" json:"input"` } @@ -2438,7 +2474,7 @@ func (q *sqlQuerier) InsertProvisionerJob(ctx context.Context, arg InsertProvisi arg.InitiatorID, arg.Provisioner, arg.StorageMethod, - arg.StorageSource, + arg.FileID, arg.Type, arg.Input, ) @@ -2455,10 +2491,10 @@ func (q *sqlQuerier) InsertProvisionerJob(ctx context.Context, arg InsertProvisi &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ) return i, err } diff --git a/coderd/database/queries/files.sql b/coderd/database/queries/files.sql index a91513d31a345..1f54386bb3ac4 100644 --- a/coderd/database/queries/files.sql +++ b/coderd/database/queries/files.sql @@ -1,15 +1,28 @@ --- name: GetFileByHash :one +-- name: GetFileByID :one +SELECT + * +FROM + files +WHERE + id = $1 +LIMIT + 1; + +-- name: GetFileByHashAndCreator :one SELECT * FROM files WHERE hash = $1 +AND + created_by = $2 LIMIT 1; + -- name: InsertFile :one INSERT INTO - files (hash, created_at, created_by, mimetype, "data") + files (id, hash, created_at, created_by, mimetype, "data") VALUES - ($1, $2, $3, $4, $5) RETURNING *; + ($1, $2, $3, $4, $5, $6) RETURNING *; diff --git a/coderd/database/queries/provisionerjobs.sql b/coderd/database/queries/provisionerjobs.sql index 4775d574e2713..027bd25bc91b3 100644 --- a/coderd/database/queries/provisionerjobs.sql +++ b/coderd/database/queries/provisionerjobs.sql @@ -59,7 +59,7 @@ INSERT INTO initiator_id, provisioner, storage_method, - storage_source, + file_id, "type", "input" ) diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index 44cfd89d43284..b4263c09b4762 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -6,6 +6,7 @@ type UniqueConstraint string // UniqueConstraint enums. const ( + UniqueFilesHashCreatedByKey UniqueConstraint = "files_hash_created_by_key" // ALTER TABLE ONLY files ADD CONSTRAINT files_hash_created_by_key UNIQUE (hash, created_by); UniqueGroupMembersUserIDGroupIDKey UniqueConstraint = "group_members_user_id_group_id_key" // ALTER TABLE ONLY group_members ADD CONSTRAINT group_members_user_id_group_id_key UNIQUE (user_id, group_id); UniqueGroupsNameOrganizationIDKey UniqueConstraint = "groups_name_organization_id_key" // ALTER TABLE ONLY groups ADD CONSTRAINT groups_name_organization_id_key UNIQUE (name, organization_id); UniqueLicensesJWTKey UniqueConstraint = "licenses_jwt_key" // ALTER TABLE ONLY licenses ADD CONSTRAINT licenses_jwt_key UNIQUE (jwt); diff --git a/coderd/files.go b/coderd/files.go index a0b7be2c8be5d..0f570379333d0 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -10,6 +10,7 @@ import ( "net/http" "github.com/go-chi/chi/v5" + "github.com/google/uuid" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" @@ -50,15 +51,20 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { } hashBytes := sha256.Sum256(data) hash := hex.EncodeToString(hashBytes[:]) - file, err := api.Database.GetFileByHash(ctx, hash) + file, err := api.Database.GetFileByHashAndCreator(ctx, database.GetFileByHashAndCreatorParams{ + Hash: hash, + CreatedBy: apiKey.UserID, + }) if err == nil { // The file already exists! httpapi.Write(ctx, rw, http.StatusOK, codersdk.UploadResponse{ - Hash: file.Hash, + ID: file.ID, }) return } + id := uuid.New() file, err = api.Database.InsertFile(ctx, database.InsertFileParams{ + ID: id, Hash: hash, CreatedBy: apiKey.UserID, CreatedAt: database.Now(), @@ -74,20 +80,30 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { } httpapi.Write(ctx, rw, http.StatusCreated, codersdk.UploadResponse{ - Hash: file.Hash, + ID: file.ID, }) } -func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) { +func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - hash := chi.URLParam(r, "hash") - if hash == "" { + + hashID := chi.URLParam(r, "hashID") + if hashID == "" { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "File hash must be provided in url.", + Message: "File id must be provided in url.", }) return } - file, err := api.Database.GetFileByHash(ctx, hash) + + id, err := uuid.Parse(hashID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "File id must be a valid UUID.", + }) + return + } + + file, err := api.Database.GetFileByID(ctx, id) if errors.Is(err, sql.ErrNoRows) { httpapi.ResourceNotFound(rw) return diff --git a/coderd/files_test.go b/coderd/files_test.go index 28d4a33104a5a..b3a3953a43972 100644 --- a/coderd/files_test.go +++ b/coderd/files_test.go @@ -5,6 +5,7 @@ import ( "net/http" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/coderdtest" @@ -64,7 +65,7 @@ func TestDownload(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - _, _, err := client.Download(ctx, "something") + _, _, err := client.Download(ctx, uuid.New()) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) @@ -80,7 +81,7 @@ func TestDownload(t *testing.T) { resp, err := client.Upload(ctx, codersdk.ContentTypeTar, make([]byte, 1024)) require.NoError(t, err) - data, contentType, err := client.Download(ctx, resp.Hash) + data, contentType, err := client.Download(ctx, resp.ID) require.NoError(t, err) require.Len(t, data, 1024) require.Equal(t, codersdk.ContentTypeTar, contentType) diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index bef1110ec3bb0..1863de54c6b98 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -315,7 +315,7 @@ func (server *provisionerdServer) AcquireJob(ctx context.Context, _ *proto.Empty } switch job.StorageMethod { case database.ProvisionerStorageMethodFile: - file, err := server.Database.GetFileByHash(ctx, job.StorageSource) + file, err := server.Database.GetFileByID(ctx, job.FileID) if err != nil { return nil, failJob(fmt.Sprintf("get file by hash: %s", err)) } diff --git a/coderd/provisionerdaemons_test.go b/coderd/provisionerdaemons_test.go index 8fe48314802c5..d3b0be35cd020 100644 --- a/coderd/provisionerdaemons_test.go +++ b/coderd/provisionerdaemons_test.go @@ -33,11 +33,11 @@ func TestProvisionerDaemons(t *testing.T) { resp, err := client.Upload(ctx, codersdk.ContentTypeTar, data) require.NoError(t, err) - t.Log(resp.Hash) + t.Log(resp.ID) version, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: resp.Hash, + FileID: resp.ID, Provisioner: codersdk.ProvisionerTypeEcho, }) require.NoError(t, err) diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index e2784112245cf..294b013e00280 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -316,10 +316,10 @@ func convertProvisionerJobLog(provisionerJobLog database.ProvisionerJobLog) code func convertProvisionerJob(provisionerJob database.ProvisionerJob) codersdk.ProvisionerJob { job := codersdk.ProvisionerJob{ - ID: provisionerJob.ID, - CreatedAt: provisionerJob.CreatedAt, - Error: provisionerJob.Error.String, - StorageSource: provisionerJob.StorageSource, + ID: provisionerJob.ID, + CreatedAt: provisionerJob.CreatedAt, + Error: provisionerJob.Error.String, + FileID: provisionerJob.FileID, } // Applying values optional to the struct. if provisionerJob.StartedAt.Valid { diff --git a/coderd/templates.go b/coderd/templates.go index 3366e02b14216..fbc25a7b5546e 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -600,6 +600,7 @@ func (api *API) autoImportTemplate(ctx context.Context, opts autoImportTemplateO now = database.Now() ) file, err := tx.InsertFile(ctx, database.InsertFileParams{ + ID: uuid.New(), Hash: hex.EncodeToString(hash[:]), CreatedAt: now, CreatedBy: opts.userID, @@ -639,7 +640,7 @@ func (api *API) autoImportTemplate(ctx context.Context, opts autoImportTemplateO InitiatorID: opts.userID, Provisioner: database.ProvisionerTypeTerraform, StorageMethod: database.ProvisionerStorageMethodFile, - StorageSource: file.Hash, + FileID: file.ID, Type: database.ProvisionerJobTypeTemplateVersionImport, Input: []byte{'{', '}'}, }) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 85494949e97dc..09f01fbb8b199 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -285,7 +285,7 @@ func (api *API) postTemplateVersionDryRun(rw http.ResponseWriter, r *http.Reques InitiatorID: apiKey.UserID, Provisioner: job.Provisioner, StorageMethod: job.StorageMethod, - StorageSource: job.StorageSource, + FileID: job.FileID, Type: database.ProvisionerJobTypeTemplateVersionDryRun, Input: input, }) @@ -716,7 +716,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht return } - file, err := api.Database.GetFileByHash(ctx, req.StorageSource) + file, err := api.Database.GetFileByID(ctx, req.FileID) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ Message: "File not found.", @@ -731,12 +731,10 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht return } - // TODO(JonA): Readd this check once we update the unique constraint - // on files to be owner + hash. - // if !api.Authorize(r, rbac.ActionRead, file) { - // httpapi.ResourceNotFound(rw) - // return - // } + if !api.Authorize(r, rbac.ActionRead, file) { + httpapi.ResourceNotFound(rw) + return + } var templateVersion database.TemplateVersion var provisionerJob database.ProvisionerJob @@ -813,7 +811,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht InitiatorID: apiKey.UserID, Provisioner: database.ProvisionerType(req.Provisioner), StorageMethod: database.ProvisionerStorageMethodFile, - StorageSource: file.Hash, + FileID: file.ID, Type: database.ProvisionerJobTypeTemplateVersionImport, Input: []byte{'{', '}'}, }) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 5521be8ec339c..1bcb5ba8a0aaa 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -66,7 +66,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { _, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ TemplateID: templateID, StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: "hash", + FileID: uuid.New(), Provisioner: codersdk.ProvisionerTypeEcho, }) var apiErr *codersdk.Error @@ -84,7 +84,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { _, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: "hash", + FileID: uuid.New(), Provisioner: codersdk.ProvisionerTypeEcho, }) var apiErr *codersdk.Error @@ -112,7 +112,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { version, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ Name: "bananas", StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: file.Hash, + FileID: file.ID, Provisioner: codersdk.ProvisionerTypeEcho, ParameterValues: []codersdk.CreateParameterRequest{{ Name: "example", @@ -842,7 +842,7 @@ func TestPaginatedTemplateVersions(t *testing.T) { eg.Go(func() error { templateVersion, err := client.CreateTemplateVersion(egCtx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ TemplateID: template.ID, - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index b2d8424907df4..6881137cf8727 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -458,7 +458,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { Provisioner: template.Provisioner, Type: database.ProvisionerJobTypeWorkspaceBuild, StorageMethod: templateVersionJob.StorageMethod, - StorageSource: templateVersionJob.StorageSource, + FileID: templateVersionJob.FileID, Input: input, }) if err != nil { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index fda034dc6eb88..00b3048b87169 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -430,7 +430,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req Provisioner: template.Provisioner, Type: database.ProvisionerJobTypeWorkspaceBuild, StorageMethod: templateVersionJob.StorageMethod, - StorageSource: templateVersionJob.StorageSource, + FileID: templateVersionJob.FileID, Input: input, }) if err != nil { diff --git a/codersdk/files.go b/codersdk/files.go index 52fcf0215081b..6d728fbfbcab2 100644 --- a/codersdk/files.go +++ b/codersdk/files.go @@ -6,6 +6,8 @@ import ( "fmt" "io" "net/http" + + "github.com/google/uuid" ) const ( @@ -14,7 +16,7 @@ const ( // UploadResponse contains the hash to reference the uploaded file. type UploadResponse struct { - Hash string `json:"hash"` + ID uuid.UUID `json:"hash"` } // Upload uploads an arbitrary file with the content type provided. @@ -35,8 +37,8 @@ func (c *Client) Upload(ctx context.Context, contentType string, content []byte) } // Download fetches a file by uploaded hash. -func (c *Client) Download(ctx context.Context, hash string) ([]byte, string, error) { - res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/files/%s", hash), nil) +func (c *Client) Download(ctx context.Context, id uuid.UUID) ([]byte, string, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/files/%s", id.String()), nil) if err != nil { return nil, "", err } diff --git a/codersdk/organizations.go b/codersdk/organizations.go index 003b9156dd7c6..de5e42122ce28 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -39,7 +39,7 @@ type CreateTemplateVersionRequest struct { TemplateID uuid.UUID `json:"template_id,omitempty"` StorageMethod ProvisionerStorageMethod `json:"storage_method" validate:"oneof=file,required"` - StorageSource string `json:"storage_source" validate:"required"` + FileID uuid.UUID `json:"file_id" validate:"required"` Provisioner ProvisionerType `json:"provisioner" validate:"oneof=terraform echo,required"` // ParameterValues allows for additional parameters to be provided // during the dry-run provision stage. diff --git a/codersdk/provisionerdaemons.go b/codersdk/provisionerdaemons.go index 5eb8872fe6620..adce0321be691 100644 --- a/codersdk/provisionerdaemons.go +++ b/codersdk/provisionerdaemons.go @@ -64,15 +64,15 @@ const ( ) type ProvisionerJob struct { - ID uuid.UUID `json:"id"` - CreatedAt time.Time `json:"created_at"` - StartedAt *time.Time `json:"started_at,omitempty"` - CompletedAt *time.Time `json:"completed_at,omitempty"` - CanceledAt *time.Time `json:"canceled_at,omitempty"` - Error string `json:"error,omitempty"` - Status ProvisionerJobStatus `json:"status"` - WorkerID *uuid.UUID `json:"worker_id,omitempty"` - StorageSource string `json:"storage_source"` + ID uuid.UUID `json:"id"` + CreatedAt time.Time `json:"created_at"` + StartedAt *time.Time `json:"started_at,omitempty"` + CompletedAt *time.Time `json:"completed_at,omitempty"` + CanceledAt *time.Time `json:"canceled_at,omitempty"` + Error string `json:"error,omitempty"` + Status ProvisionerJobStatus `json:"status"` + WorkerID *uuid.UUID `json:"worker_id,omitempty"` + FileID uuid.UUID `json:"file_id"` } type ProvisionerJobLog struct { diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index c47fae246841d..bce03bb1a1715 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -286,7 +286,7 @@ func TestTemplateACL(t *testing.T) { _, err = client1.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ Name: "testme", TemplateID: template.ID, - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) @@ -302,7 +302,7 @@ func TestTemplateACL(t *testing.T) { _, err = client1.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ Name: "testme", TemplateID: template.ID, - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) From 1530c96c822fcc618d6a19e676eaee50f609d80e Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 12 Oct 2022 01:22:33 +0000 Subject: [PATCH 2/5] make gen --- site/src/api/typesGenerated.ts | 4 ++-- site/src/testHelpers/entities.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 72abae519b469..6099ce269a6bd 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -206,7 +206,7 @@ export interface CreateTemplateVersionRequest { readonly name?: string readonly template_id?: string readonly storage_method: ProvisionerStorageMethod - readonly storage_source: string + readonly file_id: string readonly provisioner: ProvisionerType readonly parameter_values?: CreateParameterRequest[] } @@ -504,7 +504,7 @@ export interface ProvisionerJob { readonly error?: string readonly status: ProvisionerJobStatus readonly worker_id?: string - readonly storage_source: string + readonly file_id: string } // From codersdk/provisionerdaemons.go diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index a391a8ea298ea..afdc266ff6a2c 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -132,7 +132,7 @@ export const MockProvisionerJob: TypesGen.ProvisionerJob = { created_at: "", id: "test-provisioner-job", status: "succeeded", - storage_source: "asdf", + file_id: "fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0", completed_at: "2022-05-17T17:39:01.382927298Z", } From b3276355540414749ebc3a710ce04034bca990b4 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 12 Oct 2022 01:27:36 +0000 Subject: [PATCH 3/5] fix some missed tests --- coderd/coderdtest/authorize.go | 2 +- coderd/database/migrations/000059_file_id.up.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index d23a1ebd748d7..0334245095302 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -142,7 +142,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), }, "POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile}, - "GET:/api/v2/files/{hash}": { + "GET:/api/v2/files/{hashID}": { AssertAction: rbac.ActionRead, AssertObject: rbac.ResourceFile.WithOwner(a.Admin.UserID.String()), }, diff --git a/coderd/database/migrations/000059_file_id.up.sql b/coderd/database/migrations/000059_file_id.up.sql index 10a0558cf99df..7e6e919fa9bb3 100644 --- a/coderd/database/migrations/000059_file_id.up.sql +++ b/coderd/database/migrations/000059_file_id.up.sql @@ -14,7 +14,7 @@ BEGIN; -- Drop the primary key on hash. ALTER TABLE files DROP CONSTRAINT files_pkey; --- Add an 'id' colum and designate it the primary key. +-- Add an 'id' column and designate it the primary key. ALTER TABLE files ADD COLUMN id uuid NOT NULL PRIMARY KEY DEFAULT gen_random_uuid (); From 8a34858ec76aeac3255b7b30a3d90227be8daf29 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 12 Oct 2022 01:35:06 +0000 Subject: [PATCH 4/5] Fix down file --- coderd/database/migrations/000059_file_id.down.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/migrations/000059_file_id.down.sql b/coderd/database/migrations/000059_file_id.down.sql index 34187d078d78e..56dbb13eeb504 100644 --- a/coderd/database/migrations/000059_file_id.down.sql +++ b/coderd/database/migrations/000059_file_id.down.sql @@ -31,10 +31,10 @@ AND -- Drop the primary key on files.id. ALTER TABLE files DROP CONSTRAINT files_pkey; --- Drdop the id column. +-- Drop the id column. ALTER TABLE files DROP COLUMN id; -- Drop the unique constraint on hash + owner. -ALTER TABLE files DROP CONSTRAINT files_hash_owner_id_key; +ALTER TABLE files DROP CONSTRAINT files_hash_created_by_key; -- Set the primary key back to hash. ALTER TABLE files ADD PRIMARY KEY (hash); From 0930d9db8daf8e2f520cccd4711b5e50b6fc2e86 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 20:41:39 +0000 Subject: [PATCH 5/5] comments --- coderd/coderd.go | 2 +- coderd/coderdtest/authorize.go | 4 ++-- coderd/files.go | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 0b4c231c2ac87..b067035b3adb4 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -280,7 +280,7 @@ func New(options *Options) *API { // file content is expensive so it should be small. httpmw.RateLimitPerMinute(12), ) - r.Get("/{hashID}", api.fileByID) + r.Get("/{fileID}", api.fileByID) r.Post("/", api.postFile) }) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index 0334245095302..a5183f2b6e450 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -142,7 +142,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), }, "POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile}, - "GET:/api/v2/files/{hashID}": { + "GET:/api/v2/files/{fileID}": { AssertAction: rbac.ActionRead, AssertObject: rbac.ResourceFile.WithOwner(a.Admin.UserID.String()), }, @@ -369,7 +369,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a "{workspaceagent}": workspace.LatestBuild.Resources[0].Agents[0].ID.String(), "{buildnumber}": strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10), "{template}": template.ID.String(), - "{hashID}": file.ID.String(), + "{fileID}": file.ID.String(), "{workspaceresource}": workspace.LatestBuild.Resources[0].ID.String(), "{workspaceapp}": workspace.LatestBuild.Resources[0].Agents[0].Apps[0].Name, "{templateversion}": version.ID.String(), diff --git a/coderd/files.go b/coderd/files.go index 0f570379333d0..2c304921f8226 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -87,15 +87,15 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - hashID := chi.URLParam(r, "hashID") - if hashID == "" { + fileID := chi.URLParam(r, "fileID") + if fileID == "" { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "File id must be provided in url.", }) return } - id, err := uuid.Parse(hashID) + id, err := uuid.Parse(fileID) if err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "File id must be a valid UUID.",