Skip to content

Commit 330acd1

Browse files
DanielleMaywoodDanielleMaywood
authored
chore: create ResourceNotificationMessage and AsNotifier (coder#15301)
Closes coder#15213 This PR enables sending notifications without requiring the auth system context, instead using a new auth notifier context.
1 parent 9d03e04 commit 330acd1

File tree

16 files changed

+123
-52
lines changed

16 files changed

+123
-52
lines changed

‎cli/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -916,8 +916,8 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
916916
return xerrors.Errorf("failed to instantiate notification manager: %w", err)
917917
}
918918

919-
// nolint:gocritic // TODO: create own role.
920-
notificationsManager.Run(dbauthz.AsSystemRestricted(ctx))
919+
// nolint:gocritic // We need to run the manager in a notifier context.
920+
notificationsManager.Run(dbauthz.AsNotifier(ctx))
921921

922922
// Run report generator to distribute periodic reports.
923923
notificationReportGenerator := reports.NewReportGenerator(ctx, logger.Named("notifications.report_generator"), options.Database, options.NotificationsEnqueuer, quartz.NewReal())

‎coderd/apidoc/docs.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,23 @@ var (
264264
Scope: rbac.ScopeAll,
265265
}.WithCachedASTValue()
266266

267+
subjectNotifier = rbac.Subject{
268+
FriendlyName: "Notifier",
269+
ID: uuid.Nil.String(),
270+
Roles: rbac.Roles([]rbac.Role{
271+
{
272+
Identifier: rbac.RoleIdentifier{Name: "notifier"},
273+
DisplayName: "Notifier",
274+
Site: rbac.Permissions(map[string][]policy.Action{
275+
rbac.ResourceNotificationMessage.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
276+
}),
277+
Org: map[string][]rbac.Permission{},
278+
User: []rbac.Permission{},
279+
},
280+
}),
281+
Scope: rbac.ScopeAll,
282+
}.WithCachedASTValue()
283+
267284
subjectSystemRestricted = rbac.Subject{
268285
FriendlyName: "System",
269286
ID: uuid.Nil.String(),
@@ -287,6 +304,7 @@ var (
287304
rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionSSH},
288305
rbac.ResourceWorkspaceProxy.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
289306
rbac.ResourceDeploymentConfig.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
307+
rbac.ResourceNotificationMessage.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
290308
rbac.ResourceNotificationPreference.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
291309
rbac.ResourceNotificationTemplate.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
292310
rbac.ResourceCryptoKey.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
@@ -327,6 +345,12 @@ func AsKeyReader(ctx context.Context) context.Context {
327345
return context.WithValue(ctx, authContextKey{}, subjectCryptoKeyReader)
328346
}
329347

348+
// AsNotifier returns a context with an actor that has permissions required for
349+
// creating/reading/updating/deleting notifications.
350+
func AsNotifier(ctx context.Context) context.Context {
351+
return context.WithValue(ctx, authContextKey{}, subjectNotifier)
352+
}
353+
330354
// AsSystemRestricted returns a context with an actor that has permissions
331355
// required for various system operations (login, logout, metrics cache).
332356
func AsSystemRestricted(ctx context.Context) context.Context {
@@ -950,7 +974,7 @@ func (q *querier) AcquireLock(ctx context.Context, id int64) error {
950974
}
951975

952976
func (q *querier) AcquireNotificationMessages(ctx context.Context, arg database.AcquireNotificationMessagesParams) ([]database.AcquireNotificationMessagesRow, error) {
953-
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
977+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationMessage); err != nil {
954978
return nil, err
955979
}
956980
return q.db.AcquireNotificationMessages(ctx, arg)
@@ -1001,14 +1025,14 @@ func (q *querier) BatchUpdateWorkspaceLastUsedAt(ctx context.Context, arg databa
10011025
}
10021026

10031027
func (q *querier) BulkMarkNotificationMessagesFailed(ctx context.Context, arg database.BulkMarkNotificationMessagesFailedParams) (int64, error) {
1004-
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
1028+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationMessage); err != nil {
10051029
return 0, err
10061030
}
10071031
return q.db.BulkMarkNotificationMessagesFailed(ctx, arg)
10081032
}
10091033

10101034
func (q *querier) BulkMarkNotificationMessagesSent(ctx context.Context, arg database.BulkMarkNotificationMessagesSentParams) (int64, error) {
1011-
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
1035+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationMessage); err != nil {
10121036
return 0, err
10131037
}
10141038
return q.db.BulkMarkNotificationMessagesSent(ctx, arg)
@@ -1185,7 +1209,7 @@ func (q *querier) DeleteOAuth2ProviderAppTokensByAppAndUserID(ctx context.Contex
11851209
}
11861210

11871211
func (q *querier) DeleteOldNotificationMessages(ctx context.Context) error {
1188-
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
1212+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceNotificationMessage); err != nil {
11891213
return err
11901214
}
11911215
return q.db.DeleteOldNotificationMessages(ctx)
@@ -1307,7 +1331,7 @@ func (q *querier) DeleteWorkspaceAgentPortSharesByTemplate(ctx context.Context,
13071331
}
13081332

13091333
func (q *querier) EnqueueNotificationMessage(ctx context.Context, arg database.EnqueueNotificationMessageParams) error {
1310-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil {
1334+
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceNotificationMessage); err != nil {
13111335
return err
13121336
}
13131337
return q.db.EnqueueNotificationMessage(ctx, arg)
@@ -1321,7 +1345,7 @@ func (q *querier) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error {
13211345
}
13221346

13231347
func (q *querier) FetchNewMessageMetadata(ctx context.Context, arg database.FetchNewMessageMetadataParams) (database.FetchNewMessageMetadataRow, error) {
1324-
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
1348+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceNotificationMessage); err != nil {
13251349
return database.FetchNewMessageMetadataRow{}, err
13261350
}
13271351
return q.db.FetchNewMessageMetadata(ctx, arg)
@@ -1686,7 +1710,7 @@ func (q *querier) GetLogoURL(ctx context.Context) (string, error) {
16861710
}
16871711

16881712
func (q *querier) GetNotificationMessagesByStatus(ctx context.Context, arg database.GetNotificationMessagesByStatusParams) ([]database.NotificationMessage, error) {
1689-
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
1713+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceNotificationMessage); err != nil {
16901714
return nil, err
16911715
}
16921716
return q.db.GetNotificationMessagesByStatus(ctx, arg)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2888,40 +2888,33 @@ func (s *MethodTestSuite) TestSystemFunctions() {
28882888

28892889
func (s *MethodTestSuite) TestNotifications() {
28902890
// System functions
2891-
s.Run("AcquireNotificationMessages", s.Subtest(func(db database.Store, check *expects) {
2892-
// TODO: update this test once we have a specific role for notifications
2893-
check.Args(database.AcquireNotificationMessagesParams{}).Asserts(rbac.ResourceSystem, policy.ActionUpdate)
2891+
s.Run("AcquireNotificationMessages", s.Subtest(func(_ database.Store, check *expects) {
2892+
check.Args(database.AcquireNotificationMessagesParams{}).Asserts(rbac.ResourceNotificationMessage, policy.ActionUpdate)
28942893
}))
2895-
s.Run("BulkMarkNotificationMessagesFailed", s.Subtest(func(db database.Store, check *expects) {
2896-
// TODO: update this test once we have a specific role for notifications
2897-
check.Args(database.BulkMarkNotificationMessagesFailedParams{}).Asserts(rbac.ResourceSystem, policy.ActionUpdate)
2894+
s.Run("BulkMarkNotificationMessagesFailed", s.Subtest(func(_ database.Store, check *expects) {
2895+
check.Args(database.BulkMarkNotificationMessagesFailedParams{}).Asserts(rbac.ResourceNotificationMessage, policy.ActionUpdate)
28982896
}))
2899-
s.Run("BulkMarkNotificationMessagesSent", s.Subtest(func(db database.Store, check *expects) {
2900-
// TODO: update this test once we have a specific role for notifications
2901-
check.Args(database.BulkMarkNotificationMessagesSentParams{}).Asserts(rbac.ResourceSystem, policy.ActionUpdate)
2897+
s.Run("BulkMarkNotificationMessagesSent", s.Subtest(func(_ database.Store, check *expects) {
2898+
check.Args(database.BulkMarkNotificationMessagesSentParams{}).Asserts(rbac.ResourceNotificationMessage, policy.ActionUpdate)
29022899
}))
2903-
s.Run("DeleteOldNotificationMessages", s.Subtest(func(db database.Store, check *expects) {
2904-
// TODO: update this test once we have a specific role for notifications
2905-
check.Args().Asserts(rbac.ResourceSystem, policy.ActionDelete)
2900+
s.Run("DeleteOldNotificationMessages", s.Subtest(func(_ database.Store, check *expects) {
2901+
check.Args().Asserts(rbac.ResourceNotificationMessage, policy.ActionDelete)
29062902
}))
2907-
s.Run("EnqueueNotificationMessage", s.Subtest(func(db database.Store, check *expects) {
2908-
// TODO: update this test once we have a specific role for notifications
2903+
s.Run("EnqueueNotificationMessage", s.Subtest(func(_ database.Store, check *expects) {
29092904
check.Args(database.EnqueueNotificationMessageParams{
29102905
Method: database.NotificationMethodWebhook,
29112906
Payload: []byte("{}"),
2912-
}).Asserts(rbac.ResourceSystem, policy.ActionCreate)
2907+
}).Asserts(rbac.ResourceNotificationMessage, policy.ActionCreate)
29132908
}))
29142909
s.Run("FetchNewMessageMetadata", s.Subtest(func(db database.Store, check *expects) {
2915-
// TODO: update this test once we have a specific role for notifications
29162910
u := dbgen.User(s.T(), db, database.User{})
2917-
check.Args(database.FetchNewMessageMetadataParams{UserID: u.ID}).Asserts(rbac.ResourceSystem, policy.ActionRead)
2911+
check.Args(database.FetchNewMessageMetadataParams{UserID: u.ID}).Asserts(rbac.ResourceNotificationMessage, policy.ActionRead)
29182912
}))
2919-
s.Run("GetNotificationMessagesByStatus", s.Subtest(func(db database.Store, check *expects) {
2920-
// TODO: update this test once we have a specific role for notifications
2913+
s.Run("GetNotificationMessagesByStatus", s.Subtest(func(_ database.Store, check *expects) {
29212914
check.Args(database.GetNotificationMessagesByStatusParams{
29222915
Status: database.NotificationMessageStatusLeased,
29232916
Limit: 10,
2924-
}).Asserts(rbac.ResourceSystem, policy.ActionRead)
2917+
}).Asserts(rbac.ResourceNotificationMessage, policy.ActionRead)
29252918
}))
29262919

29272920
// Notification templates

‎coderd/notifications/notifications_test.go

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestBasicNotificationRoundtrip(t *testing.T) {
7171
}
7272

7373
// nolint:gocritic // Unit test.
74-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
74+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
7575
store, _ := dbtestutil.NewDB(t)
7676
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
7777
method := database.NotificationMethodSmtp
@@ -135,7 +135,7 @@ func TestSMTPDispatch(t *testing.T) {
135135
// SETUP
136136

137137
// nolint:gocritic // Unit test.
138-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
138+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
139139
store, _ := dbtestutil.NewDB(t)
140140
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
141141

@@ -197,7 +197,7 @@ func TestWebhookDispatch(t *testing.T) {
197197
// SETUP
198198

199199
// nolint:gocritic // Unit test.
200-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
200+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
201201
store, _ := dbtestutil.NewDB(t)
202202
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
203203

@@ -281,7 +281,7 @@ func TestBackpressure(t *testing.T) {
281281
store, _ := dbtestutil.NewDB(t)
282282
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
283283
// nolint:gocritic // Unit test.
284-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitShort))
284+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitShort))
285285

286286
const method = database.NotificationMethodWebhook
287287
cfg := defaultNotificationsConfig(method)
@@ -407,7 +407,7 @@ func TestRetries(t *testing.T) {
407407

408408
const maxAttempts = 3
409409
// nolint:gocritic // Unit test.
410-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
410+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
411411
store, _ := dbtestutil.NewDB(t)
412412
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
413413

@@ -501,7 +501,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) {
501501
}
502502

503503
// nolint:gocritic // Unit test.
504-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
504+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
505505
store, _ := dbtestutil.NewDB(t)
506506
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
507507

@@ -521,7 +521,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) {
521521
noopInterceptor := newNoopStoreSyncer(store)
522522

523523
// nolint:gocritic // Unit test.
524-
mgrCtx, cancelManagerCtx := context.WithCancel(dbauthz.AsSystemRestricted(context.Background()))
524+
mgrCtx, cancelManagerCtx := context.WithCancel(dbauthz.AsNotifier(context.Background()))
525525
t.Cleanup(cancelManagerCtx)
526526

527527
mgr, err := notifications.NewManager(cfg, noopInterceptor, defaultHelpers(), createMetrics(), logger.Named("manager"))
@@ -626,7 +626,7 @@ func TestNotifierPaused(t *testing.T) {
626626
// Setup.
627627

628628
// nolint:gocritic // Unit test.
629-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
629+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
630630
store, _ := dbtestutil.NewDB(t)
631631
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
632632

@@ -1081,7 +1081,7 @@ func TestNotificationTemplates_Golden(t *testing.T) {
10811081
}()
10821082

10831083
// nolint:gocritic // Unit test.
1084-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
1084+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
10851085

10861086
// smtp config shared between client and server
10871087
smtpConfig := codersdk.NotificationsEmailConfig{
@@ -1160,12 +1160,14 @@ func TestNotificationTemplates_Golden(t *testing.T) {
11601160
// as appearance changes are enterprise features and we do not want to mix those
11611161
// can't use the api
11621162
if tc.appName != "" {
1163-
err = (*db).UpsertApplicationName(ctx, "Custom Application")
1163+
// nolint:gocritic // Unit test.
1164+
err = (*db).UpsertApplicationName(dbauthz.AsSystemRestricted(ctx), "Custom Application")
11641165
require.NoError(t, err)
11651166
}
11661167

11671168
if tc.logoURL != "" {
1168-
err = (*db).UpsertLogoURL(ctx, "https://custom.application/logo.png")
1169+
// nolint:gocritic // Unit test.
1170+
err = (*db).UpsertLogoURL(dbauthz.AsSystemRestricted(ctx), "https://custom.application/logo.png")
11691171
require.NoError(t, err)
11701172
}
11711173

@@ -1248,7 +1250,7 @@ func TestNotificationTemplates_Golden(t *testing.T) {
12481250
}()
12491251

12501252
// nolint:gocritic // Unit test.
1251-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
1253+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
12521254

12531255
// Spin up the mock webhook server
12541256
var body []byte
@@ -1376,7 +1378,7 @@ func TestDisabledBeforeEnqueue(t *testing.T) {
13761378
}
13771379

13781380
// nolint:gocritic // Unit test.
1379-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
1381+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
13801382
store, _ := dbtestutil.NewDB(t)
13811383
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
13821384

@@ -1412,7 +1414,7 @@ func TestDisabledAfterEnqueue(t *testing.T) {
14121414
}
14131415

14141416
// nolint:gocritic // Unit test.
1415-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
1417+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
14161418
store, _ := dbtestutil.NewDB(t)
14171419
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
14181420

@@ -1469,7 +1471,7 @@ func TestCustomNotificationMethod(t *testing.T) {
14691471
}
14701472

14711473
// nolint:gocritic // Unit test.
1472-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
1474+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
14731475
store, _ := dbtestutil.NewDB(t)
14741476
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
14751477

@@ -1573,7 +1575,7 @@ func TestNotificationsTemplates(t *testing.T) {
15731575
}
15741576

15751577
// nolint:gocritic // Unit test.
1576-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
1578+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
15771579
api := coderdtest.New(t, createOpts(t))
15781580

15791581
// GIVEN: the first user (owner) and a regular member
@@ -1610,7 +1612,7 @@ func TestNotificationDuplicates(t *testing.T) {
16101612
}
16111613

16121614
// nolint:gocritic // Unit test.
1613-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitSuperLong))
1615+
ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong))
16141616
store, _ := dbtestutil.NewDB(t)
16151617
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
16161618

‎coderd/rbac/object_gen.go

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/rbac/policy/policy.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,14 @@ var RBACPermissions = map[string]PermissionDefinition{
262262
ActionDelete: actDef(""),
263263
},
264264
},
265+
"notification_message": {
266+
Actions: map[Action]ActionDefinition{
267+
ActionCreate: actDef("create notification messages"),
268+
ActionRead: actDef("read notification messages"),
269+
ActionUpdate: actDef("update notification messages"),
270+
ActionDelete: actDef("delete notification messages"),
271+
},
272+
},
265273
"notification_template": {
266274
Actions: map[Action]ActionDefinition{
267275
ActionRead: actDef("read notification templates"),

0 commit comments

Comments
 (0)