From 104bf2124ce121e176cf7d32156eca7063354d7d Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Mon, 4 Aug 2025 16:11:36 +0000 Subject: [PATCH 01/21] feat: add user_secrets table --- coderd/database/dump.sql | 24 +++++++++++++ coderd/database/foreign_key_constraint.go | 1 + .../000356_add_user_secrets.down.sql | 7 ++++ .../migrations/000356_add_user_secrets.up.sql | 34 +++++++++++++++++++ coderd/database/models.go | 12 +++++++ coderd/database/unique_constraint.go | 4 +++ 6 files changed, 82 insertions(+) create mode 100644 coderd/database/migrations/000356_add_user_secrets.down.sql create mode 100644 coderd/database/migrations/000356_add_user_secrets.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 053b5302d3e38..7e4a68526c4bf 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1861,6 +1861,18 @@ COMMENT ON COLUMN user_links.oauth_refresh_token_key_id IS 'The ID of the key us COMMENT ON COLUMN user_links.claims IS 'Claims from the IDP for the linked user. Includes both id_token and userinfo claims. '; +CREATE TABLE user_secrets ( + id uuid DEFAULT gen_random_uuid() NOT NULL, + user_id uuid NOT NULL, + name text NOT NULL, + description text NOT NULL, + value text NOT NULL, + env_name text DEFAULT ''::text NOT NULL, + file_path text DEFAULT ''::text NOT NULL, + created_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, + updated_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL +); + CREATE TABLE user_status_changes ( id uuid DEFAULT gen_random_uuid() NOT NULL, user_id uuid NOT NULL, @@ -2674,6 +2686,9 @@ ALTER TABLE ONLY user_deleted ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); +ALTER TABLE ONLY user_secrets + ADD CONSTRAINT user_secrets_pkey PRIMARY KEY (id); + ALTER TABLE ONLY user_status_changes ADD CONSTRAINT user_status_changes_pkey PRIMARY KEY (id); @@ -2862,6 +2877,12 @@ CREATE UNIQUE INDEX templates_organization_id_name_idx ON templates USING btree CREATE UNIQUE INDEX user_links_linked_id_login_type_idx ON user_links USING btree (linked_id, login_type) WHERE (linked_id <> ''::text); +CREATE UNIQUE INDEX user_secrets_user_env_name_idx ON user_secrets USING btree (user_id, env_name) WHERE (env_name <> ''::text); + +CREATE UNIQUE INDEX user_secrets_user_file_path_idx ON user_secrets USING btree (user_id, file_path) WHERE (file_path <> ''::text); + +CREATE UNIQUE INDEX user_secrets_user_name_idx ON user_secrets USING btree (user_id, name); + CREATE UNIQUE INDEX users_email_lower_idx ON users USING btree (lower(email)) WHERE (deleted = false); CREATE UNIQUE INDEX users_username_lower_idx ON users USING btree (lower(username)) WHERE (deleted = false); @@ -3167,6 +3188,9 @@ ALTER TABLE ONLY user_links ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY user_secrets + ADD CONSTRAINT user_secrets_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ALTER TABLE ONLY user_status_changes ADD CONSTRAINT user_status_changes_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); diff --git a/coderd/database/foreign_key_constraint.go b/coderd/database/foreign_key_constraint.go index c3aaf7342a97c..33aa8edd69032 100644 --- a/coderd/database/foreign_key_constraint.go +++ b/coderd/database/foreign_key_constraint.go @@ -63,6 +63,7 @@ const ( ForeignKeyUserLinksOauthAccessTokenKeyID ForeignKeyConstraint = "user_links_oauth_access_token_key_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_oauth_access_token_key_id_fkey FOREIGN KEY (oauth_access_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); ForeignKeyUserLinksOauthRefreshTokenKeyID ForeignKeyConstraint = "user_links_oauth_refresh_token_key_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_oauth_refresh_token_key_id_fkey FOREIGN KEY (oauth_refresh_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); ForeignKeyUserLinksUserID ForeignKeyConstraint = "user_links_user_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ForeignKeyUserSecretsUserID ForeignKeyConstraint = "user_secrets_user_id_fkey" // ALTER TABLE ONLY user_secrets ADD CONSTRAINT user_secrets_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; ForeignKeyUserStatusChangesUserID ForeignKeyConstraint = "user_status_changes_user_id_fkey" // ALTER TABLE ONLY user_status_changes ADD CONSTRAINT user_status_changes_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); ForeignKeyWebpushSubscriptionsUserID ForeignKeyConstraint = "webpush_subscriptions_user_id_fkey" // ALTER TABLE ONLY webpush_subscriptions ADD CONSTRAINT webpush_subscriptions_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; ForeignKeyWorkspaceAgentDevcontainersWorkspaceAgentID ForeignKeyConstraint = "workspace_agent_devcontainers_workspace_agent_id_fkey" // ALTER TABLE ONLY workspace_agent_devcontainers ADD CONSTRAINT workspace_agent_devcontainers_workspace_agent_id_fkey FOREIGN KEY (workspace_agent_id) REFERENCES workspace_agents(id) ON DELETE CASCADE; diff --git a/coderd/database/migrations/000356_add_user_secrets.down.sql b/coderd/database/migrations/000356_add_user_secrets.down.sql new file mode 100644 index 0000000000000..67bd30002e23a --- /dev/null +++ b/coderd/database/migrations/000356_add_user_secrets.down.sql @@ -0,0 +1,7 @@ +-- Drop the unique indexes first (in reverse order of creation) +DROP INDEX IF EXISTS user_secrets_user_file_path_idx; +DROP INDEX IF EXISTS user_secrets_user_env_name_idx; +DROP INDEX IF EXISTS user_secrets_user_name_idx; + +-- Drop the table +DROP TABLE IF EXISTS user_secrets; diff --git a/coderd/database/migrations/000356_add_user_secrets.up.sql b/coderd/database/migrations/000356_add_user_secrets.up.sql new file mode 100644 index 0000000000000..8a4d398f490eb --- /dev/null +++ b/coderd/database/migrations/000356_add_user_secrets.up.sql @@ -0,0 +1,34 @@ +-- Stores encrypted user secrets (global, available across all organizations) +CREATE TABLE user_secrets ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, + name TEXT NOT NULL, + description TEXT NOT NULL, + + -- The encrypted secret value (base64-encoded encrypted data) + value TEXT NOT NULL, + + -- Auto-injection settings + -- Environment variable name (e.g., "DATABASE_PASSWORD", "API_KEY") + -- Empty string means don't inject as env var + env_name TEXT NOT NULL DEFAULT '', + + -- File path where secret should be written (e.g., "/home/coder/.ssh/id_rsa") + -- Empty string means don't inject as file + file_path TEXT NOT NULL DEFAULT '', + + -- Timestamps + created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL, + updated_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL +); + +-- Unique constraint: user can't have duplicate secret names +CREATE UNIQUE INDEX user_secrets_user_name_idx ON user_secrets(user_id, name); + +-- Unique constraint: user can't have duplicate env names +CREATE UNIQUE INDEX user_secrets_user_env_name_idx ON user_secrets(user_id, env_name) +WHERE env_name != ''; + +-- Unique constraint: user can't have duplicate file paths +CREATE UNIQUE INDEX user_secrets_user_file_path_idx ON user_secrets(user_id, file_path) +WHERE file_path != ''; diff --git a/coderd/database/models.go b/coderd/database/models.go index 8b13c8a8af057..75d2b941dab3c 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -3812,6 +3812,18 @@ type UserLink struct { Claims UserLinkClaims `db:"claims" json:"claims"` } +type UserSecret struct { + ID uuid.UUID `db:"id" json:"id"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + Name string `db:"name" json:"name"` + Description string `db:"description" json:"description"` + Value string `db:"value" json:"value"` + EnvName string `db:"env_name" json:"env_name"` + FilePath string `db:"file_path" json:"file_path"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` +} + // Tracks the history of user status changes type UserStatusChange struct { ID uuid.UUID `db:"id" json:"id"` diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index 38c95e67410c9..3ed326102b18c 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -70,6 +70,7 @@ const ( UniqueUserConfigsPkey UniqueConstraint = "user_configs_pkey" // ALTER TABLE ONLY user_configs ADD CONSTRAINT user_configs_pkey PRIMARY KEY (user_id, key); UniqueUserDeletedPkey UniqueConstraint = "user_deleted_pkey" // ALTER TABLE ONLY user_deleted ADD CONSTRAINT user_deleted_pkey PRIMARY KEY (id); UniqueUserLinksPkey UniqueConstraint = "user_links_pkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); + UniqueUserSecretsPkey UniqueConstraint = "user_secrets_pkey" // ALTER TABLE ONLY user_secrets ADD CONSTRAINT user_secrets_pkey PRIMARY KEY (id); UniqueUserStatusChangesPkey UniqueConstraint = "user_status_changes_pkey" // ALTER TABLE ONLY user_status_changes ADD CONSTRAINT user_status_changes_pkey PRIMARY KEY (id); UniqueUsersPkey UniqueConstraint = "users_pkey" // ALTER TABLE ONLY users ADD CONSTRAINT users_pkey PRIMARY KEY (id); UniqueWebpushSubscriptionsPkey UniqueConstraint = "webpush_subscriptions_pkey" // ALTER TABLE ONLY webpush_subscriptions ADD CONSTRAINT webpush_subscriptions_pkey PRIMARY KEY (id); @@ -115,6 +116,9 @@ const ( UniqueTemplateUsageStatsStartTimeTemplateIDUserIDIndex UniqueConstraint = "template_usage_stats_start_time_template_id_user_id_idx" // CREATE UNIQUE INDEX template_usage_stats_start_time_template_id_user_id_idx ON template_usage_stats USING btree (start_time, template_id, user_id); UniqueTemplatesOrganizationIDNameIndex UniqueConstraint = "templates_organization_id_name_idx" // CREATE UNIQUE INDEX templates_organization_id_name_idx ON templates USING btree (organization_id, lower((name)::text)) WHERE (deleted = false); UniqueUserLinksLinkedIDLoginTypeIndex UniqueConstraint = "user_links_linked_id_login_type_idx" // CREATE UNIQUE INDEX user_links_linked_id_login_type_idx ON user_links USING btree (linked_id, login_type) WHERE (linked_id <> ''::text); + UniqueUserSecretsUserEnvNameIndex UniqueConstraint = "user_secrets_user_env_name_idx" // CREATE UNIQUE INDEX user_secrets_user_env_name_idx ON user_secrets USING btree (user_id, env_name) WHERE (env_name <> ''::text); + UniqueUserSecretsUserFilePathIndex UniqueConstraint = "user_secrets_user_file_path_idx" // CREATE UNIQUE INDEX user_secrets_user_file_path_idx ON user_secrets USING btree (user_id, file_path) WHERE (file_path <> ''::text); + UniqueUserSecretsUserNameIndex UniqueConstraint = "user_secrets_user_name_idx" // CREATE UNIQUE INDEX user_secrets_user_name_idx ON user_secrets USING btree (user_id, name); UniqueUsersEmailLowerIndex UniqueConstraint = "users_email_lower_idx" // CREATE UNIQUE INDEX users_email_lower_idx ON users USING btree (lower(email)) WHERE (deleted = false); UniqueUsersUsernameLowerIndex UniqueConstraint = "users_username_lower_idx" // CREATE UNIQUE INDEX users_username_lower_idx ON users USING btree (lower(username)) WHERE (deleted = false); UniqueWorkspaceAppAuditSessionsUniqueIndex UniqueConstraint = "workspace_app_audit_sessions_unique_index" // CREATE UNIQUE INDEX workspace_app_audit_sessions_unique_index ON workspace_app_audit_sessions USING btree (agent_id, app_id, user_id, ip, user_agent, slug_or_port, status_code); From b67727338e79823bccb79bbf548730c3bad90fd8 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Mon, 4 Aug 2025 20:04:47 +0000 Subject: [PATCH 02/21] feat: add SQL queries --- coderd/database/dbauthz/dbauthz.go | 24 +++ coderd/database/dbmetrics/querymetrics.go | 42 +++++ coderd/database/dbmock/dbmock.go | 89 ++++++++++ coderd/database/querier.go | 6 + coderd/database/queries.sql.go | 187 ++++++++++++++++++++++ coderd/database/queries/user_secrets.sql | 39 +++++ 6 files changed, 387 insertions(+) create mode 100644 coderd/database/queries/user_secrets.sql diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 402097f13deae..7dc6fd7de2e5d 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1387,6 +1387,10 @@ func (q *querier) CountUnreadInboxNotificationsByUserID(ctx context.Context, use return q.db.CountUnreadInboxNotificationsByUserID(ctx, userID) } +func (q *querier) CreateUserSecret(ctx context.Context, arg database.CreateUserSecretParams) (database.UserSecret, error) { + panic("not implemented") +} + // TODO: Handle org scoped lookups func (q *querier) CustomRoles(ctx context.Context, arg database.CustomRolesParams) ([]database.CustomRole, error) { roleObject := rbac.ResourceAssignRole @@ -1657,6 +1661,10 @@ func (q *querier) DeleteTailnetTunnel(ctx context.Context, arg database.DeleteTa return q.db.DeleteTailnetTunnel(ctx, arg) } +func (q *querier) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { + panic("not implemented") +} + func (q *querier) DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg database.DeleteWebpushSubscriptionByUserIDAndEndpointParams) error { if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceWebpushSubscription.WithOwner(arg.UserID.String())); err != nil { return err @@ -3075,6 +3083,14 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui return q.db.GetUserNotificationPreferences(ctx, userID) } +func (q *querier) GetUserSecret(ctx context.Context, id uuid.UUID) (database.UserSecret, error) { + panic("not implemented") +} + +func (q *querier) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) { + panic("not implemented") +} + func (q *querier) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUser); err != nil { return nil, err @@ -4158,6 +4174,10 @@ func (q *querier) ListProvisionerKeysByOrganizationExcludeReserved(ctx context.C return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.ListProvisionerKeysByOrganizationExcludeReserved)(ctx, organizationID) } +func (q *querier) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { + panic("not implemented") +} + func (q *querier) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]database.WorkspaceAgentPortShare, error) { workspace, err := q.db.GetWorkspaceByID(ctx, workspaceID) if err != nil { @@ -4871,6 +4891,10 @@ func (q *querier) UpdateUserRoles(ctx context.Context, arg database.UpdateUserRo return q.db.UpdateUserRoles(ctx, arg) } +func (q *querier) UpdateUserSecret(ctx context.Context, arg database.UpdateUserSecretParams) (database.UserSecret, error) { + panic("not implemented") +} + func (q *querier) UpdateUserStatus(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { fetch := func(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { return q.db.GetUserByID(ctx, arg.ID) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 574eeb069e47f..c061f91724451 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -215,6 +215,13 @@ func (m queryMetricsStore) CountUnreadInboxNotificationsByUserID(ctx context.Con return r0, r1 } +func (m queryMetricsStore) CreateUserSecret(ctx context.Context, arg database.CreateUserSecretParams) (database.UserSecret, error) { + start := time.Now() + r0, r1 := m.s.CreateUserSecret(ctx, arg) + m.queryLatencies.WithLabelValues("CreateUserSecret").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) CustomRoles(ctx context.Context, arg database.CustomRolesParams) ([]database.CustomRole, error) { start := time.Now() r0, r1 := m.s.CustomRoles(ctx, arg) @@ -460,6 +467,13 @@ func (m queryMetricsStore) DeleteTailnetTunnel(ctx context.Context, arg database return r0, r1 } +func (m queryMetricsStore) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { + start := time.Now() + r0 := m.s.DeleteUserSecret(ctx, id) + m.queryLatencies.WithLabelValues("DeleteUserSecret").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg database.DeleteWebpushSubscriptionByUserIDAndEndpointParams) error { start := time.Now() r0 := m.s.DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx, arg) @@ -1657,6 +1671,20 @@ func (m queryMetricsStore) GetUserNotificationPreferences(ctx context.Context, u return r0, r1 } +func (m queryMetricsStore) GetUserSecret(ctx context.Context, id uuid.UUID) (database.UserSecret, error) { + start := time.Now() + r0, r1 := m.s.GetUserSecret(ctx, id) + m.queryLatencies.WithLabelValues("GetUserSecret").Observe(time.Since(start).Seconds()) + return r0, r1 +} + +func (m queryMetricsStore) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) { + start := time.Now() + r0, r1 := m.s.GetUserSecretByUserIDAndName(ctx, arg) + m.queryLatencies.WithLabelValues("GetUserSecretByUserIDAndName").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { start := time.Now() r0, r1 := m.s.GetUserStatusCounts(ctx, arg) @@ -2546,6 +2574,13 @@ func (m queryMetricsStore) ListProvisionerKeysByOrganizationExcludeReserved(ctx return r0, r1 } +func (m queryMetricsStore) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { + start := time.Now() + r0, r1 := m.s.ListUserSecrets(ctx, userID) + m.queryLatencies.WithLabelValues("ListUserSecrets").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]database.WorkspaceAgentPortShare, error) { start := time.Now() r0, r1 := m.s.ListWorkspaceAgentPortShares(ctx, workspaceID) @@ -2994,6 +3029,13 @@ func (m queryMetricsStore) UpdateUserRoles(ctx context.Context, arg database.Upd return user, err } +func (m queryMetricsStore) UpdateUserSecret(ctx context.Context, arg database.UpdateUserSecretParams) (database.UserSecret, error) { + start := time.Now() + r0, r1 := m.s.UpdateUserSecret(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateUserSecret").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) UpdateUserStatus(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { start := time.Now() user, err := m.s.UpdateUserStatus(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 30589c9fbb8bf..d4ec6b04086de 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -338,6 +338,21 @@ func (mr *MockStoreMockRecorder) CountUnreadInboxNotificationsByUserID(ctx, user return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CountUnreadInboxNotificationsByUserID", reflect.TypeOf((*MockStore)(nil).CountUnreadInboxNotificationsByUserID), ctx, userID) } +// CreateUserSecret mocks base method. +func (m *MockStore) CreateUserSecret(ctx context.Context, arg database.CreateUserSecretParams) (database.UserSecret, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateUserSecret", ctx, arg) + ret0, _ := ret[0].(database.UserSecret) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateUserSecret indicates an expected call of CreateUserSecret. +func (mr *MockStoreMockRecorder) CreateUserSecret(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateUserSecret", reflect.TypeOf((*MockStore)(nil).CreateUserSecret), ctx, arg) +} + // CustomRoles mocks base method. func (m *MockStore) CustomRoles(ctx context.Context, arg database.CustomRolesParams) ([]database.CustomRole, error) { m.ctrl.T.Helper() @@ -835,6 +850,20 @@ func (mr *MockStoreMockRecorder) DeleteTailnetTunnel(ctx, arg any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteTailnetTunnel", reflect.TypeOf((*MockStore)(nil).DeleteTailnetTunnel), ctx, arg) } +// DeleteUserSecret mocks base method. +func (m *MockStore) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteUserSecret", ctx, id) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteUserSecret indicates an expected call of DeleteUserSecret. +func (mr *MockStoreMockRecorder) DeleteUserSecret(ctx, id any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteUserSecret", reflect.TypeOf((*MockStore)(nil).DeleteUserSecret), ctx, id) +} + // DeleteWebpushSubscriptionByUserIDAndEndpoint mocks base method. func (m *MockStore) DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg database.DeleteWebpushSubscriptionByUserIDAndEndpointParams) error { m.ctrl.T.Helper() @@ -3527,6 +3556,36 @@ func (mr *MockStoreMockRecorder) GetUserNotificationPreferences(ctx, userID any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserNotificationPreferences", reflect.TypeOf((*MockStore)(nil).GetUserNotificationPreferences), ctx, userID) } +// GetUserSecret mocks base method. +func (m *MockStore) GetUserSecret(ctx context.Context, id uuid.UUID) (database.UserSecret, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserSecret", ctx, id) + ret0, _ := ret[0].(database.UserSecret) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserSecret indicates an expected call of GetUserSecret. +func (mr *MockStoreMockRecorder) GetUserSecret(ctx, id any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserSecret", reflect.TypeOf((*MockStore)(nil).GetUserSecret), ctx, id) +} + +// GetUserSecretByUserIDAndName mocks base method. +func (m *MockStore) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserSecretByUserIDAndName", ctx, arg) + ret0, _ := ret[0].(database.UserSecret) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserSecretByUserIDAndName indicates an expected call of GetUserSecretByUserIDAndName. +func (mr *MockStoreMockRecorder) GetUserSecretByUserIDAndName(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserSecretByUserIDAndName", reflect.TypeOf((*MockStore)(nil).GetUserSecretByUserIDAndName), ctx, arg) +} + // GetUserStatusCounts mocks base method. func (m *MockStore) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { m.ctrl.T.Helper() @@ -5432,6 +5491,21 @@ func (mr *MockStoreMockRecorder) ListProvisionerKeysByOrganizationExcludeReserve return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListProvisionerKeysByOrganizationExcludeReserved", reflect.TypeOf((*MockStore)(nil).ListProvisionerKeysByOrganizationExcludeReserved), ctx, organizationID) } +// ListUserSecrets mocks base method. +func (m *MockStore) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListUserSecrets", ctx, userID) + ret0, _ := ret[0].([]database.UserSecret) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListUserSecrets indicates an expected call of ListUserSecrets. +func (mr *MockStoreMockRecorder) ListUserSecrets(ctx, userID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListUserSecrets", reflect.TypeOf((*MockStore)(nil).ListUserSecrets), ctx, userID) +} + // ListWorkspaceAgentPortShares mocks base method. func (m *MockStore) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]database.WorkspaceAgentPortShare, error) { m.ctrl.T.Helper() @@ -6387,6 +6461,21 @@ func (mr *MockStoreMockRecorder) UpdateUserRoles(ctx, arg any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserRoles", reflect.TypeOf((*MockStore)(nil).UpdateUserRoles), ctx, arg) } +// UpdateUserSecret mocks base method. +func (m *MockStore) UpdateUserSecret(ctx context.Context, arg database.UpdateUserSecretParams) (database.UserSecret, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateUserSecret", ctx, arg) + ret0, _ := ret[0].(database.UserSecret) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateUserSecret indicates an expected call of UpdateUserSecret. +func (mr *MockStoreMockRecorder) UpdateUserSecret(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserSecret", reflect.TypeOf((*MockStore)(nil).UpdateUserSecret), ctx, arg) +} + // UpdateUserStatus mocks base method. func (m *MockStore) UpdateUserStatus(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index d812ff1a96de9..4f733637a3e34 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -71,6 +71,7 @@ type sqlcQuerier interface { // Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state. CountInProgressPrebuilds(ctx context.Context) ([]CountInProgressPrebuildsRow, error) CountUnreadInboxNotificationsByUserID(ctx context.Context, userID uuid.UUID) (int64, error) + CreateUserSecret(ctx context.Context, arg CreateUserSecretParams) (UserSecret, error) CustomRoles(ctx context.Context, arg CustomRolesParams) ([]CustomRole, error) DeleteAPIKeyByID(ctx context.Context, id string) error DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error @@ -118,6 +119,7 @@ type sqlcQuerier interface { DeleteTailnetClientSubscription(ctx context.Context, arg DeleteTailnetClientSubscriptionParams) error DeleteTailnetPeer(ctx context.Context, arg DeleteTailnetPeerParams) (DeleteTailnetPeerRow, error) DeleteTailnetTunnel(ctx context.Context, arg DeleteTailnetTunnelParams) (DeleteTailnetTunnelRow, error) + DeleteUserSecret(ctx context.Context, id uuid.UUID) error DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg DeleteWebpushSubscriptionByUserIDAndEndpointParams) error DeleteWebpushSubscriptions(ctx context.Context, ids []uuid.UUID) error DeleteWorkspaceAgentPortShare(ctx context.Context, arg DeleteWorkspaceAgentPortShareParams) error @@ -383,6 +385,8 @@ type sqlcQuerier interface { GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) + GetUserSecret(ctx context.Context, id uuid.UUID) (UserSecret, error) + GetUserSecretByUserIDAndName(ctx context.Context, arg GetUserSecretByUserIDAndNameParams) (UserSecret, error) // GetUserStatusCounts returns the count of users in each status over time. // The time range is inclusively defined by the start_time and end_time parameters. // @@ -548,6 +552,7 @@ type sqlcQuerier interface { InsertWorkspaceResourceMetadata(ctx context.Context, arg InsertWorkspaceResourceMetadataParams) ([]WorkspaceResourceMetadatum, error) ListProvisionerKeysByOrganization(ctx context.Context, organizationID uuid.UUID) ([]ProvisionerKey, error) ListProvisionerKeysByOrganizationExcludeReserved(ctx context.Context, organizationID uuid.UUID) ([]ProvisionerKey, error) + ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]UserSecret, error) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]WorkspaceAgentPortShare, error) MarkAllInboxNotificationsAsRead(ctx context.Context, arg MarkAllInboxNotificationsAsReadParams) error OIDCClaimFieldValues(ctx context.Context, arg OIDCClaimFieldValuesParams) ([]string, error) @@ -623,6 +628,7 @@ type sqlcQuerier interface { UpdateUserProfile(ctx context.Context, arg UpdateUserProfileParams) (User, error) UpdateUserQuietHoursSchedule(ctx context.Context, arg UpdateUserQuietHoursScheduleParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) + UpdateUserSecret(ctx context.Context, arg UpdateUserSecretParams) (UserSecret, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) UpdateUserTerminalFont(ctx context.Context, arg UpdateUserTerminalFontParams) (UserConfig, error) UpdateUserThemePreference(ctx context.Context, arg UpdateUserThemePreferenceParams) (UserConfig, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a7b61d6eabd50..5a14dcb78e37b 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -13771,6 +13771,193 @@ func (q *sqlQuerier) UpdateUserLinkedID(ctx context.Context, arg UpdateUserLinke return i, err } +const createUserSecret = `-- name: CreateUserSecret :one +INSERT INTO user_secrets ( + user_id, + name, + description, + value, + env_name, + file_path +) VALUES ( + $1, $2, $3, $4, $5, $6 +) RETURNING id, user_id, name, description, value, env_name, file_path, created_at, updated_at +` + +type CreateUserSecretParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + Name string `db:"name" json:"name"` + Description string `db:"description" json:"description"` + Value string `db:"value" json:"value"` + EnvName string `db:"env_name" json:"env_name"` + FilePath string `db:"file_path" json:"file_path"` +} + +func (q *sqlQuerier) CreateUserSecret(ctx context.Context, arg CreateUserSecretParams) (UserSecret, error) { + row := q.db.QueryRowContext(ctx, createUserSecret, + arg.UserID, + arg.Name, + arg.Description, + arg.Value, + arg.EnvName, + arg.FilePath, + ) + var i UserSecret + err := row.Scan( + &i.ID, + &i.UserID, + &i.Name, + &i.Description, + &i.Value, + &i.EnvName, + &i.FilePath, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const deleteUserSecret = `-- name: DeleteUserSecret :exec +DELETE FROM user_secrets +WHERE id = $1 +` + +func (q *sqlQuerier) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { + _, err := q.db.ExecContext(ctx, deleteUserSecret, id) + return err +} + +const getUserSecret = `-- name: GetUserSecret :one +SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at FROM user_secrets +WHERE id = $1 +` + +func (q *sqlQuerier) GetUserSecret(ctx context.Context, id uuid.UUID) (UserSecret, error) { + row := q.db.QueryRowContext(ctx, getUserSecret, id) + var i UserSecret + err := row.Scan( + &i.ID, + &i.UserID, + &i.Name, + &i.Description, + &i.Value, + &i.EnvName, + &i.FilePath, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const getUserSecretByUserIDAndName = `-- name: GetUserSecretByUserIDAndName :one +SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at FROM user_secrets +WHERE user_id = $1 AND name = $2 +` + +type GetUserSecretByUserIDAndNameParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + Name string `db:"name" json:"name"` +} + +func (q *sqlQuerier) GetUserSecretByUserIDAndName(ctx context.Context, arg GetUserSecretByUserIDAndNameParams) (UserSecret, error) { + row := q.db.QueryRowContext(ctx, getUserSecretByUserIDAndName, arg.UserID, arg.Name) + var i UserSecret + err := row.Scan( + &i.ID, + &i.UserID, + &i.Name, + &i.Description, + &i.Value, + &i.EnvName, + &i.FilePath, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const listUserSecrets = `-- name: ListUserSecrets :many +SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at FROM user_secrets +WHERE user_id = $1 +ORDER BY name ASC +` + +func (q *sqlQuerier) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]UserSecret, error) { + rows, err := q.db.QueryContext(ctx, listUserSecrets, userID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []UserSecret + for rows.Next() { + var i UserSecret + if err := rows.Scan( + &i.ID, + &i.UserID, + &i.Name, + &i.Description, + &i.Value, + &i.EnvName, + &i.FilePath, + &i.CreatedAt, + &i.UpdatedAt, + ); 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 updateUserSecret = `-- name: UpdateUserSecret :one +UPDATE user_secrets +SET + description = $2, + value = $3, + env_name = $4, + file_path = $5, + updated_at = CURRENT_TIMESTAMP +WHERE id = $1 +RETURNING id, user_id, name, description, value, env_name, file_path, created_at, updated_at +` + +type UpdateUserSecretParams struct { + ID uuid.UUID `db:"id" json:"id"` + Description string `db:"description" json:"description"` + Value string `db:"value" json:"value"` + EnvName string `db:"env_name" json:"env_name"` + FilePath string `db:"file_path" json:"file_path"` +} + +func (q *sqlQuerier) UpdateUserSecret(ctx context.Context, arg UpdateUserSecretParams) (UserSecret, error) { + row := q.db.QueryRowContext(ctx, updateUserSecret, + arg.ID, + arg.Description, + arg.Value, + arg.EnvName, + arg.FilePath, + ) + var i UserSecret + err := row.Scan( + &i.ID, + &i.UserID, + &i.Name, + &i.Description, + &i.Value, + &i.EnvName, + &i.FilePath, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + const allUserIDs = `-- name: AllUserIDs :many SELECT DISTINCT id FROM USERS WHERE CASE WHEN $1::bool THEN TRUE ELSE is_system = false END diff --git a/coderd/database/queries/user_secrets.sql b/coderd/database/queries/user_secrets.sql new file mode 100644 index 0000000000000..8180c88ca61c4 --- /dev/null +++ b/coderd/database/queries/user_secrets.sql @@ -0,0 +1,39 @@ +-- name: GetUserSecretByUserIDAndName :one +SELECT * FROM user_secrets +WHERE user_id = $1 AND name = $2; + +-- name: GetUserSecret :one +SELECT * FROM user_secrets +WHERE id = $1; + +-- name: ListUserSecrets :many +SELECT * FROM user_secrets +WHERE user_id = $1 +ORDER BY name ASC; + +-- name: CreateUserSecret :one +INSERT INTO user_secrets ( + user_id, + name, + description, + value, + env_name, + file_path +) VALUES ( + $1, $2, $3, $4, $5, $6 +) RETURNING *; + +-- name: UpdateUserSecret :one +UPDATE user_secrets +SET + description = $2, + value = $3, + env_name = $4, + file_path = $5, + updated_at = CURRENT_TIMESTAMP +WHERE id = $1 +RETURNING *; + +-- name: DeleteUserSecret :exec +DELETE FROM user_secrets +WHERE id = $1; From a826dd3a3a9c3c682c0237885520656a0783704f Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 5 Aug 2025 12:34:25 +0000 Subject: [PATCH 03/21] feat: add DBAuthz wrappers and UserSecret resource type --- coderd/apidoc/docs.go | 2 + coderd/apidoc/swagger.json | 2 + coderd/database/dbauthz/dbauthz.go | 125 +++++++++++++++++-------- coderd/rbac/object_gen.go | 11 +++ coderd/rbac/policy/policy.go | 8 ++ codersdk/rbacresources_gen.go | 2 + docs/reference/api/members.md | 5 + docs/reference/api/schemas.md | 1 + site/src/api/rbacresourcesGenerated.ts | 6 ++ site/src/api/typesGenerated.ts | 2 + 10 files changed, 123 insertions(+), 41 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index fa2aad745ec5a..0a8b2c07793c3 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -15702,6 +15702,7 @@ const docTemplate = `{ "tailnet_coordinator", "template", "user", + "user_secret", "webpush_subscription", "workspace", "workspace_agent_devcontainers", @@ -15742,6 +15743,7 @@ const docTemplate = `{ "ResourceTailnetCoordinator", "ResourceTemplate", "ResourceUser", + "ResourceUserSecret", "ResourceWebpushSubscription", "ResourceWorkspace", "ResourceWorkspaceAgentDevcontainers", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index e1bcc5bf1013c..cd6537de0e210 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -14263,6 +14263,7 @@ "tailnet_coordinator", "template", "user", + "user_secret", "webpush_subscription", "workspace", "workspace_agent_devcontainers", @@ -14303,6 +14304,7 @@ "ResourceTailnetCoordinator", "ResourceTemplate", "ResourceUser", + "ResourceUserSecret", "ResourceWebpushSubscription", "ResourceWorkspace", "ResourceWorkspaceAgentDevcontainers", diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 7dc6fd7de2e5d..c11a3db5a6349 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -612,9 +612,9 @@ func As(ctx context.Context, actor rbac.Subject) context.Context { // running the insertFunc. The insertFunc is expected to return the object that // was inserted. func insert[ - ObjectType any, - ArgumentType any, - Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ObjectType any, +ArgumentType any, +Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -625,9 +625,9 @@ func insert[ } func insertWithAction[ - ObjectType any, - ArgumentType any, - Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ObjectType any, +ArgumentType any, +Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -654,10 +654,10 @@ func insertWithAction[ } func deleteQ[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - Delete func(ctx context.Context, arg ArgumentType) error, +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +Delete func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -669,10 +669,10 @@ func deleteQ[ } func updateWithReturn[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -683,10 +683,10 @@ func updateWithReturn[ } func update[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - Exec func(ctx context.Context, arg ArgumentType) error, +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +Exec func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -704,9 +704,9 @@ func update[ // user cannot read the resource. This is because the resource details are // required to run a proper authorization check. func fetchWithAction[ - ArgumentType any, - ObjectType rbac.Objecter, - DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ArgumentType any, +ObjectType rbac.Objecter, +DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -737,9 +737,9 @@ func fetchWithAction[ } func fetch[ - ArgumentType any, - ObjectType rbac.Objecter, - DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ArgumentType any, +ObjectType rbac.Objecter, +DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -752,10 +752,10 @@ func fetch[ // from SQL 'exec' functions which only return an error. // See fetchAndQuery for more information. func fetchAndExec[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - Exec func(ctx context.Context, arg ArgumentType) error, +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +Exec func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -778,10 +778,10 @@ func fetchAndExec[ // **before** the query runs. The returns from the fetch are only used to // assert rbac. The final return of this function comes from the Query function. func fetchAndQuery[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -815,9 +815,9 @@ func fetchAndQuery[ // fetchWithPostFilter is like fetch, but works with lists of objects. // SQL filters are much more optimal. func fetchWithPostFilter[ - ArgumentType any, - ObjectType rbac.Objecter, - DatabaseFunc func(ctx context.Context, arg ArgumentType) ([]ObjectType, error), +ArgumentType any, +ObjectType rbac.Objecter, +DatabaseFunc func(ctx context.Context, arg ArgumentType) ([]ObjectType, error), ]( authorizer rbac.Authorizer, action policy.Action, @@ -1388,7 +1388,11 @@ func (q *querier) CountUnreadInboxNotificationsByUserID(ctx context.Context, use } func (q *querier) CreateUserSecret(ctx context.Context, arg database.CreateUserSecretParams) (database.UserSecret, error) { - panic("not implemented") + obj := rbac.ResourceUserSecret.WithOwner(arg.UserID.String()) + if err := q.authorizeContext(ctx, policy.ActionCreate, obj); err != nil { + return database.UserSecret{}, err + } + return q.db.CreateUserSecret(ctx, arg) } // TODO: Handle org scoped lookups @@ -1662,7 +1666,17 @@ func (q *querier) DeleteTailnetTunnel(ctx context.Context, arg database.DeleteTa } func (q *querier) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { - panic("not implemented") + // First get the secret to check ownership + secret, err := q.db.GetUserSecret(ctx, id) + if err != nil { + return err + } + + obj := rbac.ResourceUserSecret.WithOwner(secret.UserID.String()) + if err := q.authorizeContext(ctx, policy.ActionDelete, obj); err != nil { + return err + } + return q.db.DeleteUserSecret(ctx, id) } func (q *querier) DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg database.DeleteWebpushSubscriptionByUserIDAndEndpointParams) error { @@ -3084,11 +3098,26 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui } func (q *querier) GetUserSecret(ctx context.Context, id uuid.UUID) (database.UserSecret, error) { - panic("not implemented") + // First get the secret to check ownership + secret, err := q.db.GetUserSecret(ctx, id) + if err != nil { + return database.UserSecret{}, err + } + + obj := rbac.ResourceUserSecret.WithOwner(secret.UserID.String()) + if err := q.authorizeContext(ctx, policy.ActionRead, obj); err != nil { + return database.UserSecret{}, err + } + return secret, nil } func (q *querier) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) { - panic("not implemented") + obj := rbac.ResourceUserSecret.WithOwner(arg.UserID.String()) + if err := q.authorizeContext(ctx, policy.ActionRead, obj); err != nil { + return database.UserSecret{}, err + } + + return q.db.GetUserSecretByUserIDAndName(ctx, arg) } func (q *querier) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { @@ -4175,7 +4204,11 @@ func (q *querier) ListProvisionerKeysByOrganizationExcludeReserved(ctx context.C } func (q *querier) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { - panic("not implemented") + obj := rbac.ResourceUserSecret.WithOwner(userID.String()) + if err := q.authorizeContext(ctx, policy.ActionRead, obj); err != nil { + return nil, err + } + return q.db.ListUserSecrets(ctx, userID) } func (q *querier) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]database.WorkspaceAgentPortShare, error) { @@ -4892,7 +4925,17 @@ func (q *querier) UpdateUserRoles(ctx context.Context, arg database.UpdateUserRo } func (q *querier) UpdateUserSecret(ctx context.Context, arg database.UpdateUserSecretParams) (database.UserSecret, error) { - panic("not implemented") + // First get the secret to check ownership + secret, err := q.db.GetUserSecret(ctx, arg.ID) + if err != nil { + return database.UserSecret{}, err + } + + obj := rbac.ResourceUserSecret.WithOwner(secret.UserID.String()) + if err := q.authorizeContext(ctx, policy.ActionUpdate, obj); err != nil { + return database.UserSecret{}, err + } + return q.db.UpdateUserSecret(ctx, arg) } func (q *querier) UpdateUserStatus(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 5fb3cc2bd8a3b..ca7f23b4af280 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -301,6 +301,16 @@ var ( Type: "user", } + // ResourceUserSecret + // Valid Actions + // - "ActionCreate" :: create a user secret + // - "ActionDelete" :: delete a user secret + // - "ActionRead" :: read user secret metadata and value + // - "ActionUpdate" :: update user secret metadata and value + ResourceUserSecret = Object{ + Type: "user_secret", + } + // ResourceWebpushSubscription // Valid Actions // - "ActionCreate" :: create webpush subscriptions @@ -403,6 +413,7 @@ func AllResources() []Objecter { ResourceTailnetCoordinator, ResourceTemplate, ResourceUser, + ResourceUserSecret, ResourceWebpushSubscription, ResourceWorkspace, ResourceWorkspaceAgentDevcontainers, diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 8f05bbdbe544f..5ba79c6434d44 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -343,4 +343,12 @@ var RBACPermissions = map[string]PermissionDefinition{ ActionCreate: "create workspace agent devcontainers", }, }, + "user_secret": { + Actions: map[Action]ActionDefinition{ + ActionCreate: "create a user secret", + ActionRead: "read user secret metadata and value", + ActionUpdate: "update user secret metadata and value", + ActionDelete: "delete a user secret", + }, + }, } diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index 3e22d29c73297..9dd2056b781b4 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -36,6 +36,7 @@ const ( ResourceTailnetCoordinator RBACResource = "tailnet_coordinator" ResourceTemplate RBACResource = "template" ResourceUser RBACResource = "user" + ResourceUserSecret RBACResource = "user_secret" ResourceWebpushSubscription RBACResource = "webpush_subscription" ResourceWorkspace RBACResource = "workspace" ResourceWorkspaceAgentDevcontainers RBACResource = "workspace_agent_devcontainers" @@ -100,6 +101,7 @@ var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceTailnetCoordinator: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceTemplate: {ActionCreate, ActionDelete, ActionRead, ActionUpdate, ActionUse, ActionViewInsights}, ResourceUser: {ActionCreate, ActionDelete, ActionRead, ActionReadPersonal, ActionUpdate, ActionUpdatePersonal}, + ResourceUserSecret: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceWebpushSubscription: {ActionCreate, ActionDelete, ActionRead}, ResourceWorkspace: {ActionApplicationConnect, ActionCreate, ActionCreateAgent, ActionDelete, ActionDeleteAgent, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, ResourceWorkspaceAgentDevcontainers: {ActionCreate}, diff --git a/docs/reference/api/members.md b/docs/reference/api/members.md index 4b0adbf45e338..0533da5114482 100644 --- a/docs/reference/api/members.md +++ b/docs/reference/api/members.md @@ -214,6 +214,7 @@ Status Code **200** | `resource_type` | `tailnet_coordinator` | | `resource_type` | `template` | | `resource_type` | `user` | +| `resource_type` | `user_secret` | | `resource_type` | `webpush_subscription` | | `resource_type` | `workspace` | | `resource_type` | `workspace_agent_devcontainers` | @@ -384,6 +385,7 @@ Status Code **200** | `resource_type` | `tailnet_coordinator` | | `resource_type` | `template` | | `resource_type` | `user` | +| `resource_type` | `user_secret` | | `resource_type` | `webpush_subscription` | | `resource_type` | `workspace` | | `resource_type` | `workspace_agent_devcontainers` | @@ -554,6 +556,7 @@ Status Code **200** | `resource_type` | `tailnet_coordinator` | | `resource_type` | `template` | | `resource_type` | `user` | +| `resource_type` | `user_secret` | | `resource_type` | `webpush_subscription` | | `resource_type` | `workspace` | | `resource_type` | `workspace_agent_devcontainers` | @@ -693,6 +696,7 @@ Status Code **200** | `resource_type` | `tailnet_coordinator` | | `resource_type` | `template` | | `resource_type` | `user` | +| `resource_type` | `user_secret` | | `resource_type` | `webpush_subscription` | | `resource_type` | `workspace` | | `resource_type` | `workspace_agent_devcontainers` | @@ -1054,6 +1058,7 @@ Status Code **200** | `resource_type` | `tailnet_coordinator` | | `resource_type` | `template` | | `resource_type` | `user` | +| `resource_type` | `user_secret` | | `resource_type` | `webpush_subscription` | | `resource_type` | `workspace` | | `resource_type` | `workspace_agent_devcontainers` | diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 581743ea7cc22..b3824d0c9b9b8 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -6379,6 +6379,7 @@ Only certain features set these fields: - FeatureManagedAgentLimit| | `tailnet_coordinator` | | `template` | | `user` | +| `user_secret` | | `webpush_subscription` | | `workspace` | | `workspace_agent_devcontainers` | diff --git a/site/src/api/rbacresourcesGenerated.ts b/site/src/api/rbacresourcesGenerated.ts index 5d632d57fad95..33ff18e8ce4d6 100644 --- a/site/src/api/rbacresourcesGenerated.ts +++ b/site/src/api/rbacresourcesGenerated.ts @@ -167,6 +167,12 @@ export const RBACResourceActions: Partial< update: "update an existing user", update_personal: "update personal data", }, + user_secret: { + create: "create a user secret", + delete: "delete a user secret", + read: "read user secret metadata and value", + update: "update user secret metadata and value", + }, webpush_subscription: { create: "create webpush subscriptions", delete: "delete webpush subscriptions", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index db901630b71cf..52fdb1d6effc4 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2386,6 +2386,7 @@ export type RBACResource = | "tailnet_coordinator" | "template" | "user" + | "user_secret" | "webpush_subscription" | "*" | "workspace" @@ -2426,6 +2427,7 @@ export const RBACResources: RBACResource[] = [ "tailnet_coordinator", "template", "user", + "user_secret", "webpush_subscription", "*", "workspace", From 540f2e2e81d331257f10bd555c89d5b02b1335a7 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 5 Aug 2025 16:15:35 +0000 Subject: [PATCH 04/21] feat: add DBAuthz tests --- coderd/database/dbauthz/dbauthz.go | 70 ++++++++++++------------ coderd/database/dbauthz/dbauthz_test.go | 61 +++++++++++++++++++++ coderd/database/dbgen/dbgen.go | 14 +++++ coderd/database/queries.sql.go | 5 +- coderd/database/queries/user_secrets.sql | 3 +- 5 files changed, 116 insertions(+), 37 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index c11a3db5a6349..d66d3ab3ac5ab 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -612,9 +612,9 @@ func As(ctx context.Context, actor rbac.Subject) context.Context { // running the insertFunc. The insertFunc is expected to return the object that // was inserted. func insert[ -ObjectType any, -ArgumentType any, -Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ObjectType any, + ArgumentType any, + Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -625,9 +625,9 @@ Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), } func insertWithAction[ -ObjectType any, -ArgumentType any, -Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ObjectType any, + ArgumentType any, + Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -654,10 +654,10 @@ Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), } func deleteQ[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -Delete func(ctx context.Context, arg ArgumentType) error, + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + Delete func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -669,10 +669,10 @@ Delete func(ctx context.Context, arg ArgumentType) error, } func updateWithReturn[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -683,10 +683,10 @@ UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), } func update[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -Exec func(ctx context.Context, arg ArgumentType) error, + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + Exec func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -704,9 +704,9 @@ Exec func(ctx context.Context, arg ArgumentType) error, // user cannot read the resource. This is because the resource details are // required to run a proper authorization check. func fetchWithAction[ -ArgumentType any, -ObjectType rbac.Objecter, -DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ArgumentType any, + ObjectType rbac.Objecter, + DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -737,9 +737,9 @@ DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), } func fetch[ -ArgumentType any, -ObjectType rbac.Objecter, -DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ArgumentType any, + ObjectType rbac.Objecter, + DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -752,10 +752,10 @@ DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), // from SQL 'exec' functions which only return an error. // See fetchAndQuery for more information. func fetchAndExec[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -Exec func(ctx context.Context, arg ArgumentType) error, + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + Exec func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -778,10 +778,10 @@ Exec func(ctx context.Context, arg ArgumentType) error, // **before** the query runs. The returns from the fetch are only used to // assert rbac. The final return of this function comes from the Query function. func fetchAndQuery[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -815,9 +815,9 @@ Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), // fetchWithPostFilter is like fetch, but works with lists of objects. // SQL filters are much more optimal. func fetchWithPostFilter[ -ArgumentType any, -ObjectType rbac.Objecter, -DatabaseFunc func(ctx context.Context, arg ArgumentType) ([]ObjectType, error), + ArgumentType any, + ObjectType rbac.Objecter, + DatabaseFunc func(ctx context.Context, arg ArgumentType) ([]ObjectType, error), ]( authorizer rbac.Authorizer, action policy.Action, diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 9e4f1f80fe05f..559948a202240 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -5877,3 +5877,64 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() { }).Asserts(w, policy.ActionUpdate, w.AsPrebuild(), policy.ActionUpdate) })) } + +func (s *MethodTestSuite) TestUserSecrets() { + s.Run("GetUserSecretByUserIDAndName", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + UserID: user.ID, + }) + arg := database.GetUserSecretByUserIDAndNameParams{ + UserID: user.ID, + Name: userSecret.Name, + } + check.Args(arg). + Asserts(rbac.ResourceUserSecret.WithOwner(arg.UserID.String()), policy.ActionRead). + Returns(userSecret) + })) + s.Run("GetUserSecret", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + UserID: user.ID, + }) + check.Args(userSecret.ID). + Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionRead). + Returns(userSecret) + })) + s.Run("ListUserSecrets", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + UserID: user.ID, + }) + check.Args(user.ID). + Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionRead). + Returns([]database.UserSecret{userSecret}) + })) + s.Run("CreateUserSecret", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + arg := database.CreateUserSecretParams{ + UserID: user.ID, + } + check.Args(arg). + Asserts(rbac.ResourceUserSecret.WithOwner(arg.UserID.String()), policy.ActionCreate) + })) + s.Run("UpdateUserSecret", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + UserID: user.ID, + }) + arg := database.UpdateUserSecretParams{ + ID: userSecret.ID, + } + check.Args(arg). + Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionUpdate) + })) + s.Run("DeleteUserSecret", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + UserID: user.ID, + }) + check.Args(userSecret.ID). + Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionDelete) + })) +} diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 81d9efd1cd3e3..34698e4471d3e 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -1422,6 +1422,20 @@ func PresetParameter(t testing.TB, db database.Store, seed database.InsertPreset return parameters } +func UserSecret(t testing.TB, db database.Store, seed database.CreateUserSecretParams) database.UserSecret { + userSecret, err := db.CreateUserSecret(genCtx, database.CreateUserSecretParams{ + ID: takeFirst(seed.ID, uuid.New()), + UserID: takeFirst(seed.UserID, uuid.New()), + Name: takeFirst(seed.Name, "secret-name"), + Description: takeFirst(seed.Description, "secret description"), + Value: takeFirst(seed.Value, "secret value"), + EnvName: takeFirst(seed.EnvName, "SECRET_ENV_NAME"), + FilePath: takeFirst(seed.FilePath, "~/secret/file/path"), + }) + require.NoError(t, err, "failed to insert user secret") + return userSecret +} + func ClaimPrebuild(t testing.TB, db database.Store, newUserID uuid.UUID, newName string, presetID uuid.UUID) database.ClaimPrebuiltWorkspaceRow { claimedWorkspace, err := db.ClaimPrebuiltWorkspace(genCtx, database.ClaimPrebuiltWorkspaceParams{ NewUserID: newUserID, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5a14dcb78e37b..f7beca56d4100 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -13773,6 +13773,7 @@ func (q *sqlQuerier) UpdateUserLinkedID(ctx context.Context, arg UpdateUserLinke const createUserSecret = `-- name: CreateUserSecret :one INSERT INTO user_secrets ( + id, user_id, name, description, @@ -13780,11 +13781,12 @@ INSERT INTO user_secrets ( env_name, file_path ) VALUES ( - $1, $2, $3, $4, $5, $6 + $1, $2, $3, $4, $5, $6, $7 ) RETURNING id, user_id, name, description, value, env_name, file_path, created_at, updated_at ` type CreateUserSecretParams struct { + ID uuid.UUID `db:"id" json:"id"` UserID uuid.UUID `db:"user_id" json:"user_id"` Name string `db:"name" json:"name"` Description string `db:"description" json:"description"` @@ -13795,6 +13797,7 @@ type CreateUserSecretParams struct { func (q *sqlQuerier) CreateUserSecret(ctx context.Context, arg CreateUserSecretParams) (UserSecret, error) { row := q.db.QueryRowContext(ctx, createUserSecret, + arg.ID, arg.UserID, arg.Name, arg.Description, diff --git a/coderd/database/queries/user_secrets.sql b/coderd/database/queries/user_secrets.sql index 8180c88ca61c4..303a5c096608e 100644 --- a/coderd/database/queries/user_secrets.sql +++ b/coderd/database/queries/user_secrets.sql @@ -13,6 +13,7 @@ ORDER BY name ASC; -- name: CreateUserSecret :one INSERT INTO user_secrets ( + id, user_id, name, description, @@ -20,7 +21,7 @@ INSERT INTO user_secrets ( env_name, file_path ) VALUES ( - $1, $2, $3, $4, $5, $6 + $1, $2, $3, $4, $5, $6, $7 ) RETURNING *; -- name: UpdateUserSecret :one From 363173c1139ebb5c40dfbdab84a1453a41ff6b0a Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 5 Aug 2025 19:24:03 +0000 Subject: [PATCH 05/21] test: fix role-permissions test --- coderd/rbac/roles.go | 12 +++++++++--- coderd/rbac/roles_test.go | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index b8d3f959ce477..fba6da0359515 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -269,8 +269,9 @@ func ReloadBuiltinRoles(opts *RoleOptions) { DisplayName: "Owner", Site: append( // Workspace dormancy and workspace are omitted. - // Workspace is specifically handled based on the opts.NoOwnerWorkspaceExec - allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace), + // Workspace is specifically handled based on the opts.NoOwnerWorkspaceExec. + // Owners cannot access other users' secrets. + allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUserSecret), // This adds back in the Workspace permissions. Permissions(map[string][]policy.Action{ ResourceWorkspace.Type: ownerWorkspaceActions, @@ -281,7 +282,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, })...), Org: map[string][]Permission{}, - User: []Permission{}, + User: Permissions(map[string][]policy.Action{ + // Users should be able to access their own secrets. + ResourceUserSecret.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, + }), }.withCachedRegoValue() memberRole := Role{ @@ -305,6 +309,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceOrganizationMember.Type: {policy.ActionRead}, // Users can create provisioner daemons scoped to themselves. ResourceProvisionerDaemon.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionRead, policy.ActionUpdate}, + // Users should be able to access their own secrets. + ResourceUserSecret.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, })..., ), }.withCachedRegoValue() diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 267a99993e642..f79a6408df79b 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -858,6 +858,20 @@ func TestRolePermissions(t *testing.T) { false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, }, }, + // Only the user themselves can access their own secrets — no one else. + { + Name: "UserSecrets", + Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + Resource: rbac.ResourceUserSecret.WithOwner(currentUser.String()), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {memberMe, orgMemberMe}, + false: { + owner, orgAdmin, + otherOrgAdmin, otherOrgMember, orgAuditor, orgUserAdmin, orgTemplateAdmin, + templateAdmin, userAdmin, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + }, + }, + }, } // We expect every permission to be tested above. From e152648699402e4cf914ba0ce37ea1a169dd3639 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 5 Aug 2025 19:51:18 +0000 Subject: [PATCH 06/21] test: fix migration test --- .../fixtures/000356_add_user_secrets.up.sql | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 coderd/database/migrations/testdata/fixtures/000356_add_user_secrets.up.sql diff --git a/coderd/database/migrations/testdata/fixtures/000356_add_user_secrets.up.sql b/coderd/database/migrations/testdata/fixtures/000356_add_user_secrets.up.sql new file mode 100644 index 0000000000000..bd1fb19333bc6 --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000356_add_user_secrets.up.sql @@ -0,0 +1,18 @@ +INSERT INTO user_secrets ( + id, + user_id, + name, + description, + value, + env_name, + file_path +) +VALUES ( + '4848b19e-b392-4a1b-bc7d-0b7ffb41ef87', + '30095c71-380b-457a-8995-97b8ee6e5307', + 'secret-name', + 'secret description', + 'secret value', + 'SECRET_ENV_NAME', + '~/secret/file/path' +); From 49ac807a9ad73e62f55a9d384ad33ea13453eff9 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 5 Aug 2025 20:34:33 +0000 Subject: [PATCH 07/21] fix: change user-secret permissions --- coderd/rbac/roles.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index fba6da0359515..ec804478d7c6d 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -309,8 +309,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceOrganizationMember.Type: {policy.ActionRead}, // Users can create provisioner daemons scoped to themselves. ResourceProvisionerDaemon.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionRead, policy.ActionUpdate}, - // Users should be able to access their own secrets. - ResourceUserSecret.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, })..., ), }.withCachedRegoValue() @@ -423,7 +421,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { }), Org: map[string][]Permission{ // Org admins should not have workspace exec perms. - organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceAssignRole), Permissions(map[string][]policy.Action{ + organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceAssignRole, ResourceUserSecret), Permissions(map[string][]policy.Action{ ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent}, ResourceWorkspace.Type: slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH), // PrebuiltWorkspaces are a subset of Workspaces. From 85b4f4c2534b281d670fb90381f45bc1ecd9fdbb Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 6 Aug 2025 17:06:45 +0000 Subject: [PATCH 08/21] fix: add unit-tests for sql queries --- coderd/database/querier_test.go | 222 ++++++++++++++++++++++++++++++++ 1 file changed, 222 insertions(+) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 9c88b9b3db679..883c7a990bfda 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -6003,3 +6003,225 @@ func TestGetRunningPrebuiltWorkspaces(t *testing.T) { require.Len(t, runningPrebuilds, 1, "expected only one running prebuilt workspace") require.Equal(t, runningPrebuild.ID, runningPrebuilds[0].ID, "expected the running prebuilt workspace to be returned") } + +func TestUserSecretsCRUDOperations(t *testing.T) { + t.Parallel() + + // Use raw database without dbauthz wrapper for this test + db, _ := dbtestutil.NewDB(t) + + t.Run("FullCRUDWorkflow", func(t *testing.T) { + t.Parallel() + + // Create a new user for this test + testUser := dbgen.User(t, db, database.User{}) + + // 1. CREATE + secretID := uuid.New() + createParams := database.CreateUserSecretParams{ + ID: secretID, + UserID: testUser.ID, + Name: "workflow-secret", + Description: "Secret for full CRUD workflow", + Value: "workflow-value", + EnvName: "WORKFLOW_ENV", + FilePath: "/workflow/path", + } + + createdSecret, err := db.CreateUserSecret(context.Background(), createParams) + require.NoError(t, err) + assert.Equal(t, secretID, createdSecret.ID) + + // 2. READ by ID + readSecret, err := db.GetUserSecret(context.Background(), createdSecret.ID) + require.NoError(t, err) + assert.Equal(t, createdSecret.ID, readSecret.ID) + assert.Equal(t, "workflow-secret", readSecret.Name) + + // 3. READ by UserID and Name + readByNameParams := database.GetUserSecretByUserIDAndNameParams{ + UserID: testUser.ID, + Name: "workflow-secret", + } + readByNameSecret, err := db.GetUserSecretByUserIDAndName(context.Background(), readByNameParams) + require.NoError(t, err) + assert.Equal(t, createdSecret.ID, readByNameSecret.ID) + + // 4. LIST + secrets, err := db.ListUserSecrets(context.Background(), testUser.ID) + require.NoError(t, err) + require.Len(t, secrets, 1) + assert.Equal(t, createdSecret.ID, secrets[0].ID) + + // 5. UPDATE + updateParams := database.UpdateUserSecretParams{ + ID: createdSecret.ID, + Description: "Updated workflow description", + Value: "updated-workflow-value", + EnvName: "UPDATED_WORKFLOW_ENV", + FilePath: "/updated/workflow/path", + } + + updatedSecret, err := db.UpdateUserSecret(context.Background(), updateParams) + require.NoError(t, err) + assert.Equal(t, "Updated workflow description", updatedSecret.Description) + assert.Equal(t, "updated-workflow-value", updatedSecret.Value) + + // 6. DELETE + err = db.DeleteUserSecret(context.Background(), createdSecret.ID) + require.NoError(t, err) + + // Verify deletion + _, err = db.GetUserSecret(context.Background(), createdSecret.ID) + require.Error(t, err) + assert.Contains(t, err.Error(), "no rows in result set") + + // Verify list is empty + secrets, err = db.ListUserSecrets(context.Background(), testUser.ID) + require.NoError(t, err) + assert.Len(t, secrets, 0) + }) + + t.Run("UniqueConstraints", func(t *testing.T) { + t.Parallel() + + // Create a new user for this test + testUser := dbgen.User(t, db, database.User{}) + + // Create first secret + secret1 := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ + UserID: testUser.ID, + Name: "unique-test", + Description: "First secret", + Value: "value1", + EnvName: "UNIQUE_ENV", + FilePath: "/unique/path", + }) + + // Try to create another secret with the same name (should fail) + _, err := db.CreateUserSecret(context.Background(), database.CreateUserSecretParams{ + UserID: testUser.ID, + Name: "unique-test", // Same name + Description: "Second secret", + Value: "value2", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "duplicate key value") + + // Try to create another secret with the same env_name (should fail) + _, err = db.CreateUserSecret(context.Background(), database.CreateUserSecretParams{ + UserID: testUser.ID, + Name: "unique-test-2", + Description: "Second secret", + Value: "value2", + EnvName: "UNIQUE_ENV", // Same env_name + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "duplicate key value") + + // Try to create another secret with the same file_path (should fail) + _, err = db.CreateUserSecret(context.Background(), database.CreateUserSecretParams{ + UserID: testUser.ID, + Name: "unique-test-3", + Description: "Second secret", + Value: "value2", + FilePath: "/unique/path", // Same file_path + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "duplicate key value") + + // Create secret with empty env_name and file_path (should succeed) + secret2 := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ + UserID: testUser.ID, + Name: "unique-test-4", + Description: "Second secret", + Value: "value2", + EnvName: "", // Empty env_name + FilePath: "", // Empty file_path + }) + + // Verify both secrets exist + _, err = db.GetUserSecret(context.Background(), secret1.ID) + require.NoError(t, err) + _, err = db.GetUserSecret(context.Background(), secret2.ID) + require.NoError(t, err) + }) +} + +func TestUserSecretsAuthorization(t *testing.T) { + t.Parallel() + + // Use raw database and wrap with dbauthz for authorization testing + db, _ := dbtestutil.NewDB(t) + authorizer := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry()) + authDB := dbauthz.New(db, authorizer, slogtest.Make(t, &slogtest.Options{}), coderdtest.AccessControlStorePointer()) + + // Create test users + user1 := dbgen.User(t, db, database.User{}) + user2 := dbgen.User(t, db, database.User{}) + owner := dbgen.User(t, db, database.User{}) + + // Create secrets for users + user1Secret := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ + UserID: user1.ID, + Name: "user1-secret", + Description: "User 1's secret", + Value: "user1-value", + }) + + user2Secret := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ + UserID: user2.ID, + Name: "user2-secret", + Description: "User 2's secret", + Value: "user2-value", + }) + + t.Run("UserCanAccessOwnSecrets", func(t *testing.T) { + t.Parallel() + + user1Subject := rbac.Subject{ + ID: user1.ID.String(), + Roles: rbac.RoleIdentifiers{rbac.RoleMember()}, + Scope: rbac.ScopeAll, + } + ctx := dbauthz.As(context.Background(), user1Subject) + + // Test GetUserSecret - should succeed + secret, err := authDB.GetUserSecret(ctx, user1Secret.ID) + require.NoError(t, err) + assert.Equal(t, user1Secret.ID, secret.ID) + assert.Equal(t, user1.ID, secret.UserID) + }) + + t.Run("UserCannotAccessOtherUserSecrets", func(t *testing.T) { + t.Parallel() + + user1Subject := rbac.Subject{ + ID: user1.ID.String(), + Roles: rbac.RoleIdentifiers{rbac.RoleMember()}, + Scope: rbac.ScopeAll, + } + ctx := dbauthz.As(context.Background(), user1Subject) + + // Test GetUserSecret - should fail + _, err := authDB.GetUserSecret(ctx, user2Secret.ID) + require.Error(t, err) + assert.True(t, dbauthz.IsNotAuthorizedError(err)) + }) + + t.Run("OwnerCannotAccessUserSecrets", func(t *testing.T) { + t.Parallel() + + ownerSubject := rbac.Subject{ + ID: owner.ID.String(), + Roles: rbac.RoleIdentifiers{rbac.RoleOwner()}, + Scope: rbac.ScopeAll, + } + ctx := dbauthz.As(context.Background(), ownerSubject) + + // Test GetUserSecret - should fail + _, err := authDB.GetUserSecret(ctx, user1Secret.ID) + require.Error(t, err) + assert.True(t, dbauthz.IsNotAuthorizedError(err)) + }) +} From 3f04a15875a2283a251a73a40a10b2b843f40da2 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 6 Aug 2025 18:53:50 +0000 Subject: [PATCH 09/21] test: improve unit-tests for sql queries --- coderd/database/querier_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 883c7a990bfda..786bb3673e8cb 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -6160,6 +6160,7 @@ func TestUserSecretsAuthorization(t *testing.T) { user1 := dbgen.User(t, db, database.User{}) user2 := dbgen.User(t, db, database.User{}) owner := dbgen.User(t, db, database.User{}) + orgAdmin := dbgen.User(t, db, database.User{}) // Create secrets for users user1Secret := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ @@ -6176,6 +6177,9 @@ func TestUserSecretsAuthorization(t *testing.T) { Value: "user2-value", }) + // Create organization for org-scoped roles + org := dbgen.Organization(t, db, database.Organization{}) + t.Run("UserCanAccessOwnSecrets", func(t *testing.T) { t.Parallel() @@ -6224,4 +6228,20 @@ func TestUserSecretsAuthorization(t *testing.T) { require.Error(t, err) assert.True(t, dbauthz.IsNotAuthorizedError(err)) }) + + t.Run("OrgAdminCannotAccessUserSecrets", func(t *testing.T) { + t.Parallel() + + orgAdminSubject := rbac.Subject{ + ID: orgAdmin.ID.String(), + Roles: rbac.RoleIdentifiers{rbac.ScopedRoleOrgAdmin(org.ID)}, + Scope: rbac.ScopeAll, + } + ctx := dbauthz.As(context.Background(), orgAdminSubject) + + // Test GetUserSecret - should fail + _, err := authDB.GetUserSecret(ctx, user1Secret.ID) + require.Error(t, err) + assert.True(t, dbauthz.IsNotAuthorizedError(err)) + }) } From d7605f62843b4704db7e398515600bb6b941fa1a Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 6 Aug 2025 19:08:44 +0000 Subject: [PATCH 10/21] refactor: use table-test approach for secrets-authz test --- coderd/database/querier_test.go | 128 ++++++++++++++++---------------- 1 file changed, 65 insertions(+), 63 deletions(-) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 786bb3673e8cb..3acf9542384ce 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -6162,6 +6162,9 @@ func TestUserSecretsAuthorization(t *testing.T) { owner := dbgen.User(t, db, database.User{}) orgAdmin := dbgen.User(t, db, database.User{}) + // Create organization for org-scoped roles + org := dbgen.Organization(t, db, database.Organization{}) + // Create secrets for users user1Secret := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ UserID: user1.ID, @@ -6177,71 +6180,70 @@ func TestUserSecretsAuthorization(t *testing.T) { Value: "user2-value", }) - // Create organization for org-scoped roles - org := dbgen.Organization(t, db, database.Organization{}) - - t.Run("UserCanAccessOwnSecrets", func(t *testing.T) { - t.Parallel() - - user1Subject := rbac.Subject{ - ID: user1.ID.String(), - Roles: rbac.RoleIdentifiers{rbac.RoleMember()}, - Scope: rbac.ScopeAll, - } - ctx := dbauthz.As(context.Background(), user1Subject) - - // Test GetUserSecret - should succeed - secret, err := authDB.GetUserSecret(ctx, user1Secret.ID) - require.NoError(t, err) - assert.Equal(t, user1Secret.ID, secret.ID) - assert.Equal(t, user1.ID, secret.UserID) - }) - - t.Run("UserCannotAccessOtherUserSecrets", func(t *testing.T) { - t.Parallel() - - user1Subject := rbac.Subject{ - ID: user1.ID.String(), - Roles: rbac.RoleIdentifiers{rbac.RoleMember()}, - Scope: rbac.ScopeAll, - } - ctx := dbauthz.As(context.Background(), user1Subject) - - // Test GetUserSecret - should fail - _, err := authDB.GetUserSecret(ctx, user2Secret.ID) - require.Error(t, err) - assert.True(t, dbauthz.IsNotAuthorizedError(err)) - }) - - t.Run("OwnerCannotAccessUserSecrets", func(t *testing.T) { - t.Parallel() - - ownerSubject := rbac.Subject{ - ID: owner.ID.String(), - Roles: rbac.RoleIdentifiers{rbac.RoleOwner()}, - Scope: rbac.ScopeAll, - } - ctx := dbauthz.As(context.Background(), ownerSubject) + testCases := []struct { + name string + subject rbac.Subject + secretID uuid.UUID + expectedAccess bool + }{ + { + name: "UserCanAccessOwnSecrets", + subject: rbac.Subject{ + ID: user1.ID.String(), + Roles: rbac.RoleIdentifiers{rbac.RoleMember()}, + Scope: rbac.ScopeAll, + }, + secretID: user1Secret.ID, + expectedAccess: true, + }, + { + name: "UserCannotAccessOtherUserSecrets", + subject: rbac.Subject{ + ID: user1.ID.String(), + Roles: rbac.RoleIdentifiers{rbac.RoleMember()}, + Scope: rbac.ScopeAll, + }, + secretID: user2Secret.ID, + expectedAccess: false, + }, + { + name: "OwnerCannotAccessUserSecrets", + subject: rbac.Subject{ + ID: owner.ID.String(), + Roles: rbac.RoleIdentifiers{rbac.RoleOwner()}, + Scope: rbac.ScopeAll, + }, + secretID: user1Secret.ID, + expectedAccess: false, + }, + { + name: "OrgAdminCannotAccessUserSecrets", + subject: rbac.Subject{ + ID: orgAdmin.ID.String(), + Roles: rbac.RoleIdentifiers{rbac.ScopedRoleOrgAdmin(org.ID)}, + Scope: rbac.ScopeAll, + }, + secretID: user1Secret.ID, + expectedAccess: false, + }, + } - // Test GetUserSecret - should fail - _, err := authDB.GetUserSecret(ctx, user1Secret.ID) - require.Error(t, err) - assert.True(t, dbauthz.IsNotAuthorizedError(err)) - }) + for _, tc := range testCases { + tc := tc // capture range variable + t.Run(tc.name, func(t *testing.T) { + t.Parallel() - t.Run("OrgAdminCannotAccessUserSecrets", func(t *testing.T) { - t.Parallel() + ctx := dbauthz.As(context.Background(), tc.subject) - orgAdminSubject := rbac.Subject{ - ID: orgAdmin.ID.String(), - Roles: rbac.RoleIdentifiers{rbac.ScopedRoleOrgAdmin(org.ID)}, - Scope: rbac.ScopeAll, - } - ctx := dbauthz.As(context.Background(), orgAdminSubject) + // Test GetUserSecret + _, err := authDB.GetUserSecret(ctx, tc.secretID) - // Test GetUserSecret - should fail - _, err := authDB.GetUserSecret(ctx, user1Secret.ID) - require.Error(t, err) - assert.True(t, dbauthz.IsNotAuthorizedError(err)) - }) + if tc.expectedAccess { + require.NoError(t, err, "expected access to be granted") + } else { + require.Error(t, err, "expected access to be denied") + assert.True(t, dbauthz.IsNotAuthorizedError(err), "expected authorization error") + } + }) + } } From b55be7868162e1ac20c65f9082e399c94dba15a6 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 6 Aug 2025 20:21:20 +0000 Subject: [PATCH 11/21] refactor: minor refactor in tests --- coderd/database/querier_test.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 3acf9542384ce..518c6f803196b 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -6009,6 +6009,7 @@ func TestUserSecretsCRUDOperations(t *testing.T) { // Use raw database without dbauthz wrapper for this test db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitMedium) t.Run("FullCRUDWorkflow", func(t *testing.T) { t.Parallel() @@ -6028,12 +6029,12 @@ func TestUserSecretsCRUDOperations(t *testing.T) { FilePath: "/workflow/path", } - createdSecret, err := db.CreateUserSecret(context.Background(), createParams) + createdSecret, err := db.CreateUserSecret(ctx, createParams) require.NoError(t, err) assert.Equal(t, secretID, createdSecret.ID) // 2. READ by ID - readSecret, err := db.GetUserSecret(context.Background(), createdSecret.ID) + readSecret, err := db.GetUserSecret(ctx, createdSecret.ID) require.NoError(t, err) assert.Equal(t, createdSecret.ID, readSecret.ID) assert.Equal(t, "workflow-secret", readSecret.Name) @@ -6043,12 +6044,12 @@ func TestUserSecretsCRUDOperations(t *testing.T) { UserID: testUser.ID, Name: "workflow-secret", } - readByNameSecret, err := db.GetUserSecretByUserIDAndName(context.Background(), readByNameParams) + readByNameSecret, err := db.GetUserSecretByUserIDAndName(ctx, readByNameParams) require.NoError(t, err) assert.Equal(t, createdSecret.ID, readByNameSecret.ID) // 4. LIST - secrets, err := db.ListUserSecrets(context.Background(), testUser.ID) + secrets, err := db.ListUserSecrets(ctx, testUser.ID) require.NoError(t, err) require.Len(t, secrets, 1) assert.Equal(t, createdSecret.ID, secrets[0].ID) @@ -6062,22 +6063,22 @@ func TestUserSecretsCRUDOperations(t *testing.T) { FilePath: "/updated/workflow/path", } - updatedSecret, err := db.UpdateUserSecret(context.Background(), updateParams) + updatedSecret, err := db.UpdateUserSecret(ctx, updateParams) require.NoError(t, err) assert.Equal(t, "Updated workflow description", updatedSecret.Description) assert.Equal(t, "updated-workflow-value", updatedSecret.Value) // 6. DELETE - err = db.DeleteUserSecret(context.Background(), createdSecret.ID) + err = db.DeleteUserSecret(ctx, createdSecret.ID) require.NoError(t, err) // Verify deletion - _, err = db.GetUserSecret(context.Background(), createdSecret.ID) + _, err = db.GetUserSecret(ctx, createdSecret.ID) require.Error(t, err) assert.Contains(t, err.Error(), "no rows in result set") // Verify list is empty - secrets, err = db.ListUserSecrets(context.Background(), testUser.ID) + secrets, err = db.ListUserSecrets(ctx, testUser.ID) require.NoError(t, err) assert.Len(t, secrets, 0) }) @@ -6099,7 +6100,7 @@ func TestUserSecretsCRUDOperations(t *testing.T) { }) // Try to create another secret with the same name (should fail) - _, err := db.CreateUserSecret(context.Background(), database.CreateUserSecretParams{ + _, err := db.CreateUserSecret(ctx, database.CreateUserSecretParams{ UserID: testUser.ID, Name: "unique-test", // Same name Description: "Second secret", @@ -6109,7 +6110,7 @@ func TestUserSecretsCRUDOperations(t *testing.T) { assert.Contains(t, err.Error(), "duplicate key value") // Try to create another secret with the same env_name (should fail) - _, err = db.CreateUserSecret(context.Background(), database.CreateUserSecretParams{ + _, err = db.CreateUserSecret(ctx, database.CreateUserSecretParams{ UserID: testUser.ID, Name: "unique-test-2", Description: "Second secret", @@ -6120,7 +6121,7 @@ func TestUserSecretsCRUDOperations(t *testing.T) { assert.Contains(t, err.Error(), "duplicate key value") // Try to create another secret with the same file_path (should fail) - _, err = db.CreateUserSecret(context.Background(), database.CreateUserSecretParams{ + _, err = db.CreateUserSecret(ctx, database.CreateUserSecretParams{ UserID: testUser.ID, Name: "unique-test-3", Description: "Second secret", @@ -6141,9 +6142,9 @@ func TestUserSecretsCRUDOperations(t *testing.T) { }) // Verify both secrets exist - _, err = db.GetUserSecret(context.Background(), secret1.ID) + _, err = db.GetUserSecret(ctx, secret1.ID) require.NoError(t, err) - _, err = db.GetUserSecret(context.Background(), secret2.ID) + _, err = db.GetUserSecret(ctx, secret2.ID) require.NoError(t, err) }) } @@ -6155,6 +6156,7 @@ func TestUserSecretsAuthorization(t *testing.T) { db, _ := dbtestutil.NewDB(t) authorizer := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry()) authDB := dbauthz.New(db, authorizer, slogtest.Make(t, &slogtest.Options{}), coderdtest.AccessControlStorePointer()) + ctx := testutil.Context(t, testutil.WaitMedium) // Create test users user1 := dbgen.User(t, db, database.User{}) @@ -6233,10 +6235,10 @@ func TestUserSecretsAuthorization(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - ctx := dbauthz.As(context.Background(), tc.subject) + authCtx := dbauthz.As(ctx, tc.subject) // Test GetUserSecret - _, err := authDB.GetUserSecret(ctx, tc.secretID) + _, err := authDB.GetUserSecret(authCtx, tc.secretID) if tc.expectedAccess { require.NoError(t, err, "expected access to be granted") From 3090d5255169b9c80503312f06c8019af785ae18 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 6 Aug 2025 20:41:15 +0000 Subject: [PATCH 12/21] fix: formatting --- .../migrations/testdata/fixtures/000356_add_user_secrets.up.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/migrations/testdata/fixtures/000356_add_user_secrets.up.sql b/coderd/database/migrations/testdata/fixtures/000356_add_user_secrets.up.sql index bd1fb19333bc6..a82ceb593b629 100644 --- a/coderd/database/migrations/testdata/fixtures/000356_add_user_secrets.up.sql +++ b/coderd/database/migrations/testdata/fixtures/000356_add_user_secrets.up.sql @@ -13,6 +13,6 @@ VALUES ( 'secret-name', 'secret description', 'secret value', - 'SECRET_ENV_NAME', + 'SECRET_ENV_NAME', '~/secret/file/path' ); From 789a5eb0b5ebcddd7861cc6d8bdfb3e65db8bcaf Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 6 Aug 2025 20:46:11 +0000 Subject: [PATCH 13/21] fix: formatting --- coderd/database/queries/user_secrets.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/queries/user_secrets.sql b/coderd/database/queries/user_secrets.sql index 303a5c096608e..271b97c9bb13c 100644 --- a/coderd/database/queries/user_secrets.sql +++ b/coderd/database/queries/user_secrets.sql @@ -13,7 +13,7 @@ ORDER BY name ASC; -- name: CreateUserSecret :one INSERT INTO user_secrets ( - id, + id, user_id, name, description, From 12b6c2ee3ad0b6d261c32297751ddd25f1522257 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 6 Aug 2025 20:57:22 +0000 Subject: [PATCH 14/21] ci: run make gen and make fmt --- coderd/database/queries.sql.go | 2 +- coderd/rbac/roles.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index f7beca56d4100..aaabb3db16742 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -13773,7 +13773,7 @@ func (q *sqlQuerier) UpdateUserLinkedID(ctx context.Context, arg UpdateUserLinke const createUserSecret = `-- name: CreateUserSecret :one INSERT INTO user_secrets ( - id, + id, user_id, name, description, diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index ec804478d7c6d..2e2a649332466 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -281,7 +281,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions. ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, })...), - Org: map[string][]Permission{}, + Org: map[string][]Permission{}, User: Permissions(map[string][]policy.Action{ // Users should be able to access their own secrets. ResourceUserSecret.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, From 317708bd134d098f349c3a77efb2db5a238dfb2a Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 7 Aug 2025 17:05:08 +0000 Subject: [PATCH 15/21] refactor: CR's fixes --- coderd/database/modelmethods.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index caf7ccce4c6a7..e080c7d7e4217 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -632,3 +632,7 @@ func (m WorkspaceAgentVolumeResourceMonitor) Debounce( return m.DebouncedUntil, false } + +func (s UserSecret) RBACObject() rbac.Object { + return rbac.ResourceUserSecret.WithID(s.ID).WithOwner(s.UserID.String()) +} From 72faf7a51eb2549131ff737e0a1cfdc4b9b369ec Mon Sep 17 00:00:00 2001 From: Yevhenii Shcherbina Date: Thu, 7 Aug 2025 13:06:17 -0400 Subject: [PATCH 16/21] Update coderd/database/dbauthz/dbauthz.go Co-authored-by: Steven Masley --- coderd/database/dbauthz/dbauthz.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index d66d3ab3ac5ab..252d97dd6390f 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1667,13 +1667,12 @@ func (q *querier) DeleteTailnetTunnel(ctx context.Context, arg database.DeleteTa func (q *querier) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { // First get the secret to check ownership - secret, err := q.db.GetUserSecret(ctx, id) + secret, err := q.GetUserSecret(ctx, id) if err != nil { return err } - obj := rbac.ResourceUserSecret.WithOwner(secret.UserID.String()) - if err := q.authorizeContext(ctx, policy.ActionDelete, obj); err != nil { + if err := q.authorizeContext(ctx, policy.ActionDelete, secret); err != nil { return err } return q.db.DeleteUserSecret(ctx, id) From a715310ed61b8858d38599e3c42a211212798b96 Mon Sep 17 00:00:00 2001 From: Yevhenii Shcherbina Date: Thu, 7 Aug 2025 13:11:21 -0400 Subject: [PATCH 17/21] Update coderd/database/dbauthz/dbauthz.go Co-authored-by: Steven Masley --- coderd/database/dbauthz/dbauthz.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 252d97dd6390f..34f92fdd100e9 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -4930,8 +4930,7 @@ func (q *querier) UpdateUserSecret(ctx context.Context, arg database.UpdateUserS return database.UserSecret{}, err } - obj := rbac.ResourceUserSecret.WithOwner(secret.UserID.String()) - if err := q.authorizeContext(ctx, policy.ActionUpdate, obj); err != nil { + if err := q.authorizeContext(ctx, policy.ActionUpdate, secret); err != nil { return database.UserSecret{}, err } return q.db.UpdateUserSecret(ctx, arg) From 8ea3cda0af7467f56a674b665faeab72d72c62dd Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 7 Aug 2025 18:12:15 +0000 Subject: [PATCH 18/21] refactor: CR's fixes --- coderd/database/dbauthz/dbauthz.go | 3 +-- coderd/database/dbauthz/dbauthz_test.go | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 34f92fdd100e9..e44b3534de241 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3103,8 +3103,7 @@ func (q *querier) GetUserSecret(ctx context.Context, id uuid.UUID) (database.Use return database.UserSecret{}, err } - obj := rbac.ResourceUserSecret.WithOwner(secret.UserID.String()) - if err := q.authorizeContext(ctx, policy.ActionRead, obj); err != nil { + if err := q.authorizeContext(ctx, policy.ActionRead, secret); err != nil { return database.UserSecret{}, err } return secret, nil diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 559948a202240..18a93804a0a22 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -5898,7 +5898,7 @@ func (s *MethodTestSuite) TestUserSecrets() { UserID: user.ID, }) check.Args(userSecret.ID). - Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionRead). + Asserts(userSecret, policy.ActionRead). Returns(userSecret) })) s.Run("ListUserSecrets", s.Subtest(func(db database.Store, check *expects) { @@ -5927,7 +5927,7 @@ func (s *MethodTestSuite) TestUserSecrets() { ID: userSecret.ID, } check.Args(arg). - Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionUpdate) + Asserts(userSecret, policy.ActionUpdate) })) s.Run("DeleteUserSecret", s.Subtest(func(db database.Store, check *expects) { user := dbgen.User(s.T(), db, database.User{}) @@ -5935,6 +5935,6 @@ func (s *MethodTestSuite) TestUserSecrets() { UserID: user.ID, }) check.Args(userSecret.ID). - Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionDelete) + Asserts(userSecret, policy.ActionRead, userSecret, policy.ActionDelete) })) } From b21f07c9206eb781cbcdaa6c93ffc89ae58c71fb Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 7 Aug 2025 18:37:58 +0000 Subject: [PATCH 19/21] refactor: CR's fixes --- coderd/rbac/roles.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 2e2a649332466..7bd7cdfe352b3 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -282,10 +282,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, })...), Org: map[string][]Permission{}, - User: Permissions(map[string][]policy.Action{ - // Users should be able to access their own secrets. - ResourceUserSecret.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, - }), + User: Permissions(map[string][]policy.Action{}), }.withCachedRegoValue() memberRole := Role{ From 0f47bae0fb7b329a3472eef0a7ce1bb9c6cca452 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 7 Aug 2025 18:48:31 +0000 Subject: [PATCH 20/21] refactor: CR's fixes --- coderd/rbac/roles.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 7bd7cdfe352b3..33635f34e5914 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -281,8 +281,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions. ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, })...), - Org: map[string][]Permission{}, - User: Permissions(map[string][]policy.Action{}), + Org: map[string][]Permission{}, + User: []Permission{}, }.withCachedRegoValue() memberRole := Role{ From 1d6f2fa65ae93236cdd42de7e7a652516421596f Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 7 Aug 2025 19:30:52 +0000 Subject: [PATCH 21/21] refactor: CR's fixes --- coderd/database/dbauthz/dbauthz_test.go | 10 +++++----- coderd/database/dbgen/dbgen.go | 2 +- coderd/database/querier_test.go | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 18a93804a0a22..c01050a010052 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -5881,7 +5881,7 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() { func (s *MethodTestSuite) TestUserSecrets() { s.Run("GetUserSecretByUserIDAndName", s.Subtest(func(db database.Store, check *expects) { user := dbgen.User(s.T(), db, database.User{}) - userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + userSecret := dbgen.UserSecret(s.T(), db, database.UserSecret{ UserID: user.ID, }) arg := database.GetUserSecretByUserIDAndNameParams{ @@ -5894,7 +5894,7 @@ func (s *MethodTestSuite) TestUserSecrets() { })) s.Run("GetUserSecret", s.Subtest(func(db database.Store, check *expects) { user := dbgen.User(s.T(), db, database.User{}) - userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + userSecret := dbgen.UserSecret(s.T(), db, database.UserSecret{ UserID: user.ID, }) check.Args(userSecret.ID). @@ -5903,7 +5903,7 @@ func (s *MethodTestSuite) TestUserSecrets() { })) s.Run("ListUserSecrets", s.Subtest(func(db database.Store, check *expects) { user := dbgen.User(s.T(), db, database.User{}) - userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + userSecret := dbgen.UserSecret(s.T(), db, database.UserSecret{ UserID: user.ID, }) check.Args(user.ID). @@ -5920,7 +5920,7 @@ func (s *MethodTestSuite) TestUserSecrets() { })) s.Run("UpdateUserSecret", s.Subtest(func(db database.Store, check *expects) { user := dbgen.User(s.T(), db, database.User{}) - userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + userSecret := dbgen.UserSecret(s.T(), db, database.UserSecret{ UserID: user.ID, }) arg := database.UpdateUserSecretParams{ @@ -5931,7 +5931,7 @@ func (s *MethodTestSuite) TestUserSecrets() { })) s.Run("DeleteUserSecret", s.Subtest(func(db database.Store, check *expects) { user := dbgen.User(s.T(), db, database.User{}) - userSecret := dbgen.UserSecret(s.T(), db, database.CreateUserSecretParams{ + userSecret := dbgen.UserSecret(s.T(), db, database.UserSecret{ UserID: user.ID, }) check.Args(userSecret.ID). diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 34698e4471d3e..11e02d0f651e9 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -1422,7 +1422,7 @@ func PresetParameter(t testing.TB, db database.Store, seed database.InsertPreset return parameters } -func UserSecret(t testing.TB, db database.Store, seed database.CreateUserSecretParams) database.UserSecret { +func UserSecret(t testing.TB, db database.Store, seed database.UserSecret) database.UserSecret { userSecret, err := db.CreateUserSecret(genCtx, database.CreateUserSecretParams{ ID: takeFirst(seed.ID, uuid.New()), UserID: takeFirst(seed.UserID, uuid.New()), diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 518c6f803196b..39d9559509100 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -6090,7 +6090,7 @@ func TestUserSecretsCRUDOperations(t *testing.T) { testUser := dbgen.User(t, db, database.User{}) // Create first secret - secret1 := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ + secret1 := dbgen.UserSecret(t, db, database.UserSecret{ UserID: testUser.ID, Name: "unique-test", Description: "First secret", @@ -6132,7 +6132,7 @@ func TestUserSecretsCRUDOperations(t *testing.T) { assert.Contains(t, err.Error(), "duplicate key value") // Create secret with empty env_name and file_path (should succeed) - secret2 := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ + secret2 := dbgen.UserSecret(t, db, database.UserSecret{ UserID: testUser.ID, Name: "unique-test-4", Description: "Second secret", @@ -6168,14 +6168,14 @@ func TestUserSecretsAuthorization(t *testing.T) { org := dbgen.Organization(t, db, database.Organization{}) // Create secrets for users - user1Secret := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ + user1Secret := dbgen.UserSecret(t, db, database.UserSecret{ UserID: user1.ID, Name: "user1-secret", Description: "User 1's secret", Value: "user1-value", }) - user2Secret := dbgen.UserSecret(t, db, database.CreateUserSecretParams{ + user2Secret := dbgen.UserSecret(t, db, database.UserSecret{ UserID: user2.ID, Name: "user2-secret", Description: "User 2's secret",