From 019de1008ad9bbe3d90905d5c141a3d5e88de6f1 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 25 Jul 2024 21:46:54 +0200 Subject: [PATCH 01/20] Implement notification template update auditing Signed-off-by: Danny Kopping --- coderd/audit/diff.go | 3 ++- coderd/audit/request.go | 9 +++++++++ coderd/database/models.go | 5 ++++- codersdk/audit.go | 3 +++ docs/admin/audit-logs.md | 1 + enterprise/audit/diff.go | 7 +++++++ enterprise/audit/table.go | 9 +++++++++ site/src/api/typesGenerated.ts | 2 ++ 8 files changed, 37 insertions(+), 2 deletions(-) diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go index 129b904c75b03..04943c760a55e 100644 --- a/coderd/audit/diff.go +++ b/coderd/audit/diff.go @@ -25,7 +25,8 @@ type Auditable interface { database.OAuth2ProviderAppSecret | database.CustomRole | database.AuditableOrganizationMember | - database.Organization + database.Organization | + database.NotificationTemplate } // Map is a map of changed fields in an audited resource. It maps field names to diff --git a/coderd/audit/request.go b/coderd/audit/request.go index 6c862c6e11103..adaf3ce1f573c 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -16,6 +16,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpmw" @@ -117,6 +118,8 @@ func ResourceTarget[T Auditable](tgt T) string { return typed.Username case database.Organization: return typed.Name + case database.NotificationTemplate: + return typed.Name default: panic(fmt.Sprintf("unknown resource %T for ResourceTarget", tgt)) } @@ -163,6 +166,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID { return typed.UserID case database.Organization: return typed.ID + case database.NotificationTemplate: + return typed.ID default: panic(fmt.Sprintf("unknown resource %T for ResourceID", tgt)) } @@ -206,6 +211,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType { return database.ResourceTypeOrganizationMember case database.Organization: return database.ResourceTypeOrganization + case database.NotificationTemplate: + return database.ResourceTypeNotificationTemplate default: panic(fmt.Sprintf("unknown resource %T for ResourceType", typed)) } @@ -251,6 +258,8 @@ func ResourceRequiresOrgID[T Auditable]() bool { return true case database.Organization: return true + case database.NotificationTemplate: + return false default: panic(fmt.Sprintf("unknown resource %T for ResourceRequiresOrgID", tgt)) } diff --git a/coderd/database/models.go b/coderd/database/models.go index 70350f54a704f..2ac87d6ca80b5 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1353,6 +1353,7 @@ const ( ResourceTypeCustomRole ResourceType = "custom_role" ResourceTypeOrganizationMember ResourceType = "organization_member" ResourceTypeNotificationsSettings ResourceType = "notifications_settings" + ResourceTypeNotificationTemplate ResourceType = "notification_template" ) func (e *ResourceType) Scan(src interface{}) error { @@ -1409,7 +1410,8 @@ func (e ResourceType) Valid() bool { ResourceTypeOauth2ProviderAppSecret, ResourceTypeCustomRole, ResourceTypeOrganizationMember, - ResourceTypeNotificationsSettings: + ResourceTypeNotificationsSettings, + ResourceTypeNotificationTemplate: return true } return false @@ -1435,6 +1437,7 @@ func AllResourceTypeValues() []ResourceType { ResourceTypeCustomRole, ResourceTypeOrganizationMember, ResourceTypeNotificationsSettings, + ResourceTypeNotificationTemplate, } } diff --git a/codersdk/audit.go b/codersdk/audit.go index 33b4714f03df6..be2959127fd4c 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -33,6 +33,7 @@ const ( ResourceTypeOAuth2ProviderAppSecret ResourceType = "oauth2_provider_app_secret" ResourceTypeCustomRole ResourceType = "custom_role" ResourceTypeOrganizationMember = "organization_member" + ResourceTypeNotificationTemplate = "notification_template" ) func (r ResourceType) FriendlyString() string { @@ -75,6 +76,8 @@ func (r ResourceType) FriendlyString() string { return "custom role" case ResourceTypeOrganizationMember: return "organization member" + case ResourceTypeNotificationTemplate: + return "notification template" default: return "unknown" } diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index a6f8e4e5117da..ddabf13d39810 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -18,6 +18,7 @@ We track the following resources: | GitSSHKey
create |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| | HealthSettings
|
FieldTracked
dismissed_healthcheckstrue
idfalse
| | License
create, delete |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| +| NotificationTemplate
|
FieldTracked
actionstrue
body_templatetrue
grouptrue
idfalse
methodtrue
nametrue
title_templatetrue
| | NotificationsSettings
|
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
|
FieldTracked
callback_urltrue
created_atfalse
icontrue
idfalse
nametrue
updated_atfalse
| | OAuth2ProviderAppSecret
|
FieldTracked
app_idfalse
created_atfalse
display_secretfalse
hashed_secretfalse
idfalse
last_used_atfalse
secret_prefixfalse
| diff --git a/enterprise/audit/diff.go b/enterprise/audit/diff.go index 007f475f6f5eb..07cd8a5fdcb87 100644 --- a/enterprise/audit/diff.go +++ b/enterprise/audit/diff.go @@ -142,6 +142,13 @@ func convertDiffType(left, right any) (newLeft, newRight any, changed bool) { } return leftInt64Ptr, rightInt64Ptr, true + case database.NullNotificationMethod: + vl, vr := string(typedLeft.NotificationMethod), "" + if val, ok := right.(database.NullNotificationMethod); ok { + vr = string(val.NotificationMethod) + } + + return vl, vr, true case database.TemplateACL: return fmt.Sprintf("%+v", left), fmt.Sprintf("%+v", right), true case database.CustomRolePermissions: diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index dcecd88971af8..1a46e561751da 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -272,6 +272,15 @@ var auditableResourcesTypes = map[any]map[string]Action{ "display_name": ActionTrack, "icon": ActionTrack, }, + &database.NotificationTemplate{}: { + "id": ActionIgnore, + "name": ActionTrack, + "title_template": ActionTrack, + "body_template": ActionTrack, + "actions": ActionTrack, + "group": ActionTrack, + "method": ActionTrack, + }, } // auditMap converts a map of struct pointers to a map of struct names as diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 6aa052ad94c2c..4ae1e65b42f0c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2269,6 +2269,7 @@ export type RBACResource = | "file" | "group" | "license" + | "notification_template" | "oauth2_app" | "oauth2_app_code_token" | "oauth2_app_secret" @@ -2296,6 +2297,7 @@ export const RBACResources: RBACResource[] = [ "file", "group", "license", + "notification_template", "oauth2_app", "oauth2_app_code_token", "oauth2_app_secret", From 3ac40be09fbf1d424d92d68d4f2838d70d35e71f Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 23 Jul 2024 14:42:09 +0200 Subject: [PATCH 02/20] Add notification preferences migrations Signed-off-by: Danny Kopping --- coderd/database/dump.sql | 44 +++++++++++++++++- .../000229_notification_preferences.down.sql | 7 +++ .../000229_notification_preferences.up.sql | 45 +++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 coderd/database/migrations/000229_notification_preferences.down.sql create mode 100644 coderd/database/migrations/000229_notification_preferences.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index c3b74732dd825..33eb9dd8f1b07 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -84,7 +84,8 @@ CREATE TYPE notification_message_status AS ENUM ( 'sent', 'permanent_failure', 'temporary_failure', - 'unknown' + 'unknown', + 'inhibited' ); CREATE TYPE notification_method AS ENUM ( @@ -249,6 +250,23 @@ BEGIN END; $$; +CREATE FUNCTION inhibit_enqueue_if_disabled() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN + -- Fail the insertion if the user has disabled this notification + IF EXISTS (SELECT 1 + FROM notification_preferences + WHERE disabled = TRUE + AND user_id = NEW.user_id + AND notification_template_id = NEW.notification_template_id) THEN + RAISE EXCEPTION 'cannot enqueue message: user has disabled this notification'; + END IF; + + RETURN NEW; +END; +$$; + CREATE FUNCTION insert_apikey_fail_if_user_deleted() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -567,17 +585,28 @@ CREATE TABLE notification_messages ( queued_seconds double precision ); +CREATE TABLE notification_preferences ( + user_id uuid NOT NULL, + notification_template_id uuid NOT NULL, + disabled boolean DEFAULT false 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 notification_templates ( id uuid NOT NULL, name text NOT NULL, title_template text NOT NULL, body_template text NOT NULL, actions jsonb, - "group" text + "group" text, + method notification_method ); COMMENT ON TABLE notification_templates IS 'Templates from which to create notification messages.'; +COMMENT ON COLUMN notification_templates.method IS 'NULL defers to the deployment-level method'; + CREATE TABLE oauth2_provider_app_codes ( id uuid NOT NULL, created_at timestamp with time zone NOT NULL, @@ -1641,6 +1670,9 @@ ALTER TABLE ONLY template_versions ALTER TABLE ONLY templates ADD CONSTRAINT templates_pkey PRIMARY KEY (id); +ALTER TABLE ONLY notification_preferences + ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); + ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); @@ -1798,6 +1830,8 @@ CREATE INDEX workspace_resources_job_id_idx ON workspace_resources USING btree ( CREATE UNIQUE INDEX workspaces_owner_id_lower_idx ON workspaces USING btree (owner_id, lower((name)::text)) WHERE (deleted = false); +CREATE TRIGGER inhibit_enqueue_if_disabled_trigger BEFORE INSERT ON notification_messages FOR EACH ROW EXECUTE FUNCTION inhibit_enqueue_if_disabled(); + CREATE TRIGGER tailnet_notify_agent_change AFTER INSERT OR DELETE OR UPDATE ON tailnet_agents FOR EACH ROW EXECUTE FUNCTION tailnet_notify_agent_change(); CREATE TRIGGER tailnet_notify_client_change AFTER INSERT OR DELETE OR UPDATE ON tailnet_clients FOR EACH ROW EXECUTE FUNCTION tailnet_notify_client_change(); @@ -1851,6 +1885,12 @@ ALTER TABLE ONLY notification_messages ALTER TABLE ONLY notification_messages ADD CONSTRAINT notification_messages_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY notification_preferences + ADD CONSTRAINT notification_preferences_notification_template_id_fkey FOREIGN KEY (notification_template_id) REFERENCES notification_templates(id) ON DELETE CASCADE; + +ALTER TABLE ONLY notification_preferences + ADD CONSTRAINT notification_preferences_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ALTER TABLE ONLY oauth2_provider_app_codes ADD CONSTRAINT oauth2_provider_app_codes_app_id_fkey FOREIGN KEY (app_id) REFERENCES oauth2_provider_apps(id) ON DELETE CASCADE; diff --git a/coderd/database/migrations/000229_notification_preferences.down.sql b/coderd/database/migrations/000229_notification_preferences.down.sql new file mode 100644 index 0000000000000..f882f1f59fe59 --- /dev/null +++ b/coderd/database/migrations/000229_notification_preferences.down.sql @@ -0,0 +1,7 @@ +DROP TABLE IF EXISTS notification_preferences; + +ALTER TABLE notification_templates + DROP COLUMN IF EXISTS method; + +DROP TRIGGER IF EXISTS inhibit_enqueue_if_disabled_trigger ON notification_messages; +DROP FUNCTION IF EXISTS inhibit_enqueue_if_disabled; diff --git a/coderd/database/migrations/000229_notification_preferences.up.sql b/coderd/database/migrations/000229_notification_preferences.up.sql new file mode 100644 index 0000000000000..6e85285eb7fa4 --- /dev/null +++ b/coderd/database/migrations/000229_notification_preferences.up.sql @@ -0,0 +1,45 @@ +CREATE TABLE notification_preferences +( + user_id uuid REFERENCES users ON DELETE CASCADE NOT NULL, + notification_template_id uuid REFERENCES notification_templates ON DELETE CASCADE NOT NULL, + disabled bool NOT NULL DEFAULT FALSE, + created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +-- Ensure we cannot insert multiple entries for the same user/template combination +ALTER TABLE notification_preferences + ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); + +-- Allow per-template notification method (enterprise only) +ALTER TABLE notification_templates + ADD COLUMN method notification_method; +COMMENT ON COLUMN notification_templates.method IS 'NULL defers to the deployment-level method'; + +-- No equivalent in down migration because ENUM values cannot be deleted +ALTER TYPE notification_message_status ADD VALUE IF NOT EXISTS 'inhibited'; + +-- Function to prevent enqueuing notifications unnecessarily +CREATE OR REPLACE FUNCTION inhibit_enqueue_if_disabled() + RETURNS TRIGGER AS +$$ +BEGIN + -- Fail the insertion if the user has disabled this notification + IF EXISTS (SELECT 1 + FROM notification_preferences + WHERE disabled = TRUE + AND user_id = NEW.user_id + AND notification_template_id = NEW.notification_template_id) THEN + RAISE EXCEPTION 'cannot enqueue message: user has disabled this notification'; + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +-- Trigger to execute above function on insertion +CREATE TRIGGER inhibit_enqueue_if_disabled_trigger + BEFORE INSERT + ON notification_messages + FOR EACH ROW +EXECUTE FUNCTION inhibit_enqueue_if_disabled(); \ No newline at end of file From 8556c38fca7b01e7e1efbe0b078b08e8798628c6 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 23 Jul 2024 15:37:04 +0200 Subject: [PATCH 03/20] Add preferences queries Signed-off-by: Danny Kopping --- coderd/apidoc/docs.go | 2 + coderd/apidoc/swagger.json | 2 + coderd/database/dbauthz/dbauthz.go | 29 +++++ coderd/database/dbmem/dbmem.go | 71 +++++++++++++ coderd/database/dbmetrics/dbmetrics.go | 28 +++++ coderd/database/dbmock/dbmock.go | 60 +++++++++++ coderd/database/foreign_key_constraint.go | 2 + coderd/database/models.go | 17 ++- coderd/database/querier.go | 6 +- coderd/database/queries.sql.go | 123 ++++++++++++++++++++-- coderd/database/queries/notifications.sql | 46 ++++++-- coderd/database/unique_constraint.go | 1 + coderd/rbac/object_gen.go | 9 ++ coderd/rbac/policy/policy.go | 6 ++ codersdk/rbacresources_gen.go | 106 ++++++++++--------- docs/api/members.md | 3 + docs/api/schemas.md | 1 + 17 files changed, 445 insertions(+), 67 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 981be686df469..5205565b6d281 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11217,6 +11217,7 @@ const docTemplate = `{ "file", "group", "license", + "notification_template", "oauth2_app", "oauth2_app_code_token", "oauth2_app_secret", @@ -11245,6 +11246,7 @@ const docTemplate = `{ "ResourceFile", "ResourceGroup", "ResourceLicense", + "ResourceNotificationTemplate", "ResourceOauth2App", "ResourceOauth2AppCodeToken", "ResourceOauth2AppSecret", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 14efc71711687..3bffe32254841 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10119,6 +10119,7 @@ "file", "group", "license", + "notification_template", "oauth2_app", "oauth2_app_code_token", "oauth2_app_secret", @@ -10147,6 +10148,7 @@ "ResourceFile", "ResourceGroup", "ResourceLicense", + "ResourceNotificationTemplate", "ResourceOauth2App", "ResourceOauth2AppCodeToken", "ResourceOauth2AppSecret", diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 941ab4caccfac..3e2f3ae305230 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1474,6 +1474,13 @@ func (q *querier) GetNotificationMessagesByStatus(ctx context.Context, arg datab return q.db.GetNotificationMessagesByStatus(ctx, arg) } +func (q *querier) GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (database.NotificationTemplate, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceNotificationTemplate); err != nil { + return database.NotificationTemplate{}, err + } + return q.db.GetNotificationTemplateById(ctx, id) +} + func (q *querier) GetNotificationsSettings(ctx context.Context) (string, error) { // No authz checks return q.db.GetNotificationsSettings(ctx) @@ -2085,6 +2092,13 @@ func (q *querier) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([ return q.db.GetUserLinksByUserID(ctx, userID) } +func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]database.NotificationPreference, error) { + if err := q.authorizeContext(ctx, policy.ActionReadPersonal, rbac.ResourceUserObject(userID)); err != nil { + return nil, err + } + return q.db.GetUserNotificationPreferences(ctx, userID) +} + func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { u, err := q.db.GetUserByID(ctx, params.OwnerID) if err != nil { @@ -3011,6 +3025,14 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb return q.db.UpdateMemberRoles(ctx, arg) } +// TODO: how to restrict this to admins? +func (q *querier) UpdateNotificationTemplateMethod(ctx context.Context, arg database.UpdateNotificationTemplateMethodParams) (int64, error) { + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationTemplate); err != nil { + return -1, err + } + return q.db.UpdateNotificationTemplateMethod(ctx, arg) +} + func (q *querier) UpdateOAuth2ProviderAppByID(ctx context.Context, arg database.UpdateOAuth2ProviderAppByIDParams) (database.OAuth2ProviderApp, error) { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceOauth2App); err != nil { return database.OAuth2ProviderApp{}, err @@ -3326,6 +3348,13 @@ func (q *querier) UpdateUserLoginType(ctx context.Context, arg database.UpdateUs return q.db.UpdateUserLoginType(ctx, arg) } +func (q *querier) UpdateUserNotificationPreferences(ctx context.Context, arg database.UpdateUserNotificationPreferencesParams) (int64, error) { + if err := q.authorizeContext(ctx, policy.ActionUpdatePersonal, rbac.ResourceUserObject(arg.UserID)); err != nil { + return -1, err + } + return q.db.UpdateUserNotificationPreferences(ctx, arg) +} + func (q *querier) UpdateUserProfile(ctx context.Context, arg database.UpdateUserProfileParams) (database.User, error) { u, err := q.db.GetUserByID(ctx, arg.ID) if err != nil { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 09c0585964795..430e0ff8f2af7 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -160,6 +160,7 @@ type data struct { jfrogXRayScans []database.JfrogXrayScan licenses []database.License notificationMessages []database.NotificationMessage + notificationPreferences []database.NotificationPreference oauth2ProviderApps []database.OAuth2ProviderApp oauth2ProviderAppSecrets []database.OAuth2ProviderAppSecret oauth2ProviderAppCodes []database.OAuth2ProviderAppCode @@ -2708,6 +2709,10 @@ func (q *FakeQuerier) GetNotificationMessagesByStatus(_ context.Context, arg dat return out, nil } +func (q *FakeQuerier) GetNotificationTemplateById(_ context.Context, id uuid.UUID) (database.NotificationTemplate, error) { + return database.NotificationTemplate{ID: id, Name: "fake"}, nil +} + func (q *FakeQuerier) GetNotificationsSettings(_ context.Context) (string, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -4853,6 +4858,22 @@ func (q *FakeQuerier) GetUserLinksByUserID(_ context.Context, userID uuid.UUID) return uls, nil } +func (q *FakeQuerier) GetUserNotificationPreferences(_ context.Context, userID uuid.UUID) ([]database.NotificationPreference, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + out := make([]database.NotificationPreference, 0, len(q.notificationPreferences)) + for _, np := range q.notificationPreferences { + if np.UserID != userID { + continue + } + + out = append(out, np) + } + + return out, nil +} + func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -7520,6 +7541,15 @@ func (q *FakeQuerier) UpdateMemberRoles(_ context.Context, arg database.UpdateMe return database.OrganizationMember{}, sql.ErrNoRows } +func (q *FakeQuerier) UpdateNotificationTemplateMethod(ctx context.Context, arg database.UpdateNotificationTemplateMethodParams) (int64, error) { + err := validateDatabaseType(arg) + if err != nil { + return 0, err + } + + return 1, nil +} + func (q *FakeQuerier) UpdateOAuth2ProviderAppByID(_ context.Context, arg database.UpdateOAuth2ProviderAppByIDParams) (database.OAuth2ProviderApp, error) { err := validateDatabaseType(arg) if err != nil { @@ -8114,6 +8144,47 @@ func (q *FakeQuerier) UpdateUserLoginType(_ context.Context, arg database.Update return database.User{}, sql.ErrNoRows } +func (q *FakeQuerier) UpdateUserNotificationPreferences(ctx context.Context, arg database.UpdateUserNotificationPreferencesParams) (int64, error) { + err := validateDatabaseType(arg) + if err != nil { + return 0, err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + var upserted int64 + for i, templateID := range arg.NotificationTemplateIds { + var ( + found *database.NotificationPreference + disabled = arg.Disableds[i] + ) + + for _, np := range q.notificationPreferences { + if np.UserID != arg.UserID && np.NotificationTemplateID != templateID { + continue + } + + found = &np + } + + if found != nil { + found.Disabled = disabled + found.UpdatedAt = time.Now() + } else { + q.notificationPreferences = append(q.notificationPreferences, database.NotificationPreference{ + Disabled: disabled, + UserID: arg.UserID, + NotificationTemplateID: templateID, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }) + } + } + + return upserted, nil +} + func (q *FakeQuerier) UpdateUserProfile(_ context.Context, arg database.UpdateUserProfileParams) (database.User, error) { if err := validateDatabaseType(arg); err != nil { return database.User{}, err diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 1a13ff7f0b5a9..339d135e5e6ff 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -746,6 +746,13 @@ func (m metricsStore) GetNotificationMessagesByStatus(ctx context.Context, arg d return r0, r1 } +func (m metricsStore) GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (database.NotificationTemplate, error) { + start := time.Now() + r0, r1 := m.s.GetNotificationTemplateById(ctx, id) + m.queryLatencies.WithLabelValues("GetNotificationTemplateById").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetNotificationsSettings(ctx context.Context) (string, error) { start := time.Now() r0, r1 := m.s.GetNotificationsSettings(ctx) @@ -1222,6 +1229,13 @@ func (m metricsStore) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID return r0, r1 } +func (m metricsStore) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]database.NotificationPreference, error) { + start := time.Now() + r0, r1 := m.s.GetUserNotificationPreferences(ctx, userID) + m.queryLatencies.WithLabelValues("GetUserNotificationPreferences").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { start := time.Now() r0, r1 := m.s.GetUserWorkspaceBuildParameters(ctx, ownerID) @@ -1957,6 +1971,13 @@ func (m metricsStore) UpdateMemberRoles(ctx context.Context, arg database.Update return member, err } +func (m metricsStore) UpdateNotificationTemplateMethod(ctx context.Context, arg database.UpdateNotificationTemplateMethodParams) (int64, error) { + start := time.Now() + r0, r1 := m.s.UpdateNotificationTemplateMethod(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateNotificationTemplateMethod").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) UpdateOAuth2ProviderAppByID(ctx context.Context, arg database.UpdateOAuth2ProviderAppByIDParams) (database.OAuth2ProviderApp, error) { start := time.Now() r0, r1 := m.s.UpdateOAuth2ProviderAppByID(ctx, arg) @@ -2139,6 +2160,13 @@ func (m metricsStore) UpdateUserLoginType(ctx context.Context, arg database.Upda return r0, r1 } +func (m metricsStore) UpdateUserNotificationPreferences(ctx context.Context, arg database.UpdateUserNotificationPreferencesParams) (int64, error) { + start := time.Now() + r0, r1 := m.s.UpdateUserNotificationPreferences(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateUserNotificationPreferences").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) UpdateUserProfile(ctx context.Context, arg database.UpdateUserProfileParams) (database.User, error) { start := time.Now() user, err := m.s.UpdateUserProfile(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index b4aa6043510f1..db29195375a58 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1495,6 +1495,21 @@ func (mr *MockStoreMockRecorder) GetNotificationMessagesByStatus(arg0, arg1 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNotificationMessagesByStatus", reflect.TypeOf((*MockStore)(nil).GetNotificationMessagesByStatus), arg0, arg1) } +// GetNotificationTemplateById mocks base method. +func (m *MockStore) GetNotificationTemplateById(arg0 context.Context, arg1 uuid.UUID) (database.NotificationTemplate, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNotificationTemplateById", arg0, arg1) + ret0, _ := ret[0].(database.NotificationTemplate) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNotificationTemplateById indicates an expected call of GetNotificationTemplateById. +func (mr *MockStoreMockRecorder) GetNotificationTemplateById(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNotificationTemplateById", reflect.TypeOf((*MockStore)(nil).GetNotificationTemplateById), arg0, arg1) +} + // GetNotificationsSettings mocks base method. func (m *MockStore) GetNotificationsSettings(arg0 context.Context) (string, error) { m.ctrl.T.Helper() @@ -2545,6 +2560,21 @@ func (mr *MockStoreMockRecorder) GetUserLinksByUserID(arg0, arg1 any) *gomock.Ca return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserLinksByUserID", reflect.TypeOf((*MockStore)(nil).GetUserLinksByUserID), arg0, arg1) } +// GetUserNotificationPreferences mocks base method. +func (m *MockStore) GetUserNotificationPreferences(arg0 context.Context, arg1 uuid.UUID) ([]database.NotificationPreference, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserNotificationPreferences", arg0, arg1) + ret0, _ := ret[0].([]database.NotificationPreference) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserNotificationPreferences indicates an expected call of GetUserNotificationPreferences. +func (mr *MockStoreMockRecorder) GetUserNotificationPreferences(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserNotificationPreferences", reflect.TypeOf((*MockStore)(nil).GetUserNotificationPreferences), arg0, arg1) +} + // GetUserWorkspaceBuildParameters mocks base method. func (m *MockStore) GetUserWorkspaceBuildParameters(arg0 context.Context, arg1 database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { m.ctrl.T.Helper() @@ -4131,6 +4161,21 @@ func (mr *MockStoreMockRecorder) UpdateMemberRoles(arg0, arg1 any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateMemberRoles", reflect.TypeOf((*MockStore)(nil).UpdateMemberRoles), arg0, arg1) } +// UpdateNotificationTemplateMethod mocks base method. +func (m *MockStore) UpdateNotificationTemplateMethod(arg0 context.Context, arg1 database.UpdateNotificationTemplateMethodParams) (int64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateNotificationTemplateMethod", arg0, arg1) + ret0, _ := ret[0].(int64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateNotificationTemplateMethod indicates an expected call of UpdateNotificationTemplateMethod. +func (mr *MockStoreMockRecorder) UpdateNotificationTemplateMethod(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNotificationTemplateMethod", reflect.TypeOf((*MockStore)(nil).UpdateNotificationTemplateMethod), arg0, arg1) +} + // UpdateOAuth2ProviderAppByID mocks base method. func (m *MockStore) UpdateOAuth2ProviderAppByID(arg0 context.Context, arg1 database.UpdateOAuth2ProviderAppByIDParams) (database.OAuth2ProviderApp, error) { m.ctrl.T.Helper() @@ -4504,6 +4549,21 @@ func (mr *MockStoreMockRecorder) UpdateUserLoginType(arg0, arg1 any) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserLoginType", reflect.TypeOf((*MockStore)(nil).UpdateUserLoginType), arg0, arg1) } +// UpdateUserNotificationPreferences mocks base method. +func (m *MockStore) UpdateUserNotificationPreferences(arg0 context.Context, arg1 database.UpdateUserNotificationPreferencesParams) (int64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateUserNotificationPreferences", arg0, arg1) + ret0, _ := ret[0].(int64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateUserNotificationPreferences indicates an expected call of UpdateUserNotificationPreferences. +func (mr *MockStoreMockRecorder) UpdateUserNotificationPreferences(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserNotificationPreferences", reflect.TypeOf((*MockStore)(nil).UpdateUserNotificationPreferences), arg0, arg1) +} + // UpdateUserProfile mocks base method. func (m *MockStore) UpdateUserProfile(arg0 context.Context, arg1 database.UpdateUserProfileParams) (database.User, error) { m.ctrl.T.Helper() diff --git a/coderd/database/foreign_key_constraint.go b/coderd/database/foreign_key_constraint.go index 6e6eef8862b72..011d39bdc5b91 100644 --- a/coderd/database/foreign_key_constraint.go +++ b/coderd/database/foreign_key_constraint.go @@ -17,6 +17,8 @@ const ( ForeignKeyJfrogXrayScansWorkspaceID ForeignKeyConstraint = "jfrog_xray_scans_workspace_id_fkey" // ALTER TABLE ONLY jfrog_xray_scans ADD CONSTRAINT jfrog_xray_scans_workspace_id_fkey FOREIGN KEY (workspace_id) REFERENCES workspaces(id) ON DELETE CASCADE; ForeignKeyNotificationMessagesNotificationTemplateID ForeignKeyConstraint = "notification_messages_notification_template_id_fkey" // ALTER TABLE ONLY notification_messages ADD CONSTRAINT notification_messages_notification_template_id_fkey FOREIGN KEY (notification_template_id) REFERENCES notification_templates(id) ON DELETE CASCADE; ForeignKeyNotificationMessagesUserID ForeignKeyConstraint = "notification_messages_user_id_fkey" // ALTER TABLE ONLY notification_messages ADD CONSTRAINT notification_messages_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ForeignKeyNotificationPreferencesNotificationTemplateID ForeignKeyConstraint = "notification_preferences_notification_template_id_fkey" // ALTER TABLE ONLY notification_preferences ADD CONSTRAINT notification_preferences_notification_template_id_fkey FOREIGN KEY (notification_template_id) REFERENCES notification_templates(id) ON DELETE CASCADE; + ForeignKeyNotificationPreferencesUserID ForeignKeyConstraint = "notification_preferences_user_id_fkey" // ALTER TABLE ONLY notification_preferences ADD CONSTRAINT notification_preferences_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; ForeignKeyOauth2ProviderAppCodesAppID ForeignKeyConstraint = "oauth2_provider_app_codes_app_id_fkey" // ALTER TABLE ONLY oauth2_provider_app_codes ADD CONSTRAINT oauth2_provider_app_codes_app_id_fkey FOREIGN KEY (app_id) REFERENCES oauth2_provider_apps(id) ON DELETE CASCADE; ForeignKeyOauth2ProviderAppCodesUserID ForeignKeyConstraint = "oauth2_provider_app_codes_user_id_fkey" // ALTER TABLE ONLY oauth2_provider_app_codes ADD CONSTRAINT oauth2_provider_app_codes_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; ForeignKeyOauth2ProviderAppSecretsAppID ForeignKeyConstraint = "oauth2_provider_app_secrets_app_id_fkey" // ALTER TABLE ONLY oauth2_provider_app_secrets ADD CONSTRAINT oauth2_provider_app_secrets_app_id_fkey FOREIGN KEY (app_id) REFERENCES oauth2_provider_apps(id) ON DELETE CASCADE; diff --git a/coderd/database/models.go b/coderd/database/models.go index 2ac87d6ca80b5..175c1480e5159 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.25.0 +// sqlc v1.26.0 package database @@ -669,6 +669,7 @@ const ( NotificationMessageStatusPermanentFailure NotificationMessageStatus = "permanent_failure" NotificationMessageStatusTemporaryFailure NotificationMessageStatus = "temporary_failure" NotificationMessageStatusUnknown NotificationMessageStatus = "unknown" + NotificationMessageStatusInhibited NotificationMessageStatus = "inhibited" ) func (e *NotificationMessageStatus) Scan(src interface{}) error { @@ -713,7 +714,8 @@ func (e NotificationMessageStatus) Valid() bool { NotificationMessageStatusSent, NotificationMessageStatusPermanentFailure, NotificationMessageStatusTemporaryFailure, - NotificationMessageStatusUnknown: + NotificationMessageStatusUnknown, + NotificationMessageStatusInhibited: return true } return false @@ -727,6 +729,7 @@ func AllNotificationMessageStatusValues() []NotificationMessageStatus { NotificationMessageStatusPermanentFailure, NotificationMessageStatusTemporaryFailure, NotificationMessageStatusUnknown, + NotificationMessageStatusInhibited, } } @@ -2037,6 +2040,14 @@ type NotificationMessage struct { QueuedSeconds sql.NullFloat64 `db:"queued_seconds" json:"queued_seconds"` } +type NotificationPreference struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + NotificationTemplateID uuid.UUID `db:"notification_template_id" json:"notification_template_id"` + Disabled bool `db:"disabled" json:"disabled"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` +} + // Templates from which to create notification messages. type NotificationTemplate struct { ID uuid.UUID `db:"id" json:"id"` @@ -2045,6 +2056,8 @@ type NotificationTemplate struct { BodyTemplate string `db:"body_template" json:"body_template"` Actions []byte `db:"actions" json:"actions"` Group sql.NullString `db:"group" json:"group"` + // NULL defers to the deployment-level method + Method NullNotificationMethod `db:"method" json:"method"` } // A table used to configure apps that can use Coder as an OAuth2 provider, the reverse of what we are calling external authentication. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 95015aa706348..4da7b76f978ef 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.25.0 +// sqlc v1.26.0 package database @@ -162,6 +162,7 @@ type sqlcQuerier interface { GetLicenses(ctx context.Context) ([]License, error) GetLogoURL(ctx context.Context) (string, error) GetNotificationMessagesByStatus(ctx context.Context, arg GetNotificationMessagesByStatusParams) ([]NotificationMessage, error) + GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (NotificationTemplate, error) GetNotificationsSettings(ctx context.Context) (string, error) GetOAuth2ProviderAppByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderApp, error) GetOAuth2ProviderAppCodeByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderAppCode, error) @@ -265,6 +266,7 @@ type sqlcQuerier interface { GetUserLinkByLinkedID(ctx context.Context, linkedID string) (UserLink, error) 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) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) @@ -401,6 +403,7 @@ type sqlcQuerier interface { UpdateGroupByID(ctx context.Context, arg UpdateGroupByIDParams) (Group, error) UpdateInactiveUsersToDormant(ctx context.Context, arg UpdateInactiveUsersToDormantParams) ([]UpdateInactiveUsersToDormantRow, error) UpdateMemberRoles(ctx context.Context, arg UpdateMemberRolesParams) (OrganizationMember, error) + UpdateNotificationTemplateMethod(ctx context.Context, arg UpdateNotificationTemplateMethodParams) (int64, error) UpdateOAuth2ProviderAppByID(ctx context.Context, arg UpdateOAuth2ProviderAppByIDParams) (OAuth2ProviderApp, error) UpdateOAuth2ProviderAppSecretByID(ctx context.Context, arg UpdateOAuth2ProviderAppSecretByIDParams) (OAuth2ProviderAppSecret, error) UpdateOrganization(ctx context.Context, arg UpdateOrganizationParams) (Organization, error) @@ -427,6 +430,7 @@ type sqlcQuerier interface { UpdateUserLink(ctx context.Context, arg UpdateUserLinkParams) (UserLink, error) UpdateUserLinkedID(ctx context.Context, arg UpdateUserLinkedIDParams) (UserLink, error) UpdateUserLoginType(ctx context.Context, arg UpdateUserLoginTypeParams) (User, error) + UpdateUserNotificationPreferences(ctx context.Context, arg UpdateUserNotificationPreferencesParams) (int64, error) UpdateUserProfile(ctx context.Context, arg UpdateUserProfileParams) (User, error) UpdateUserQuietHoursSchedule(ctx context.Context, arg UpdateUserQuietHoursScheduleParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4e7e0ceb3150d..3c744e358c8f7 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.25.0 +// sqlc v1.26.0 package database @@ -3335,14 +3335,18 @@ SELECT nm.id, nm.payload, nm.method, - nm.attempt_count::int AS attempt_count, - nm.queued_seconds::float AS queued_seconds, + nm.attempt_count::int AS attempt_count, + nm.queued_seconds::float AS queued_seconds, -- template - nt.id AS template_id, + nt.id AS template_id, nt.title_template, - nt.body_template + nt.body_template, + -- preferences + (CASE WHEN np.disabled IS NULL THEN false ELSE np.disabled END)::bool AS disabled FROM acquired nm JOIN notification_templates nt ON nm.notification_template_id = nt.id + LEFT JOIN notification_preferences AS np + ON (np.user_id = nm.user_id AND np.notification_template_id = nm.notification_template_id) ` type AcquireNotificationMessagesParams struct { @@ -3361,6 +3365,7 @@ type AcquireNotificationMessagesRow struct { TemplateID uuid.UUID `db:"template_id" json:"template_id"` TitleTemplate string `db:"title_template" json:"title_template"` BodyTemplate string `db:"body_template" json:"body_template"` + Disabled bool `db:"disabled" json:"disabled"` } // Acquires the lease for a given count of notification messages, to enable concurrent dequeuing and subsequent sending. @@ -3396,6 +3401,7 @@ func (q *sqlQuerier) AcquireNotificationMessages(ctx context.Context, arg Acquir &i.TemplateID, &i.TitleTemplate, &i.BodyTemplate, + &i.Disabled, ); err != nil { return nil, err } @@ -3574,7 +3580,10 @@ func (q *sqlQuerier) FetchNewMessageMetadata(ctx context.Context, arg FetchNewMe } const getNotificationMessagesByStatus = `-- name: GetNotificationMessagesByStatus :many -SELECT id, notification_template_id, user_id, method, status, status_reason, created_by, payload, attempt_count, targets, created_at, updated_at, leased_until, next_retry_after, queued_seconds FROM notification_messages WHERE status = $1 LIMIT $2::int +SELECT id, notification_template_id, user_id, method, status, status_reason, created_by, payload, attempt_count, targets, created_at, updated_at, leased_until, next_retry_after, queued_seconds +FROM notification_messages +WHERE status = $1 +LIMIT $2::int ` type GetNotificationMessagesByStatusParams struct { @@ -3621,6 +3630,108 @@ func (q *sqlQuerier) GetNotificationMessagesByStatus(ctx context.Context, arg Ge return items, nil } +const getNotificationTemplateById = `-- name: GetNotificationTemplateById :one +SELECT id, name, title_template, body_template, actions, "group", method +FROM notification_templates +WHERE id = $1::uuid +` + +func (q *sqlQuerier) GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (NotificationTemplate, error) { + row := q.db.QueryRowContext(ctx, getNotificationTemplateById, id) + var i NotificationTemplate + err := row.Scan( + &i.ID, + &i.Name, + &i.TitleTemplate, + &i.BodyTemplate, + &i.Actions, + &i.Group, + &i.Method, + ) + return i, err +} + +const getUserNotificationPreferences = `-- name: GetUserNotificationPreferences :many +SELECT user_id, notification_template_id, disabled, created_at, updated_at +FROM notification_preferences +WHERE user_id = $1::uuid +` + +func (q *sqlQuerier) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) { + rows, err := q.db.QueryContext(ctx, getUserNotificationPreferences, userID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []NotificationPreference + for rows.Next() { + var i NotificationPreference + if err := rows.Scan( + &i.UserID, + &i.NotificationTemplateID, + &i.Disabled, + &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 updateNotificationTemplateMethod = `-- name: UpdateNotificationTemplateMethod :execrows +UPDATE notification_templates +SET method = $1::notification_method +WHERE id = $2::uuid +` + +type UpdateNotificationTemplateMethodParams struct { + Method NullNotificationMethod `db:"method" json:"method"` + ID uuid.UUID `db:"id" json:"id"` +} + +func (q *sqlQuerier) UpdateNotificationTemplateMethod(ctx context.Context, arg UpdateNotificationTemplateMethodParams) (int64, error) { + result, err := q.db.ExecContext(ctx, updateNotificationTemplateMethod, arg.Method, arg.ID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const updateUserNotificationPreferences = `-- name: UpdateUserNotificationPreferences :execrows +WITH new_values AS + (SELECT UNNEST($2::uuid[]) AS notification_template_id, + UNNEST($3::bool[]) AS disabled) +INSERT +INTO notification_preferences (user_id, notification_template_id, disabled) +SELECT $1::uuid, new_values.notification_template_id, new_values.disabled +FROM new_values +ON CONFLICT (user_id, notification_template_id) DO UPDATE + SET disabled = EXCLUDED.disabled, + updated_at = CURRENT_TIMESTAMP +` + +type UpdateUserNotificationPreferencesParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + NotificationTemplateIds []uuid.UUID `db:"notification_template_ids" json:"notification_template_ids"` + Disableds []bool `db:"disableds" json:"disableds"` +} + +func (q *sqlQuerier) UpdateUserNotificationPreferences(ctx context.Context, arg UpdateUserNotificationPreferencesParams) (int64, error) { + result, err := q.db.ExecContext(ctx, updateUserNotificationPreferences, arg.UserID, pq.Array(arg.NotificationTemplateIds), pq.Array(arg.Disableds)) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + const deleteOAuth2ProviderAppByID = `-- name: DeleteOAuth2ProviderAppByID :exec DELETE FROM oauth2_provider_apps WHERE id = $1 ` diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index c0a2f25323957..6476edaaabd41 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -79,14 +79,18 @@ SELECT nm.id, nm.payload, nm.method, - nm.attempt_count::int AS attempt_count, - nm.queued_seconds::float AS queued_seconds, + nm.attempt_count::int AS attempt_count, + nm.queued_seconds::float AS queued_seconds, -- template - nt.id AS template_id, + nt.id AS template_id, nt.title_template, - nt.body_template + nt.body_template, + -- preferences + (CASE WHEN np.disabled IS NULL THEN false ELSE np.disabled END)::bool AS disabled FROM acquired nm - JOIN notification_templates nt ON nm.notification_template_id = nt.id; + JOIN notification_templates nt ON nm.notification_template_id = nt.id + LEFT JOIN notification_preferences AS np + ON (np.user_id = nm.user_id AND np.notification_template_id = nm.notification_template_id); -- name: BulkMarkNotificationMessagesFailed :execrows UPDATE notification_messages @@ -131,4 +135,34 @@ WHERE id IN WHERE nested.updated_at < NOW() - INTERVAL '7 days'); -- name: GetNotificationMessagesByStatus :many -SELECT * FROM notification_messages WHERE status = @status LIMIT sqlc.arg('limit')::int; +SELECT * +FROM notification_messages +WHERE status = @status +LIMIT sqlc.arg('limit')::int; + +-- name: GetUserNotificationPreferences :many +SELECT * +FROM notification_preferences +WHERE user_id = @user_id::uuid; + +-- name: UpdateUserNotificationPreferences :execrows +WITH new_values AS + (SELECT UNNEST(@notification_template_ids::uuid[]) AS notification_template_id, + UNNEST(@disableds::bool[]) AS disabled) +INSERT +INTO notification_preferences (user_id, notification_template_id, disabled) +SELECT @user_id::uuid, new_values.notification_template_id, new_values.disabled +FROM new_values +ON CONFLICT (user_id, notification_template_id) DO UPDATE + SET disabled = EXCLUDED.disabled, + updated_at = CURRENT_TIMESTAMP; + +-- name: UpdateNotificationTemplateMethod :execrows +UPDATE notification_templates +SET method = sqlc.narg('method')::notification_method +WHERE id = @id::uuid; + +-- name: GetNotificationTemplateById :one +SELECT * +FROM notification_templates +WHERE id = @id::uuid; diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index aecae02d572ff..f5c7254361b00 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -59,6 +59,7 @@ const ( UniqueTemplateVersionsPkey UniqueConstraint = "template_versions_pkey" // ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_pkey PRIMARY KEY (id); UniqueTemplateVersionsTemplateIDNameKey UniqueConstraint = "template_versions_template_id_name_key" // ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_template_id_name_key UNIQUE (template_id, name); UniqueTemplatesPkey UniqueConstraint = "templates_pkey" // ALTER TABLE ONLY templates ADD CONSTRAINT templates_pkey PRIMARY KEY (id); + UniqueUniqueUserNotificationTemplate UniqueConstraint = "unique_user_notification_template" // ALTER TABLE ONLY notification_preferences ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); UniqueUserLinksPkey UniqueConstraint = "user_links_pkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); UniqueUsersPkey UniqueConstraint = "users_pkey" // ALTER TABLE ONLY users ADD CONSTRAINT users_pkey PRIMARY KEY (id); UniqueWorkspaceAgentLogSourcesPkey UniqueConstraint = "workspace_agent_log_sources_pkey" // ALTER TABLE ONLY workspace_agent_log_sources ADD CONSTRAINT workspace_agent_log_sources_pkey PRIMARY KEY (workspace_agent_id, id); diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index bc2846da49564..938e116479234 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -102,6 +102,14 @@ var ( Type: "license", } + // ResourceNotificationTemplate + // Valid Actions + // - "ActionRead" :: read notification templates + // - "ActionUpdate" :: update notification templates + ResourceNotificationTemplate = Object{ + Type: "notification_template", + } + // ResourceOauth2App // Valid Actions // - "ActionCreate" :: make an OAuth2 app. @@ -272,6 +280,7 @@ func AllResources() []Objecter { ResourceFile, ResourceGroup, ResourceLicense, + ResourceNotificationTemplate, ResourceOauth2App, ResourceOauth2AppCodeToken, ResourceOauth2AppSecret, diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 2390c9e30c785..566185283a55f 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -255,4 +255,10 @@ var RBACPermissions = map[string]PermissionDefinition{ ActionDelete: actDef(""), }, }, + "notification_template": { + Actions: map[Action]ActionDefinition{ + ActionRead: actDef("read notification templates"), + ActionUpdate: actDef("update notification templates"), + }, + }, } diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index 573fea66b8c80..009b1f4e7c735 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -4,32 +4,33 @@ package codersdk type RBACResource string const ( - ResourceWildcard RBACResource = "*" - ResourceApiKey RBACResource = "api_key" - ResourceAssignOrgRole RBACResource = "assign_org_role" - ResourceAssignRole RBACResource = "assign_role" - ResourceAuditLog RBACResource = "audit_log" - ResourceDebugInfo RBACResource = "debug_info" - ResourceDeploymentConfig RBACResource = "deployment_config" - ResourceDeploymentStats RBACResource = "deployment_stats" - ResourceFile RBACResource = "file" - ResourceGroup RBACResource = "group" - ResourceLicense RBACResource = "license" - ResourceOauth2App RBACResource = "oauth2_app" - ResourceOauth2AppCodeToken RBACResource = "oauth2_app_code_token" - ResourceOauth2AppSecret RBACResource = "oauth2_app_secret" - ResourceOrganization RBACResource = "organization" - ResourceOrganizationMember RBACResource = "organization_member" - ResourceProvisionerDaemon RBACResource = "provisioner_daemon" - ResourceProvisionerKeys RBACResource = "provisioner_keys" - ResourceReplicas RBACResource = "replicas" - ResourceSystem RBACResource = "system" - ResourceTailnetCoordinator RBACResource = "tailnet_coordinator" - ResourceTemplate RBACResource = "template" - ResourceUser RBACResource = "user" - ResourceWorkspace RBACResource = "workspace" - ResourceWorkspaceDormant RBACResource = "workspace_dormant" - ResourceWorkspaceProxy RBACResource = "workspace_proxy" + ResourceWildcard RBACResource = "*" + ResourceApiKey RBACResource = "api_key" + ResourceAssignOrgRole RBACResource = "assign_org_role" + ResourceAssignRole RBACResource = "assign_role" + ResourceAuditLog RBACResource = "audit_log" + ResourceDebugInfo RBACResource = "debug_info" + ResourceDeploymentConfig RBACResource = "deployment_config" + ResourceDeploymentStats RBACResource = "deployment_stats" + ResourceFile RBACResource = "file" + ResourceGroup RBACResource = "group" + ResourceLicense RBACResource = "license" + ResourceNotificationTemplate RBACResource = "notification_template" + ResourceOauth2App RBACResource = "oauth2_app" + ResourceOauth2AppCodeToken RBACResource = "oauth2_app_code_token" + ResourceOauth2AppSecret RBACResource = "oauth2_app_secret" + ResourceOrganization RBACResource = "organization" + ResourceOrganizationMember RBACResource = "organization_member" + ResourceProvisionerDaemon RBACResource = "provisioner_daemon" + ResourceProvisionerKeys RBACResource = "provisioner_keys" + ResourceReplicas RBACResource = "replicas" + ResourceSystem RBACResource = "system" + ResourceTailnetCoordinator RBACResource = "tailnet_coordinator" + ResourceTemplate RBACResource = "template" + ResourceUser RBACResource = "user" + ResourceWorkspace RBACResource = "workspace" + ResourceWorkspaceDormant RBACResource = "workspace_dormant" + ResourceWorkspaceProxy RBACResource = "workspace_proxy" ) type RBACAction string @@ -53,30 +54,31 @@ const ( // RBACResourceActions is the mapping of resources to which actions are valid for // said resource type. var RBACResourceActions = map[RBACResource][]RBACAction{ - ResourceWildcard: {}, - ResourceApiKey: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceAssignOrgRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, - ResourceAssignRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, - ResourceAuditLog: {ActionCreate, ActionRead}, - ResourceDebugInfo: {ActionRead}, - ResourceDeploymentConfig: {ActionRead, ActionUpdate}, - ResourceDeploymentStats: {ActionRead}, - ResourceFile: {ActionCreate, ActionRead}, - ResourceGroup: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceLicense: {ActionCreate, ActionDelete, ActionRead}, - ResourceOauth2App: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceOauth2AppCodeToken: {ActionCreate, ActionDelete, ActionRead}, - ResourceOauth2AppSecret: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceOrganization: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceOrganizationMember: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceProvisionerDaemon: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceProvisionerKeys: {ActionCreate, ActionDelete, ActionRead}, - ResourceReplicas: {ActionRead}, - ResourceSystem: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceTailnetCoordinator: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceTemplate: {ActionCreate, ActionDelete, ActionRead, ActionUpdate, ActionViewInsights}, - ResourceUser: {ActionCreate, ActionDelete, ActionRead, ActionReadPersonal, ActionUpdate, ActionUpdatePersonal}, - ResourceWorkspace: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, - ResourceWorkspaceDormant: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, - ResourceWorkspaceProxy: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceWildcard: {}, + ResourceApiKey: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceAssignOrgRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, + ResourceAssignRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, + ResourceAuditLog: {ActionCreate, ActionRead}, + ResourceDebugInfo: {ActionRead}, + ResourceDeploymentConfig: {ActionRead, ActionUpdate}, + ResourceDeploymentStats: {ActionRead}, + ResourceFile: {ActionCreate, ActionRead}, + ResourceGroup: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceLicense: {ActionCreate, ActionDelete, ActionRead}, + ResourceNotificationTemplate: {ActionRead, ActionUpdate}, + ResourceOauth2App: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOauth2AppCodeToken: {ActionCreate, ActionDelete, ActionRead}, + ResourceOauth2AppSecret: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOrganization: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOrganizationMember: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceProvisionerDaemon: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceProvisionerKeys: {ActionCreate, ActionDelete, ActionRead}, + ResourceReplicas: {ActionRead}, + ResourceSystem: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceTailnetCoordinator: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceTemplate: {ActionCreate, ActionDelete, ActionRead, ActionUpdate, ActionViewInsights}, + ResourceUser: {ActionCreate, ActionDelete, ActionRead, ActionReadPersonal, ActionUpdate, ActionUpdatePersonal}, + ResourceWorkspace: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, + ResourceWorkspaceDormant: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, + ResourceWorkspaceProxy: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, } diff --git a/docs/api/members.md b/docs/api/members.md index 1ecf490738f00..b0cd8f956a475 100644 --- a/docs/api/members.md +++ b/docs/api/members.md @@ -190,6 +190,7 @@ Status Code **200** | `resource_type` | `file` | | `resource_type` | `group` | | `resource_type` | `license` | +| `resource_type` | `notification_template` | | `resource_type` | `oauth2_app` | | `resource_type` | `oauth2_app_code_token` | | `resource_type` | `oauth2_app_secret` | @@ -313,6 +314,7 @@ Status Code **200** | `resource_type` | `file` | | `resource_type` | `group` | | `resource_type` | `license` | +| `resource_type` | `notification_template` | | `resource_type` | `oauth2_app` | | `resource_type` | `oauth2_app_code_token` | | `resource_type` | `oauth2_app_secret` | @@ -567,6 +569,7 @@ Status Code **200** | `resource_type` | `file` | | `resource_type` | `group` | | `resource_type` | `license` | +| `resource_type` | `notification_template` | | `resource_type` | `oauth2_app` | | `resource_type` | `oauth2_app_code_token` | | `resource_type` | `oauth2_app_secret` | diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 53ad820daf60c..46c9b6aa4d338 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -4166,6 +4166,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `file` | | `group` | | `license` | +| `notification_template` | | `oauth2_app` | | `oauth2_app_code_token` | | `oauth2_app_secret` | From 9e0adbf8884515f3d6f4cfd52b36df333d2ae3fd Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 25 Jul 2024 14:33:09 +0200 Subject: [PATCH 04/20] Allow notification templates to be auditable Signed-off-by: Danny Kopping --- coderd/database/dbauthz/dbauthz.go | 6 ++--- coderd/database/dbmem/dbmem.go | 9 ++++--- coderd/database/dbmetrics/dbmetrics.go | 6 ++--- ... 000231_notification_preferences.down.sql} | 0 ...=> 000231_notification_preferences.up.sql} | 17 +++++++------ coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 24 ++++++++++++------- coderd/database/queries/notifications.sql | 5 ++-- 8 files changed, 42 insertions(+), 27 deletions(-) rename coderd/database/migrations/{000229_notification_preferences.down.sql => 000231_notification_preferences.down.sql} (100%) rename coderd/database/migrations/{000229_notification_preferences.up.sql => 000231_notification_preferences.up.sql} (81%) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 3e2f3ae305230..81f8db294307c 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3026,11 +3026,11 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb } // TODO: how to restrict this to admins? -func (q *querier) UpdateNotificationTemplateMethod(ctx context.Context, arg database.UpdateNotificationTemplateMethodParams) (int64, error) { +func (q *querier) UpdateNotificationTemplateMethodById(ctx context.Context, arg database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationTemplate); err != nil { - return -1, err + return database.NotificationTemplate{}, err } - return q.db.UpdateNotificationTemplateMethod(ctx, arg) + return q.db.UpdateNotificationTemplateMethodById(ctx, arg) } func (q *querier) UpdateOAuth2ProviderAppByID(ctx context.Context, arg database.UpdateOAuth2ProviderAppByIDParams) (database.OAuth2ProviderApp, error) { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 430e0ff8f2af7..a5324ca02512e 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -7541,13 +7541,16 @@ func (q *FakeQuerier) UpdateMemberRoles(_ context.Context, arg database.UpdateMe return database.OrganizationMember{}, sql.ErrNoRows } -func (q *FakeQuerier) UpdateNotificationTemplateMethod(ctx context.Context, arg database.UpdateNotificationTemplateMethodParams) (int64, error) { +func (q *FakeQuerier) UpdateNotificationTemplateMethodById(_ context.Context, arg database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { err := validateDatabaseType(arg) if err != nil { - return 0, err + return database.NotificationTemplate{}, err } - return 1, nil + return database.NotificationTemplate{ + ID: arg.ID, + Method: arg.Method, + }, nil } func (q *FakeQuerier) UpdateOAuth2ProviderAppByID(_ context.Context, arg database.UpdateOAuth2ProviderAppByIDParams) (database.OAuth2ProviderApp, error) { diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 339d135e5e6ff..c0262013a863e 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1971,10 +1971,10 @@ func (m metricsStore) UpdateMemberRoles(ctx context.Context, arg database.Update return member, err } -func (m metricsStore) UpdateNotificationTemplateMethod(ctx context.Context, arg database.UpdateNotificationTemplateMethodParams) (int64, error) { +func (m metricsStore) UpdateNotificationTemplateMethodById(ctx context.Context, arg database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { start := time.Now() - r0, r1 := m.s.UpdateNotificationTemplateMethod(ctx, arg) - m.queryLatencies.WithLabelValues("UpdateNotificationTemplateMethod").Observe(time.Since(start).Seconds()) + r0, r1 := m.s.UpdateNotificationTemplateMethodById(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateNotificationTemplateMethodById").Observe(time.Since(start).Seconds()) return r0, r1 } diff --git a/coderd/database/migrations/000229_notification_preferences.down.sql b/coderd/database/migrations/000231_notification_preferences.down.sql similarity index 100% rename from coderd/database/migrations/000229_notification_preferences.down.sql rename to coderd/database/migrations/000231_notification_preferences.down.sql diff --git a/coderd/database/migrations/000229_notification_preferences.up.sql b/coderd/database/migrations/000231_notification_preferences.up.sql similarity index 81% rename from coderd/database/migrations/000229_notification_preferences.up.sql rename to coderd/database/migrations/000231_notification_preferences.up.sql index 6e85285eb7fa4..604009b334163 100644 --- a/coderd/database/migrations/000229_notification_preferences.up.sql +++ b/coderd/database/migrations/000231_notification_preferences.up.sql @@ -7,24 +7,24 @@ CREATE TABLE notification_preferences updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP ); --- Ensure we cannot insert multiple entries for the same user/template combination +-- Ensure we cannot insert multiple entries for the same user/template combination. ALTER TABLE notification_preferences ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); --- Allow per-template notification method (enterprise only) +-- Allow per-template notification method (enterprise only). ALTER TABLE notification_templates ADD COLUMN method notification_method; COMMENT ON COLUMN notification_templates.method IS 'NULL defers to the deployment-level method'; --- No equivalent in down migration because ENUM values cannot be deleted +-- No equivalent in down migration because ENUM values cannot be deleted. ALTER TYPE notification_message_status ADD VALUE IF NOT EXISTS 'inhibited'; --- Function to prevent enqueuing notifications unnecessarily +-- Function to prevent enqueuing notifications unnecessarily. CREATE OR REPLACE FUNCTION inhibit_enqueue_if_disabled() RETURNS TRIGGER AS $$ BEGIN - -- Fail the insertion if the user has disabled this notification + -- Fail the insertion if the user has disabled this notification. IF EXISTS (SELECT 1 FROM notification_preferences WHERE disabled = TRUE @@ -37,9 +37,12 @@ BEGIN END; $$ LANGUAGE plpgsql; --- Trigger to execute above function on insertion +-- Trigger to execute above function on insertion. CREATE TRIGGER inhibit_enqueue_if_disabled_trigger BEFORE INSERT ON notification_messages FOR EACH ROW -EXECUTE FUNCTION inhibit_enqueue_if_disabled(); \ No newline at end of file +EXECUTE FUNCTION inhibit_enqueue_if_disabled(); + +-- Allow modifications to notification templates to be audited. +ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'notification_template'; \ No newline at end of file diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 4da7b76f978ef..b7cb50fa4d4c6 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -403,7 +403,7 @@ type sqlcQuerier interface { UpdateGroupByID(ctx context.Context, arg UpdateGroupByIDParams) (Group, error) UpdateInactiveUsersToDormant(ctx context.Context, arg UpdateInactiveUsersToDormantParams) ([]UpdateInactiveUsersToDormantRow, error) UpdateMemberRoles(ctx context.Context, arg UpdateMemberRolesParams) (OrganizationMember, error) - UpdateNotificationTemplateMethod(ctx context.Context, arg UpdateNotificationTemplateMethodParams) (int64, error) + UpdateNotificationTemplateMethodById(ctx context.Context, arg UpdateNotificationTemplateMethodByIdParams) (NotificationTemplate, error) UpdateOAuth2ProviderAppByID(ctx context.Context, arg UpdateOAuth2ProviderAppByIDParams) (OAuth2ProviderApp, error) UpdateOAuth2ProviderAppSecretByID(ctx context.Context, arg UpdateOAuth2ProviderAppSecretByIDParams) (OAuth2ProviderAppSecret, error) UpdateOrganization(ctx context.Context, arg UpdateOrganizationParams) (Organization, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3c744e358c8f7..c47940fc293e6 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3686,23 +3686,31 @@ func (q *sqlQuerier) GetUserNotificationPreferences(ctx context.Context, userID return items, nil } -const updateNotificationTemplateMethod = `-- name: UpdateNotificationTemplateMethod :execrows +const updateNotificationTemplateMethodById = `-- name: UpdateNotificationTemplateMethodById :one UPDATE notification_templates SET method = $1::notification_method WHERE id = $2::uuid +RETURNING id, name, title_template, body_template, actions, "group", method ` -type UpdateNotificationTemplateMethodParams struct { +type UpdateNotificationTemplateMethodByIdParams struct { Method NullNotificationMethod `db:"method" json:"method"` ID uuid.UUID `db:"id" json:"id"` } -func (q *sqlQuerier) UpdateNotificationTemplateMethod(ctx context.Context, arg UpdateNotificationTemplateMethodParams) (int64, error) { - result, err := q.db.ExecContext(ctx, updateNotificationTemplateMethod, arg.Method, arg.ID) - if err != nil { - return 0, err - } - return result.RowsAffected() +func (q *sqlQuerier) UpdateNotificationTemplateMethodById(ctx context.Context, arg UpdateNotificationTemplateMethodByIdParams) (NotificationTemplate, error) { + row := q.db.QueryRowContext(ctx, updateNotificationTemplateMethodById, arg.Method, arg.ID) + var i NotificationTemplate + err := row.Scan( + &i.ID, + &i.Name, + &i.TitleTemplate, + &i.BodyTemplate, + &i.Actions, + &i.Group, + &i.Method, + ) + return i, err } const updateUserNotificationPreferences = `-- name: UpdateUserNotificationPreferences :execrows diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index 6476edaaabd41..13fb38721c108 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -157,10 +157,11 @@ ON CONFLICT (user_id, notification_template_id) DO UPDATE SET disabled = EXCLUDED.disabled, updated_at = CURRENT_TIMESTAMP; --- name: UpdateNotificationTemplateMethod :execrows +-- name: UpdateNotificationTemplateMethodById :one UPDATE notification_templates SET method = sqlc.narg('method')::notification_method -WHERE id = @id::uuid; +WHERE id = @id::uuid +RETURNING *; -- name: GetNotificationTemplateById :one SELECT * From 88294a3b1c3610d2eb4693ead266b289009db0b3 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 26 Jul 2024 11:51:59 +0200 Subject: [PATCH 05/20] Add kind to notification_templates and query to retrieve templates by kind Signed-off-by: Danny Kopping --- coderd/database/dbauthz/dbauthz.go | 10 +++- coderd/database/dbmem/dbmem.go | 20 +++---- coderd/database/dbmetrics/dbmetrics.go | 7 +++ coderd/database/dbmock/dbmock.go | 29 +++++++--- coderd/database/dump.sql | 12 +++- ... 000232_notification_preferences.down.sql} | 8 ++- ...=> 000232_notification_preferences.up.sql} | 13 ++++- coderd/database/models.go | 58 ++++++++++++++++++- coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 43 +++++++++++++- coderd/database/queries/notifications.sql | 4 ++ docs/admin/audit-logs.md | 2 +- enterprise/audit/table.go | 1 + site/src/api/rbacresources_gen.ts | 4 ++ 14 files changed, 179 insertions(+), 33 deletions(-) rename coderd/database/migrations/{000231_notification_preferences.down.sql => 000232_notification_preferences.down.sql} (66%) rename coderd/database/migrations/{000231_notification_preferences.up.sql => 000232_notification_preferences.up.sql} (81%) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 81f8db294307c..629b7c213fa6e 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1481,6 +1481,14 @@ func (q *querier) GetNotificationTemplateById(ctx context.Context, id uuid.UUID) return q.db.GetNotificationTemplateById(ctx, id) } +func (q *querier) GetNotificationTemplatesByKind(ctx context.Context, kind database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { + // TODO: restrict 'system' kind to admins only? + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceNotificationTemplate); err != nil { + return nil, err + } + return q.db.GetNotificationTemplatesByKind(ctx, kind) +} + func (q *querier) GetNotificationsSettings(ctx context.Context) (string, error) { // No authz checks return q.db.GetNotificationsSettings(ctx) @@ -3025,8 +3033,8 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb return q.db.UpdateMemberRoles(ctx, arg) } -// TODO: how to restrict this to admins? func (q *querier) UpdateNotificationTemplateMethodById(ctx context.Context, arg database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { + // TODO: how to restrict this to admins? if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationTemplate); err != nil { return database.NotificationTemplate{}, err } diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index a5324ca02512e..98c1dad235eef 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -2709,8 +2709,12 @@ func (q *FakeQuerier) GetNotificationMessagesByStatus(_ context.Context, arg dat return out, nil } -func (q *FakeQuerier) GetNotificationTemplateById(_ context.Context, id uuid.UUID) (database.NotificationTemplate, error) { - return database.NotificationTemplate{ID: id, Name: "fake"}, nil +func (*FakeQuerier) GetNotificationTemplateById(_ context.Context, _ uuid.UUID) (database.NotificationTemplate, error) { + return database.NotificationTemplate{}, ErrUnimplemented +} + +func (q *FakeQuerier) GetNotificationTemplatesByKind(ctx context.Context, kind database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { + return nil, ErrUnimplemented } func (q *FakeQuerier) GetNotificationsSettings(_ context.Context) (string, error) { @@ -7541,16 +7545,8 @@ func (q *FakeQuerier) UpdateMemberRoles(_ context.Context, arg database.UpdateMe return database.OrganizationMember{}, sql.ErrNoRows } -func (q *FakeQuerier) UpdateNotificationTemplateMethodById(_ context.Context, arg database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { - err := validateDatabaseType(arg) - if err != nil { - return database.NotificationTemplate{}, err - } - - return database.NotificationTemplate{ - ID: arg.ID, - Method: arg.Method, - }, nil +func (*FakeQuerier) UpdateNotificationTemplateMethodById(_ context.Context, _ database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { + return database.NotificationTemplate{}, ErrUnimplemented } func (q *FakeQuerier) UpdateOAuth2ProviderAppByID(_ context.Context, arg database.UpdateOAuth2ProviderAppByIDParams) (database.OAuth2ProviderApp, error) { diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index c0262013a863e..e00cb2fbf5c4d 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -753,6 +753,13 @@ func (m metricsStore) GetNotificationTemplateById(ctx context.Context, id uuid.U return r0, r1 } +func (m metricsStore) GetNotificationTemplatesByKind(ctx context.Context, kind database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { + start := time.Now() + r0, r1 := m.s.GetNotificationTemplatesByKind(ctx, kind) + m.queryLatencies.WithLabelValues("GetNotificationTemplatesByKind").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetNotificationsSettings(ctx context.Context) (string, error) { start := time.Now() r0, r1 := m.s.GetNotificationsSettings(ctx) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index db29195375a58..a1aecc531aa26 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1510,6 +1510,21 @@ func (mr *MockStoreMockRecorder) GetNotificationTemplateById(arg0, arg1 any) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNotificationTemplateById", reflect.TypeOf((*MockStore)(nil).GetNotificationTemplateById), arg0, arg1) } +// GetNotificationTemplatesByKind mocks base method. +func (m *MockStore) GetNotificationTemplatesByKind(arg0 context.Context, arg1 database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNotificationTemplatesByKind", arg0, arg1) + ret0, _ := ret[0].([]database.NotificationTemplate) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNotificationTemplatesByKind indicates an expected call of GetNotificationTemplatesByKind. +func (mr *MockStoreMockRecorder) GetNotificationTemplatesByKind(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNotificationTemplatesByKind", reflect.TypeOf((*MockStore)(nil).GetNotificationTemplatesByKind), arg0, arg1) +} + // GetNotificationsSettings mocks base method. func (m *MockStore) GetNotificationsSettings(arg0 context.Context) (string, error) { m.ctrl.T.Helper() @@ -4161,19 +4176,19 @@ func (mr *MockStoreMockRecorder) UpdateMemberRoles(arg0, arg1 any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateMemberRoles", reflect.TypeOf((*MockStore)(nil).UpdateMemberRoles), arg0, arg1) } -// UpdateNotificationTemplateMethod mocks base method. -func (m *MockStore) UpdateNotificationTemplateMethod(arg0 context.Context, arg1 database.UpdateNotificationTemplateMethodParams) (int64, error) { +// UpdateNotificationTemplateMethodById mocks base method. +func (m *MockStore) UpdateNotificationTemplateMethodById(arg0 context.Context, arg1 database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateNotificationTemplateMethod", arg0, arg1) - ret0, _ := ret[0].(int64) + ret := m.ctrl.Call(m, "UpdateNotificationTemplateMethodById", arg0, arg1) + ret0, _ := ret[0].(database.NotificationTemplate) ret1, _ := ret[1].(error) return ret0, ret1 } -// UpdateNotificationTemplateMethod indicates an expected call of UpdateNotificationTemplateMethod. -func (mr *MockStoreMockRecorder) UpdateNotificationTemplateMethod(arg0, arg1 any) *gomock.Call { +// UpdateNotificationTemplateMethodById indicates an expected call of UpdateNotificationTemplateMethodById. +func (mr *MockStoreMockRecorder) UpdateNotificationTemplateMethodById(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNotificationTemplateMethod", reflect.TypeOf((*MockStore)(nil).UpdateNotificationTemplateMethod), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNotificationTemplateMethodById", reflect.TypeOf((*MockStore)(nil).UpdateNotificationTemplateMethodById), arg0, arg1) } // UpdateOAuth2ProviderAppByID mocks base method. diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 33eb9dd8f1b07..673f557405e0c 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -93,6 +93,10 @@ CREATE TYPE notification_method AS ENUM ( 'webhook' ); +CREATE TYPE notification_template_kind AS ENUM ( + 'system' +); + CREATE TYPE parameter_destination_scheme AS ENUM ( 'none', 'environment_variable', @@ -165,7 +169,8 @@ CREATE TYPE resource_type AS ENUM ( 'oauth2_provider_app_secret', 'custom_role', 'organization_member', - 'notifications_settings' + 'notifications_settings', + 'notification_template' ); CREATE TYPE startup_script_behavior AS ENUM ( @@ -254,7 +259,7 @@ CREATE FUNCTION inhibit_enqueue_if_disabled() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN - -- Fail the insertion if the user has disabled this notification + -- Fail the insertion if the user has disabled this notification. IF EXISTS (SELECT 1 FROM notification_preferences WHERE disabled = TRUE @@ -600,7 +605,8 @@ CREATE TABLE notification_templates ( body_template text NOT NULL, actions jsonb, "group" text, - method notification_method + method notification_method, + kind notification_template_kind DEFAULT 'system'::notification_template_kind NOT NULL ); COMMENT ON TABLE notification_templates IS 'Templates from which to create notification messages.'; diff --git a/coderd/database/migrations/000231_notification_preferences.down.sql b/coderd/database/migrations/000232_notification_preferences.down.sql similarity index 66% rename from coderd/database/migrations/000231_notification_preferences.down.sql rename to coderd/database/migrations/000232_notification_preferences.down.sql index f882f1f59fe59..b2494971e1051 100644 --- a/coderd/database/migrations/000231_notification_preferences.down.sql +++ b/coderd/database/migrations/000232_notification_preferences.down.sql @@ -1,7 +1,9 @@ -DROP TABLE IF EXISTS notification_preferences; - ALTER TABLE notification_templates - DROP COLUMN IF EXISTS method; + DROP COLUMN IF EXISTS method, + DROP COLUMN IF EXISTS kind; + +DROP TABLE IF EXISTS notification_preferences; +DROP TYPE IF EXISTS notification_template_kind; DROP TRIGGER IF EXISTS inhibit_enqueue_if_disabled_trigger ON notification_messages; DROP FUNCTION IF EXISTS inhibit_enqueue_if_disabled; diff --git a/coderd/database/migrations/000231_notification_preferences.up.sql b/coderd/database/migrations/000232_notification_preferences.up.sql similarity index 81% rename from coderd/database/migrations/000231_notification_preferences.up.sql rename to coderd/database/migrations/000232_notification_preferences.up.sql index 604009b334163..3d8de38fefe4b 100644 --- a/coderd/database/migrations/000231_notification_preferences.up.sql +++ b/coderd/database/migrations/000232_notification_preferences.up.sql @@ -11,9 +11,16 @@ CREATE TABLE notification_preferences ALTER TABLE notification_preferences ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); --- Allow per-template notification method (enterprise only). +-- Add a new type (to be expanded upon later) which specifies the kind of notification template. +CREATE TYPE notification_template_kind AS ENUM ( + 'system' + ); + ALTER TABLE notification_templates - ADD COLUMN method notification_method; + -- Allow per-template notification method (enterprise only). + ADD COLUMN method notification_method, + -- Update all existing notification templates to be system templates. + ADD COLUMN kind notification_template_kind DEFAULT 'system'::notification_template_kind NOT NULL; COMMENT ON COLUMN notification_templates.method IS 'NULL defers to the deployment-level method'; -- No equivalent in down migration because ENUM values cannot be deleted. @@ -45,4 +52,4 @@ CREATE TRIGGER inhibit_enqueue_if_disabled_trigger EXECUTE FUNCTION inhibit_enqueue_if_disabled(); -- Allow modifications to notification templates to be audited. -ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'notification_template'; \ No newline at end of file +ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'notification_template'; diff --git a/coderd/database/models.go b/coderd/database/models.go index 175c1480e5159..51fb4f2ffa335 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -791,6 +791,61 @@ func AllNotificationMethodValues() []NotificationMethod { } } +type NotificationTemplateKind string + +const ( + NotificationTemplateKindSystem NotificationTemplateKind = "system" +) + +func (e *NotificationTemplateKind) Scan(src interface{}) error { + switch s := src.(type) { + case []byte: + *e = NotificationTemplateKind(s) + case string: + *e = NotificationTemplateKind(s) + default: + return fmt.Errorf("unsupported scan type for NotificationTemplateKind: %T", src) + } + return nil +} + +type NullNotificationTemplateKind struct { + NotificationTemplateKind NotificationTemplateKind `json:"notification_template_kind"` + Valid bool `json:"valid"` // Valid is true if NotificationTemplateKind is not NULL +} + +// Scan implements the Scanner interface. +func (ns *NullNotificationTemplateKind) Scan(value interface{}) error { + if value == nil { + ns.NotificationTemplateKind, ns.Valid = "", false + return nil + } + ns.Valid = true + return ns.NotificationTemplateKind.Scan(value) +} + +// Value implements the driver Valuer interface. +func (ns NullNotificationTemplateKind) Value() (driver.Value, error) { + if !ns.Valid { + return nil, nil + } + return string(ns.NotificationTemplateKind), nil +} + +func (e NotificationTemplateKind) Valid() bool { + switch e { + case NotificationTemplateKindSystem: + return true + } + return false +} + +func AllNotificationTemplateKindValues() []NotificationTemplateKind { + return []NotificationTemplateKind{ + NotificationTemplateKindSystem, + } +} + type ParameterDestinationScheme string const ( @@ -2057,7 +2112,8 @@ type NotificationTemplate struct { Actions []byte `db:"actions" json:"actions"` Group sql.NullString `db:"group" json:"group"` // NULL defers to the deployment-level method - Method NullNotificationMethod `db:"method" json:"method"` + Method NullNotificationMethod `db:"method" json:"method"` + Kind NotificationTemplateKind `db:"kind" json:"kind"` } // A table used to configure apps that can use Coder as an OAuth2 provider, the reverse of what we are calling external authentication. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index b7cb50fa4d4c6..6bebf112851c1 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -163,6 +163,7 @@ type sqlcQuerier interface { GetLogoURL(ctx context.Context) (string, error) GetNotificationMessagesByStatus(ctx context.Context, arg GetNotificationMessagesByStatusParams) ([]NotificationMessage, error) GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (NotificationTemplate, error) + GetNotificationTemplatesByKind(ctx context.Context, kind NotificationTemplateKind) ([]NotificationTemplate, error) GetNotificationsSettings(ctx context.Context) (string, error) GetOAuth2ProviderAppByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderApp, error) GetOAuth2ProviderAppCodeByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderAppCode, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index c47940fc293e6..4f7f30cb706b7 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3631,7 +3631,7 @@ func (q *sqlQuerier) GetNotificationMessagesByStatus(ctx context.Context, arg Ge } const getNotificationTemplateById = `-- name: GetNotificationTemplateById :one -SELECT id, name, title_template, body_template, actions, "group", method +SELECT id, name, title_template, body_template, actions, "group", method, kind FROM notification_templates WHERE id = $1::uuid ` @@ -3647,10 +3647,48 @@ func (q *sqlQuerier) GetNotificationTemplateById(ctx context.Context, id uuid.UU &i.Actions, &i.Group, &i.Method, + &i.Kind, ) return i, err } +const getNotificationTemplatesByKind = `-- name: GetNotificationTemplatesByKind :many +SELECT id, name, title_template, body_template, actions, "group", method, kind FROM notification_templates +WHERE kind = $1::notification_template_kind +` + +func (q *sqlQuerier) GetNotificationTemplatesByKind(ctx context.Context, kind NotificationTemplateKind) ([]NotificationTemplate, error) { + rows, err := q.db.QueryContext(ctx, getNotificationTemplatesByKind, kind) + if err != nil { + return nil, err + } + defer rows.Close() + var items []NotificationTemplate + for rows.Next() { + var i NotificationTemplate + if err := rows.Scan( + &i.ID, + &i.Name, + &i.TitleTemplate, + &i.BodyTemplate, + &i.Actions, + &i.Group, + &i.Method, + &i.Kind, + ); 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 getUserNotificationPreferences = `-- name: GetUserNotificationPreferences :many SELECT user_id, notification_template_id, disabled, created_at, updated_at FROM notification_preferences @@ -3690,7 +3728,7 @@ const updateNotificationTemplateMethodById = `-- name: UpdateNotificationTemplat UPDATE notification_templates SET method = $1::notification_method WHERE id = $2::uuid -RETURNING id, name, title_template, body_template, actions, "group", method +RETURNING id, name, title_template, body_template, actions, "group", method, kind ` type UpdateNotificationTemplateMethodByIdParams struct { @@ -3709,6 +3747,7 @@ func (q *sqlQuerier) UpdateNotificationTemplateMethodById(ctx context.Context, a &i.Actions, &i.Group, &i.Method, + &i.Kind, ) return i, err } diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index 13fb38721c108..cb4aa948eefc9 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -167,3 +167,7 @@ RETURNING *; SELECT * FROM notification_templates WHERE id = @id::uuid; + +-- name: GetNotificationTemplatesByKind :many +SELECT * FROM notification_templates +WHERE kind = @kind::notification_template_kind; diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index ddabf13d39810..564a4f8594a0e 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -18,7 +18,7 @@ We track the following resources: | GitSSHKey
create |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| | HealthSettings
|
FieldTracked
dismissed_healthcheckstrue
idfalse
| | License
create, delete |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| -| NotificationTemplate
|
FieldTracked
actionstrue
body_templatetrue
grouptrue
idfalse
methodtrue
nametrue
title_templatetrue
| +| NotificationTemplate
|
FieldTracked
actionstrue
body_templatetrue
grouptrue
idfalse
kindtrue
methodtrue
nametrue
title_templatetrue
| | NotificationsSettings
|
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
|
FieldTracked
callback_urltrue
created_atfalse
icontrue
idfalse
nametrue
updated_atfalse
| | OAuth2ProviderAppSecret
|
FieldTracked
app_idfalse
created_atfalse
display_secretfalse
hashed_secretfalse
idfalse
last_used_atfalse
secret_prefixfalse
| diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 1a46e561751da..7310848f4f959 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -280,6 +280,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "actions": ActionTrack, "group": ActionTrack, "method": ActionTrack, + "kind": ActionTrack, }, } diff --git a/site/src/api/rbacresources_gen.ts b/site/src/api/rbacresources_gen.ts index 37fe508fde89c..0e68e1cf7a9cc 100644 --- a/site/src/api/rbacresources_gen.ts +++ b/site/src/api/rbacresources_gen.ts @@ -55,6 +55,10 @@ export const RBACResourceActions: Partial< delete: "delete license", read: "read licenses", }, + notification_template: { + read: "read notification templates", + update: "update notification templates", + }, oauth2_app: { create: "make an OAuth2 app.", delete: "delete an OAuth2 app", From 2fce7ce950894465352bb2f5541c9f4fd875c0f2 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 29 Jul 2024 11:21:28 +0200 Subject: [PATCH 06/20] Remove redundant suffix on trigger name Signed-off-by: Danny Kopping --- coderd/database/dump.sql | 2 +- .../migrations/000232_notification_preferences.down.sql | 2 +- .../database/migrations/000232_notification_preferences.up.sql | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 673f557405e0c..2d2b783c8b6af 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1836,7 +1836,7 @@ CREATE INDEX workspace_resources_job_id_idx ON workspace_resources USING btree ( CREATE UNIQUE INDEX workspaces_owner_id_lower_idx ON workspaces USING btree (owner_id, lower((name)::text)) WHERE (deleted = false); -CREATE TRIGGER inhibit_enqueue_if_disabled_trigger BEFORE INSERT ON notification_messages FOR EACH ROW EXECUTE FUNCTION inhibit_enqueue_if_disabled(); +CREATE TRIGGER inhibit_enqueue_if_disabled BEFORE INSERT ON notification_messages FOR EACH ROW EXECUTE FUNCTION inhibit_enqueue_if_disabled(); CREATE TRIGGER tailnet_notify_agent_change AFTER INSERT OR DELETE OR UPDATE ON tailnet_agents FOR EACH ROW EXECUTE FUNCTION tailnet_notify_agent_change(); diff --git a/coderd/database/migrations/000232_notification_preferences.down.sql b/coderd/database/migrations/000232_notification_preferences.down.sql index b2494971e1051..5e894d93e5289 100644 --- a/coderd/database/migrations/000232_notification_preferences.down.sql +++ b/coderd/database/migrations/000232_notification_preferences.down.sql @@ -5,5 +5,5 @@ ALTER TABLE notification_templates DROP TABLE IF EXISTS notification_preferences; DROP TYPE IF EXISTS notification_template_kind; -DROP TRIGGER IF EXISTS inhibit_enqueue_if_disabled_trigger ON notification_messages; +DROP TRIGGER IF EXISTS inhibit_enqueue_if_disabled ON notification_messages; DROP FUNCTION IF EXISTS inhibit_enqueue_if_disabled; diff --git a/coderd/database/migrations/000232_notification_preferences.up.sql b/coderd/database/migrations/000232_notification_preferences.up.sql index 3d8de38fefe4b..182d9a72a57e3 100644 --- a/coderd/database/migrations/000232_notification_preferences.up.sql +++ b/coderd/database/migrations/000232_notification_preferences.up.sql @@ -45,7 +45,7 @@ END; $$ LANGUAGE plpgsql; -- Trigger to execute above function on insertion. -CREATE TRIGGER inhibit_enqueue_if_disabled_trigger +CREATE TRIGGER inhibit_enqueue_if_disabled BEFORE INSERT ON notification_messages FOR EACH ROW From a30ee5c6a7f6c045d0f1119c86a6c1d11c054016 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 29 Jul 2024 13:11:29 +0200 Subject: [PATCH 07/20] Add notification preferences RBAC role Signed-off-by: Danny Kopping --- coderd/apidoc/docs.go | 2 + coderd/apidoc/swagger.json | 2 + coderd/rbac/object_gen.go | 9 ++ coderd/rbac/policy/policy.go | 6 + codersdk/rbacresources_gen.go | 110 ++++++------- docs/api/members.md | 255 +++++++++++++++--------------- docs/api/schemas.md | 59 +++---- site/src/api/rbacresources_gen.ts | 4 + site/src/api/typesGenerated.ts | 2 + 9 files changed, 240 insertions(+), 209 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 5205565b6d281..6671fdb796836 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11217,6 +11217,7 @@ const docTemplate = `{ "file", "group", "license", + "notification_preference", "notification_template", "oauth2_app", "oauth2_app_code_token", @@ -11246,6 +11247,7 @@ const docTemplate = `{ "ResourceFile", "ResourceGroup", "ResourceLicense", + "ResourceNotificationPreference", "ResourceNotificationTemplate", "ResourceOauth2App", "ResourceOauth2AppCodeToken", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 3bffe32254841..02d0247df7362 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10119,6 +10119,7 @@ "file", "group", "license", + "notification_preference", "notification_template", "oauth2_app", "oauth2_app_code_token", @@ -10148,6 +10149,7 @@ "ResourceFile", "ResourceGroup", "ResourceLicense", + "ResourceNotificationPreference", "ResourceNotificationTemplate", "ResourceOauth2App", "ResourceOauth2AppCodeToken", diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 938e116479234..1c539ef09567d 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -102,6 +102,14 @@ var ( Type: "license", } + // ResourceNotificationPreference + // Valid Actions + // - "ActionReadPersonal" :: read own notification preferences + // - "ActionUpdatePersonal" :: update own notification preferences + ResourceNotificationPreference = Object{ + Type: "notification_preference", + } + // ResourceNotificationTemplate // Valid Actions // - "ActionRead" :: read notification templates @@ -280,6 +288,7 @@ func AllResources() []Objecter { ResourceFile, ResourceGroup, ResourceLicense, + ResourceNotificationPreference, ResourceNotificationTemplate, ResourceOauth2App, ResourceOauth2AppCodeToken, diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 566185283a55f..c55d9e8cf5294 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -261,4 +261,10 @@ var RBACPermissions = map[string]PermissionDefinition{ ActionUpdate: actDef("update notification templates"), }, }, + "notification_preference": { + Actions: map[Action]ActionDefinition{ + ActionReadPersonal: actDef("read own notification preferences"), + ActionUpdatePersonal: actDef("update own notification preferences"), + }, + }, } diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index 009b1f4e7c735..bfe228a5477d9 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -4,33 +4,34 @@ package codersdk type RBACResource string const ( - ResourceWildcard RBACResource = "*" - ResourceApiKey RBACResource = "api_key" - ResourceAssignOrgRole RBACResource = "assign_org_role" - ResourceAssignRole RBACResource = "assign_role" - ResourceAuditLog RBACResource = "audit_log" - ResourceDebugInfo RBACResource = "debug_info" - ResourceDeploymentConfig RBACResource = "deployment_config" - ResourceDeploymentStats RBACResource = "deployment_stats" - ResourceFile RBACResource = "file" - ResourceGroup RBACResource = "group" - ResourceLicense RBACResource = "license" - ResourceNotificationTemplate RBACResource = "notification_template" - ResourceOauth2App RBACResource = "oauth2_app" - ResourceOauth2AppCodeToken RBACResource = "oauth2_app_code_token" - ResourceOauth2AppSecret RBACResource = "oauth2_app_secret" - ResourceOrganization RBACResource = "organization" - ResourceOrganizationMember RBACResource = "organization_member" - ResourceProvisionerDaemon RBACResource = "provisioner_daemon" - ResourceProvisionerKeys RBACResource = "provisioner_keys" - ResourceReplicas RBACResource = "replicas" - ResourceSystem RBACResource = "system" - ResourceTailnetCoordinator RBACResource = "tailnet_coordinator" - ResourceTemplate RBACResource = "template" - ResourceUser RBACResource = "user" - ResourceWorkspace RBACResource = "workspace" - ResourceWorkspaceDormant RBACResource = "workspace_dormant" - ResourceWorkspaceProxy RBACResource = "workspace_proxy" + ResourceWildcard RBACResource = "*" + ResourceApiKey RBACResource = "api_key" + ResourceAssignOrgRole RBACResource = "assign_org_role" + ResourceAssignRole RBACResource = "assign_role" + ResourceAuditLog RBACResource = "audit_log" + ResourceDebugInfo RBACResource = "debug_info" + ResourceDeploymentConfig RBACResource = "deployment_config" + ResourceDeploymentStats RBACResource = "deployment_stats" + ResourceFile RBACResource = "file" + ResourceGroup RBACResource = "group" + ResourceLicense RBACResource = "license" + ResourceNotificationPreference RBACResource = "notification_preference" + ResourceNotificationTemplate RBACResource = "notification_template" + ResourceOauth2App RBACResource = "oauth2_app" + ResourceOauth2AppCodeToken RBACResource = "oauth2_app_code_token" + ResourceOauth2AppSecret RBACResource = "oauth2_app_secret" + ResourceOrganization RBACResource = "organization" + ResourceOrganizationMember RBACResource = "organization_member" + ResourceProvisionerDaemon RBACResource = "provisioner_daemon" + ResourceProvisionerKeys RBACResource = "provisioner_keys" + ResourceReplicas RBACResource = "replicas" + ResourceSystem RBACResource = "system" + ResourceTailnetCoordinator RBACResource = "tailnet_coordinator" + ResourceTemplate RBACResource = "template" + ResourceUser RBACResource = "user" + ResourceWorkspace RBACResource = "workspace" + ResourceWorkspaceDormant RBACResource = "workspace_dormant" + ResourceWorkspaceProxy RBACResource = "workspace_proxy" ) type RBACAction string @@ -54,31 +55,32 @@ const ( // RBACResourceActions is the mapping of resources to which actions are valid for // said resource type. var RBACResourceActions = map[RBACResource][]RBACAction{ - ResourceWildcard: {}, - ResourceApiKey: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceAssignOrgRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, - ResourceAssignRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, - ResourceAuditLog: {ActionCreate, ActionRead}, - ResourceDebugInfo: {ActionRead}, - ResourceDeploymentConfig: {ActionRead, ActionUpdate}, - ResourceDeploymentStats: {ActionRead}, - ResourceFile: {ActionCreate, ActionRead}, - ResourceGroup: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceLicense: {ActionCreate, ActionDelete, ActionRead}, - ResourceNotificationTemplate: {ActionRead, ActionUpdate}, - ResourceOauth2App: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceOauth2AppCodeToken: {ActionCreate, ActionDelete, ActionRead}, - ResourceOauth2AppSecret: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceOrganization: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceOrganizationMember: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceProvisionerDaemon: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceProvisionerKeys: {ActionCreate, ActionDelete, ActionRead}, - ResourceReplicas: {ActionRead}, - ResourceSystem: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceTailnetCoordinator: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceTemplate: {ActionCreate, ActionDelete, ActionRead, ActionUpdate, ActionViewInsights}, - ResourceUser: {ActionCreate, ActionDelete, ActionRead, ActionReadPersonal, ActionUpdate, ActionUpdatePersonal}, - ResourceWorkspace: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, - ResourceWorkspaceDormant: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, - ResourceWorkspaceProxy: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceWildcard: {}, + ResourceApiKey: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceAssignOrgRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, + ResourceAssignRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, + ResourceAuditLog: {ActionCreate, ActionRead}, + ResourceDebugInfo: {ActionRead}, + ResourceDeploymentConfig: {ActionRead, ActionUpdate}, + ResourceDeploymentStats: {ActionRead}, + ResourceFile: {ActionCreate, ActionRead}, + ResourceGroup: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceLicense: {ActionCreate, ActionDelete, ActionRead}, + ResourceNotificationPreference: {ActionReadPersonal, ActionUpdatePersonal}, + ResourceNotificationTemplate: {ActionRead, ActionUpdate}, + ResourceOauth2App: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOauth2AppCodeToken: {ActionCreate, ActionDelete, ActionRead}, + ResourceOauth2AppSecret: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOrganization: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOrganizationMember: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceProvisionerDaemon: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceProvisionerKeys: {ActionCreate, ActionDelete, ActionRead}, + ResourceReplicas: {ActionRead}, + ResourceSystem: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceTailnetCoordinator: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceTemplate: {ActionCreate, ActionDelete, ActionRead, ActionUpdate, ActionViewInsights}, + ResourceUser: {ActionCreate, ActionDelete, ActionRead, ActionReadPersonal, ActionUpdate, ActionUpdatePersonal}, + ResourceWorkspace: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, + ResourceWorkspaceDormant: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, + ResourceWorkspaceProxy: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, } diff --git a/docs/api/members.md b/docs/api/members.md index b0cd8f956a475..63bf06b9b0f8c 100644 --- a/docs/api/members.md +++ b/docs/api/members.md @@ -164,48 +164,49 @@ Status Code **200** #### Enumerated Values -| Property | Value | -| --------------- | ----------------------- | -| `action` | `application_connect` | -| `action` | `assign` | -| `action` | `create` | -| `action` | `delete` | -| `action` | `read` | -| `action` | `read_personal` | -| `action` | `ssh` | -| `action` | `update` | -| `action` | `update_personal` | -| `action` | `use` | -| `action` | `view_insights` | -| `action` | `start` | -| `action` | `stop` | -| `resource_type` | `*` | -| `resource_type` | `api_key` | -| `resource_type` | `assign_org_role` | -| `resource_type` | `assign_role` | -| `resource_type` | `audit_log` | -| `resource_type` | `debug_info` | -| `resource_type` | `deployment_config` | -| `resource_type` | `deployment_stats` | -| `resource_type` | `file` | -| `resource_type` | `group` | -| `resource_type` | `license` | -| `resource_type` | `notification_template` | -| `resource_type` | `oauth2_app` | -| `resource_type` | `oauth2_app_code_token` | -| `resource_type` | `oauth2_app_secret` | -| `resource_type` | `organization` | -| `resource_type` | `organization_member` | -| `resource_type` | `provisioner_daemon` | -| `resource_type` | `provisioner_keys` | -| `resource_type` | `replicas` | -| `resource_type` | `system` | -| `resource_type` | `tailnet_coordinator` | -| `resource_type` | `template` | -| `resource_type` | `user` | -| `resource_type` | `workspace` | -| `resource_type` | `workspace_dormant` | -| `resource_type` | `workspace_proxy` | +| Property | Value | +| --------------- | ------------------------- | +| `action` | `application_connect` | +| `action` | `assign` | +| `action` | `create` | +| `action` | `delete` | +| `action` | `read` | +| `action` | `read_personal` | +| `action` | `ssh` | +| `action` | `update` | +| `action` | `update_personal` | +| `action` | `use` | +| `action` | `view_insights` | +| `action` | `start` | +| `action` | `stop` | +| `resource_type` | `*` | +| `resource_type` | `api_key` | +| `resource_type` | `assign_org_role` | +| `resource_type` | `assign_role` | +| `resource_type` | `audit_log` | +| `resource_type` | `debug_info` | +| `resource_type` | `deployment_config` | +| `resource_type` | `deployment_stats` | +| `resource_type` | `file` | +| `resource_type` | `group` | +| `resource_type` | `license` | +| `resource_type` | `notification_preference` | +| `resource_type` | `notification_template` | +| `resource_type` | `oauth2_app` | +| `resource_type` | `oauth2_app_code_token` | +| `resource_type` | `oauth2_app_secret` | +| `resource_type` | `organization` | +| `resource_type` | `organization_member` | +| `resource_type` | `provisioner_daemon` | +| `resource_type` | `provisioner_keys` | +| `resource_type` | `replicas` | +| `resource_type` | `system` | +| `resource_type` | `tailnet_coordinator` | +| `resource_type` | `template` | +| `resource_type` | `user` | +| `resource_type` | `workspace` | +| `resource_type` | `workspace_dormant` | +| `resource_type` | `workspace_proxy` | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -288,48 +289,49 @@ Status Code **200** #### Enumerated Values -| Property | Value | -| --------------- | ----------------------- | -| `action` | `application_connect` | -| `action` | `assign` | -| `action` | `create` | -| `action` | `delete` | -| `action` | `read` | -| `action` | `read_personal` | -| `action` | `ssh` | -| `action` | `update` | -| `action` | `update_personal` | -| `action` | `use` | -| `action` | `view_insights` | -| `action` | `start` | -| `action` | `stop` | -| `resource_type` | `*` | -| `resource_type` | `api_key` | -| `resource_type` | `assign_org_role` | -| `resource_type` | `assign_role` | -| `resource_type` | `audit_log` | -| `resource_type` | `debug_info` | -| `resource_type` | `deployment_config` | -| `resource_type` | `deployment_stats` | -| `resource_type` | `file` | -| `resource_type` | `group` | -| `resource_type` | `license` | -| `resource_type` | `notification_template` | -| `resource_type` | `oauth2_app` | -| `resource_type` | `oauth2_app_code_token` | -| `resource_type` | `oauth2_app_secret` | -| `resource_type` | `organization` | -| `resource_type` | `organization_member` | -| `resource_type` | `provisioner_daemon` | -| `resource_type` | `provisioner_keys` | -| `resource_type` | `replicas` | -| `resource_type` | `system` | -| `resource_type` | `tailnet_coordinator` | -| `resource_type` | `template` | -| `resource_type` | `user` | -| `resource_type` | `workspace` | -| `resource_type` | `workspace_dormant` | -| `resource_type` | `workspace_proxy` | +| Property | Value | +| --------------- | ------------------------- | +| `action` | `application_connect` | +| `action` | `assign` | +| `action` | `create` | +| `action` | `delete` | +| `action` | `read` | +| `action` | `read_personal` | +| `action` | `ssh` | +| `action` | `update` | +| `action` | `update_personal` | +| `action` | `use` | +| `action` | `view_insights` | +| `action` | `start` | +| `action` | `stop` | +| `resource_type` | `*` | +| `resource_type` | `api_key` | +| `resource_type` | `assign_org_role` | +| `resource_type` | `assign_role` | +| `resource_type` | `audit_log` | +| `resource_type` | `debug_info` | +| `resource_type` | `deployment_config` | +| `resource_type` | `deployment_stats` | +| `resource_type` | `file` | +| `resource_type` | `group` | +| `resource_type` | `license` | +| `resource_type` | `notification_preference` | +| `resource_type` | `notification_template` | +| `resource_type` | `oauth2_app` | +| `resource_type` | `oauth2_app_code_token` | +| `resource_type` | `oauth2_app_secret` | +| `resource_type` | `organization` | +| `resource_type` | `organization_member` | +| `resource_type` | `provisioner_daemon` | +| `resource_type` | `provisioner_keys` | +| `resource_type` | `replicas` | +| `resource_type` | `system` | +| `resource_type` | `tailnet_coordinator` | +| `resource_type` | `template` | +| `resource_type` | `user` | +| `resource_type` | `workspace` | +| `resource_type` | `workspace_dormant` | +| `resource_type` | `workspace_proxy` | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -543,47 +545,48 @@ Status Code **200** #### Enumerated Values -| Property | Value | -| --------------- | ----------------------- | -| `action` | `application_connect` | -| `action` | `assign` | -| `action` | `create` | -| `action` | `delete` | -| `action` | `read` | -| `action` | `read_personal` | -| `action` | `ssh` | -| `action` | `update` | -| `action` | `update_personal` | -| `action` | `use` | -| `action` | `view_insights` | -| `action` | `start` | -| `action` | `stop` | -| `resource_type` | `*` | -| `resource_type` | `api_key` | -| `resource_type` | `assign_org_role` | -| `resource_type` | `assign_role` | -| `resource_type` | `audit_log` | -| `resource_type` | `debug_info` | -| `resource_type` | `deployment_config` | -| `resource_type` | `deployment_stats` | -| `resource_type` | `file` | -| `resource_type` | `group` | -| `resource_type` | `license` | -| `resource_type` | `notification_template` | -| `resource_type` | `oauth2_app` | -| `resource_type` | `oauth2_app_code_token` | -| `resource_type` | `oauth2_app_secret` | -| `resource_type` | `organization` | -| `resource_type` | `organization_member` | -| `resource_type` | `provisioner_daemon` | -| `resource_type` | `provisioner_keys` | -| `resource_type` | `replicas` | -| `resource_type` | `system` | -| `resource_type` | `tailnet_coordinator` | -| `resource_type` | `template` | -| `resource_type` | `user` | -| `resource_type` | `workspace` | -| `resource_type` | `workspace_dormant` | -| `resource_type` | `workspace_proxy` | +| Property | Value | +| --------------- | ------------------------- | +| `action` | `application_connect` | +| `action` | `assign` | +| `action` | `create` | +| `action` | `delete` | +| `action` | `read` | +| `action` | `read_personal` | +| `action` | `ssh` | +| `action` | `update` | +| `action` | `update_personal` | +| `action` | `use` | +| `action` | `view_insights` | +| `action` | `start` | +| `action` | `stop` | +| `resource_type` | `*` | +| `resource_type` | `api_key` | +| `resource_type` | `assign_org_role` | +| `resource_type` | `assign_role` | +| `resource_type` | `audit_log` | +| `resource_type` | `debug_info` | +| `resource_type` | `deployment_config` | +| `resource_type` | `deployment_stats` | +| `resource_type` | `file` | +| `resource_type` | `group` | +| `resource_type` | `license` | +| `resource_type` | `notification_preference` | +| `resource_type` | `notification_template` | +| `resource_type` | `oauth2_app` | +| `resource_type` | `oauth2_app_code_token` | +| `resource_type` | `oauth2_app_secret` | +| `resource_type` | `organization` | +| `resource_type` | `organization_member` | +| `resource_type` | `provisioner_daemon` | +| `resource_type` | `provisioner_keys` | +| `resource_type` | `replicas` | +| `resource_type` | `system` | +| `resource_type` | `tailnet_coordinator` | +| `resource_type` | `template` | +| `resource_type` | `user` | +| `resource_type` | `workspace` | +| `resource_type` | `workspace_dormant` | +| `resource_type` | `workspace_proxy` | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 46c9b6aa4d338..e0d208895eb75 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -4153,35 +4153,36 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o #### Enumerated Values -| Value | -| ----------------------- | -| `*` | -| `api_key` | -| `assign_org_role` | -| `assign_role` | -| `audit_log` | -| `debug_info` | -| `deployment_config` | -| `deployment_stats` | -| `file` | -| `group` | -| `license` | -| `notification_template` | -| `oauth2_app` | -| `oauth2_app_code_token` | -| `oauth2_app_secret` | -| `organization` | -| `organization_member` | -| `provisioner_daemon` | -| `provisioner_keys` | -| `replicas` | -| `system` | -| `tailnet_coordinator` | -| `template` | -| `user` | -| `workspace` | -| `workspace_dormant` | -| `workspace_proxy` | +| Value | +| ------------------------- | +| `*` | +| `api_key` | +| `assign_org_role` | +| `assign_role` | +| `audit_log` | +| `debug_info` | +| `deployment_config` | +| `deployment_stats` | +| `file` | +| `group` | +| `license` | +| `notification_preference` | +| `notification_template` | +| `oauth2_app` | +| `oauth2_app_code_token` | +| `oauth2_app_secret` | +| `organization` | +| `organization_member` | +| `provisioner_daemon` | +| `provisioner_keys` | +| `replicas` | +| `system` | +| `tailnet_coordinator` | +| `template` | +| `user` | +| `workspace` | +| `workspace_dormant` | +| `workspace_proxy` | ## codersdk.RateLimitConfig diff --git a/site/src/api/rbacresources_gen.ts b/site/src/api/rbacresources_gen.ts index 0e68e1cf7a9cc..c526a95ec891e 100644 --- a/site/src/api/rbacresources_gen.ts +++ b/site/src/api/rbacresources_gen.ts @@ -55,6 +55,10 @@ export const RBACResourceActions: Partial< delete: "delete license", read: "read licenses", }, + notification_preference: { + read_personal: "read own notification preferences", + update_personal: "update own notification preferences", + }, notification_template: { read: "read notification templates", update: "update notification templates", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4ae1e65b42f0c..804ee8ce8cec0 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2269,6 +2269,7 @@ export type RBACResource = | "file" | "group" | "license" + | "notification_preference" | "notification_template" | "oauth2_app" | "oauth2_app_code_token" @@ -2297,6 +2298,7 @@ export const RBACResources: RBACResource[] = [ "file", "group", "license", + "notification_preference", "notification_template", "oauth2_app", "oauth2_app_code_token", From 81d92612e59eff7c51c32f04322ee55630085131 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 29 Jul 2024 16:10:25 +0200 Subject: [PATCH 08/20] UpdateUserNotificationPreferences Signed-off-by: Danny Kopping --- coderd/database/dbmem/dbmem.go | 39 ++++++++++++------- ... 000233_notification_preferences.down.sql} | 0 ...=> 000233_notification_preferences.up.sql} | 0 3 files changed, 25 insertions(+), 14 deletions(-) rename coderd/database/migrations/{000232_notification_preferences.down.sql => 000233_notification_preferences.down.sql} (100%) rename coderd/database/migrations/{000232_notification_preferences.up.sql => 000233_notification_preferences.up.sql} (100%) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 98c1dad235eef..f1911648b3ce2 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -65,6 +65,7 @@ func New() database.Store { files: make([]database.File, 0), gitSSHKey: make([]database.GitSSHKey, 0), notificationMessages: make([]database.NotificationMessage, 0), + notificationPreferences: make([]database.NotificationPreference, 0), parameterSchemas: make([]database.ParameterSchema, 0), provisionerDaemons: make([]database.ProvisionerDaemon, 0), workspaceAgents: make([]database.WorkspaceAgent, 0), @@ -8143,41 +8144,51 @@ func (q *FakeQuerier) UpdateUserLoginType(_ context.Context, arg database.Update return database.User{}, sql.ErrNoRows } -func (q *FakeQuerier) UpdateUserNotificationPreferences(ctx context.Context, arg database.UpdateUserNotificationPreferencesParams) (int64, error) { +func (q *FakeQuerier) UpdateUserNotificationPreferences(_ context.Context, arg database.UpdateUserNotificationPreferencesParams) (int64, error) { err := validateDatabaseType(arg) if err != nil { - return 0, err + return -1, err } q.mutex.Lock() defer q.mutex.Unlock() var upserted int64 - for i, templateID := range arg.NotificationTemplateIds { + for i := range arg.NotificationTemplateIds { var ( - found *database.NotificationPreference - disabled = arg.Disableds[i] + found bool + templateID = arg.NotificationTemplateIds[i] + disabled = arg.Disableds[i] ) - for _, np := range q.notificationPreferences { - if np.UserID != arg.UserID && np.NotificationTemplateID != templateID { + for j, np := range q.notificationPreferences { + if np.UserID != arg.UserID { + continue + } + + if np.NotificationTemplateID != templateID { continue } - found = &np + np.Disabled = disabled + np.UpdatedAt = time.Now() + q.notificationPreferences[j] = np + + upserted++ + found = true + break } - if found != nil { - found.Disabled = disabled - found.UpdatedAt = time.Now() - } else { - q.notificationPreferences = append(q.notificationPreferences, database.NotificationPreference{ + if !found { + np := database.NotificationPreference{ Disabled: disabled, UserID: arg.UserID, NotificationTemplateID: templateID, CreatedAt: time.Now(), UpdatedAt: time.Now(), - }) + } + q.notificationPreferences = append(q.notificationPreferences, np) + upserted++ } } diff --git a/coderd/database/migrations/000232_notification_preferences.down.sql b/coderd/database/migrations/000233_notification_preferences.down.sql similarity index 100% rename from coderd/database/migrations/000232_notification_preferences.down.sql rename to coderd/database/migrations/000233_notification_preferences.down.sql diff --git a/coderd/database/migrations/000232_notification_preferences.up.sql b/coderd/database/migrations/000233_notification_preferences.up.sql similarity index 100% rename from coderd/database/migrations/000232_notification_preferences.up.sql rename to coderd/database/migrations/000233_notification_preferences.up.sql From 0c662fbb5faaf12c45dfefb68180ab4eba6afce1 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 1 Aug 2024 12:28:33 +0200 Subject: [PATCH 09/20] RBAC for notification templates & preferences Signed-off-by: Danny Kopping --- coderd/database/dbauthz/dbauthz.go | 4 +- ... 000234_notification_preferences.down.sql} | 0 ...=> 000234_notification_preferences.up.sql} | 0 coderd/rbac/object_gen.go | 4 +- coderd/rbac/policy/policy.go | 4 +- coderd/rbac/roles_test.go | 68 +++++++++++++++++++ codersdk/rbacresources_gen.go | 2 +- site/src/api/rbacresources_gen.ts | 4 +- 8 files changed, 77 insertions(+), 9 deletions(-) rename coderd/database/migrations/{000233_notification_preferences.down.sql => 000234_notification_preferences.down.sql} (100%) rename coderd/database/migrations/{000233_notification_preferences.up.sql => 000234_notification_preferences.up.sql} (100%) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 629b7c213fa6e..9e41c95fef238 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2101,7 +2101,7 @@ func (q *querier) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([ } func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]database.NotificationPreference, error) { - if err := q.authorizeContext(ctx, policy.ActionReadPersonal, rbac.ResourceUserObject(userID)); err != nil { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceNotificationPreference.WithOwner(userID.String())); err != nil { return nil, err } return q.db.GetUserNotificationPreferences(ctx, userID) @@ -3357,7 +3357,7 @@ func (q *querier) UpdateUserLoginType(ctx context.Context, arg database.UpdateUs } func (q *querier) UpdateUserNotificationPreferences(ctx context.Context, arg database.UpdateUserNotificationPreferencesParams) (int64, error) { - if err := q.authorizeContext(ctx, policy.ActionUpdatePersonal, rbac.ResourceUserObject(arg.UserID)); err != nil { + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationPreference.WithOwner(arg.UserID.String())); err != nil { return -1, err } return q.db.UpdateUserNotificationPreferences(ctx, arg) diff --git a/coderd/database/migrations/000233_notification_preferences.down.sql b/coderd/database/migrations/000234_notification_preferences.down.sql similarity index 100% rename from coderd/database/migrations/000233_notification_preferences.down.sql rename to coderd/database/migrations/000234_notification_preferences.down.sql diff --git a/coderd/database/migrations/000233_notification_preferences.up.sql b/coderd/database/migrations/000234_notification_preferences.up.sql similarity index 100% rename from coderd/database/migrations/000233_notification_preferences.up.sql rename to coderd/database/migrations/000234_notification_preferences.up.sql diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 1c539ef09567d..64dc7fc3016f8 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -104,8 +104,8 @@ var ( // ResourceNotificationPreference // Valid Actions - // - "ActionReadPersonal" :: read own notification preferences - // - "ActionUpdatePersonal" :: update own notification preferences + // - "ActionRead" :: read own notification preferences + // - "ActionUpdate" :: update own notification preferences ResourceNotificationPreference = Object{ Type: "notification_preference", } diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index c55d9e8cf5294..e8eb96e86e82e 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -263,8 +263,8 @@ var RBACPermissions = map[string]PermissionDefinition{ }, "notification_preference": { Actions: map[Action]ActionDefinition{ - ActionReadPersonal: actDef("read own notification preferences"), - ActionUpdatePersonal: actDef("update own notification preferences"), + ActionRead: actDef("read own notification preferences"), + ActionUpdate: actDef("update own notification preferences"), }, }, } diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 225e5eb9d311e..f25b8adc383eb 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -590,6 +590,34 @@ func TestRolePermissions(t *testing.T) { false: {}, }, }, + { + Name: "NotificationPreferencesOwn", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, + Resource: rbac.ResourceNotificationPreference.WithOwner(currentUser.String()), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {memberMe, orgMemberMe, owner}, + false: { + userAdmin, orgUserAdmin, templateAdmin, + orgAuditor, orgTemplateAdmin, + otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + orgAdmin, otherOrgAdmin, + }, + }, + }, + { + Name: "NotificationTemplates", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, + Resource: rbac.ResourceNotificationTemplate, + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {owner}, + false: { + memberMe, orgMemberMe, userAdmin, orgUserAdmin, templateAdmin, + orgAuditor, orgTemplateAdmin, + otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + orgAdmin, otherOrgAdmin, + }, + }, + }, // AnyOrganization tests { Name: "CreateOrgMember", @@ -630,6 +658,46 @@ func TestRolePermissions(t *testing.T) { }, }, }, + { + Name: "NotificationPreferencesAnyOrg", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, + Resource: rbac.ResourceNotificationPreference.AnyOrganization().WithOwner(currentUser.String()), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {orgMemberMe, orgAdmin, otherOrgAdmin, owner}, + false: { + memberMe, templateAdmin, otherOrgUserAdmin, userAdmin, orgUserAdmin, + orgAuditor, orgTemplateAdmin, + otherOrgMember, otherOrgAuditor, otherOrgTemplateAdmin, + }, + }, + }, + { + Name: "NotificationPreferencesOtherUser", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, + Resource: rbac.ResourceNotificationPreference.InOrg(orgID).WithOwner(uuid.NewString()), // some other user + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {orgAdmin, owner}, + false: { + memberMe, templateAdmin, orgUserAdmin, userAdmin, + orgAuditor, orgTemplateAdmin, + otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + otherOrgAdmin, orgMemberMe, + }, + }, + }, + { + Name: "NotificationTemplateAnyOrg", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, + Resource: rbac.ResourceNotificationPreference.AnyOrganization(), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {orgAdmin, otherOrgAdmin, owner}, + false: { + orgMemberMe, memberMe, templateAdmin, orgUserAdmin, userAdmin, + orgAuditor, orgTemplateAdmin, + otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + }, + }, + }, } // We expect every permission to be tested above. diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index bfe228a5477d9..788cab912643b 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -66,7 +66,7 @@ var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceFile: {ActionCreate, ActionRead}, ResourceGroup: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceLicense: {ActionCreate, ActionDelete, ActionRead}, - ResourceNotificationPreference: {ActionReadPersonal, ActionUpdatePersonal}, + ResourceNotificationPreference: {ActionRead, ActionUpdate}, ResourceNotificationTemplate: {ActionRead, ActionUpdate}, ResourceOauth2App: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceOauth2AppCodeToken: {ActionCreate, ActionDelete, ActionRead}, diff --git a/site/src/api/rbacresources_gen.ts b/site/src/api/rbacresources_gen.ts index c526a95ec891e..f5538a5ae73bb 100644 --- a/site/src/api/rbacresources_gen.ts +++ b/site/src/api/rbacresources_gen.ts @@ -56,8 +56,8 @@ export const RBACResourceActions: Partial< read: "read licenses", }, notification_preference: { - read_personal: "read own notification preferences", - update_personal: "update own notification preferences", + read: "read own notification preferences", + update: "update own notification preferences", }, notification_template: { read: "read notification templates", From fdce56432f787c3174ad62f48c16c5ac663d9e81 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 1 Aug 2024 21:04:15 +0200 Subject: [PATCH 10/20] CI Signed-off-by: Danny Kopping --- codersdk/audit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codersdk/audit.go b/codersdk/audit.go index be2959127fd4c..7d83c8e238ce0 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -33,7 +33,7 @@ const ( ResourceTypeOAuth2ProviderAppSecret ResourceType = "oauth2_provider_app_secret" ResourceTypeCustomRole ResourceType = "custom_role" ResourceTypeOrganizationMember = "organization_member" - ResourceTypeNotificationTemplate = "notification_template" + ResourceTypeNotificationTemplate = "notification_template" ) func (r ResourceType) FriendlyString() string { From ee77df8c7d3ff87fecef08af0430dae5e5dbba3b Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 1 Aug 2024 21:17:34 +0200 Subject: [PATCH 11/20] Self-review, appeasing CI Signed-off-by: Danny Kopping --- coderd/database/dbauthz/dbauthz_test.go | 39 +++++++++++++++++++++++++ coderd/rbac/object_gen.go | 4 +-- coderd/rbac/policy/policy.go | 4 +-- coderd/rbac/roles_test.go | 39 ++++++++++++++++--------- site/src/api/rbacresources_gen.ts | 4 +-- 5 files changed, 70 insertions(+), 20 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 627558dbe1f73..badc03459ee1c 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -16,6 +16,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database/db2sdk" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/codersdk" @@ -2561,6 +2562,10 @@ func (s *MethodTestSuite) TestSystemFunctions() { AgentID: uuid.New(), }).Asserts(tpl, policy.ActionCreate) })) +} + +func (s *MethodTestSuite) TestNotifications() { + // System functions s.Run("AcquireNotificationMessages", s.Subtest(func(db database.Store, check *expects) { // TODO: update this test once we have a specific role for notifications check.Args(database.AcquireNotificationMessagesParams{}).Asserts(rbac.ResourceSystem, policy.ActionUpdate) @@ -2596,6 +2601,40 @@ func (s *MethodTestSuite) TestSystemFunctions() { Limit: 10, }).Asserts(rbac.ResourceSystem, policy.ActionRead) })) + + // Notification templates + s.Run("GetNotificationTemplateById", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + check.Args(user.ID).Asserts(rbac.ResourceNotificationTemplate, policy.ActionRead). + Errors(dbmem.ErrUnimplemented) + })) + s.Run("GetNotificationTemplatesByKind", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.NotificationTemplateKindSystem). + Asserts(rbac.ResourceNotificationTemplate, policy.ActionRead). + Errors(dbmem.ErrUnimplemented) + })) + s.Run("UpdateNotificationTemplateMethodById", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.UpdateNotificationTemplateMethodByIdParams{ + Method: database.NullNotificationMethod{NotificationMethod: database.NotificationMethodWebhook, Valid: true}, + ID: notifications.TemplateWorkspaceDormant, + }).Asserts(rbac.ResourceNotificationTemplate, policy.ActionUpdate). + Errors(dbmem.ErrUnimplemented) + })) + + // Notification preferences + s.Run("GetUserNotificationPreferences", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + check.Args(user.ID). + Asserts(rbac.ResourceNotificationPreference.WithOwner(user.ID.String()), policy.ActionRead) + })) + s.Run("UpdateUserNotificationPreferences", s.Subtest(func(db database.Store, check *expects) { + user := dbgen.User(s.T(), db, database.User{}) + check.Args(database.UpdateUserNotificationPreferencesParams{ + UserID: user.ID, + NotificationTemplateIds: []uuid.UUID{notifications.TemplateWorkspaceAutoUpdated, notifications.TemplateWorkspaceDeleted}, + Disableds: []bool{true, false}, + }).Asserts(rbac.ResourceNotificationPreference.WithOwner(user.ID.String()), policy.ActionUpdate) + })) } func (s *MethodTestSuite) TestOAuth2ProviderApps() { diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 64dc7fc3016f8..7645f65c5c502 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -104,8 +104,8 @@ var ( // ResourceNotificationPreference // Valid Actions - // - "ActionRead" :: read own notification preferences - // - "ActionUpdate" :: update own notification preferences + // - "ActionRead" :: read notification preferences + // - "ActionUpdate" :: update notification preferences ResourceNotificationPreference = Object{ Type: "notification_preference", } diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index e8eb96e86e82e..54dcbe358007b 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -263,8 +263,8 @@ var RBACPermissions = map[string]PermissionDefinition{ }, "notification_preference": { Actions: map[Action]ActionDefinition{ - ActionRead: actDef("read own notification preferences"), - ActionUpdate: actDef("update own notification preferences"), + ActionRead: actDef("read notification preferences"), + ActionUpdate: actDef("update notification preferences"), }, }, } diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index f25b8adc383eb..b52fc0beb9c6b 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -591,6 +591,8 @@ func TestRolePermissions(t *testing.T) { }, }, { + // Any owner/admin across may access any users' preferences + // Members may not access other members' preferences Name: "NotificationPreferencesOwn", Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceNotificationPreference.WithOwner(currentUser.String()), @@ -605,6 +607,7 @@ func TestRolePermissions(t *testing.T) { }, }, { + // Any owner/admin may access notification templates Name: "NotificationTemplates", Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceNotificationTemplate, @@ -618,6 +621,23 @@ func TestRolePermissions(t *testing.T) { }, }, }, + { + // Notification preferences are currently not organization-scoped + // Any owner/admin may access any users' preferences + // Members may not access other members' preferences + Name: "NotificationPreferencesOtherUser", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, + Resource: rbac.ResourceNotificationPreference.InOrg(orgID).WithOwner(uuid.NewString()), // some other user + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {orgAdmin, owner}, + false: { + memberMe, templateAdmin, orgUserAdmin, userAdmin, + orgAuditor, orgTemplateAdmin, + otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + otherOrgAdmin, orgMemberMe, + }, + }, + }, // AnyOrganization tests { Name: "CreateOrgMember", @@ -659,6 +679,9 @@ func TestRolePermissions(t *testing.T) { }, }, { + // Notification preferences are currently not organization-scoped + // Any owner/admin across any organization may access any users' preferences + // Members may access their own preferences Name: "NotificationPreferencesAnyOrg", Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceNotificationPreference.AnyOrganization().WithOwner(currentUser.String()), @@ -672,20 +695,8 @@ func TestRolePermissions(t *testing.T) { }, }, { - Name: "NotificationPreferencesOtherUser", - Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, - Resource: rbac.ResourceNotificationPreference.InOrg(orgID).WithOwner(uuid.NewString()), // some other user - AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {orgAdmin, owner}, - false: { - memberMe, templateAdmin, orgUserAdmin, userAdmin, - orgAuditor, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, - otherOrgAdmin, orgMemberMe, - }, - }, - }, - { + // Notification templates are currently not organization-scoped + // Any owner/admin across any organization may access notification templates Name: "NotificationTemplateAnyOrg", Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceNotificationPreference.AnyOrganization(), diff --git a/site/src/api/rbacresources_gen.ts b/site/src/api/rbacresources_gen.ts index f5538a5ae73bb..6cee389dfbc7a 100644 --- a/site/src/api/rbacresources_gen.ts +++ b/site/src/api/rbacresources_gen.ts @@ -56,8 +56,8 @@ export const RBACResourceActions: Partial< read: "read licenses", }, notification_preference: { - read: "read own notification preferences", - update: "update own notification preferences", + read: "read notification preferences", + update: "update notification preferences", }, notification_template: { read: "read notification templates", From 71dd626fe485c4add3895205f051fb1222232285 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 1 Aug 2024 21:17:34 +0200 Subject: [PATCH 12/20] Self-review, appeasing CI Signed-off-by: Danny Kopping --- coderd/database/dbauthz/dbauthz.go | 8 ++++---- coderd/database/dbauthz/dbauthz_test.go | 6 +++--- coderd/database/dbmem/dbmem.go | 6 +++--- coderd/database/dbmetrics/dbmetrics.go | 12 ++++++------ coderd/database/dbmock/dbmock.go | 24 +++++++++++------------ coderd/database/querier.go | 4 ++-- coderd/database/queries.sql.go | 14 ++++++------- coderd/database/queries/notifications.sql | 4 ++-- 8 files changed, 39 insertions(+), 39 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9e41c95fef238..216ae0c9eeacb 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1474,11 +1474,11 @@ func (q *querier) GetNotificationMessagesByStatus(ctx context.Context, arg datab return q.db.GetNotificationMessagesByStatus(ctx, arg) } -func (q *querier) GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (database.NotificationTemplate, error) { +func (q *querier) GetNotificationTemplateByID(ctx context.Context, id uuid.UUID) (database.NotificationTemplate, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceNotificationTemplate); err != nil { return database.NotificationTemplate{}, err } - return q.db.GetNotificationTemplateById(ctx, id) + return q.db.GetNotificationTemplateByID(ctx, id) } func (q *querier) GetNotificationTemplatesByKind(ctx context.Context, kind database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { @@ -3033,12 +3033,12 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb return q.db.UpdateMemberRoles(ctx, arg) } -func (q *querier) UpdateNotificationTemplateMethodById(ctx context.Context, arg database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { +func (q *querier) UpdateNotificationTemplateMethodByID(ctx context.Context, arg database.UpdateNotificationTemplateMethodByIDParams) (database.NotificationTemplate, error) { // TODO: how to restrict this to admins? if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationTemplate); err != nil { return database.NotificationTemplate{}, err } - return q.db.UpdateNotificationTemplateMethodById(ctx, arg) + return q.db.UpdateNotificationTemplateMethodByID(ctx, arg) } func (q *querier) UpdateOAuth2ProviderAppByID(ctx context.Context, arg database.UpdateOAuth2ProviderAppByIDParams) (database.OAuth2ProviderApp, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index badc03459ee1c..95d1bbcdb7f18 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2603,7 +2603,7 @@ func (s *MethodTestSuite) TestNotifications() { })) // Notification templates - s.Run("GetNotificationTemplateById", s.Subtest(func(db database.Store, check *expects) { + s.Run("GetNotificationTemplateByID", s.Subtest(func(db database.Store, check *expects) { user := dbgen.User(s.T(), db, database.User{}) check.Args(user.ID).Asserts(rbac.ResourceNotificationTemplate, policy.ActionRead). Errors(dbmem.ErrUnimplemented) @@ -2613,8 +2613,8 @@ func (s *MethodTestSuite) TestNotifications() { Asserts(rbac.ResourceNotificationTemplate, policy.ActionRead). Errors(dbmem.ErrUnimplemented) })) - s.Run("UpdateNotificationTemplateMethodById", s.Subtest(func(db database.Store, check *expects) { - check.Args(database.UpdateNotificationTemplateMethodByIdParams{ + s.Run("UpdateNotificationTemplateMethodByID", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.UpdateNotificationTemplateMethodByIDParams{ Method: database.NullNotificationMethod{NotificationMethod: database.NotificationMethodWebhook, Valid: true}, ID: notifications.TemplateWorkspaceDormant, }).Asserts(rbac.ResourceNotificationTemplate, policy.ActionUpdate). diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index f1911648b3ce2..c80f6648ad0c8 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -2710,11 +2710,11 @@ func (q *FakeQuerier) GetNotificationMessagesByStatus(_ context.Context, arg dat return out, nil } -func (*FakeQuerier) GetNotificationTemplateById(_ context.Context, _ uuid.UUID) (database.NotificationTemplate, error) { +func (*FakeQuerier) GetNotificationTemplateByID(_ context.Context, _ uuid.UUID) (database.NotificationTemplate, error) { return database.NotificationTemplate{}, ErrUnimplemented } -func (q *FakeQuerier) GetNotificationTemplatesByKind(ctx context.Context, kind database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { +func (*FakeQuerier) GetNotificationTemplatesByKind(_ context.Context, _ database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { return nil, ErrUnimplemented } @@ -7546,7 +7546,7 @@ func (q *FakeQuerier) UpdateMemberRoles(_ context.Context, arg database.UpdateMe return database.OrganizationMember{}, sql.ErrNoRows } -func (*FakeQuerier) UpdateNotificationTemplateMethodById(_ context.Context, _ database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { +func (*FakeQuerier) UpdateNotificationTemplateMethodByID(_ context.Context, _ database.UpdateNotificationTemplateMethodByIDParams) (database.NotificationTemplate, error) { return database.NotificationTemplate{}, ErrUnimplemented } diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index e00cb2fbf5c4d..7b6cdb147dcf9 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -746,10 +746,10 @@ func (m metricsStore) GetNotificationMessagesByStatus(ctx context.Context, arg d return r0, r1 } -func (m metricsStore) GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (database.NotificationTemplate, error) { +func (m metricsStore) GetNotificationTemplateByID(ctx context.Context, id uuid.UUID) (database.NotificationTemplate, error) { start := time.Now() - r0, r1 := m.s.GetNotificationTemplateById(ctx, id) - m.queryLatencies.WithLabelValues("GetNotificationTemplateById").Observe(time.Since(start).Seconds()) + r0, r1 := m.s.GetNotificationTemplateByID(ctx, id) + m.queryLatencies.WithLabelValues("GetNotificationTemplateByID").Observe(time.Since(start).Seconds()) return r0, r1 } @@ -1978,10 +1978,10 @@ func (m metricsStore) UpdateMemberRoles(ctx context.Context, arg database.Update return member, err } -func (m metricsStore) UpdateNotificationTemplateMethodById(ctx context.Context, arg database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { +func (m metricsStore) UpdateNotificationTemplateMethodByID(ctx context.Context, arg database.UpdateNotificationTemplateMethodByIDParams) (database.NotificationTemplate, error) { start := time.Now() - r0, r1 := m.s.UpdateNotificationTemplateMethodById(ctx, arg) - m.queryLatencies.WithLabelValues("UpdateNotificationTemplateMethodById").Observe(time.Since(start).Seconds()) + r0, r1 := m.s.UpdateNotificationTemplateMethodByID(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateNotificationTemplateMethodByID").Observe(time.Since(start).Seconds()) return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index a1aecc531aa26..bda8186a26a4f 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1495,19 +1495,19 @@ func (mr *MockStoreMockRecorder) GetNotificationMessagesByStatus(arg0, arg1 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNotificationMessagesByStatus", reflect.TypeOf((*MockStore)(nil).GetNotificationMessagesByStatus), arg0, arg1) } -// GetNotificationTemplateById mocks base method. -func (m *MockStore) GetNotificationTemplateById(arg0 context.Context, arg1 uuid.UUID) (database.NotificationTemplate, error) { +// GetNotificationTemplateByID mocks base method. +func (m *MockStore) GetNotificationTemplateByID(arg0 context.Context, arg1 uuid.UUID) (database.NotificationTemplate, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetNotificationTemplateById", arg0, arg1) + ret := m.ctrl.Call(m, "GetNotificationTemplateByID", arg0, arg1) ret0, _ := ret[0].(database.NotificationTemplate) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetNotificationTemplateById indicates an expected call of GetNotificationTemplateById. -func (mr *MockStoreMockRecorder) GetNotificationTemplateById(arg0, arg1 any) *gomock.Call { +// GetNotificationTemplateByID indicates an expected call of GetNotificationTemplateByID. +func (mr *MockStoreMockRecorder) GetNotificationTemplateByID(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNotificationTemplateById", reflect.TypeOf((*MockStore)(nil).GetNotificationTemplateById), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNotificationTemplateByID", reflect.TypeOf((*MockStore)(nil).GetNotificationTemplateByID), arg0, arg1) } // GetNotificationTemplatesByKind mocks base method. @@ -4176,19 +4176,19 @@ func (mr *MockStoreMockRecorder) UpdateMemberRoles(arg0, arg1 any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateMemberRoles", reflect.TypeOf((*MockStore)(nil).UpdateMemberRoles), arg0, arg1) } -// UpdateNotificationTemplateMethodById mocks base method. -func (m *MockStore) UpdateNotificationTemplateMethodById(arg0 context.Context, arg1 database.UpdateNotificationTemplateMethodByIdParams) (database.NotificationTemplate, error) { +// UpdateNotificationTemplateMethodByID mocks base method. +func (m *MockStore) UpdateNotificationTemplateMethodByID(arg0 context.Context, arg1 database.UpdateNotificationTemplateMethodByIDParams) (database.NotificationTemplate, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateNotificationTemplateMethodById", arg0, arg1) + ret := m.ctrl.Call(m, "UpdateNotificationTemplateMethodByID", arg0, arg1) ret0, _ := ret[0].(database.NotificationTemplate) ret1, _ := ret[1].(error) return ret0, ret1 } -// UpdateNotificationTemplateMethodById indicates an expected call of UpdateNotificationTemplateMethodById. -func (mr *MockStoreMockRecorder) UpdateNotificationTemplateMethodById(arg0, arg1 any) *gomock.Call { +// UpdateNotificationTemplateMethodByID indicates an expected call of UpdateNotificationTemplateMethodByID. +func (mr *MockStoreMockRecorder) UpdateNotificationTemplateMethodByID(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNotificationTemplateMethodById", reflect.TypeOf((*MockStore)(nil).UpdateNotificationTemplateMethodById), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNotificationTemplateMethodByID", reflect.TypeOf((*MockStore)(nil).UpdateNotificationTemplateMethodByID), arg0, arg1) } // UpdateOAuth2ProviderAppByID mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 6bebf112851c1..044aa0aac5669 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -162,7 +162,7 @@ type sqlcQuerier interface { GetLicenses(ctx context.Context) ([]License, error) GetLogoURL(ctx context.Context) (string, error) GetNotificationMessagesByStatus(ctx context.Context, arg GetNotificationMessagesByStatusParams) ([]NotificationMessage, error) - GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (NotificationTemplate, error) + GetNotificationTemplateByID(ctx context.Context, id uuid.UUID) (NotificationTemplate, error) GetNotificationTemplatesByKind(ctx context.Context, kind NotificationTemplateKind) ([]NotificationTemplate, error) GetNotificationsSettings(ctx context.Context) (string, error) GetOAuth2ProviderAppByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderApp, error) @@ -404,7 +404,7 @@ type sqlcQuerier interface { UpdateGroupByID(ctx context.Context, arg UpdateGroupByIDParams) (Group, error) UpdateInactiveUsersToDormant(ctx context.Context, arg UpdateInactiveUsersToDormantParams) ([]UpdateInactiveUsersToDormantRow, error) UpdateMemberRoles(ctx context.Context, arg UpdateMemberRolesParams) (OrganizationMember, error) - UpdateNotificationTemplateMethodById(ctx context.Context, arg UpdateNotificationTemplateMethodByIdParams) (NotificationTemplate, error) + UpdateNotificationTemplateMethodByID(ctx context.Context, arg UpdateNotificationTemplateMethodByIDParams) (NotificationTemplate, error) UpdateOAuth2ProviderAppByID(ctx context.Context, arg UpdateOAuth2ProviderAppByIDParams) (OAuth2ProviderApp, error) UpdateOAuth2ProviderAppSecretByID(ctx context.Context, arg UpdateOAuth2ProviderAppSecretByIDParams) (OAuth2ProviderAppSecret, error) UpdateOrganization(ctx context.Context, arg UpdateOrganizationParams) (Organization, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4f7f30cb706b7..8461d0af47321 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3630,14 +3630,14 @@ func (q *sqlQuerier) GetNotificationMessagesByStatus(ctx context.Context, arg Ge return items, nil } -const getNotificationTemplateById = `-- name: GetNotificationTemplateById :one +const getNotificationTemplateByID = `-- name: GetNotificationTemplateByID :one SELECT id, name, title_template, body_template, actions, "group", method, kind FROM notification_templates WHERE id = $1::uuid ` -func (q *sqlQuerier) GetNotificationTemplateById(ctx context.Context, id uuid.UUID) (NotificationTemplate, error) { - row := q.db.QueryRowContext(ctx, getNotificationTemplateById, id) +func (q *sqlQuerier) GetNotificationTemplateByID(ctx context.Context, id uuid.UUID) (NotificationTemplate, error) { + row := q.db.QueryRowContext(ctx, getNotificationTemplateByID, id) var i NotificationTemplate err := row.Scan( &i.ID, @@ -3724,20 +3724,20 @@ func (q *sqlQuerier) GetUserNotificationPreferences(ctx context.Context, userID return items, nil } -const updateNotificationTemplateMethodById = `-- name: UpdateNotificationTemplateMethodById :one +const updateNotificationTemplateMethodByID = `-- name: UpdateNotificationTemplateMethodByID :one UPDATE notification_templates SET method = $1::notification_method WHERE id = $2::uuid RETURNING id, name, title_template, body_template, actions, "group", method, kind ` -type UpdateNotificationTemplateMethodByIdParams struct { +type UpdateNotificationTemplateMethodByIDParams struct { Method NullNotificationMethod `db:"method" json:"method"` ID uuid.UUID `db:"id" json:"id"` } -func (q *sqlQuerier) UpdateNotificationTemplateMethodById(ctx context.Context, arg UpdateNotificationTemplateMethodByIdParams) (NotificationTemplate, error) { - row := q.db.QueryRowContext(ctx, updateNotificationTemplateMethodById, arg.Method, arg.ID) +func (q *sqlQuerier) UpdateNotificationTemplateMethodByID(ctx context.Context, arg UpdateNotificationTemplateMethodByIDParams) (NotificationTemplate, error) { + row := q.db.QueryRowContext(ctx, updateNotificationTemplateMethodByID, arg.Method, arg.ID) var i NotificationTemplate err := row.Scan( &i.ID, diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index cb4aa948eefc9..283e417dc8971 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -157,13 +157,13 @@ ON CONFLICT (user_id, notification_template_id) DO UPDATE SET disabled = EXCLUDED.disabled, updated_at = CURRENT_TIMESTAMP; --- name: UpdateNotificationTemplateMethodById :one +-- name: UpdateNotificationTemplateMethodByID :one UPDATE notification_templates SET method = sqlc.narg('method')::notification_method WHERE id = @id::uuid RETURNING *; --- name: GetNotificationTemplateById :one +-- name: GetNotificationTemplateByID :one SELECT * FROM notification_templates WHERE id = @id::uuid; From 36306afb7d077459d1d2191651c9eb8ebf2d15af Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 2 Aug 2024 09:15:27 +0000 Subject: [PATCH 13/20] CI Signed-off-by: Danny Kopping --- .../testdata/fixtures/000234_notifications.preferences.sql | 6 ++++++ coderd/database/models.go | 2 +- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 coderd/database/migrations/testdata/fixtures/000234_notifications.preferences.sql diff --git a/coderd/database/migrations/testdata/fixtures/000234_notifications.preferences.sql b/coderd/database/migrations/testdata/fixtures/000234_notifications.preferences.sql new file mode 100644 index 0000000000000..c2db895e0c39b --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000234_notifications.preferences.sql @@ -0,0 +1,6 @@ +INSERT INTO notification_preferences (user_id, notification_template_id, disabled, created_at, updated_at) VALUES + ('a3d5b8f6-1f29-4a0f-a1c1-33d5a8e4e8f1', 'c9f5a2b4-3d18-4b8b-a2c4-45f1a9e2d3b7', FALSE, '2024-07-15 10:30:00+00', '2024-07-15 10:30:00+00'), + ('b7c3f9e5-6d8b-4b3a-b2f5-56e1c8d4f2a9', 'd2e7f3a8-4b7c-4d9b-b7e1-67a2b9d5c4e8', TRUE, '2024-07-20 12:45:00+00', '2024-07-20 12:45:00+00'), + ('c2f6d4e7-5b8a-4c9e-b5a7-78f4a9d3b2c1', 'e9d8a7b2-5c8d-4e1f-a7b5-78d3c9f6e4b7', FALSE, '2024-07-25 14:00:00+00', '2024-07-25 14:00:00+00'), + ('d4a8b5f6-8d7b-4a9e-b4c5-89f2e7c1d3a8', 'f7b6c3a4-6e9a-4b7c-b3f2-90e1d2c5a7b6', TRUE, '2024-07-30 16:15:00+00', '2024-07-30 16:15:00+00'), + ('e7c6b9f2-9d8a-4c7b-b8a4-91f3e8a2d5b9', 'a2d7f4b8-7e1f-4a3c-a8b5-12b4d9c6e3f7', FALSE, '2024-08-01 18:30:00+00', '2024-08-01 18:30:00+00'); diff --git a/coderd/database/models.go b/coderd/database/models.go index 51fb4f2ffa335..4bd012e258fbd 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.26.0 +// sqlc v1.25.0 package database diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 044aa0aac5669..2d45e154b532d 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.26.0 +// sqlc v1.25.0 package database diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8461d0af47321..2f88a76b3af3e 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.26.0 +// sqlc v1.25.0 package database From 17d27bed94b3dc9b83abfd601954ab222f83f6d2 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 2 Aug 2024 09:26:36 +0000 Subject: [PATCH 14/20] Fixture, add composite primary key Signed-off-by: Danny Kopping --- coderd/database/dump.sql | 23 ++++---- .../000234_notification_preferences.up.sql | 53 ++++++++++--------- .../000234_notifications.preferences.sql | 6 --- .../000234_notifications_preferences.up.sql | 9 ++++ coderd/database/unique_constraint.go | 1 + 5 files changed, 50 insertions(+), 42 deletions(-) delete mode 100644 coderd/database/migrations/testdata/fixtures/000234_notifications.preferences.sql create mode 100644 coderd/database/migrations/testdata/fixtures/000234_notifications_preferences.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 2d2b783c8b6af..589c8ae55e5fa 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -259,16 +259,16 @@ CREATE FUNCTION inhibit_enqueue_if_disabled() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN - -- Fail the insertion if the user has disabled this notification. - IF EXISTS (SELECT 1 - FROM notification_preferences - WHERE disabled = TRUE - AND user_id = NEW.user_id - AND notification_template_id = NEW.notification_template_id) THEN - RAISE EXCEPTION 'cannot enqueue message: user has disabled this notification'; - END IF; - - RETURN NEW; + -- Fail the insertion if the user has disabled this notification. + IF EXISTS (SELECT 1 + FROM notification_preferences + WHERE disabled = TRUE + AND user_id = NEW.user_id + AND notification_template_id = NEW.notification_template_id) THEN + RAISE EXCEPTION 'cannot enqueue message: user has disabled this notification'; + END IF; + + RETURN NEW; END; $$; @@ -1571,6 +1571,9 @@ ALTER TABLE ONLY licenses ALTER TABLE ONLY notification_messages ADD CONSTRAINT notification_messages_pkey PRIMARY KEY (id); +ALTER TABLE ONLY notification_preferences + ADD CONSTRAINT notification_preferences_pkey PRIMARY KEY (user_id, notification_template_id); + ALTER TABLE ONLY notification_templates ADD CONSTRAINT notification_templates_name_key UNIQUE (name); diff --git a/coderd/database/migrations/000234_notification_preferences.up.sql b/coderd/database/migrations/000234_notification_preferences.up.sql index 182d9a72a57e3..417786d2fe6ff 100644 --- a/coderd/database/migrations/000234_notification_preferences.up.sql +++ b/coderd/database/migrations/000234_notification_preferences.up.sql @@ -1,26 +1,27 @@ CREATE TABLE notification_preferences ( - user_id uuid REFERENCES users ON DELETE CASCADE NOT NULL, - notification_template_id uuid REFERENCES notification_templates ON DELETE CASCADE NOT NULL, - disabled bool NOT NULL DEFAULT FALSE, - created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, - updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP + user_id uuid REFERENCES users ON DELETE CASCADE NOT NULL, + notification_template_id uuid REFERENCES notification_templates ON DELETE CASCADE NOT NULL, + disabled bool NOT NULL DEFAULT FALSE, + created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (user_id, notification_template_id) ); -- Ensure we cannot insert multiple entries for the same user/template combination. ALTER TABLE notification_preferences - ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); + ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); -- Add a new type (to be expanded upon later) which specifies the kind of notification template. CREATE TYPE notification_template_kind AS ENUM ( - 'system' - ); + 'system' + ); ALTER TABLE notification_templates - -- Allow per-template notification method (enterprise only). - ADD COLUMN method notification_method, - -- Update all existing notification templates to be system templates. - ADD COLUMN kind notification_template_kind DEFAULT 'system'::notification_template_kind NOT NULL; + -- Allow per-template notification method (enterprise only). + ADD COLUMN method notification_method, + -- Update all existing notification templates to be system templates. + ADD COLUMN kind notification_template_kind DEFAULT 'system'::notification_template_kind NOT NULL; COMMENT ON COLUMN notification_templates.method IS 'NULL defers to the deployment-level method'; -- No equivalent in down migration because ENUM values cannot be deleted. @@ -28,27 +29,27 @@ ALTER TYPE notification_message_status ADD VALUE IF NOT EXISTS 'inhibited'; -- Function to prevent enqueuing notifications unnecessarily. CREATE OR REPLACE FUNCTION inhibit_enqueue_if_disabled() - RETURNS TRIGGER AS + RETURNS TRIGGER AS $$ BEGIN - -- Fail the insertion if the user has disabled this notification. - IF EXISTS (SELECT 1 - FROM notification_preferences - WHERE disabled = TRUE - AND user_id = NEW.user_id - AND notification_template_id = NEW.notification_template_id) THEN - RAISE EXCEPTION 'cannot enqueue message: user has disabled this notification'; - END IF; - - RETURN NEW; + -- Fail the insertion if the user has disabled this notification. + IF EXISTS (SELECT 1 + FROM notification_preferences + WHERE disabled = TRUE + AND user_id = NEW.user_id + AND notification_template_id = NEW.notification_template_id) THEN + RAISE EXCEPTION 'cannot enqueue message: user has disabled this notification'; + END IF; + + RETURN NEW; END; $$ LANGUAGE plpgsql; -- Trigger to execute above function on insertion. CREATE TRIGGER inhibit_enqueue_if_disabled - BEFORE INSERT - ON notification_messages - FOR EACH ROW + BEFORE INSERT + ON notification_messages + FOR EACH ROW EXECUTE FUNCTION inhibit_enqueue_if_disabled(); -- Allow modifications to notification templates to be audited. diff --git a/coderd/database/migrations/testdata/fixtures/000234_notifications.preferences.sql b/coderd/database/migrations/testdata/fixtures/000234_notifications.preferences.sql deleted file mode 100644 index c2db895e0c39b..0000000000000 --- a/coderd/database/migrations/testdata/fixtures/000234_notifications.preferences.sql +++ /dev/null @@ -1,6 +0,0 @@ -INSERT INTO notification_preferences (user_id, notification_template_id, disabled, created_at, updated_at) VALUES - ('a3d5b8f6-1f29-4a0f-a1c1-33d5a8e4e8f1', 'c9f5a2b4-3d18-4b8b-a2c4-45f1a9e2d3b7', FALSE, '2024-07-15 10:30:00+00', '2024-07-15 10:30:00+00'), - ('b7c3f9e5-6d8b-4b3a-b2f5-56e1c8d4f2a9', 'd2e7f3a8-4b7c-4d9b-b7e1-67a2b9d5c4e8', TRUE, '2024-07-20 12:45:00+00', '2024-07-20 12:45:00+00'), - ('c2f6d4e7-5b8a-4c9e-b5a7-78f4a9d3b2c1', 'e9d8a7b2-5c8d-4e1f-a7b5-78d3c9f6e4b7', FALSE, '2024-07-25 14:00:00+00', '2024-07-25 14:00:00+00'), - ('d4a8b5f6-8d7b-4a9e-b4c5-89f2e7c1d3a8', 'f7b6c3a4-6e9a-4b7c-b3f2-90e1d2c5a7b6', TRUE, '2024-07-30 16:15:00+00', '2024-07-30 16:15:00+00'), - ('e7c6b9f2-9d8a-4c7b-b8a4-91f3e8a2d5b9', 'a2d7f4b8-7e1f-4a3c-a8b5-12b4d9c6e3f7', FALSE, '2024-08-01 18:30:00+00', '2024-08-01 18:30:00+00'); diff --git a/coderd/database/migrations/testdata/fixtures/000234_notifications_preferences.up.sql b/coderd/database/migrations/testdata/fixtures/000234_notifications_preferences.up.sql new file mode 100644 index 0000000000000..5795ca15dc5f8 --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000234_notifications_preferences.up.sql @@ -0,0 +1,9 @@ +INSERT INTO users(id, email, username, hashed_password, created_at, updated_at, status, rbac_roles, deleted) +VALUES ('fc1511ef-4fcf-4a3b-98a1-8df64160e35a', 'githubuser@coder.com', 'githubuser', '\x', + '2022-11-02 13:05:21.445455+02', '2022-11-02 13:05:21.445455+02', 'active', '{}', false) ON CONFLICT DO NOTHING; + +INSERT INTO notification_templates (id, name, title_template, body_template, "group") +VALUES ('a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11', 'A', 'title', 'body', 'Group 1') ON CONFLICT DO NOTHING; + +INSERT INTO notification_preferences (user_id, notification_template_id, disabled, created_at, updated_at) +VALUES ('a0061a8e-7db7-4585-838c-3116a003dd21', 'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11', FALSE, '2024-07-15 10:30:00+00', '2024-07-15 10:30:00+00'); diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index f5c7254361b00..f6d37d9bf67a3 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -24,6 +24,7 @@ const ( UniqueLicensesJWTKey UniqueConstraint = "licenses_jwt_key" // ALTER TABLE ONLY licenses ADD CONSTRAINT licenses_jwt_key UNIQUE (jwt); UniqueLicensesPkey UniqueConstraint = "licenses_pkey" // ALTER TABLE ONLY licenses ADD CONSTRAINT licenses_pkey PRIMARY KEY (id); UniqueNotificationMessagesPkey UniqueConstraint = "notification_messages_pkey" // ALTER TABLE ONLY notification_messages ADD CONSTRAINT notification_messages_pkey PRIMARY KEY (id); + UniqueNotificationPreferencesPkey UniqueConstraint = "notification_preferences_pkey" // ALTER TABLE ONLY notification_preferences ADD CONSTRAINT notification_preferences_pkey PRIMARY KEY (user_id, notification_template_id); UniqueNotificationTemplatesNameKey UniqueConstraint = "notification_templates_name_key" // ALTER TABLE ONLY notification_templates ADD CONSTRAINT notification_templates_name_key UNIQUE (name); UniqueNotificationTemplatesPkey UniqueConstraint = "notification_templates_pkey" // ALTER TABLE ONLY notification_templates ADD CONSTRAINT notification_templates_pkey PRIMARY KEY (id); UniqueOauth2ProviderAppCodesPkey UniqueConstraint = "oauth2_provider_app_codes_pkey" // ALTER TABLE ONLY oauth2_provider_app_codes ADD CONSTRAINT oauth2_provider_app_codes_pkey PRIMARY KEY (id); From 52d3108485f97c86de96bf5ca6142ce5098be03c Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 2 Aug 2024 11:48:51 +0200 Subject: [PATCH 15/20] Self-review Signed-off-by: Danny Kopping --- coderd/database/dbauthz/dbauthz.go | 1 - coderd/database/queries.sql.go | 6 ++---- coderd/database/queries/notifications.sql | 6 ++---- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 216ae0c9eeacb..95ef2d2f0a807 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3034,7 +3034,6 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb } func (q *querier) UpdateNotificationTemplateMethodByID(ctx context.Context, arg database.UpdateNotificationTemplateMethodByIDParams) (database.NotificationTemplate, error) { - // TODO: how to restrict this to admins? if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationTemplate); err != nil { return database.NotificationTemplate{}, err } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2f88a76b3af3e..b635ee0fd40fe 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3753,13 +3753,11 @@ func (q *sqlQuerier) UpdateNotificationTemplateMethodByID(ctx context.Context, a } const updateUserNotificationPreferences = `-- name: UpdateUserNotificationPreferences :execrows -WITH new_values AS - (SELECT UNNEST($2::uuid[]) AS notification_template_id, - UNNEST($3::bool[]) AS disabled) INSERT INTO notification_preferences (user_id, notification_template_id, disabled) SELECT $1::uuid, new_values.notification_template_id, new_values.disabled -FROM new_values +FROM (SELECT UNNEST($2::uuid[]) AS notification_template_id, + UNNEST($3::bool[]) AS disabled) AS new_values ON CONFLICT (user_id, notification_template_id) DO UPDATE SET disabled = EXCLUDED.disabled, updated_at = CURRENT_TIMESTAMP diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index 283e417dc8971..d62564ea6edbf 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -146,13 +146,11 @@ FROM notification_preferences WHERE user_id = @user_id::uuid; -- name: UpdateUserNotificationPreferences :execrows -WITH new_values AS - (SELECT UNNEST(@notification_template_ids::uuid[]) AS notification_template_id, - UNNEST(@disableds::bool[]) AS disabled) INSERT INTO notification_preferences (user_id, notification_template_id, disabled) SELECT @user_id::uuid, new_values.notification_template_id, new_values.disabled -FROM new_values +FROM (SELECT UNNEST(@notification_template_ids::uuid[]) AS notification_template_id, + UNNEST(@disableds::bool[]) AS disabled) AS new_values ON CONFLICT (user_id, notification_template_id) DO UPDATE SET disabled = EXCLUDED.disabled, updated_at = CURRENT_TIMESTAMP; From 2d451874640c001a1bf63a323ccea61cb9cb2d23 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 2 Aug 2024 10:18:23 +0000 Subject: [PATCH 16/20] Rename migration Signed-off-by: Danny Kopping --- ...ferences.down.sql => 000236_notification_preferences.down.sql} | 0 ..._preferences.up.sql => 000236_notification_preferences.up.sql} | 0 ...preferences.up.sql => 000236_notifications_preferences.up.sql} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000234_notification_preferences.down.sql => 000236_notification_preferences.down.sql} (100%) rename coderd/database/migrations/{000234_notification_preferences.up.sql => 000236_notification_preferences.up.sql} (100%) rename coderd/database/migrations/testdata/fixtures/{000234_notifications_preferences.up.sql => 000236_notifications_preferences.up.sql} (100%) diff --git a/coderd/database/migrations/000234_notification_preferences.down.sql b/coderd/database/migrations/000236_notification_preferences.down.sql similarity index 100% rename from coderd/database/migrations/000234_notification_preferences.down.sql rename to coderd/database/migrations/000236_notification_preferences.down.sql diff --git a/coderd/database/migrations/000234_notification_preferences.up.sql b/coderd/database/migrations/000236_notification_preferences.up.sql similarity index 100% rename from coderd/database/migrations/000234_notification_preferences.up.sql rename to coderd/database/migrations/000236_notification_preferences.up.sql diff --git a/coderd/database/migrations/testdata/fixtures/000234_notifications_preferences.up.sql b/coderd/database/migrations/testdata/fixtures/000236_notifications_preferences.up.sql similarity index 100% rename from coderd/database/migrations/testdata/fixtures/000234_notifications_preferences.up.sql rename to coderd/database/migrations/testdata/fixtures/000236_notifications_preferences.up.sql From 27dfc51bec9fa52c3c46291aee0c666e68e10812 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 2 Aug 2024 16:23:44 +0200 Subject: [PATCH 17/20] Rename migration, again Signed-off-by: Danny Kopping --- ...ferences.down.sql => 000237_notification_preferences.down.sql} | 0 ..._preferences.up.sql => 000237_notification_preferences.up.sql} | 0 ...preferences.up.sql => 000237_notifications_preferences.up.sql} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000236_notification_preferences.down.sql => 000237_notification_preferences.down.sql} (100%) rename coderd/database/migrations/{000236_notification_preferences.up.sql => 000237_notification_preferences.up.sql} (100%) rename coderd/database/migrations/testdata/fixtures/{000236_notifications_preferences.up.sql => 000237_notifications_preferences.up.sql} (100%) diff --git a/coderd/database/migrations/000236_notification_preferences.down.sql b/coderd/database/migrations/000237_notification_preferences.down.sql similarity index 100% rename from coderd/database/migrations/000236_notification_preferences.down.sql rename to coderd/database/migrations/000237_notification_preferences.down.sql diff --git a/coderd/database/migrations/000236_notification_preferences.up.sql b/coderd/database/migrations/000237_notification_preferences.up.sql similarity index 100% rename from coderd/database/migrations/000236_notification_preferences.up.sql rename to coderd/database/migrations/000237_notification_preferences.up.sql diff --git a/coderd/database/migrations/testdata/fixtures/000236_notifications_preferences.up.sql b/coderd/database/migrations/testdata/fixtures/000237_notifications_preferences.up.sql similarity index 100% rename from coderd/database/migrations/testdata/fixtures/000236_notifications_preferences.up.sql rename to coderd/database/migrations/testdata/fixtures/000237_notifications_preferences.up.sql From c744d333716058743f4387938f0887d76e962fc3 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 5 Aug 2024 08:52:01 +0200 Subject: [PATCH 18/20] Review feedback Signed-off-by: Danny Kopping --- coderd/database/dbauthz/dbauthz.go | 2 + coderd/database/dump.sql | 3 -- ... 000238_notification_preferences.down.sql} | 0 ...=> 000238_notification_preferences.up.sql} | 4 -- ...> 000238_notifications_preferences.up.sql} | 0 coderd/database/unique_constraint.go | 1 - coderd/rbac/roles_test.go | 37 ++----------------- 7 files changed, 5 insertions(+), 42 deletions(-) rename coderd/database/migrations/{000237_notification_preferences.down.sql => 000238_notification_preferences.down.sql} (100%) rename coderd/database/migrations/{000237_notification_preferences.up.sql => 000238_notification_preferences.up.sql} (91%) rename coderd/database/migrations/testdata/fixtures/{000237_notifications_preferences.up.sql => 000238_notifications_preferences.up.sql} (100%) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 95ef2d2f0a807..2f3567455fed8 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1483,6 +1483,8 @@ func (q *querier) GetNotificationTemplateByID(ctx context.Context, id uuid.UUID) func (q *querier) GetNotificationTemplatesByKind(ctx context.Context, kind database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { // TODO: restrict 'system' kind to admins only? + // All notification templates share the same rbac.Object, so there is no need + // to authorize them individually. If this passes, all notification templates can be read. if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceNotificationTemplate); err != nil { return nil, err } diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 589c8ae55e5fa..0bcad08271da5 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1679,9 +1679,6 @@ ALTER TABLE ONLY template_versions ALTER TABLE ONLY templates ADD CONSTRAINT templates_pkey PRIMARY KEY (id); -ALTER TABLE ONLY notification_preferences - ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); - ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); diff --git a/coderd/database/migrations/000237_notification_preferences.down.sql b/coderd/database/migrations/000238_notification_preferences.down.sql similarity index 100% rename from coderd/database/migrations/000237_notification_preferences.down.sql rename to coderd/database/migrations/000238_notification_preferences.down.sql diff --git a/coderd/database/migrations/000237_notification_preferences.up.sql b/coderd/database/migrations/000238_notification_preferences.up.sql similarity index 91% rename from coderd/database/migrations/000237_notification_preferences.up.sql rename to coderd/database/migrations/000238_notification_preferences.up.sql index 417786d2fe6ff..c6e38a3ab69fd 100644 --- a/coderd/database/migrations/000237_notification_preferences.up.sql +++ b/coderd/database/migrations/000238_notification_preferences.up.sql @@ -8,10 +8,6 @@ CREATE TABLE notification_preferences PRIMARY KEY (user_id, notification_template_id) ); --- Ensure we cannot insert multiple entries for the same user/template combination. -ALTER TABLE notification_preferences - ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); - -- Add a new type (to be expanded upon later) which specifies the kind of notification template. CREATE TYPE notification_template_kind AS ENUM ( 'system' diff --git a/coderd/database/migrations/testdata/fixtures/000237_notifications_preferences.up.sql b/coderd/database/migrations/testdata/fixtures/000238_notifications_preferences.up.sql similarity index 100% rename from coderd/database/migrations/testdata/fixtures/000237_notifications_preferences.up.sql rename to coderd/database/migrations/testdata/fixtures/000238_notifications_preferences.up.sql diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index f6d37d9bf67a3..f3f42ea0b72ad 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -60,7 +60,6 @@ const ( UniqueTemplateVersionsPkey UniqueConstraint = "template_versions_pkey" // ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_pkey PRIMARY KEY (id); UniqueTemplateVersionsTemplateIDNameKey UniqueConstraint = "template_versions_template_id_name_key" // ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_template_id_name_key UNIQUE (template_id, name); UniqueTemplatesPkey UniqueConstraint = "templates_pkey" // ALTER TABLE ONLY templates ADD CONSTRAINT templates_pkey PRIMARY KEY (id); - UniqueUniqueUserNotificationTemplate UniqueConstraint = "unique_user_notification_template" // ALTER TABLE ONLY notification_preferences ADD CONSTRAINT unique_user_notification_template UNIQUE (user_id, notification_template_id); UniqueUserLinksPkey UniqueConstraint = "user_links_pkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); UniqueUsersPkey UniqueConstraint = "users_pkey" // ALTER TABLE ONLY users ADD CONSTRAINT users_pkey PRIMARY KEY (id); UniqueWorkspaceAgentLogSourcesPkey UniqueConstraint = "workspace_agent_log_sources_pkey" // ALTER TABLE ONLY workspace_agent_log_sources ADD CONSTRAINT workspace_agent_log_sources_pkey PRIMARY KEY (workspace_agent_id, id); diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index b52fc0beb9c6b..9d71629d95d9f 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -627,12 +627,12 @@ func TestRolePermissions(t *testing.T) { // Members may not access other members' preferences Name: "NotificationPreferencesOtherUser", Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, - Resource: rbac.ResourceNotificationPreference.InOrg(orgID).WithOwner(uuid.NewString()), // some other user + Resource: rbac.ResourceNotificationPreference.WithOwner(uuid.NewString()), // some other user AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {orgAdmin, owner}, + true: {owner}, false: { memberMe, templateAdmin, orgUserAdmin, userAdmin, - orgAuditor, orgTemplateAdmin, + orgAdmin, orgAuditor, orgTemplateAdmin, otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, otherOrgAdmin, orgMemberMe, }, @@ -678,37 +678,6 @@ func TestRolePermissions(t *testing.T) { }, }, }, - { - // Notification preferences are currently not organization-scoped - // Any owner/admin across any organization may access any users' preferences - // Members may access their own preferences - Name: "NotificationPreferencesAnyOrg", - Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, - Resource: rbac.ResourceNotificationPreference.AnyOrganization().WithOwner(currentUser.String()), - AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {orgMemberMe, orgAdmin, otherOrgAdmin, owner}, - false: { - memberMe, templateAdmin, otherOrgUserAdmin, userAdmin, orgUserAdmin, - orgAuditor, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgTemplateAdmin, - }, - }, - }, - { - // Notification templates are currently not organization-scoped - // Any owner/admin across any organization may access notification templates - Name: "NotificationTemplateAnyOrg", - Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, - Resource: rbac.ResourceNotificationPreference.AnyOrganization(), - AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {orgAdmin, otherOrgAdmin, owner}, - false: { - orgMemberMe, memberMe, templateAdmin, orgUserAdmin, userAdmin, - orgAuditor, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, - }, - }, - }, } // We expect every permission to be tested above. From e4103c3cd7b2e047d864fa2d93049b9bb4fbe8d0 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 5 Aug 2024 15:32:14 +0200 Subject: [PATCH 19/20] feat: allow admins to modify notification template delivery method (#14116) --- cli/notifications_test.go | 16 +- coderd/apidoc/docs.go | 249 ++++++++++++++++- coderd/apidoc/swagger.json | 227 +++++++++++++++- coderd/coderd.go | 15 +- coderd/database/queries.sql.go | 20 +- coderd/database/queries/notifications.sql | 6 +- coderd/httpmw/notificationtemplateparam.go | 49 ++++ coderd/notifications.go | 204 +++++++++++++- coderd/notifications/enqueuer.go | 53 ++-- coderd/notifications/manager.go | 18 +- coderd/notifications/metrics_test.go | 75 ++++++ coderd/notifications/notifications_test.go | 190 ++++++++++++- coderd/notifications/notifier.go | 39 ++- coderd/notifications_test.go | 231 +++++++++++++++- codersdk/notifications.go | 161 +++++++++++ docs/api/enterprise.md | 27 ++ docs/api/general.md | 78 ------ docs/api/notifications.md | 296 +++++++++++++++++++++ docs/api/schemas.md | 80 ++++++ docs/manifest.json | 4 + enterprise/coderd/coderd.go | 10 +- enterprise/coderd/notifications.go | 98 +++++++ enterprise/coderd/notifications_test.go | 180 +++++++++++++ site/src/api/typesGenerated.ts | 35 +++ 24 files changed, 2211 insertions(+), 150 deletions(-) create mode 100644 coderd/httpmw/notificationtemplateparam.go create mode 100644 docs/api/notifications.md create mode 100644 enterprise/coderd/notifications.go create mode 100644 enterprise/coderd/notifications_test.go diff --git a/cli/notifications_test.go b/cli/notifications_test.go index 9ea4d7072e4c3..9d7ff8a37abc3 100644 --- a/cli/notifications_test.go +++ b/cli/notifications_test.go @@ -16,6 +16,16 @@ import ( "github.com/coder/coder/v2/testutil" ) +func createOpts(t *testing.T) *coderdtest.Options { + t.Helper() + + dt := coderdtest.DeploymentValues(t) + dt.Experiments = []string{string(codersdk.ExperimentNotifications)} + return &coderdtest.Options{ + DeploymentValues: dt, + } +} + func TestNotifications(t *testing.T) { t.Parallel() @@ -42,7 +52,7 @@ func TestNotifications(t *testing.T) { t.Parallel() // given - ownerClient, db := coderdtest.NewWithDatabase(t, nil) + ownerClient, db := coderdtest.NewWithDatabase(t, createOpts(t)) _ = coderdtest.CreateFirstUser(t, ownerClient) // when @@ -72,7 +82,7 @@ func TestPauseNotifications_RegularUser(t *testing.T) { t.Parallel() // given - ownerClient, db := coderdtest.NewWithDatabase(t, nil) + ownerClient, db := coderdtest.NewWithDatabase(t, createOpts(t)) owner := coderdtest.CreateFirstUser(t, ownerClient) anotherClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID) @@ -87,7 +97,7 @@ func TestPauseNotifications_RegularUser(t *testing.T) { require.Error(t, err) require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error") assert.Equal(t, http.StatusForbidden, sdkError.StatusCode()) - assert.Contains(t, sdkError.Message, "Insufficient permissions to update notifications settings.") + assert.Contains(t, sdkError.Message, "Forbidden.") // then ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 6671fdb796836..962fccae0a4ea 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -1547,6 +1547,34 @@ const docTemplate = `{ } } }, + "/notifications/dispatch-methods": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Notifications" + ], + "summary": "Get notification dispatch methods", + "operationId": "get-notification-dispatch-methods", + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.NotificationMethodsResponse" + } + } + } + } + } + }, "/notifications/settings": { "get": { "security": [ @@ -1558,7 +1586,7 @@ const docTemplate = `{ "application/json" ], "tags": [ - "General" + "Notifications" ], "summary": "Get notifications settings", "operationId": "get-notifications-settings", @@ -1584,7 +1612,7 @@ const docTemplate = `{ "application/json" ], "tags": [ - "General" + "Notifications" ], "summary": "Update notifications settings", "operationId": "update-notifications-settings", @@ -1612,6 +1640,68 @@ const docTemplate = `{ } } }, + "/notifications/templates/system": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Notifications" + ], + "summary": "Get system notification templates", + "operationId": "get-system-notification-templates", + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.NotificationTemplate" + } + } + } + } + } + }, + "/notifications/templates/{notification_template}/method": { + "put": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Update notification template dispatch method", + "operationId": "update-notification-template-dispatch-method", + "parameters": [ + { + "type": "string", + "description": "Notification template UUID", + "name": "notification_template", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "Success" + }, + "304": { + "description": "Not modified" + } + } + } + }, "/oauth2-provider/apps": { "get": { "security": [ @@ -5354,6 +5444,90 @@ const docTemplate = `{ } } }, + "/users/{user}/notifications/preferences": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Notifications" + ], + "summary": "Get user notification preferences", + "operationId": "get-user-notification-preferences", + "parameters": [ + { + "type": "string", + "description": "User ID, name, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.NotificationPreference" + } + } + } + } + }, + "put": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Notifications" + ], + "summary": "Update user notification preferences", + "operationId": "update-user-notification-preferences", + "parameters": [ + { + "description": "Preferences", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.UpdateUserNotificationPreferences" + } + }, + { + "type": "string", + "description": "User ID, name, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.NotificationPreference" + } + } + } + } + } + }, "/users/{user}/organizations": { "get": { "security": [ @@ -10202,6 +10376,66 @@ const docTemplate = `{ } } }, + "codersdk.NotificationMethodsResponse": { + "type": "object", + "properties": { + "available": { + "type": "array", + "items": { + "type": "string" + } + }, + "default": { + "type": "string" + } + } + }, + "codersdk.NotificationPreference": { + "type": "object", + "properties": { + "disabled": { + "type": "boolean" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + } + }, + "codersdk.NotificationTemplate": { + "type": "object", + "properties": { + "actions": { + "type": "string" + }, + "body_template": { + "type": "string" + }, + "group": { + "type": "string" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "kind": { + "type": "string" + }, + "method": { + "type": "string" + }, + "name": { + "type": "string" + }, + "title_template": { + "type": "string" + } + } + }, "codersdk.NotificationsConfig": { "type": "object", "properties": { @@ -12517,6 +12751,17 @@ const docTemplate = `{ } } }, + "codersdk.UpdateUserNotificationPreferences": { + "type": "object", + "properties": { + "template_disabled_map": { + "type": "object", + "additionalProperties": { + "type": "boolean" + } + } + } + }, "codersdk.UpdateUserPasswordRequest": { "type": "object", "required": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 02d0247df7362..35b8b82a21888 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1344,6 +1344,30 @@ } } }, + "/notifications/dispatch-methods": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Notifications"], + "summary": "Get notification dispatch methods", + "operationId": "get-notification-dispatch-methods", + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.NotificationMethodsResponse" + } + } + } + } + } + }, "/notifications/settings": { "get": { "security": [ @@ -1352,7 +1376,7 @@ } ], "produces": ["application/json"], - "tags": ["General"], + "tags": ["Notifications"], "summary": "Get notifications settings", "operationId": "get-notifications-settings", "responses": { @@ -1372,7 +1396,7 @@ ], "consumes": ["application/json"], "produces": ["application/json"], - "tags": ["General"], + "tags": ["Notifications"], "summary": "Update notifications settings", "operationId": "update-notifications-settings", "parameters": [ @@ -1399,6 +1423,60 @@ } } }, + "/notifications/templates/system": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Notifications"], + "summary": "Get system notification templates", + "operationId": "get-system-notification-templates", + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.NotificationTemplate" + } + } + } + } + } + }, + "/notifications/templates/{notification_template}/method": { + "put": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Update notification template dispatch method", + "operationId": "update-notification-template-dispatch-method", + "parameters": [ + { + "type": "string", + "description": "Notification template UUID", + "name": "notification_template", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "Success" + }, + "304": { + "description": "Not modified" + } + } + } + }, "/oauth2-provider/apps": { "get": { "security": [ @@ -4726,6 +4804,80 @@ } } }, + "/users/{user}/notifications/preferences": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Notifications"], + "summary": "Get user notification preferences", + "operationId": "get-user-notification-preferences", + "parameters": [ + { + "type": "string", + "description": "User ID, name, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.NotificationPreference" + } + } + } + } + }, + "put": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "tags": ["Notifications"], + "summary": "Update user notification preferences", + "operationId": "update-user-notification-preferences", + "parameters": [ + { + "description": "Preferences", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.UpdateUserNotificationPreferences" + } + }, + { + "type": "string", + "description": "User ID, name, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.NotificationPreference" + } + } + } + } + } + }, "/users/{user}/organizations": { "get": { "security": [ @@ -9143,6 +9295,66 @@ } } }, + "codersdk.NotificationMethodsResponse": { + "type": "object", + "properties": { + "available": { + "type": "array", + "items": { + "type": "string" + } + }, + "default": { + "type": "string" + } + } + }, + "codersdk.NotificationPreference": { + "type": "object", + "properties": { + "disabled": { + "type": "boolean" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + } + }, + "codersdk.NotificationTemplate": { + "type": "object", + "properties": { + "actions": { + "type": "string" + }, + "body_template": { + "type": "string" + }, + "group": { + "type": "string" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "kind": { + "type": "string" + }, + "method": { + "type": "string" + }, + "name": { + "type": "string" + }, + "title_template": { + "type": "string" + } + } + }, "codersdk.NotificationsConfig": { "type": "object", "properties": { @@ -11366,6 +11578,17 @@ } } }, + "codersdk.UpdateUserNotificationPreferences": { + "type": "object", + "properties": { + "template_disabled_map": { + "type": "object", + "additionalProperties": { + "type": "boolean" + } + } + } + }, "codersdk.UpdateUserPasswordRequest": { "type": "object", "required": ["password"], diff --git a/coderd/coderd.go b/coderd/coderd.go index 6f8a59ad6efc6..7fbfe7d477f06 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1050,6 +1050,12 @@ func New(options *Options) *API { }) r.Get("/gitsshkey", api.gitSSHKey) r.Put("/gitsshkey", api.regenerateGitSSHKey) + r.Route("/notifications", func(r chi.Router) { + r.Route("/preferences", func(r chi.Router) { + r.Get("/", api.userNotificationPreferences) + r.Put("/", api.putUserNotificationPreferences) + }) + }) }) }) }) @@ -1243,9 +1249,16 @@ func New(options *Options) *API { }) }) r.Route("/notifications", func(r chi.Router) { - r.Use(apiKeyMiddleware) + r.Use( + apiKeyMiddleware, + httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentNotifications), + ) r.Get("/settings", api.notificationsSettings) r.Put("/settings", api.putNotificationsSettings) + r.Route("/templates", func(r chi.Router) { + r.Get("/system", api.systemNotificationTemplates) + }) + r.Get("/dispatch-methods", api.notificationDispatchMethods) }) }) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index b635ee0fd40fe..d8a6e3a1abb03 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3540,10 +3540,11 @@ func (q *sqlQuerier) EnqueueNotificationMessage(ctx context.Context, arg Enqueue const fetchNewMessageMetadata = `-- name: FetchNewMessageMetadata :one SELECT nt.name AS notification_name, nt.actions AS actions, + nt.method AS custom_method, u.id AS user_id, u.email AS user_email, COALESCE(NULLIF(u.name, ''), NULLIF(u.username, ''))::text AS user_name, - COALESCE(u.username, '') AS user_username + u.username AS user_username FROM notification_templates nt, users u WHERE nt.id = $1 @@ -3556,12 +3557,13 @@ type FetchNewMessageMetadataParams struct { } type FetchNewMessageMetadataRow struct { - NotificationName string `db:"notification_name" json:"notification_name"` - Actions []byte `db:"actions" json:"actions"` - UserID uuid.UUID `db:"user_id" json:"user_id"` - UserEmail string `db:"user_email" json:"user_email"` - UserName string `db:"user_name" json:"user_name"` - UserUsername string `db:"user_username" json:"user_username"` + NotificationName string `db:"notification_name" json:"notification_name"` + Actions []byte `db:"actions" json:"actions"` + CustomMethod NullNotificationMethod `db:"custom_method" json:"custom_method"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + UserEmail string `db:"user_email" json:"user_email"` + UserName string `db:"user_name" json:"user_name"` + UserUsername string `db:"user_username" json:"user_username"` } // This is used to build up the notification_message's JSON payload. @@ -3571,6 +3573,7 @@ func (q *sqlQuerier) FetchNewMessageMetadata(ctx context.Context, arg FetchNewMe err := row.Scan( &i.NotificationName, &i.Actions, + &i.CustomMethod, &i.UserID, &i.UserEmail, &i.UserName, @@ -3653,7 +3656,8 @@ func (q *sqlQuerier) GetNotificationTemplateByID(ctx context.Context, id uuid.UU } const getNotificationTemplatesByKind = `-- name: GetNotificationTemplatesByKind :many -SELECT id, name, title_template, body_template, actions, "group", method, kind FROM notification_templates +SELECT id, name, title_template, body_template, actions, "group", method, kind +FROM notification_templates WHERE kind = $1::notification_template_kind ` diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index d62564ea6edbf..f5b8601871ccc 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -2,10 +2,11 @@ -- This is used to build up the notification_message's JSON payload. SELECT nt.name AS notification_name, nt.actions AS actions, + nt.method AS custom_method, u.id AS user_id, u.email AS user_email, COALESCE(NULLIF(u.name, ''), NULLIF(u.username, ''))::text AS user_name, - COALESCE(u.username, '') AS user_username + u.username AS user_username FROM notification_templates nt, users u WHERE nt.id = @notification_template_id @@ -167,5 +168,6 @@ FROM notification_templates WHERE id = @id::uuid; -- name: GetNotificationTemplatesByKind :many -SELECT * FROM notification_templates +SELECT * +FROM notification_templates WHERE kind = @kind::notification_template_kind; diff --git a/coderd/httpmw/notificationtemplateparam.go b/coderd/httpmw/notificationtemplateparam.go new file mode 100644 index 0000000000000..5466c3b7403d9 --- /dev/null +++ b/coderd/httpmw/notificationtemplateparam.go @@ -0,0 +1,49 @@ +package httpmw + +import ( + "context" + "net/http" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/codersdk" +) + +type notificationTemplateParamContextKey struct{} + +// NotificationTemplateParam returns the template from the ExtractNotificationTemplateParam handler. +func NotificationTemplateParam(r *http.Request) database.NotificationTemplate { + template, ok := r.Context().Value(notificationTemplateParamContextKey{}).(database.NotificationTemplate) + if !ok { + panic("developer error: notification template middleware not used") + } + return template +} + +// ExtractNotificationTemplateParam grabs a notification template from the "notification_template" URL parameter. +func ExtractNotificationTemplateParam(db database.Store) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + notifTemplateID, parsed := ParseUUIDParam(rw, r, "notification_template") + if !parsed { + return + } + nt, err := db.GetNotificationTemplateByID(r.Context(), notifTemplateID) + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching notification template.", + Detail: err.Error(), + }) + return + } + + ctx = context.WithValue(ctx, notificationTemplateParamContextKey{}, nt) + next.ServeHTTP(rw, r.WithContext(ctx)) + }) + } +} diff --git a/coderd/notifications.go b/coderd/notifications.go index f6bcbe0c7183d..bdf71f99cab98 100644 --- a/coderd/notifications.go +++ b/coderd/notifications.go @@ -7,11 +7,13 @@ import ( "github.com/google/uuid" + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac" - "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/codersdk" ) @@ -19,7 +21,7 @@ import ( // @ID get-notifications-settings // @Security CoderSessionToken // @Produce json -// @Tags General +// @Tags Notifications // @Success 200 {object} codersdk.NotificationsSettings // @Router /notifications/settings [get] func (api *API) notificationsSettings(rw http.ResponseWriter, r *http.Request) { @@ -51,7 +53,7 @@ func (api *API) notificationsSettings(rw http.ResponseWriter, r *http.Request) { // @Security CoderSessionToken // @Accept json // @Produce json -// @Tags General +// @Tags Notifications // @Param request body codersdk.NotificationsSettings true "Notifications settings request" // @Success 200 {object} codersdk.NotificationsSettings // @Success 304 @@ -59,13 +61,6 @@ func (api *API) notificationsSettings(rw http.ResponseWriter, r *http.Request) { func (api *API) putNotificationsSettings(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - if !api.Authorize(r, policy.ActionUpdate, rbac.ResourceDeploymentConfig) { - httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: "Insufficient permissions to update notifications settings.", - }) - return - } - var settings codersdk.NotificationsSettings if !httpapi.Read(ctx, rw, r, &settings) { return @@ -80,9 +75,9 @@ func (api *API) putNotificationsSettings(rw http.ResponseWriter, r *http.Request return } - currentSettingsJSON, err := api.Database.GetNotificationsSettings(r.Context()) + currentSettingsJSON, err := api.Database.GetNotificationsSettings(ctx) if err != nil { - httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to fetch current notifications settings.", Detail: err.Error(), }) @@ -91,7 +86,7 @@ func (api *API) putNotificationsSettings(rw http.ResponseWriter, r *http.Request if bytes.Equal(settingsJSON, []byte(currentSettingsJSON)) { // See: https://www.rfc-editor.org/rfc/rfc7232#section-4.1 - httpapi.Write(r.Context(), rw, http.StatusNotModified, nil) + httpapi.Write(ctx, rw, http.StatusNotModified, nil) return } @@ -111,12 +106,193 @@ func (api *API) putNotificationsSettings(rw http.ResponseWriter, r *http.Request err = api.Database.UpsertNotificationsSettings(ctx, string(settingsJSON)) if err != nil { - httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + if rbac.IsUnauthorizedError(err) { + httpapi.Forbidden(rw) + return + } + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to update notifications settings.", Detail: err.Error(), }) + return } httpapi.Write(r.Context(), rw, http.StatusOK, settings) } + +// @Summary Get system notification templates +// @ID get-system-notification-templates +// @Security CoderSessionToken +// @Produce json +// @Tags Notifications +// @Success 200 {array} codersdk.NotificationTemplate +// @Router /notifications/templates/system [get] +func (api *API) systemNotificationTemplates(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + templates, err := api.Database.GetNotificationTemplatesByKind(ctx, database.NotificationTemplateKindSystem) + if err != nil { + httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve system notifications templates.", + Detail: err.Error(), + }) + return + } + + out := convertNotificationTemplates(templates) + httpapi.Write(r.Context(), rw, http.StatusOK, out) +} + +// @Summary Get notification dispatch methods +// @ID get-notification-dispatch-methods +// @Security CoderSessionToken +// @Produce json +// @Tags Notifications +// @Success 200 {array} codersdk.NotificationMethodsResponse +// @Router /notifications/dispatch-methods [get] +func (api *API) notificationDispatchMethods(rw http.ResponseWriter, r *http.Request) { + var methods []string + for _, nm := range database.AllNotificationMethodValues() { + methods = append(methods, string(nm)) + } + + httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.NotificationMethodsResponse{ + AvailableNotificationMethods: methods, + DefaultNotificationMethod: api.DeploymentValues.Notifications.Method.Value(), + }) +} + +// @Summary Get user notification preferences +// @ID get-user-notification-preferences +// @Security CoderSessionToken +// @Produce json +// @Tags Notifications +// @Param user path string true "User ID, name, or me" +// @Success 200 {array} codersdk.NotificationPreference +// @Router /users/{user}/notifications/preferences [get] +func (api *API) userNotificationPreferences(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + user = httpmw.UserParam(r) + logger = api.Logger.Named("notifications.preferences").With(slog.F("user_id", user.ID)) + ) + + prefs, err := api.Database.GetUserNotificationPreferences(ctx, user.ID) + if err != nil { + logger.Error(ctx, "failed to retrieve preferences", slog.Error(err)) + + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve user notification preferences.", + Detail: err.Error(), + }) + return + } + + out := convertNotificationPreferences(prefs) + httpapi.Write(ctx, rw, http.StatusOK, out) +} + +// @Summary Update user notification preferences +// @ID update-user-notification-preferences +// @Security CoderSessionToken +// @Accept json +// @Produce json +// @Tags Notifications +// @Param request body codersdk.UpdateUserNotificationPreferences true "Preferences" +// @Param user path string true "User ID, name, or me" +// @Success 200 {array} codersdk.NotificationPreference +// @Router /users/{user}/notifications/preferences [put] +func (api *API) putUserNotificationPreferences(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + user = httpmw.UserParam(r) + logger = api.Logger.Named("notifications.preferences").With(slog.F("user_id", user.ID)) + ) + + // Parse request. + var prefs codersdk.UpdateUserNotificationPreferences + if !httpapi.Read(ctx, rw, r, &prefs) { + return + } + + // Build query params. + input := database.UpdateUserNotificationPreferencesParams{ + UserID: user.ID, + NotificationTemplateIds: make([]uuid.UUID, 0, len(prefs.TemplateDisabledMap)), + Disableds: make([]bool, 0, len(prefs.TemplateDisabledMap)), + } + for tmplID, disabled := range prefs.TemplateDisabledMap { + id, err := uuid.Parse(tmplID) + if err != nil { + logger.Warn(ctx, "failed to parse notification template UUID", slog.F("input", tmplID), slog.Error(err)) + + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Unable to parse notification template UUID.", + Detail: err.Error(), + }) + return + } + + input.NotificationTemplateIds = append(input.NotificationTemplateIds, id) + input.Disableds = append(input.Disableds, disabled) + } + + // Update preferences with params. + updated, err := api.Database.UpdateUserNotificationPreferences(ctx, input) + if err != nil { + logger.Error(ctx, "failed to update preferences", slog.Error(err)) + + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to update user notifications preferences.", + Detail: err.Error(), + }) + return + } + + // Preferences updated, now fetch all preferences belonging to this user. + logger.Info(ctx, "updated preferences", slog.F("count", updated)) + + userPrefs, err := api.Database.GetUserNotificationPreferences(ctx, user.ID) + if err != nil { + logger.Error(ctx, "failed to retrieve preferences", slog.Error(err)) + + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve user notifications preferences.", + Detail: err.Error(), + }) + return + } + + out := convertNotificationPreferences(userPrefs) + httpapi.Write(ctx, rw, http.StatusOK, out) +} + +func convertNotificationTemplates(in []database.NotificationTemplate) (out []codersdk.NotificationTemplate) { + for _, tmpl := range in { + out = append(out, codersdk.NotificationTemplate{ + ID: tmpl.ID, + Name: tmpl.Name, + TitleTemplate: tmpl.TitleTemplate, + BodyTemplate: tmpl.BodyTemplate, + Actions: string(tmpl.Actions), + Group: tmpl.Group.String, + Method: string(tmpl.Method.NotificationMethod), + Kind: string(tmpl.Kind), + }) + } + + return out +} + +func convertNotificationPreferences(in []database.NotificationPreference) (out []codersdk.NotificationPreference) { + for _, pref := range in { + out = append(out, codersdk.NotificationPreference{ + NotificationTemplateID: pref.NotificationTemplateID, + Disabled: pref.Disabled, + UpdatedAt: pref.UpdatedAt, + }) + } + + return out +} diff --git a/coderd/notifications/enqueuer.go b/coderd/notifications/enqueuer.go index 32822dd6ab9d7..d990a71bdb5ad 100644 --- a/coderd/notifications/enqueuer.go +++ b/coderd/notifications/enqueuer.go @@ -3,6 +3,7 @@ package notifications import ( "context" "encoding/json" + "strings" "text/template" "github.com/google/uuid" @@ -16,14 +17,13 @@ import ( "github.com/coder/coder/v2/codersdk" ) +var ErrCannotEnqueueDisabledNotification = xerrors.New("user has disabled this notification") + type StoreEnqueuer struct { store Store log slog.Logger - // TODO: expand this to allow for each notification to have custom delivery methods, or multiple, or none. - // For example, Larry might want email notifications for "workspace deleted" notifications, but Harry wants - // Slack notifications, and Mary doesn't want any. - method database.NotificationMethod + defaultMethod database.NotificationMethod // helpers holds a map of template funcs which are used when rendering templates. These need to be passed in because // the template funcs will return values which are inappropriately encapsulated in this struct. helpers template.FuncMap @@ -37,17 +37,31 @@ func NewStoreEnqueuer(cfg codersdk.NotificationsConfig, store Store, helpers tem } return &StoreEnqueuer{ - store: store, - log: log, - method: method, - helpers: helpers, + store: store, + log: log, + defaultMethod: method, + helpers: helpers, }, nil } // Enqueue queues a notification message for later delivery. // Messages will be dequeued by a notifier later and dispatched. func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { - payload, err := s.buildPayload(ctx, userID, templateID, labels) + metadata, err := s.store.FetchNewMessageMetadata(ctx, database.FetchNewMessageMetadataParams{ + UserID: userID, + NotificationTemplateID: templateID, + }) + if err != nil { + s.log.Warn(ctx, "failed to fetch message metadata", slog.F("template_id", templateID), slog.F("user_id", userID), slog.Error(err)) + return nil, xerrors.Errorf("new message metadata: %w", err) + } + + dispatchMethod := s.defaultMethod + if metadata.CustomMethod.Valid { + dispatchMethod = metadata.CustomMethod.NotificationMethod + } + + payload, err := s.buildPayload(metadata, labels) if err != nil { s.log.Warn(ctx, "failed to build payload", slog.F("template_id", templateID), slog.F("user_id", userID), slog.Error(err)) return nil, xerrors.Errorf("enqueue notification (payload build): %w", err) @@ -63,12 +77,21 @@ func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUI ID: id, UserID: userID, NotificationTemplateID: templateID, - Method: s.method, + Method: dispatchMethod, Payload: input, Targets: targets, CreatedBy: createdBy, }) if err != nil { + // We have a trigger on the notification_messages table named `inhibit_enqueue_if_disabled` which prevents messages + // from being enqueued if the user has disabled them via notification_preferences. The trigger will fail the insertion + // with the message "cannot enqueue message: user has disabled this notification". + // + // This is more efficient than fetching the user's preferences for each enqueue, and centralizes the business logic. + if strings.Contains(err.Error(), ErrCannotEnqueueDisabledNotification.Error()) { + return nil, ErrCannotEnqueueDisabledNotification + } + s.log.Warn(ctx, "failed to enqueue notification", slog.F("template_id", templateID), slog.F("input", input), slog.Error(err)) return nil, xerrors.Errorf("enqueue notification: %w", err) } @@ -80,15 +103,7 @@ func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUI // buildPayload creates the payload that the notification will for variable substitution and/or routing. // The payload contains information about the recipient, the event that triggered the notification, and any subsequent // actions which can be taken by the recipient. -func (s *StoreEnqueuer) buildPayload(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string) (*types.MessagePayload, error) { - metadata, err := s.store.FetchNewMessageMetadata(ctx, database.FetchNewMessageMetadataParams{ - UserID: userID, - NotificationTemplateID: templateID, - }) - if err != nil { - return nil, xerrors.Errorf("new message metadata: %w", err) - } - +func (s *StoreEnqueuer) buildPayload(metadata database.FetchNewMessageMetadataRow, labels map[string]string) (*types.MessagePayload, error) { payload := types.MessagePayload{ Version: "1.0", diff --git a/coderd/notifications/manager.go b/coderd/notifications/manager.go index 5f5d30974a302..91580d5fc4fb7 100644 --- a/coderd/notifications/manager.go +++ b/coderd/notifications/manager.go @@ -149,7 +149,7 @@ func (m *Manager) loop(ctx context.Context) error { var eg errgroup.Group // Create a notifier to run concurrently, which will handle dequeueing and dispatching notifications. - m.notifier = newNotifier(m.cfg, uuid.New(), m.log, m.store, m.handlers, m.method, m.metrics) + m.notifier = newNotifier(m.cfg, uuid.New(), m.log, m.store, m.handlers, m.metrics) eg.Go(func() error { return m.notifier.run(ctx, m.success, m.failure) }) @@ -249,15 +249,24 @@ func (m *Manager) syncUpdates(ctx context.Context) { for i := 0; i < nFailure; i++ { res := <-m.failure - status := database.NotificationMessageStatusPermanentFailure - if res.retryable { + var ( + reason string + status database.NotificationMessageStatus + ) + + switch { + case res.retryable: status = database.NotificationMessageStatusTemporaryFailure + case res.inhibited: + status = database.NotificationMessageStatusInhibited + reason = "disabled by user" + default: + status = database.NotificationMessageStatusPermanentFailure } failureParams.IDs = append(failureParams.IDs, res.msg) failureParams.FailedAts = append(failureParams.FailedAts, res.ts) failureParams.Statuses = append(failureParams.Statuses, status) - var reason string if res.err != nil { reason = res.err.Error() } @@ -367,4 +376,5 @@ type dispatchResult struct { ts time.Time err error retryable bool + inhibited bool } diff --git a/coderd/notifications/metrics_test.go b/coderd/notifications/metrics_test.go index 6c360dd2919d0..139f7ae18c6c6 100644 --- a/coderd/notifications/metrics_test.go +++ b/coderd/notifications/metrics_test.go @@ -339,6 +339,81 @@ func TestInflightDispatchesMetric(t *testing.T) { }, testutil.WaitShort, testutil.IntervalFast) } +func TestCustomMethodMetricCollection(t *testing.T) { + t.Parallel() + + // SETUP + if !dbtestutil.WillUsePostgres() { + // UpdateNotificationTemplateMethodByID only makes sense with a real database. + t.Skip("This test requires postgres; it relies on business-logic only implemented in the database") + } + ctx, logger, store := setup(t) + + var ( + reg = prometheus.NewRegistry() + metrics = notifications.NewMetrics(reg) + template = notifications.TemplateWorkspaceDeleted + anotherTemplate = notifications.TemplateWorkspaceDormant + ) + + const ( + customMethod = database.NotificationMethodWebhook + defaultMethod = database.NotificationMethodSmtp + ) + + // GIVEN: a template whose notification method differs from the default. + out, err := store.UpdateNotificationTemplateMethodByID(ctx, database.UpdateNotificationTemplateMethodByIDParams{ + ID: template, + Method: database.NullNotificationMethod{NotificationMethod: customMethod, Valid: true}, + }) + require.NoError(t, err) + require.Equal(t, customMethod, out.Method.NotificationMethod) + + // WHEN: two notifications (each with different templates) are enqueued. + cfg := defaultNotificationsConfig(defaultMethod) + mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager")) + require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, mgr.Stop(ctx)) + }) + + smtpHandler := &fakeHandler{} + webhookHandler := &fakeHandler{} + mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ + defaultMethod: smtpHandler, + customMethod: webhookHandler, + }) + + enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer")) + require.NoError(t, err) + + user := createSampleUser(t, store) + + _, err = enq.Enqueue(ctx, user.ID, template, map[string]string{"type": "success"}, "test") + require.NoError(t, err) + _, err = enq.Enqueue(ctx, user.ID, anotherTemplate, map[string]string{"type": "success"}, "test") + require.NoError(t, err) + + mgr.Run(ctx) + + // THEN: the fake handlers to "dispatch" the notifications. + require.Eventually(t, func() bool { + smtpHandler.mu.RLock() + webhookHandler.mu.RLock() + defer smtpHandler.mu.RUnlock() + defer webhookHandler.mu.RUnlock() + + return len(smtpHandler.succeeded) == 1 && len(smtpHandler.failed) == 0 && + len(webhookHandler.succeeded) == 1 && len(webhookHandler.failed) == 0 + }, testutil.WaitShort, testutil.IntervalFast) + + // THEN: we should have metric series for both the default and custom notification methods. + require.Eventually(t, func() bool { + return promtest.ToFloat64(metrics.DispatchAttempts.WithLabelValues(string(defaultMethod), anotherTemplate.String(), notifications.ResultSuccess)) > 0 && + promtest.ToFloat64(metrics.DispatchAttempts.WithLabelValues(string(customMethod), template.String(), notifications.ResultSuccess)) > 0 + }, testutil.WaitShort, testutil.IntervalFast) +} + // hasMatchingFingerprint checks if the given metric's series fingerprint matches the reference fingerprint. func hasMatchingFingerprint(metric *dto.Metric, fp model.Fingerprint) bool { return fingerprintLabelPairs(metric.Label) == fp diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index 37fe4a2ce5ce3..d73edbf7c453b 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -604,7 +604,7 @@ func TestNotifierPaused(t *testing.T) { }, testutil.WaitShort, testutil.IntervalFast) } -func TestNotifcationTemplatesBody(t *testing.T) { +func TestNotificationTemplatesBody(t *testing.T) { t.Parallel() if !dbtestutil.WillUsePostgres() { @@ -705,6 +705,194 @@ func TestNotifcationTemplatesBody(t *testing.T) { } } +// TestDisabledBeforeEnqueue ensures that notifications cannot be enqueued once a user has disabled that notification template +func TestDisabledBeforeEnqueue(t *testing.T) { + t.Parallel() + + // SETUP + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres; it is testing business-logic implemented in the database") + } + + ctx, logger, db := setup(t) + + // GIVEN: an enqueuer & a sample user + cfg := defaultNotificationsConfig(database.NotificationMethodSmtp) + enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer")) + require.NoError(t, err) + user := createSampleUser(t, db) + + // WHEN: the user has a preference set to not receive the "workspace deleted" notification + templateID := notifications.TemplateWorkspaceDeleted + n, err := db.UpdateUserNotificationPreferences(ctx, database.UpdateUserNotificationPreferencesParams{ + UserID: user.ID, + NotificationTemplateIds: []uuid.UUID{templateID}, + Disableds: []bool{true}, + }) + require.NoError(t, err, "failed to set preferences") + require.EqualValues(t, 1, n, "unexpected number of affected rows") + + // THEN: enqueuing the "workspace deleted" notification should fail with an error + _, err = enq.Enqueue(ctx, user.ID, templateID, map[string]string{}, "test") + require.ErrorIs(t, err, notifications.ErrCannotEnqueueDisabledNotification, "enqueueing did not fail with expected error") +} + +// TestDisabledAfterEnqueue ensures that notifications enqueued before a notification template was disabled will not be +// sent, and will instead be marked as "inhibited". +func TestDisabledAfterEnqueue(t *testing.T) { + t.Parallel() + + // SETUP + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres; it is testing business-logic implemented in the database") + } + + ctx, logger, db := setup(t) + + method := database.NotificationMethodSmtp + cfg := defaultNotificationsConfig(method) + + mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) + require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, mgr.Stop(ctx)) + }) + + enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer")) + require.NoError(t, err) + user := createSampleUser(t, db) + + // GIVEN: a notification is enqueued which has not (yet) been disabled + templateID := notifications.TemplateWorkspaceDeleted + msgID, err := enq.Enqueue(ctx, user.ID, templateID, map[string]string{}, "test") + require.NoError(t, err) + + // Disable the notification template. + n, err := db.UpdateUserNotificationPreferences(ctx, database.UpdateUserNotificationPreferencesParams{ + UserID: user.ID, + NotificationTemplateIds: []uuid.UUID{templateID}, + Disableds: []bool{true}, + }) + require.NoError(t, err, "failed to set preferences") + require.EqualValues(t, 1, n, "unexpected number of affected rows") + + // WHEN: running the manager to trigger dequeueing of (now-disabled) messages + mgr.Run(ctx) + + // THEN: the message should not be sent, and must be set to "inhibited" + require.EventuallyWithT(t, func(ct *assert.CollectT) { + m, err := db.GetNotificationMessagesByStatus(ctx, database.GetNotificationMessagesByStatusParams{ + Status: database.NotificationMessageStatusInhibited, + Limit: 10, + }) + assert.NoError(ct, err) + if assert.Equal(ct, len(m), 1) { + assert.Equal(ct, m[0].ID.String(), msgID.String()) + assert.Contains(ct, m[0].StatusReason.String, "disabled by user") + } + }, testutil.WaitLong, testutil.IntervalFast, "did not find the expected inhibited message") +} + +func TestCustomNotificationMethod(t *testing.T) { + t.Parallel() + + // SETUP + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres; it relies on business-logic only implemented in the database") + } + + ctx, logger, db := setup(t) + + received := make(chan uuid.UUID, 1) + + // SETUP: + // Start mock server to simulate webhook endpoint. + mockWebhookSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var payload dispatch.WebhookPayload + err := json.NewDecoder(r.Body).Decode(&payload) + assert.NoError(t, err) + + received <- payload.MsgID + close(received) + + w.WriteHeader(http.StatusOK) + _, err = w.Write([]byte("noted.")) + require.NoError(t, err) + })) + defer mockWebhookSrv.Close() + + // Start mock SMTP server. + mockSMTPSrv := smtpmock.New(smtpmock.ConfigurationAttr{ + LogToStdout: false, + LogServerActivity: true, + }) + require.NoError(t, mockSMTPSrv.Start()) + t.Cleanup(func() { + assert.NoError(t, mockSMTPSrv.Stop()) + }) + + endpoint, err := url.Parse(mockWebhookSrv.URL) + require.NoError(t, err) + + // GIVEN: a notification template which has a method explicitly set + var ( + template = notifications.TemplateWorkspaceDormant + defaultMethod = database.NotificationMethodSmtp + customMethod = database.NotificationMethodWebhook + ) + out, err := db.UpdateNotificationTemplateMethodByID(ctx, database.UpdateNotificationTemplateMethodByIDParams{ + ID: template, + Method: database.NullNotificationMethod{NotificationMethod: customMethod, Valid: true}, + }) + require.NoError(t, err) + require.Equal(t, customMethod, out.Method.NotificationMethod) + + // GIVEN: a manager configured with multiple dispatch methods + cfg := defaultNotificationsConfig(defaultMethod) + cfg.SMTP = codersdk.NotificationsEmailConfig{ + From: "danny@coder.com", + Hello: "localhost", + Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%d", mockSMTPSrv.PortNumber())}, + } + cfg.Webhook = codersdk.NotificationsWebhookConfig{ + Endpoint: *serpent.URLOf(endpoint), + } + + mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) + require.NoError(t, err) + t.Cleanup(func() { + _ = mgr.Stop(ctx) + }) + + enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger) + require.NoError(t, err) + + // WHEN: a notification of that template is enqueued, it should be delivered with the configured method - not the default. + user := createSampleUser(t, db) + msgID, err := enq.Enqueue(ctx, user.ID, template, map[string]string{}, "test") + require.NoError(t, err) + + // THEN: the notification should be received by the custom dispatch method + mgr.Run(ctx) + + receivedMsgID := testutil.RequireRecvCtx(ctx, t, received) + require.Equal(t, msgID.String(), receivedMsgID.String()) + + // Ensure no messages received by default method (SMTP): + msgs := mockSMTPSrv.MessagesAndPurge() + require.Len(t, msgs, 0) + + // Enqueue a notification which does not have a custom method set to ensure default works correctly. + msgID, err = enq.Enqueue(ctx, user.ID, notifications.TemplateWorkspaceDeleted, map[string]string{}, "test") + require.NoError(t, err) + require.EventuallyWithT(t, func(ct *assert.CollectT) { + msgs := mockSMTPSrv.MessagesAndPurge() + if assert.Len(ct, msgs, 1) { + assert.Contains(ct, msgs[0].MsgRequest(), fmt.Sprintf("Message-Id: %s", msgID)) + } + }, testutil.WaitLong, testutil.IntervalFast) +} + type fakeHandler struct { mu sync.RWMutex succeeded, failed []string diff --git a/coderd/notifications/notifier.go b/coderd/notifications/notifier.go index c39de6168db81..ac7ed8b2d5f4a 100644 --- a/coderd/notifications/notifier.go +++ b/coderd/notifications/notifier.go @@ -10,6 +10,7 @@ import ( "golang.org/x/sync/errgroup" "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications/dispatch" "github.com/coder/coder/v2/coderd/notifications/render" "github.com/coder/coder/v2/coderd/notifications/types" @@ -33,12 +34,11 @@ type notifier struct { quit chan any done chan any - method database.NotificationMethod handlers map[database.NotificationMethod]Handler metrics *Metrics } -func newNotifier(cfg codersdk.NotificationsConfig, id uuid.UUID, log slog.Logger, db Store, hr map[database.NotificationMethod]Handler, method database.NotificationMethod, metrics *Metrics) *notifier { +func newNotifier(cfg codersdk.NotificationsConfig, id uuid.UUID, log slog.Logger, db Store, hr map[database.NotificationMethod]Handler, metrics *Metrics) *notifier { return ¬ifier{ id: id, cfg: cfg, @@ -48,7 +48,6 @@ func newNotifier(cfg codersdk.NotificationsConfig, id uuid.UUID, log slog.Logger tick: time.NewTicker(cfg.FetchInterval.Value()), store: db, handlers: hr, - method: method, metrics: metrics, } } @@ -144,6 +143,12 @@ func (n *notifier) process(ctx context.Context, success chan<- dispatchResult, f var eg errgroup.Group for _, msg := range msgs { + // If a notification template has been disabled by the user after a notification was enqueued, mark it as inhibited + if msg.Disabled { + failure <- n.newInhibitedDispatch(msg) + continue + } + // A message failing to be prepared correctly should not affect other messages. deliverFn, err := n.prepare(ctx, msg) if err != nil { @@ -234,17 +239,17 @@ func (n *notifier) deliver(ctx context.Context, msg database.AcquireNotification logger := n.log.With(slog.F("msg_id", msg.ID), slog.F("method", msg.Method), slog.F("attempt", msg.AttemptCount+1)) if msg.AttemptCount > 0 { - n.metrics.RetryCount.WithLabelValues(string(n.method), msg.TemplateID.String()).Inc() + n.metrics.RetryCount.WithLabelValues(string(msg.Method), msg.TemplateID.String()).Inc() } - n.metrics.InflightDispatches.WithLabelValues(string(n.method), msg.TemplateID.String()).Inc() - n.metrics.QueuedSeconds.WithLabelValues(string(n.method)).Observe(msg.QueuedSeconds) + n.metrics.InflightDispatches.WithLabelValues(string(msg.Method), msg.TemplateID.String()).Inc() + n.metrics.QueuedSeconds.WithLabelValues(string(msg.Method)).Observe(msg.QueuedSeconds) start := time.Now() retryable, err := deliver(ctx, msg.ID) - n.metrics.DispatcherSendSeconds.WithLabelValues(string(n.method)).Observe(time.Since(start).Seconds()) - n.metrics.InflightDispatches.WithLabelValues(string(n.method), msg.TemplateID.String()).Dec() + n.metrics.DispatcherSendSeconds.WithLabelValues(string(msg.Method)).Observe(time.Since(start).Seconds()) + n.metrics.InflightDispatches.WithLabelValues(string(msg.Method), msg.TemplateID.String()).Dec() if err != nil { // Don't try to accumulate message responses if the context has been canceled. @@ -281,12 +286,12 @@ func (n *notifier) deliver(ctx context.Context, msg database.AcquireNotification } func (n *notifier) newSuccessfulDispatch(msg database.AcquireNotificationMessagesRow) dispatchResult { - n.metrics.DispatchAttempts.WithLabelValues(string(n.method), msg.TemplateID.String(), ResultSuccess).Inc() + n.metrics.DispatchAttempts.WithLabelValues(string(msg.Method), msg.TemplateID.String(), ResultSuccess).Inc() return dispatchResult{ notifier: n.id, msg: msg.ID, - ts: time.Now(), + ts: dbtime.Now(), } } @@ -301,17 +306,27 @@ func (n *notifier) newFailedDispatch(msg database.AcquireNotificationMessagesRow result = ResultPermFail } - n.metrics.DispatchAttempts.WithLabelValues(string(n.method), msg.TemplateID.String(), result).Inc() + n.metrics.DispatchAttempts.WithLabelValues(string(msg.Method), msg.TemplateID.String(), result).Inc() return dispatchResult{ notifier: n.id, msg: msg.ID, - ts: time.Now(), + ts: dbtime.Now(), err: err, retryable: retryable, } } +func (n *notifier) newInhibitedDispatch(msg database.AcquireNotificationMessagesRow) dispatchResult { + return dispatchResult{ + notifier: n.id, + msg: msg.ID, + ts: dbtime.Now(), + retryable: false, + inhibited: true, + } +} + // stop stops the notifier from processing any new notifications. // This is a graceful stop, so any in-flight notifications will be completed before the notifier stops. // Once a notifier has stopped, it cannot be restarted. diff --git a/coderd/notifications_test.go b/coderd/notifications_test.go index 7690154a0db80..537e8cef095d1 100644 --- a/coderd/notifications_test.go +++ b/coderd/notifications_test.go @@ -5,19 +5,34 @@ import ( "testing" "github.com/stretchr/testify/require" + "golang.org/x/exp/slices" + + "github.com/coder/serpent" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) +func createOpts(t *testing.T) *coderdtest.Options { + t.Helper() + + dt := coderdtest.DeploymentValues(t) + dt.Experiments = []string{string(codersdk.ExperimentNotifications)} + return &coderdtest.Options{ + DeploymentValues: dt, + } +} + func TestUpdateNotificationsSettings(t *testing.T) { t.Parallel() t.Run("Permissions denied", func(t *testing.T) { t.Parallel() - api := coderdtest.New(t, nil) + api := coderdtest.New(t, createOpts(t)) firstUser := coderdtest.CreateFirstUser(t, api) anotherClient, _ := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) @@ -41,7 +56,7 @@ func TestUpdateNotificationsSettings(t *testing.T) { t.Run("Settings modified", func(t *testing.T) { t.Parallel() - client := coderdtest.New(t, nil) + client := coderdtest.New(t, createOpts(t)) _ = coderdtest.CreateFirstUser(t, client) // given @@ -65,7 +80,7 @@ func TestUpdateNotificationsSettings(t *testing.T) { t.Parallel() // Empty state: notifications Settings are undefined now (default). - client := coderdtest.New(t, nil) + client := coderdtest.New(t, createOpts(t)) _ = coderdtest.CreateFirstUser(t, client) ctx := testutil.Context(t, testutil.WaitShort) @@ -93,3 +108,213 @@ func TestUpdateNotificationsSettings(t *testing.T) { require.Equal(t, expected.NotifierPaused, actual.NotifierPaused) }) } + +func TestNotificationPreferences(t *testing.T) { + t.Parallel() + + t.Run("Initial state", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + api := coderdtest.New(t, createOpts(t)) + firstUser := coderdtest.CreateFirstUser(t, api) + + // Given: a member in its initial state. + memberClient, member := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) + + // When: calling the API. + prefs, err := memberClient.GetUserNotificationPreferences(ctx, member.ID) + require.NoError(t, err) + + // Then: no preferences will be returned. + require.Len(t, prefs, 0) + }) + + t.Run("Insufficient permissions", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + api := coderdtest.New(t, createOpts(t)) + firstUser := coderdtest.CreateFirstUser(t, api) + + // Given: 2 members. + _, member1 := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) + member2Client, _ := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) + + // When: attempting to retrieve the preferences of another member. + _, err := member2Client.GetUserNotificationPreferences(ctx, member1.ID) + + // Then: the API should reject the request. + var sdkError *codersdk.Error + require.Error(t, err) + require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error") + // NOTE: ExtractUserParam gets in the way here, and returns a 400 Bad Request instead of a 403 Forbidden. + // This is not ideal, and we should probably change this behavior. + require.Equal(t, http.StatusBadRequest, sdkError.StatusCode()) + }) + + t.Run("Admin may read any users' preferences", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + api := coderdtest.New(t, createOpts(t)) + firstUser := coderdtest.CreateFirstUser(t, api) + + // Given: a member. + _, member := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) + + // When: attempting to retrieve the preferences of another member as an admin. + prefs, err := api.GetUserNotificationPreferences(ctx, member.ID) + + // Then: the API should not reject the request. + require.NoError(t, err) + require.Len(t, prefs, 0) + }) + + t.Run("Admin may update any users' preferences", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + api := coderdtest.New(t, createOpts(t)) + firstUser := coderdtest.CreateFirstUser(t, api) + + // Given: a member. + memberClient, member := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) + + // When: attempting to modify and subsequently retrieve the preferences of another member as an admin. + prefs, err := api.UpdateUserNotificationPreferences(ctx, member.ID, codersdk.UpdateUserNotificationPreferences{ + TemplateDisabledMap: map[string]bool{ + notifications.TemplateWorkspaceMarkedForDeletion.String(): true, + }, + }) + + // Then: the request should succeed and the user should be able to query their own preferences to see the same result. + require.NoError(t, err) + require.Len(t, prefs, 1) + + memberPrefs, err := memberClient.GetUserNotificationPreferences(ctx, member.ID) + require.NoError(t, err) + require.Len(t, memberPrefs, 1) + require.Equal(t, prefs[0].NotificationTemplateID, memberPrefs[0].NotificationTemplateID) + require.Equal(t, prefs[0].Disabled, memberPrefs[0].Disabled) + }) + + t.Run("Add preferences", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + api := coderdtest.New(t, createOpts(t)) + firstUser := coderdtest.CreateFirstUser(t, api) + + // Given: a member with no preferences. + memberClient, member := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) + prefs, err := memberClient.GetUserNotificationPreferences(ctx, member.ID) + require.NoError(t, err) + require.Len(t, prefs, 0) + + // When: attempting to add new preferences. + template := notifications.TemplateWorkspaceDeleted + prefs, err = memberClient.UpdateUserNotificationPreferences(ctx, member.ID, codersdk.UpdateUserNotificationPreferences{ + TemplateDisabledMap: map[string]bool{ + template.String(): true, + }, + }) + + // Then: the returning preferences should be set as expected. + require.NoError(t, err) + require.Len(t, prefs, 1) + require.Equal(t, prefs[0].NotificationTemplateID, template) + require.True(t, prefs[0].Disabled) + }) + + t.Run("Modify preferences", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + api := coderdtest.New(t, createOpts(t)) + firstUser := coderdtest.CreateFirstUser(t, api) + + // Given: a member with preferences. + memberClient, member := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) + prefs, err := memberClient.UpdateUserNotificationPreferences(ctx, member.ID, codersdk.UpdateUserNotificationPreferences{ + TemplateDisabledMap: map[string]bool{ + notifications.TemplateWorkspaceDeleted.String(): true, + notifications.TemplateWorkspaceDormant.String(): true, + }, + }) + require.NoError(t, err) + require.Len(t, prefs, 2) + + // When: attempting to modify their preferences. + prefs, err = memberClient.UpdateUserNotificationPreferences(ctx, member.ID, codersdk.UpdateUserNotificationPreferences{ + TemplateDisabledMap: map[string]bool{ + notifications.TemplateWorkspaceDeleted.String(): true, + notifications.TemplateWorkspaceDormant.String(): false, // <--- this one was changed + }, + }) + require.NoError(t, err) + require.Len(t, prefs, 2) + + // Then: the modified preferences should be set as expected. + var found bool + for _, p := range prefs { + switch p.NotificationTemplateID { + case notifications.TemplateWorkspaceDormant: + found = true + require.False(t, p.Disabled) + case notifications.TemplateWorkspaceDeleted: + require.True(t, p.Disabled) + } + } + require.True(t, found, "dormant notification preference was not found") + }) +} + +func TestNotificationDispatchMethods(t *testing.T) { + t.Parallel() + + defaultOpts := createOpts(t) + webhookOpts := createOpts(t) + webhookOpts.DeploymentValues.Notifications.Method = serpent.String(database.NotificationMethodWebhook) + + tests := []struct { + name string + opts *coderdtest.Options + expectedDefault string + }{ + { + name: "default", + opts: defaultOpts, + expectedDefault: string(database.NotificationMethodSmtp), + }, + { + name: "non-default", + opts: webhookOpts, + expectedDefault: string(database.NotificationMethodWebhook), + }, + } + + var allMethods []string + for _, nm := range database.AllNotificationMethodValues() { + allMethods = append(allMethods, string(nm)) + } + slices.Sort(allMethods) + + // nolint:paralleltest // Not since Go v1.22. + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + api := coderdtest.New(t, tc.opts) + _ = coderdtest.CreateFirstUser(t, api) + + resp, err := api.GetNotificationDispatchMethods(ctx) + require.NoError(t, err) + + slices.Sort(resp.AvailableNotificationMethods) + require.EqualValues(t, resp.AvailableNotificationMethods, allMethods) + require.Equal(t, tc.expectedDefault, resp.DefaultNotificationMethod) + }) + } +} diff --git a/codersdk/notifications.go b/codersdk/notifications.go index 58829eed57891..92870b4dd2b95 100644 --- a/codersdk/notifications.go +++ b/codersdk/notifications.go @@ -3,13 +3,43 @@ package codersdk import ( "context" "encoding/json" + "fmt" + "io" "net/http" + "time" + + "github.com/google/uuid" + "golang.org/x/xerrors" ) type NotificationsSettings struct { NotifierPaused bool `json:"notifier_paused"` } +type NotificationTemplate struct { + ID uuid.UUID `json:"id" format:"uuid"` + Name string `json:"name"` + TitleTemplate string `json:"title_template"` + BodyTemplate string `json:"body_template"` + Actions string `json:"actions" format:""` + Group string `json:"group"` + Method string `json:"method"` + Kind string `json:"kind"` +} + +type NotificationMethodsResponse struct { + AvailableNotificationMethods []string `json:"available"` + DefaultNotificationMethod string `json:"default"` +} + +type NotificationPreference struct { + NotificationTemplateID uuid.UUID `json:"id" format:"uuid"` + Disabled bool `json:"disabled"` + UpdatedAt time.Time `json:"updated_at" format:"date-time"` +} + +// GetNotificationsSettings retrieves the notifications settings, which currently just describes whether all +// notifications are paused from sending. func (c *Client) GetNotificationsSettings(ctx context.Context) (NotificationsSettings, error) { res, err := c.Request(ctx, http.MethodGet, "/api/v2/notifications/settings", nil) if err != nil { @@ -23,6 +53,8 @@ func (c *Client) GetNotificationsSettings(ctx context.Context) (NotificationsSet return settings, json.NewDecoder(res.Body).Decode(&settings) } +// PutNotificationsSettings modifies the notifications settings, which currently just controls whether all +// notifications are paused from sending. func (c *Client) PutNotificationsSettings(ctx context.Context, settings NotificationsSettings) error { res, err := c.Request(ctx, http.MethodPut, "/api/v2/notifications/settings", settings) if err != nil { @@ -38,3 +70,132 @@ func (c *Client) PutNotificationsSettings(ctx context.Context, settings Notifica } return nil } + +// UpdateNotificationTemplateMethod modifies a notification template to use a specific notification method, overriding +// the method set in the deployment configuration. +func (c *Client) UpdateNotificationTemplateMethod(ctx context.Context, notificationTemplateID uuid.UUID, method string) error { + res, err := c.Request(ctx, http.MethodPut, + fmt.Sprintf("/api/v2/notifications/templates/%s/method", notificationTemplateID), + UpdateNotificationTemplateMethod{Method: method}, + ) + if err != nil { + return err + } + defer res.Body.Close() + + if res.StatusCode == http.StatusNotModified { + return nil + } + if res.StatusCode != http.StatusOK { + return ReadBodyAsError(res) + } + return nil +} + +// GetSystemNotificationTemplates retrieves all notification templates pertaining to internal system events. +func (c *Client) GetSystemNotificationTemplates(ctx context.Context) ([]NotificationTemplate, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/v2/notifications/templates/system", nil) + if err != nil { + return nil, err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, ReadBodyAsError(res) + } + + var templates []NotificationTemplate + body, err := io.ReadAll(res.Body) + if err != nil { + return nil, xerrors.Errorf("read response body: %w", err) + } + + if err := json.Unmarshal(body, &templates); err != nil { + return nil, xerrors.Errorf("unmarshal response body: %w", err) + } + + return templates, nil +} + +// GetUserNotificationPreferences retrieves notification preferences for a given user. +func (c *Client) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/notifications/preferences", userID.String()), nil) + if err != nil { + return nil, err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, ReadBodyAsError(res) + } + + var prefs []NotificationPreference + body, err := io.ReadAll(res.Body) + if err != nil { + return nil, xerrors.Errorf("read response body: %w", err) + } + + if err := json.Unmarshal(body, &prefs); err != nil { + return nil, xerrors.Errorf("unmarshal response body: %w", err) + } + + return prefs, nil +} + +// UpdateUserNotificationPreferences updates notification preferences for a given user. +func (c *Client) UpdateUserNotificationPreferences(ctx context.Context, userID uuid.UUID, req UpdateUserNotificationPreferences) ([]NotificationPreference, error) { + res, err := c.Request(ctx, http.MethodPut, fmt.Sprintf("/api/v2/users/%s/notifications/preferences", userID.String()), req) + if err != nil { + return nil, err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, ReadBodyAsError(res) + } + + var prefs []NotificationPreference + body, err := io.ReadAll(res.Body) + if err != nil { + return nil, xerrors.Errorf("read response body: %w", err) + } + + if err := json.Unmarshal(body, &prefs); err != nil { + return nil, xerrors.Errorf("unmarshal response body: %w", err) + } + + return prefs, nil +} + +// GetNotificationDispatchMethods the available and default notification dispatch methods. +func (c *Client) GetNotificationDispatchMethods(ctx context.Context) (NotificationMethodsResponse, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/v2/notifications/dispatch-methods", nil) + if err != nil { + return NotificationMethodsResponse{}, err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return NotificationMethodsResponse{}, ReadBodyAsError(res) + } + + var resp NotificationMethodsResponse + body, err := io.ReadAll(res.Body) + if err != nil { + return NotificationMethodsResponse{}, xerrors.Errorf("read response body: %w", err) + } + + if err := json.Unmarshal(body, &resp); err != nil { + return NotificationMethodsResponse{}, xerrors.Errorf("unmarshal response body: %w", err) + } + + return resp, nil +} + +type UpdateNotificationTemplateMethod struct { + Method string `json:"method,omitempty" example:"webhook"` +} + +type UpdateUserNotificationPreferences struct { + TemplateDisabledMap map[string]bool `json:"template_disabled_map"` +} diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md index dec875eebaac3..be30b790d4aef 100644 --- a/docs/api/enterprise.md +++ b/docs/api/enterprise.md @@ -537,6 +537,33 @@ curl -X DELETE http://coder-server:8080/api/v2/licenses/{id} \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Update notification template dispatch method + +### Code samples + +```shell +# Example request using curl +curl -X PUT http://coder-server:8080/api/v2/notifications/templates/{notification_template}/method \ + -H 'Coder-Session-Token: API_KEY' +``` + +`PUT /notifications/templates/{notification_template}/method` + +### Parameters + +| Name | In | Type | Required | Description | +| ----------------------- | ---- | ------ | -------- | -------------------------- | +| `notification_template` | path | string | true | Notification template UUID | + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | --------------------------------------------------------------- | ------------ | ------ | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | Success | | +| 304 | [Not Modified](https://tools.ietf.org/html/rfc7232#section-4.1) | Not modified | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get OAuth2 applications. ### Code samples diff --git a/docs/api/general.md b/docs/api/general.md index e913a4c804cd6..52cfd25f4c46c 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -667,84 +667,6 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Get notifications settings - -### Code samples - -```shell -# Example request using curl -curl -X GET http://coder-server:8080/api/v2/notifications/settings \ - -H 'Accept: application/json' \ - -H 'Coder-Session-Token: API_KEY' -``` - -`GET /notifications/settings` - -### Example responses - -> 200 Response - -```json -{ - "notifier_paused": true -} -``` - -### Responses - -| Status | Meaning | Description | Schema | -| ------ | ------------------------------------------------------- | ----------- | -------------------------------------------------------------------------- | -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.NotificationsSettings](schemas.md#codersdknotificationssettings) | - -To perform this operation, you must be authenticated. [Learn more](authentication.md). - -## Update notifications settings - -### Code samples - -```shell -# Example request using curl -curl -X PUT http://coder-server:8080/api/v2/notifications/settings \ - -H 'Content-Type: application/json' \ - -H 'Accept: application/json' \ - -H 'Coder-Session-Token: API_KEY' -``` - -`PUT /notifications/settings` - -> Body parameter - -```json -{ - "notifier_paused": true -} -``` - -### Parameters - -| Name | In | Type | Required | Description | -| ------ | ---- | -------------------------------------------------------------------------- | -------- | ------------------------------ | -| `body` | body | [codersdk.NotificationsSettings](schemas.md#codersdknotificationssettings) | true | Notifications settings request | - -### Example responses - -> 200 Response - -```json -{ - "notifier_paused": true -} -``` - -### Responses - -| Status | Meaning | Description | Schema | -| ------ | --------------------------------------------------------------- | ------------ | -------------------------------------------------------------------------- | -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.NotificationsSettings](schemas.md#codersdknotificationssettings) | -| 304 | [Not Modified](https://tools.ietf.org/html/rfc7232#section-4.1) | Not Modified | | - -To perform this operation, you must be authenticated. [Learn more](authentication.md). - ## Update check ### Code samples diff --git a/docs/api/notifications.md b/docs/api/notifications.md new file mode 100644 index 0000000000000..528153ebd103b --- /dev/null +++ b/docs/api/notifications.md @@ -0,0 +1,296 @@ +# Notifications + +## Get notification dispatch methods + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/notifications/dispatch-methods \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /notifications/dispatch-methods` + +### Example responses + +> 200 Response + +```json +[ + { + "available": ["string"], + "default": "string" + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ----------------------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.NotificationMethodsResponse](schemas.md#codersdknotificationmethodsresponse) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| -------------- | ------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» available` | array | false | | | +| `» default` | string | false | | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Get notifications settings + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/notifications/settings \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /notifications/settings` + +### Example responses + +> 200 Response + +```json +{ + "notifier_paused": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | -------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.NotificationsSettings](schemas.md#codersdknotificationssettings) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Update notifications settings + +### Code samples + +```shell +# Example request using curl +curl -X PUT http://coder-server:8080/api/v2/notifications/settings \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`PUT /notifications/settings` + +> Body parameter + +```json +{ + "notifier_paused": true +} +``` + +### Parameters + +| Name | In | Type | Required | Description | +| ------ | ---- | -------------------------------------------------------------------------- | -------- | ------------------------------ | +| `body` | body | [codersdk.NotificationsSettings](schemas.md#codersdknotificationssettings) | true | Notifications settings request | + +### Example responses + +> 200 Response + +```json +{ + "notifier_paused": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | --------------------------------------------------------------- | ------------ | -------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.NotificationsSettings](schemas.md#codersdknotificationssettings) | +| 304 | [Not Modified](https://tools.ietf.org/html/rfc7232#section-4.1) | Not Modified | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Get system notification templates + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/notifications/templates/system \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /notifications/templates/system` + +### Example responses + +> 200 Response + +```json +[ + { + "actions": "string", + "body_template": "string", + "group": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "kind": "string", + "method": "string", + "name": "string", + "title_template": "string" + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | --------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.NotificationTemplate](schemas.md#codersdknotificationtemplate) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| ------------------ | ------------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» actions` | string | false | | | +| `» body_template` | string | false | | | +| `» group` | string | false | | | +| `» id` | string(uuid) | false | | | +| `» kind` | string | false | | | +| `» method` | string | false | | | +| `» name` | string | false | | | +| `» title_template` | string | false | | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Get user notification preferences + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/users/{user}/notifications/preferences \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /users/{user}/notifications/preferences` + +### Parameters + +| Name | In | Type | Required | Description | +| ------ | ---- | ------ | -------- | -------------------- | +| `user` | path | string | true | User ID, name, or me | + +### Example responses + +> 200 Response + +```json +[ + { + "disabled": true, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "updated_at": "2019-08-24T14:15:22Z" + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.NotificationPreference](schemas.md#codersdknotificationpreference) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| -------------- | ----------------- | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» disabled` | boolean | false | | | +| `» id` | string(uuid) | false | | | +| `» updated_at` | string(date-time) | false | | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Update user notification preferences + +### Code samples + +```shell +# Example request using curl +curl -X PUT http://coder-server:8080/api/v2/users/{user}/notifications/preferences \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`PUT /users/{user}/notifications/preferences` + +> Body parameter + +```json +{ + "template_disabled_map": { + "property1": true, + "property2": true + } +} +``` + +### Parameters + +| Name | In | Type | Required | Description | +| ------ | ---- | -------------------------------------------------------------------------------------------------- | -------- | -------------------- | +| `user` | path | string | true | User ID, name, or me | +| `body` | body | [codersdk.UpdateUserNotificationPreferences](schemas.md#codersdkupdateusernotificationpreferences) | true | Preferences | + +### Example responses + +> 200 Response + +```json +[ + { + "disabled": true, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "updated_at": "2019-08-24T14:15:22Z" + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.NotificationPreference](schemas.md#codersdknotificationpreference) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| -------------- | ----------------- | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» disabled` | boolean | false | | | +| `» id` | string(uuid) | false | | | +| `» updated_at` | string(date-time) | false | | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/api/schemas.md b/docs/api/schemas.md index e0d208895eb75..7406d135112f1 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -3141,6 +3141,68 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `id` | string | true | | | | `username` | string | true | | | +## codersdk.NotificationMethodsResponse + +```json +{ + "available": ["string"], + "default": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ----------- | --------------- | -------- | ------------ | ----------- | +| `available` | array of string | false | | | +| `default` | string | false | | | + +## codersdk.NotificationPreference + +```json +{ + "disabled": true, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "updated_at": "2019-08-24T14:15:22Z" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------------ | ------- | -------- | ------------ | ----------- | +| `disabled` | boolean | false | | | +| `id` | string | false | | | +| `updated_at` | string | false | | | + +## codersdk.NotificationTemplate + +```json +{ + "actions": "string", + "body_template": "string", + "group": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "kind": "string", + "method": "string", + "name": "string", + "title_template": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ---------------- | ------ | -------- | ------------ | ----------- | +| `actions` | string | false | | | +| `body_template` | string | false | | | +| `group` | string | false | | | +| `id` | string | false | | | +| `kind` | string | false | | | +| `method` | string | false | | | +| `name` | string | false | | | +| `title_template` | string | false | | | + ## codersdk.NotificationsConfig ```json @@ -5537,6 +5599,24 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | ------------------ | ------ | -------- | ------------ | ----------- | | `theme_preference` | string | true | | | +## codersdk.UpdateUserNotificationPreferences + +```json +{ + "template_disabled_map": { + "property1": true, + "property2": true + } +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ----------------------- | ------- | -------- | ------------ | ----------- | +| `template_disabled_map` | object | false | | | +| » `[any property]` | boolean | false | | | + ## codersdk.UpdateUserPasswordRequest ```json diff --git a/docs/manifest.json b/docs/manifest.json index 82dd73ada47c8..4b686ed9598b6 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -601,6 +601,10 @@ "title": "Members", "path": "./api/members.md" }, + { + "title": "Notifications", + "path": "./api/notifications.md" + }, { "title": "Organizations", "path": "./api/organizations.md" diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index e9e8d7d196af0..5fbd1569d0207 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -368,7 +368,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { r.Put("/", api.putAppearance) }) }) - r.Route("/users/{user}/quiet-hours", func(r chi.Router) { r.Use( api.autostopRequirementEnabledMW, @@ -388,6 +387,15 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { r.Post("/jfrog/xray-scan", api.postJFrogXrayScan) r.Get("/jfrog/xray-scan", api.jFrogXrayScan) }) + + // The /notifications base route is mounted by the AGPL router, so we can't group it here. + // Additionally, because we have a static route for /notifications/templates/system which conflicts + // with the below route, we need to register this route without any mounts or groups to make both work. + r.With( + apiKeyMiddleware, + httpmw.RequireExperiment(api.AGPL.Experiments, codersdk.ExperimentNotifications), + httpmw.ExtractNotificationTemplateParam(options.Database), + ).Put("/notifications/templates/{notification_template}/method", api.updateNotificationTemplateMethod) }) if len(options.SCIMAPIKey) != 0 { diff --git a/enterprise/coderd/notifications.go b/enterprise/coderd/notifications.go new file mode 100644 index 0000000000000..3f3ea2b911026 --- /dev/null +++ b/enterprise/coderd/notifications.go @@ -0,0 +1,98 @@ +package coderd + +import ( + "fmt" + "net/http" + "strings" + + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/codersdk" +) + +// @Summary Update notification template dispatch method +// @ID update-notification-template-dispatch-method +// @Security CoderSessionToken +// @Produce json +// @Param notification_template path string true "Notification template UUID" +// @Tags Enterprise +// @Success 200 "Success" +// @Success 304 "Not modified" +// @Router /notifications/templates/{notification_template}/method [put] +func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + template = httpmw.NotificationTemplateParam(r) + auditor = api.AGPL.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.NotificationTemplate](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + }) + ) + + var req codersdk.UpdateNotificationTemplateMethod + if !httpapi.Read(ctx, rw, r, &req) { + return + } + + var nm database.NullNotificationMethod + if err := nm.Scan(req.Method); err != nil || !nm.Valid || !nm.NotificationMethod.Valid() { + vals := database.AllNotificationMethodValues() + acceptable := make([]string, len(vals)) + for i, v := range vals { + acceptable[i] = string(v) + } + + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid request to update notification template method", + Validations: []codersdk.ValidationError{ + { + Field: "method", + Detail: fmt.Sprintf("%q is not a valid method; %s are the available options", + req.Method, strings.Join(acceptable, ", "), + ), + }, + }, + }) + return + } + + if template.Method == nm { + httpapi.Write(ctx, rw, http.StatusNotModified, codersdk.Response{ + Message: "Notification template method unchanged.", + }) + return + } + + defer commitAudit() + aReq.Old = template + + err := api.Database.InTx(func(tx database.Store) error { + var err error + template, err = api.Database.UpdateNotificationTemplateMethodByID(r.Context(), database.UpdateNotificationTemplateMethodByIDParams{ + ID: template.ID, + Method: nm, + }) + if err != nil { + return xerrors.Errorf("failed to update notification template ID: %w", err) + } + + return err + }, nil) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + aReq.New = template + + httpapi.Write(ctx, rw, http.StatusOK, codersdk.Response{ + Message: "Successfully updated notification template method.", + }) +} diff --git a/enterprise/coderd/notifications_test.go b/enterprise/coderd/notifications_test.go new file mode 100644 index 0000000000000..5546bec1dcb79 --- /dev/null +++ b/enterprise/coderd/notifications_test.go @@ -0,0 +1,180 @@ +package coderd_test + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/testutil" +) + +func createOpts(t *testing.T) *coderdenttest.Options { + t.Helper() + + dt := coderdtest.DeploymentValues(t) + dt.Experiments = []string{string(codersdk.ExperimentNotifications)} + return &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dt, + }, + } +} + +func TestUpdateNotificationTemplateMethod(t *testing.T) { + t.Parallel() + + t.Run("Happy path", func(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres; it relies on read from and writing to the notification_templates table") + } + + ctx := testutil.Context(t, testutil.WaitSuperLong) + api, _ := coderdenttest.New(t, createOpts(t)) + + var ( + method = string(database.NotificationMethodSmtp) + templateID = notifications.TemplateWorkspaceDeleted + ) + + // Given: a template whose method is initially empty (i.e. deferring to the global method value). + template, err := getTemplateByID(t, ctx, api, templateID) + require.NoError(t, err) + require.NotNil(t, template) + require.Empty(t, template.Method) + + // When: calling the API to update the method. + require.NoError(t, api.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, method), "initial request to set the method failed") + + // Then: the method should be set. + template, err = getTemplateByID(t, ctx, api, templateID) + require.NoError(t, err) + require.NotNil(t, template) + require.Equal(t, method, template.Method) + }) + + t.Run("Insufficient permissions", func(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres; it relies on read from and writing to the notification_templates table") + } + + ctx := testutil.Context(t, testutil.WaitSuperLong) + + // Given: the first user which has an "owner" role, and another user which does not. + api, firstUser := coderdenttest.New(t, createOpts(t)) + anotherClient, _ := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) + + // When: calling the API as an unprivileged user. + err := anotherClient.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, string(database.NotificationMethodWebhook)) + + // Then: the request is denied because of insufficient permissions. + var sdkError *codersdk.Error + require.Error(t, err) + require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error") + require.Equal(t, http.StatusNotFound, sdkError.StatusCode()) + require.Equal(t, "Resource not found or you do not have access to this resource", sdkError.Response.Message) + }) + + t.Run("Invalid notification method", func(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres; it relies on read from and writing to the notification_templates table") + } + + ctx := testutil.Context(t, testutil.WaitSuperLong) + + // Given: the first user which has an "owner" role + api, _ := coderdenttest.New(t, createOpts(t)) + + // When: calling the API with an invalid method. + const method = "nope" + + // nolint:gocritic // Using an owner-scope user is kinda the point. + err := api.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, method) + + // Then: the request is invalid because of the unacceptable method. + var sdkError *codersdk.Error + require.Error(t, err) + require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error") + require.Equal(t, http.StatusBadRequest, sdkError.StatusCode()) + require.Equal(t, "Invalid request to update notification template method", sdkError.Response.Message) + require.Len(t, sdkError.Response.Validations, 1) + require.Equal(t, "method", sdkError.Response.Validations[0].Field) + require.Equal(t, fmt.Sprintf("%q is not a valid method; smtp, webhook are the available options", method), sdkError.Response.Validations[0].Detail) + }) + + t.Run("Not modified", func(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres; it relies on read from and writing to the notification_templates table") + } + + ctx := testutil.Context(t, testutil.WaitSuperLong) + api, _ := coderdenttest.New(t, createOpts(t)) + + var ( + method = string(database.NotificationMethodSmtp) + templateID = notifications.TemplateWorkspaceDeleted + ) + + template, err := getTemplateByID(t, ctx, api, templateID) + require.NoError(t, err) + require.NotNil(t, template) + + // Given: a template whose method is initially empty (i.e. deferring to the global method value). + require.Empty(t, template.Method) + + // When: calling the API to update the method, it should set it. + require.NoError(t, api.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, method), "initial request to set the method failed") + template, err = getTemplateByID(t, ctx, api, templateID) + require.NoError(t, err) + require.NotNil(t, template) + require.Equal(t, method, template.Method) + + // Then: when calling the API again with the same method, the method will remain unchanged. + require.NoError(t, api.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, method), "second request to set the method failed") + template, err = getTemplateByID(t, ctx, api, templateID) + require.NoError(t, err) + require.NotNil(t, template) + require.Equal(t, method, template.Method) + }) +} + +// nolint:revive // t takes precedence. +func getTemplateByID(t *testing.T, ctx context.Context, api *codersdk.Client, id uuid.UUID) (*codersdk.NotificationTemplate, error) { + t.Helper() + + var template codersdk.NotificationTemplate + templates, err := api.GetSystemNotificationTemplates(ctx) + if err != nil { + return nil, err + } + + for _, tmpl := range templates { + if tmpl.ID == id { + template = tmpl + } + } + + if template.ID == uuid.Nil { + return nil, xerrors.Errorf("template not found: %q", id.String()) + } + + return &template, nil +} diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 804ee8ce8cec0..5c2dc816fea1e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -709,6 +709,31 @@ export interface MinimalUser { readonly avatar_url: string; } +// From codersdk/notifications.go +export interface NotificationMethodsResponse { + readonly available: readonly string[]; + readonly default: string; +} + +// From codersdk/notifications.go +export interface NotificationPreference { + readonly id: string; + readonly disabled: boolean; + readonly updated_at: string; +} + +// From codersdk/notifications.go +export interface NotificationTemplate { + readonly id: string; + readonly name: string; + readonly title_template: string; + readonly body_template: string; + readonly actions: string; + readonly group: string; + readonly method: string; + readonly kind: string; +} + // From codersdk/deployment.go export interface NotificationsConfig { readonly max_send_attempts: number; @@ -1447,6 +1472,11 @@ export interface UpdateCheckResponse { readonly url: string; } +// From codersdk/notifications.go +export interface UpdateNotificationTemplateMethod { + readonly method?: string; +} + // From codersdk/organizations.go export interface UpdateOrganizationRequest { readonly name?: string; @@ -1495,6 +1525,11 @@ export interface UpdateUserAppearanceSettingsRequest { readonly theme_preference: string; } +// From codersdk/notifications.go +export interface UpdateUserNotificationPreferences { + readonly template_disabled_map: Record; +} + // From codersdk/users.go export interface UpdateUserPasswordRequest { readonly old_password: string; From 5d683d4938971e7ce09044b1b2087f88671c91e5 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 5 Aug 2024 16:04:00 +0200 Subject: [PATCH 20/20] Review comments Signed-off-by: Danny Kopping --- coderd/database/dbmem/dbmem.go | 14 ++++++++++---- .../000238_notifications_preferences.up.sql | 4 ---- coderd/notifications_test.go | 12 ++++++------ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index c80f6648ad0c8..5768379535668 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -2711,10 +2711,14 @@ func (q *FakeQuerier) GetNotificationMessagesByStatus(_ context.Context, arg dat } func (*FakeQuerier) GetNotificationTemplateByID(_ context.Context, _ uuid.UUID) (database.NotificationTemplate, error) { + // Not implementing this function because it relies on state in the database which is created with migrations. + // We could consider using code-generation to align the database state and dbmem, but it's not worth it right now. return database.NotificationTemplate{}, ErrUnimplemented } func (*FakeQuerier) GetNotificationTemplatesByKind(_ context.Context, _ database.NotificationTemplateKind) ([]database.NotificationTemplate, error) { + // Not implementing this function because it relies on state in the database which is created with migrations. + // We could consider using code-generation to align the database state and dbmem, but it's not worth it right now. return nil, ErrUnimplemented } @@ -7547,6 +7551,8 @@ func (q *FakeQuerier) UpdateMemberRoles(_ context.Context, arg database.UpdateMe } func (*FakeQuerier) UpdateNotificationTemplateMethodByID(_ context.Context, _ database.UpdateNotificationTemplateMethodByIDParams) (database.NotificationTemplate, error) { + // Not implementing this function because it relies on state in the database which is created with migrations. + // We could consider using code-generation to align the database state and dbmem, but it's not worth it right now. return database.NotificationTemplate{}, ErrUnimplemented } @@ -8147,7 +8153,7 @@ func (q *FakeQuerier) UpdateUserLoginType(_ context.Context, arg database.Update func (q *FakeQuerier) UpdateUserNotificationPreferences(_ context.Context, arg database.UpdateUserNotificationPreferencesParams) (int64, error) { err := validateDatabaseType(arg) if err != nil { - return -1, err + return 0, err } q.mutex.Lock() @@ -8171,7 +8177,7 @@ func (q *FakeQuerier) UpdateUserNotificationPreferences(_ context.Context, arg d } np.Disabled = disabled - np.UpdatedAt = time.Now() + np.UpdatedAt = dbtime.Now() q.notificationPreferences[j] = np upserted++ @@ -8184,8 +8190,8 @@ func (q *FakeQuerier) UpdateUserNotificationPreferences(_ context.Context, arg d Disabled: disabled, UserID: arg.UserID, NotificationTemplateID: templateID, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), } q.notificationPreferences = append(q.notificationPreferences, np) upserted++ diff --git a/coderd/database/migrations/testdata/fixtures/000238_notifications_preferences.up.sql b/coderd/database/migrations/testdata/fixtures/000238_notifications_preferences.up.sql index 5795ca15dc5f8..74b70cf29792e 100644 --- a/coderd/database/migrations/testdata/fixtures/000238_notifications_preferences.up.sql +++ b/coderd/database/migrations/testdata/fixtures/000238_notifications_preferences.up.sql @@ -1,7 +1,3 @@ -INSERT INTO users(id, email, username, hashed_password, created_at, updated_at, status, rbac_roles, deleted) -VALUES ('fc1511ef-4fcf-4a3b-98a1-8df64160e35a', 'githubuser@coder.com', 'githubuser', '\x', - '2022-11-02 13:05:21.445455+02', '2022-11-02 13:05:21.445455+02', 'active', '{}', false) ON CONFLICT DO NOTHING; - INSERT INTO notification_templates (id, name, title_template, body_template, "group") VALUES ('a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11', 'A', 'title', 'body', 'Group 1') ON CONFLICT DO NOTHING; diff --git a/coderd/notifications_test.go b/coderd/notifications_test.go index 537e8cef095d1..6cea84af11cc2 100644 --- a/coderd/notifications_test.go +++ b/coderd/notifications_test.go @@ -115,7 +115,7 @@ func TestNotificationPreferences(t *testing.T) { t.Run("Initial state", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) + ctx := testutil.Context(t, testutil.WaitLong) api := coderdtest.New(t, createOpts(t)) firstUser := coderdtest.CreateFirstUser(t, api) @@ -133,7 +133,7 @@ func TestNotificationPreferences(t *testing.T) { t.Run("Insufficient permissions", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) + ctx := testutil.Context(t, testutil.WaitLong) api := coderdtest.New(t, createOpts(t)) firstUser := coderdtest.CreateFirstUser(t, api) @@ -156,7 +156,7 @@ func TestNotificationPreferences(t *testing.T) { t.Run("Admin may read any users' preferences", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) + ctx := testutil.Context(t, testutil.WaitLong) api := coderdtest.New(t, createOpts(t)) firstUser := coderdtest.CreateFirstUser(t, api) @@ -174,7 +174,7 @@ func TestNotificationPreferences(t *testing.T) { t.Run("Admin may update any users' preferences", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) + ctx := testutil.Context(t, testutil.WaitLong) api := coderdtest.New(t, createOpts(t)) firstUser := coderdtest.CreateFirstUser(t, api) @@ -202,7 +202,7 @@ func TestNotificationPreferences(t *testing.T) { t.Run("Add preferences", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) + ctx := testutil.Context(t, testutil.WaitLong) api := coderdtest.New(t, createOpts(t)) firstUser := coderdtest.CreateFirstUser(t, api) @@ -230,7 +230,7 @@ func TestNotificationPreferences(t *testing.T) { t.Run("Modify preferences", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) + ctx := testutil.Context(t, testutil.WaitLong) api := coderdtest.New(t, createOpts(t)) firstUser := coderdtest.CreateFirstUser(t, api)