From 565a15bc9b62ec4619e5c47c17946db306090c87 Mon Sep 17 00:00:00 2001 From: Zach <3724288+zedkipp@users.noreply.github.com> Date: Tue, 7 Apr 2026 09:03:28 -0600 Subject: [PATCH] feat: update user secrets queries for REST API and injection (#23998) Update queries as prep work for user secrets API development: - Switch all lookups and mutations from ID-based to user_id + name - Split list query into metadata-only (for API responses) and with-values (for provisioner/agent) - Add partial update support using CASE WHEN pattern for write-only value fields - Include value_key_id in create for dbcrypt encryption support - Update dbauthz wrappers and remove stale methods from dbmetrics --- coderd/database/dbauthz/dbauthz.go | 51 +++---- coderd/database/dbauthz/dbauthz_test.go | 44 +++--- coderd/database/dbgen/dbgen.go | 1 + coderd/database/dbmetrics/querymetrics.go | 34 ++--- coderd/database/dbmock/dbmock.go | 58 +++---- coderd/database/querier.go | 13 +- coderd/database/querier_test.go | 75 ++++++---- coderd/database/queries.sql.go | 175 +++++++++++++++------- coderd/database/queries/user_secrets.sql | 57 ++++--- 9 files changed, 301 insertions(+), 207 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index aae2ce2aa3..f2020172f2 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2155,17 +2155,12 @@ func (q *querier) DeleteUserChatProviderKey(ctx context.Context, arg database.De return q.db.DeleteUserChatProviderKey(ctx, arg) } -func (q *querier) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { - // First get the secret to check ownership - secret, err := q.GetUserSecret(ctx, id) - if err != nil { +func (q *querier) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) error { + obj := rbac.ResourceUserSecret.WithOwner(arg.UserID.String()) + if err := q.authorizeContext(ctx, policy.ActionDelete, obj); err != nil { return err } - - if err := q.authorizeContext(ctx, policy.ActionDelete, secret); err != nil { - return err - } - return q.db.DeleteUserSecret(ctx, id) + return q.db.DeleteUserSecretByUserIDAndName(ctx, arg) } func (q *querier) DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg database.DeleteWebpushSubscriptionByUserIDAndEndpointParams) error { @@ -4128,19 +4123,6 @@ 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) { - // First get the secret to check ownership - secret, err := q.db.GetUserSecret(ctx, id) - if err != nil { - return database.UserSecret{}, err - } - - if err := q.authorizeContext(ctx, policy.ActionRead, secret); err != nil { - return database.UserSecret{}, err - } - return secret, nil -} - func (q *querier) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) { obj := rbac.ResourceUserSecret.WithOwner(arg.UserID.String()) if err := q.authorizeContext(ctx, policy.ActionRead, obj); err != nil { @@ -5524,7 +5506,7 @@ func (q *querier) ListUserChatCompactionThresholds(ctx context.Context, userID u return q.db.ListUserChatCompactionThresholds(ctx, userID) } -func (q *querier) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { +func (q *querier) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.ListUserSecretsRow, error) { obj := rbac.ResourceUserSecret.WithOwner(userID.String()) if err := q.authorizeContext(ctx, policy.ActionRead, obj); err != nil { return nil, err @@ -5532,6 +5514,16 @@ func (q *querier) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]data return q.db.ListUserSecrets(ctx, userID) } +func (q *querier) ListUserSecretsWithValues(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { + // This query returns decrypted secret values and must only be called + // from system contexts (provisioner, agent manifest). REST API + // handlers should use ListUserSecrets (metadata only). + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.ListUserSecretsWithValues(ctx, userID) +} + func (q *querier) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]database.WorkspaceAgentPortShare, error) { workspace, err := q.db.GetWorkspaceByID(ctx, workspaceID) if err != nil { @@ -6632,17 +6624,12 @@ 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) { - // First get the secret to check ownership - secret, err := q.db.GetUserSecret(ctx, arg.ID) - if err != nil { +func (q *querier) UpdateUserSecretByUserIDAndName(ctx context.Context, arg database.UpdateUserSecretByUserIDAndNameParams) (database.UserSecret, error) { + obj := rbac.ResourceUserSecret.WithOwner(arg.UserID.String()) + if err := q.authorizeContext(ctx, policy.ActionUpdate, obj); err != nil { return database.UserSecret{}, err } - - if err := q.authorizeContext(ctx, policy.ActionUpdate, secret); err != nil { - return database.UserSecret{}, err - } - return q.db.UpdateUserSecret(ctx, arg) + return q.db.UpdateUserSecretByUserIDAndName(ctx, arg) } func (q *querier) UpdateUserStatus(ctx context.Context, arg database.UpdateUserStatusParams) (database.User, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 01449b4f06..5e0cf7dbf7 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -5346,19 +5346,20 @@ func (s *MethodTestSuite) TestUserSecrets() { Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionRead). Returns(secret) })) - s.Run("GetUserSecret", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { - secret := testutil.Fake(s.T(), faker, database.UserSecret{}) - dbm.EXPECT().GetUserSecret(gomock.Any(), secret.ID).Return(secret, nil).AnyTimes() - check.Args(secret.ID). - Asserts(secret, policy.ActionRead). - Returns(secret) - })) s.Run("ListUserSecrets", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { user := testutil.Fake(s.T(), faker, database.User{}) - secret := testutil.Fake(s.T(), faker, database.UserSecret{UserID: user.ID}) - dbm.EXPECT().ListUserSecrets(gomock.Any(), user.ID).Return([]database.UserSecret{secret}, nil).AnyTimes() + row := testutil.Fake(s.T(), faker, database.ListUserSecretsRow{UserID: user.ID}) + dbm.EXPECT().ListUserSecrets(gomock.Any(), user.ID).Return([]database.ListUserSecretsRow{row}, nil).AnyTimes() check.Args(user.ID). Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionRead). + Returns([]database.ListUserSecretsRow{row}) + })) + s.Run("ListUserSecretsWithValues", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + user := testutil.Fake(s.T(), faker, database.User{}) + secret := testutil.Fake(s.T(), faker, database.UserSecret{UserID: user.ID}) + dbm.EXPECT().ListUserSecretsWithValues(gomock.Any(), user.ID).Return([]database.UserSecret{secret}, nil).AnyTimes() + check.Args(user.ID). + Asserts(rbac.ResourceSystem, policy.ActionRead). Returns([]database.UserSecret{secret}) })) s.Run("CreateUserSecret", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { @@ -5370,22 +5371,21 @@ func (s *MethodTestSuite) TestUserSecrets() { Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionCreate). Returns(ret) })) - s.Run("UpdateUserSecret", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { - secret := testutil.Fake(s.T(), faker, database.UserSecret{}) - updated := testutil.Fake(s.T(), faker, database.UserSecret{ID: secret.ID}) - arg := database.UpdateUserSecretParams{ID: secret.ID} - dbm.EXPECT().GetUserSecret(gomock.Any(), secret.ID).Return(secret, nil).AnyTimes() - dbm.EXPECT().UpdateUserSecret(gomock.Any(), arg).Return(updated, nil).AnyTimes() + s.Run("UpdateUserSecretByUserIDAndName", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + user := testutil.Fake(s.T(), faker, database.User{}) + updated := testutil.Fake(s.T(), faker, database.UserSecret{UserID: user.ID}) + arg := database.UpdateUserSecretByUserIDAndNameParams{UserID: user.ID, Name: "test"} + dbm.EXPECT().UpdateUserSecretByUserIDAndName(gomock.Any(), arg).Return(updated, nil).AnyTimes() check.Args(arg). - Asserts(secret, policy.ActionUpdate). + Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionUpdate). Returns(updated) })) - s.Run("DeleteUserSecret", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { - secret := testutil.Fake(s.T(), faker, database.UserSecret{}) - dbm.EXPECT().GetUserSecret(gomock.Any(), secret.ID).Return(secret, nil).AnyTimes() - dbm.EXPECT().DeleteUserSecret(gomock.Any(), secret.ID).Return(nil).AnyTimes() - check.Args(secret.ID). - Asserts(secret, policy.ActionRead, secret, policy.ActionDelete). + s.Run("DeleteUserSecretByUserIDAndName", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + user := testutil.Fake(s.T(), faker, database.User{}) + arg := database.DeleteUserSecretByUserIDAndNameParams{UserID: user.ID, Name: "test"} + dbm.EXPECT().DeleteUserSecretByUserIDAndName(gomock.Any(), arg).Return(nil).AnyTimes() + check.Args(arg). + Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionDelete). Returns() })) } diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 69a50ede9c..fcf2e22ebf 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -1597,6 +1597,7 @@ func UserSecret(t testing.TB, db database.Store, seed database.UserSecret) datab Name: takeFirst(seed.Name, "secret-name"), Description: takeFirst(seed.Description, "secret description"), Value: takeFirst(seed.Value, "secret value"), + ValueKeyID: seed.ValueKeyID, EnvName: takeFirst(seed.EnvName, "SECRET_ENV_NAME"), FilePath: takeFirst(seed.FilePath, "~/secret/file/path"), }) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index f91b6f6efa..1bae104b09 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -712,11 +712,11 @@ func (m queryMetricsStore) DeleteUserChatProviderKey(ctx context.Context, arg da return r0 } -func (m queryMetricsStore) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { +func (m queryMetricsStore) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) error { start := time.Now() - r0 := m.s.DeleteUserSecret(ctx, id) - m.queryLatencies.WithLabelValues("DeleteUserSecret").Observe(time.Since(start).Seconds()) - m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "DeleteUserSecret").Inc() + r0 := m.s.DeleteUserSecretByUserIDAndName(ctx, arg) + m.queryLatencies.WithLabelValues("DeleteUserSecretByUserIDAndName").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "DeleteUserSecretByUserIDAndName").Inc() return r0 } @@ -2624,14 +2624,6 @@ 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()) - m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetUserSecret").Inc() - 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) @@ -3920,7 +3912,7 @@ func (m queryMetricsStore) ListUserChatCompactionThresholds(ctx context.Context, return r0, r1 } -func (m queryMetricsStore) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { +func (m queryMetricsStore) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.ListUserSecretsRow, error) { start := time.Now() r0, r1 := m.s.ListUserSecrets(ctx, userID) m.queryLatencies.WithLabelValues("ListUserSecrets").Observe(time.Since(start).Seconds()) @@ -3928,6 +3920,14 @@ func (m queryMetricsStore) ListUserSecrets(ctx context.Context, userID uuid.UUID return r0, r1 } +func (m queryMetricsStore) ListUserSecretsWithValues(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { + start := time.Now() + r0, r1 := m.s.ListUserSecretsWithValues(ctx, userID) + m.queryLatencies.WithLabelValues("ListUserSecretsWithValues").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "ListUserSecretsWithValues").Inc() + 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) @@ -4696,11 +4696,11 @@ func (m queryMetricsStore) UpdateUserRoles(ctx context.Context, arg database.Upd return r0, r1 } -func (m queryMetricsStore) UpdateUserSecret(ctx context.Context, arg database.UpdateUserSecretParams) (database.UserSecret, error) { +func (m queryMetricsStore) UpdateUserSecretByUserIDAndName(ctx context.Context, arg database.UpdateUserSecretByUserIDAndNameParams) (database.UserSecret, error) { start := time.Now() - r0, r1 := m.s.UpdateUserSecret(ctx, arg) - m.queryLatencies.WithLabelValues("UpdateUserSecret").Observe(time.Since(start).Seconds()) - m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpdateUserSecret").Inc() + r0, r1 := m.s.UpdateUserSecretByUserIDAndName(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateUserSecretByUserIDAndName").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpdateUserSecretByUserIDAndName").Inc() return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 9aa4a08bb4..494961d08c 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1199,18 +1199,18 @@ func (mr *MockStoreMockRecorder) DeleteUserChatProviderKey(ctx, arg any) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteUserChatProviderKey", reflect.TypeOf((*MockStore)(nil).DeleteUserChatProviderKey), ctx, arg) } -// DeleteUserSecret mocks base method. -func (m *MockStore) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { +// DeleteUserSecretByUserIDAndName mocks base method. +func (m *MockStore) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteUserSecret", ctx, id) + ret := m.ctrl.Call(m, "DeleteUserSecretByUserIDAndName", ctx, arg) ret0, _ := ret[0].(error) return ret0 } -// DeleteUserSecret indicates an expected call of DeleteUserSecret. -func (mr *MockStoreMockRecorder) DeleteUserSecret(ctx, id any) *gomock.Call { +// DeleteUserSecretByUserIDAndName indicates an expected call of DeleteUserSecretByUserIDAndName. +func (mr *MockStoreMockRecorder) DeleteUserSecretByUserIDAndName(ctx, arg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteUserSecret", reflect.TypeOf((*MockStore)(nil).DeleteUserSecret), ctx, id) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteUserSecretByUserIDAndName", reflect.TypeOf((*MockStore)(nil).DeleteUserSecretByUserIDAndName), ctx, arg) } // DeleteWebpushSubscriptionByUserIDAndEndpoint mocks base method. @@ -4907,21 +4907,6 @@ 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() @@ -7412,10 +7397,10 @@ func (mr *MockStoreMockRecorder) ListUserChatCompactionThresholds(ctx, userID an } // ListUserSecrets mocks base method. -func (m *MockStore) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { +func (m *MockStore) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]database.ListUserSecretsRow, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListUserSecrets", ctx, userID) - ret0, _ := ret[0].([]database.UserSecret) + ret0, _ := ret[0].([]database.ListUserSecretsRow) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -7426,6 +7411,21 @@ func (mr *MockStoreMockRecorder) ListUserSecrets(ctx, userID any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListUserSecrets", reflect.TypeOf((*MockStore)(nil).ListUserSecrets), ctx, userID) } +// ListUserSecretsWithValues mocks base method. +func (m *MockStore) ListUserSecretsWithValues(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListUserSecretsWithValues", ctx, userID) + ret0, _ := ret[0].([]database.UserSecret) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListUserSecretsWithValues indicates an expected call of ListUserSecretsWithValues. +func (mr *MockStoreMockRecorder) ListUserSecretsWithValues(ctx, userID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListUserSecretsWithValues", reflect.TypeOf((*MockStore)(nil).ListUserSecretsWithValues), ctx, userID) +} + // ListWorkspaceAgentPortShares mocks base method. func (m *MockStore) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]database.WorkspaceAgentPortShare, error) { m.ctrl.T.Helper() @@ -8854,19 +8854,19 @@ 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) { +// UpdateUserSecretByUserIDAndName mocks base method. +func (m *MockStore) UpdateUserSecretByUserIDAndName(ctx context.Context, arg database.UpdateUserSecretByUserIDAndNameParams) (database.UserSecret, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateUserSecret", ctx, arg) + ret := m.ctrl.Call(m, "UpdateUserSecretByUserIDAndName", 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 { +// UpdateUserSecretByUserIDAndName indicates an expected call of UpdateUserSecretByUserIDAndName. +func (mr *MockStoreMockRecorder) UpdateUserSecretByUserIDAndName(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) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserSecretByUserIDAndName", reflect.TypeOf((*MockStore)(nil).UpdateUserSecretByUserIDAndName), ctx, arg) } // UpdateUserStatus mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 6719cb8862..049cde3c0f 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -152,7 +152,7 @@ type sqlcQuerier interface { DeleteTask(ctx context.Context, arg DeleteTaskParams) (uuid.UUID, error) DeleteUserChatCompactionThreshold(ctx context.Context, arg DeleteUserChatCompactionThresholdParams) error DeleteUserChatProviderKey(ctx context.Context, arg DeleteUserChatProviderKeyParams) error - DeleteUserSecret(ctx context.Context, id uuid.UUID) error + DeleteUserSecretByUserIDAndName(ctx context.Context, arg DeleteUserSecretByUserIDAndNameParams) error DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg DeleteWebpushSubscriptionByUserIDAndEndpointParams) error DeleteWebpushSubscriptions(ctx context.Context, ids []uuid.UUID) error DeleteWorkspaceACLByID(ctx context.Context, id uuid.UUID) error @@ -598,7 +598,6 @@ 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. @@ -818,7 +817,13 @@ type sqlcQuerier interface { ListProvisionerKeysByOrganizationExcludeReserved(ctx context.Context, organizationID uuid.UUID) ([]ProvisionerKey, error) ListTasks(ctx context.Context, arg ListTasksParams) ([]Task, error) ListUserChatCompactionThresholds(ctx context.Context, userID uuid.UUID) ([]UserConfig, error) - ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]UserSecret, error) + // Returns metadata only (no value or value_key_id) for the + // REST API list and get endpoints. + ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]ListUserSecretsRow, error) + // Returns all columns including the secret value. Used by the + // provisioner (build-time injection) and the agent manifest + // (runtime injection). + ListUserSecretsWithValues(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) @@ -957,7 +962,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) + UpdateUserSecretByUserIDAndName(ctx context.Context, arg UpdateUserSecretByUserIDAndNameParams) (UserSecret, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) UpdateUserTaskNotificationAlertDismissed(ctx context.Context, arg UpdateUserTaskNotificationAlertDismissedParams) (bool, error) UpdateUserTerminalFont(ctx context.Context, arg UpdateUserTerminalFontParams) (UserConfig, error) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index dd68a4ce9c..73cf0afc97 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -7339,13 +7339,7 @@ func TestUserSecretsCRUDOperations(t *testing.T) { require.NoError(t, err) assert.Equal(t, secretID, createdSecret.ID) - // 2. READ by 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) - - // 3. READ by UserID and Name + // 2. READ by UserID and Name readByNameParams := database.GetUserSecretByUserIDAndNameParams{ UserID: testUser.ID, Name: "workflow-secret", @@ -7353,33 +7347,43 @@ func TestUserSecretsCRUDOperations(t *testing.T) { readByNameSecret, err := db.GetUserSecretByUserIDAndName(ctx, readByNameParams) require.NoError(t, err) assert.Equal(t, createdSecret.ID, readByNameSecret.ID) + assert.Equal(t, "workflow-secret", readByNameSecret.Name) - // 4. LIST + // 3. LIST (metadata only) secrets, err := db.ListUserSecrets(ctx, 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", + // 4. LIST with values + secretsWithValues, err := db.ListUserSecretsWithValues(ctx, testUser.ID) + require.NoError(t, err) + require.Len(t, secretsWithValues, 1) + assert.Equal(t, "workflow-value", secretsWithValues[0].Value) + + // 5. UPDATE (partial - only description) + updateParams := database.UpdateUserSecretByUserIDAndNameParams{ + UserID: testUser.ID, + Name: "workflow-secret", + UpdateDescription: true, + Description: "Updated workflow description", } - updatedSecret, err := db.UpdateUserSecret(ctx, updateParams) + updatedSecret, err := db.UpdateUserSecretByUserIDAndName(ctx, updateParams) require.NoError(t, err) assert.Equal(t, "Updated workflow description", updatedSecret.Description) - assert.Equal(t, "updated-workflow-value", updatedSecret.Value) + assert.Equal(t, "workflow-value", updatedSecret.Value) // Value unchanged + assert.Equal(t, "WORKFLOW_ENV", updatedSecret.EnvName) // EnvName unchanged // 6. DELETE - err = db.DeleteUserSecret(ctx, createdSecret.ID) + err = db.DeleteUserSecretByUserIDAndName(ctx, database.DeleteUserSecretByUserIDAndNameParams{ + UserID: testUser.ID, + Name: "workflow-secret", + }) require.NoError(t, err) // Verify deletion - _, err = db.GetUserSecret(ctx, createdSecret.ID) + _, err = db.GetUserSecretByUserIDAndName(ctx, readByNameParams) require.Error(t, err) assert.Contains(t, err.Error(), "no rows in result set") @@ -7449,9 +7453,13 @@ func TestUserSecretsCRUDOperations(t *testing.T) { }) // Verify both secrets exist - _, err = db.GetUserSecret(ctx, secret1.ID) + _, err = db.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: testUser.ID, Name: secret1.Name, + }) require.NoError(t, err) - _, err = db.GetUserSecret(ctx, secret2.ID) + _, err = db.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: testUser.ID, Name: secret2.Name, + }) require.NoError(t, err) }) } @@ -7474,14 +7482,14 @@ func TestUserSecretsAuthorization(t *testing.T) { org := dbgen.Organization(t, db, database.Organization{}) // Create secrets for users - user1Secret := dbgen.UserSecret(t, db, database.UserSecret{ + _ = 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.UserSecret{ + _ = dbgen.UserSecret(t, db, database.UserSecret{ UserID: user2.ID, Name: "user2-secret", Description: "User 2's secret", @@ -7491,7 +7499,8 @@ func TestUserSecretsAuthorization(t *testing.T) { testCases := []struct { name string subject rbac.Subject - secretID uuid.UUID + lookupUserID uuid.UUID + lookupName string expectedAccess bool }{ { @@ -7501,7 +7510,8 @@ func TestUserSecretsAuthorization(t *testing.T) { Roles: rbac.RoleIdentifiers{rbac.RoleMember()}, Scope: rbac.ScopeAll, }, - secretID: user1Secret.ID, + lookupUserID: user1.ID, + lookupName: "user1-secret", expectedAccess: true, }, { @@ -7511,7 +7521,8 @@ func TestUserSecretsAuthorization(t *testing.T) { Roles: rbac.RoleIdentifiers{rbac.RoleMember()}, Scope: rbac.ScopeAll, }, - secretID: user2Secret.ID, + lookupUserID: user2.ID, + lookupName: "user2-secret", expectedAccess: false, }, { @@ -7521,7 +7532,8 @@ func TestUserSecretsAuthorization(t *testing.T) { Roles: rbac.RoleIdentifiers{rbac.RoleOwner()}, Scope: rbac.ScopeAll, }, - secretID: user1Secret.ID, + lookupUserID: user1.ID, + lookupName: "user1-secret", expectedAccess: false, }, { @@ -7531,7 +7543,8 @@ func TestUserSecretsAuthorization(t *testing.T) { Roles: rbac.RoleIdentifiers{rbac.ScopedRoleOrgAdmin(org.ID)}, Scope: rbac.ScopeAll, }, - secretID: user1Secret.ID, + lookupUserID: user1.ID, + lookupName: "user1-secret", expectedAccess: false, }, } @@ -7543,8 +7556,10 @@ func TestUserSecretsAuthorization(t *testing.T) { authCtx := dbauthz.As(ctx, tc.subject) - // Test GetUserSecret - _, err := authDB.GetUserSecret(authCtx, tc.secretID) + _, err := authDB.GetUserSecretByUserIDAndName(authCtx, database.GetUserSecretByUserIDAndNameParams{ + UserID: tc.lookupUserID, + Name: tc.lookupName, + }) if tc.expectedAccess { require.NoError(t, err, "expected access to be granted") diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4549830418..9aa787c6f4 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -22639,21 +22639,30 @@ INSERT INTO user_secrets ( name, description, value, + value_key_id, env_name, file_path ) VALUES ( - $1, $2, $3, $4, $5, $6, $7 + $1, + $2, + $3, + $4, + $5, + $6, + $7, + $8 ) RETURNING id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id ` 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"` - Value string `db:"value" json:"value"` - EnvName string `db:"env_name" json:"env_name"` - FilePath string `db:"file_path" json:"file_path"` + 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"` + ValueKeyID sql.NullString `db:"value_key_id" json:"value_key_id"` + 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) { @@ -22663,6 +22672,7 @@ func (q *sqlQuerier) CreateUserSecret(ctx context.Context, arg CreateUserSecretP arg.Name, arg.Description, arg.Value, + arg.ValueKeyID, arg.EnvName, arg.FilePath, ) @@ -22682,41 +22692,24 @@ func (q *sqlQuerier) CreateUserSecret(ctx context.Context, arg CreateUserSecretP return i, err } -const deleteUserSecret = `-- name: DeleteUserSecret :exec +const deleteUserSecretByUserIDAndName = `-- name: DeleteUserSecretByUserIDAndName :exec DELETE FROM user_secrets -WHERE id = $1 +WHERE user_id = $1 AND name = $2 ` -func (q *sqlQuerier) DeleteUserSecret(ctx context.Context, id uuid.UUID) error { - _, err := q.db.ExecContext(ctx, deleteUserSecret, id) +type DeleteUserSecretByUserIDAndNameParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + Name string `db:"name" json:"name"` +} + +func (q *sqlQuerier) DeleteUserSecretByUserIDAndName(ctx context.Context, arg DeleteUserSecretByUserIDAndNameParams) error { + _, err := q.db.ExecContext(ctx, deleteUserSecretByUserIDAndName, arg.UserID, arg.Name) return err } -const getUserSecret = `-- name: GetUserSecret :one -SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id 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, - &i.ValueKeyID, - ) - return i, err -} - const getUserSecretByUserIDAndName = `-- name: GetUserSecretByUserIDAndName :one -SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id FROM user_secrets +SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id +FROM user_secrets WHERE user_id = $1 AND name = $2 ` @@ -22744,17 +22737,76 @@ func (q *sqlQuerier) GetUserSecretByUserIDAndName(ctx context.Context, arg GetUs } const listUserSecrets = `-- name: ListUserSecrets :many -SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id FROM user_secrets +SELECT + id, user_id, name, description, + 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) { +type ListUserSecretsRow 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"` + 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"` +} + +// Returns metadata only (no value or value_key_id) for the +// REST API list and get endpoints. +func (q *sqlQuerier) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]ListUserSecretsRow, error) { rows, err := q.db.QueryContext(ctx, listUserSecrets, userID) if err != nil { return nil, err } defer rows.Close() + var items []ListUserSecretsRow + for rows.Next() { + var i ListUserSecretsRow + if err := rows.Scan( + &i.ID, + &i.UserID, + &i.Name, + &i.Description, + &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 listUserSecretsWithValues = `-- name: ListUserSecretsWithValues :many +SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id +FROM user_secrets +WHERE user_id = $1 +ORDER BY name ASC +` + +// Returns all columns including the secret value. Used by the +// provisioner (build-time injection) and the agent manifest +// (runtime injection). +func (q *sqlQuerier) ListUserSecretsWithValues(ctx context.Context, userID uuid.UUID) ([]UserSecret, error) { + rows, err := q.db.QueryContext(ctx, listUserSecretsWithValues, userID) + if err != nil { + return nil, err + } + defer rows.Close() var items []UserSecret for rows.Next() { var i UserSecret @@ -22783,33 +22835,46 @@ func (q *sqlQuerier) ListUserSecrets(ctx context.Context, userID uuid.UUID) ([]U return items, nil } -const updateUserSecret = `-- name: UpdateUserSecret :one +const updateUserSecretByUserIDAndName = `-- name: UpdateUserSecretByUserIDAndName :one UPDATE user_secrets SET - description = $2, - value = $3, - env_name = $4, - file_path = $5, - updated_at = CURRENT_TIMESTAMP -WHERE id = $1 + value = CASE WHEN $1::bool THEN $2 ELSE value END, + value_key_id = CASE WHEN $1::bool THEN $3 ELSE value_key_id END, + description = CASE WHEN $4::bool THEN $5 ELSE description END, + env_name = CASE WHEN $6::bool THEN $7 ELSE env_name END, + file_path = CASE WHEN $8::bool THEN $9 ELSE file_path END, + updated_at = CURRENT_TIMESTAMP +WHERE user_id = $10 AND name = $11 RETURNING id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id ` -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"` +type UpdateUserSecretByUserIDAndNameParams struct { + UpdateValue bool `db:"update_value" json:"update_value"` + Value string `db:"value" json:"value"` + ValueKeyID sql.NullString `db:"value_key_id" json:"value_key_id"` + UpdateDescription bool `db:"update_description" json:"update_description"` + Description string `db:"description" json:"description"` + UpdateEnvName bool `db:"update_env_name" json:"update_env_name"` + EnvName string `db:"env_name" json:"env_name"` + UpdateFilePath bool `db:"update_file_path" json:"update_file_path"` + FilePath string `db:"file_path" json:"file_path"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + Name string `db:"name" json:"name"` } -func (q *sqlQuerier) UpdateUserSecret(ctx context.Context, arg UpdateUserSecretParams) (UserSecret, error) { - row := q.db.QueryRowContext(ctx, updateUserSecret, - arg.ID, - arg.Description, +func (q *sqlQuerier) UpdateUserSecretByUserIDAndName(ctx context.Context, arg UpdateUserSecretByUserIDAndNameParams) (UserSecret, error) { + row := q.db.QueryRowContext(ctx, updateUserSecretByUserIDAndName, + arg.UpdateValue, arg.Value, + arg.ValueKeyID, + arg.UpdateDescription, + arg.Description, + arg.UpdateEnvName, arg.EnvName, + arg.UpdateFilePath, arg.FilePath, + arg.UserID, + arg.Name, ) var i UserSecret err := row.Scan( diff --git a/coderd/database/queries/user_secrets.sql b/coderd/database/queries/user_secrets.sql index 271b97c9bb..9a68843b36 100644 --- a/coderd/database/queries/user_secrets.sql +++ b/coderd/database/queries/user_secrets.sql @@ -1,14 +1,26 @@ -- name: GetUserSecretByUserIDAndName :one -SELECT * FROM user_secrets -WHERE user_id = $1 AND name = $2; - --- name: GetUserSecret :one -SELECT * FROM user_secrets -WHERE id = $1; +SELECT * +FROM user_secrets +WHERE user_id = @user_id AND name = @name; -- name: ListUserSecrets :many -SELECT * FROM user_secrets -WHERE user_id = $1 +-- Returns metadata only (no value or value_key_id) for the +-- REST API list and get endpoints. +SELECT + id, user_id, name, description, + env_name, file_path, + created_at, updated_at +FROM user_secrets +WHERE user_id = @user_id +ORDER BY name ASC; + +-- name: ListUserSecretsWithValues :many +-- Returns all columns including the secret value. Used by the +-- provisioner (build-time injection) and the agent manifest +-- (runtime injection). +SELECT * +FROM user_secrets +WHERE user_id = @user_id ORDER BY name ASC; -- name: CreateUserSecret :one @@ -18,23 +30,32 @@ INSERT INTO user_secrets ( name, description, value, + value_key_id, env_name, file_path ) VALUES ( - $1, $2, $3, $4, $5, $6, $7 + @id, + @user_id, + @name, + @description, + @value, + @value_key_id, + @env_name, + @file_path ) RETURNING *; --- name: UpdateUserSecret :one +-- name: UpdateUserSecretByUserIDAndName :one UPDATE user_secrets SET - description = $2, - value = $3, - env_name = $4, - file_path = $5, - updated_at = CURRENT_TIMESTAMP -WHERE id = $1 + value = CASE WHEN @update_value::bool THEN @value ELSE value END, + value_key_id = CASE WHEN @update_value::bool THEN @value_key_id ELSE value_key_id END, + description = CASE WHEN @update_description::bool THEN @description ELSE description END, + env_name = CASE WHEN @update_env_name::bool THEN @env_name ELSE env_name END, + file_path = CASE WHEN @update_file_path::bool THEN @file_path ELSE file_path END, + updated_at = CURRENT_TIMESTAMP +WHERE user_id = @user_id AND name = @name RETURNING *; --- name: DeleteUserSecret :exec +-- name: DeleteUserSecretByUserIDAndName :exec DELETE FROM user_secrets -WHERE id = $1; +WHERE user_id = @user_id AND name = @name;