diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index af9daf7f8de63..667b20dd9fd4f 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -19,6 +19,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" @@ -116,7 +117,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { tickCh = make(chan time.Time) statsCh = make(chan autobuild.Stats) logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: !tc.expectStart}).Leveled(slog.LevelDebug) - enqueuer = testutil.FakeNotificationsEnqueuer{} + enqueuer = notificationstest.FakeEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, @@ -202,17 +203,18 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { } if tc.expectNotification { - require.Len(t, enqueuer.Sent, 1) - require.Equal(t, enqueuer.Sent[0].UserID, workspace.OwnerID) - require.Contains(t, enqueuer.Sent[0].Targets, workspace.TemplateID) - require.Contains(t, enqueuer.Sent[0].Targets, workspace.ID) - require.Contains(t, enqueuer.Sent[0].Targets, workspace.OrganizationID) - require.Contains(t, enqueuer.Sent[0].Targets, workspace.OwnerID) - require.Equal(t, newVersion.Name, enqueuer.Sent[0].Labels["template_version_name"]) - require.Equal(t, "autobuild", enqueuer.Sent[0].Labels["initiator"]) - require.Equal(t, "autostart", enqueuer.Sent[0].Labels["reason"]) + sent := enqueuer.Sent() + require.Len(t, sent, 1) + require.Equal(t, sent[0].UserID, workspace.OwnerID) + require.Contains(t, sent[0].Targets, workspace.TemplateID) + require.Contains(t, sent[0].Targets, workspace.ID) + require.Contains(t, sent[0].Targets, workspace.OrganizationID) + require.Contains(t, sent[0].Targets, workspace.OwnerID) + require.Equal(t, newVersion.Name, sent[0].Labels["template_version_name"]) + require.Equal(t, "autobuild", sent[0].Labels["initiator"]) + require.Equal(t, "autostart", sent[0].Labels["reason"]) } else { - require.Len(t, enqueuer.Sent, 0) + require.Empty(t, enqueuer.Sent()) } }) } @@ -1073,7 +1075,7 @@ func TestNotifications(t *testing.T) { var ( ticker = make(chan time.Time) statCh = make(chan autobuild.Stats) - notifyEnq = testutil.FakeNotificationsEnqueuer{} + notifyEnq = notificationstest.FakeEnqueuer{} timeTilDormant = time.Minute client = coderdtest.New(t, &coderdtest.Options{ AutobuildTicker: ticker, @@ -1107,6 +1109,7 @@ func TestNotifications(t *testing.T) { _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) // Wait for workspace to become dormant + notifyEnq.Clear() ticker <- workspace.LastUsedAt.Add(timeTilDormant * 3) _ = testutil.RequireRecvCtx(testutil.Context(t, testutil.WaitShort), t, statCh) @@ -1115,14 +1118,14 @@ func TestNotifications(t *testing.T) { require.NotNil(t, workspace.DormantAt) // Check that a notification was enqueued - require.Len(t, notifyEnq.Sent, 2) - // notifyEnq.Sent[0] is an event for created user account - require.Equal(t, notifyEnq.Sent[1].UserID, workspace.OwnerID) - require.Equal(t, notifyEnq.Sent[1].TemplateID, notifications.TemplateWorkspaceDormant) - require.Contains(t, notifyEnq.Sent[1].Targets, template.ID) - require.Contains(t, notifyEnq.Sent[1].Targets, workspace.ID) - require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OrganizationID) - require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OwnerID) + sent := notifyEnq.Sent() + require.Len(t, sent, 1) + require.Equal(t, sent[0].UserID, workspace.OwnerID) + require.Equal(t, sent[0].TemplateID, notifications.TemplateWorkspaceDormant) + require.Contains(t, sent[0].Targets, template.ID) + require.Contains(t, sent[0].Targets, workspace.ID) + require.Contains(t, sent[0].Targets, workspace.OrganizationID) + require.Contains(t, sent[0].Targets, workspace.OwnerID) }) } @@ -1168,7 +1171,7 @@ func mustSchedule(t *testing.T, s string) *cron.Schedule { } func mustWorkspaceParameters(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) { - ctx := context.Background() + ctx := testutil.Context(t, testutil.WaitShort) buildParameters, err := client.WorkspaceBuildParameters(ctx, workspaceID) require.NoError(t, err) require.NotEmpty(t, buildParameters) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index f3868bf14d54b..7c1e6a4962a8c 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -66,6 +66,7 @@ import ( "github.com/coder/coder/v2/coderd/gitsshkey" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/runtimeconfig" @@ -251,7 +252,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can } if options.NotificationsEnqueuer == nil { - options.NotificationsEnqueuer = new(testutil.FakeNotificationsEnqueuer) + options.NotificationsEnqueuer = ¬ificationstest.FakeEnqueuer{} } accessControlStore := &atomic.Pointer[dbauthz.AccessControlStore]{} @@ -311,7 +312,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can t.Cleanup(closeBatcher) } if options.NotificationsEnqueuer == nil { - options.NotificationsEnqueuer = &testutil.FakeNotificationsEnqueuer{} + options.NotificationsEnqueuer = ¬ificationstest.FakeEnqueuer{} } if options.OneTimePasscodeValidityPeriod == 0 { diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9afa8afcb01d9..637a928824b13 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -178,6 +178,8 @@ var ( // this can be reduced to read a specific org. rbac.ResourceOrganization.Type: {policy.ActionRead}, rbac.ResourceGroup.Type: {policy.ActionRead}, + // Provisionerd creates notification messages + rbac.ResourceNotificationMessage.Type: {policy.ActionCreate, policy.ActionRead}, }), Org: map[string][]rbac.Permission{}, User: []rbac.Permission{}, @@ -194,11 +196,12 @@ var ( Identifier: rbac.RoleIdentifier{Name: "autostart"}, DisplayName: "Autostart Daemon", Site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceSystem.Type: {policy.WildcardSymbol}, - rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate}, - rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop}, - rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop}, - rbac.ResourceUser.Type: {policy.ActionRead}, + rbac.ResourceNotificationMessage.Type: {policy.ActionCreate, policy.ActionRead}, + rbac.ResourceSystem.Type: {policy.WildcardSymbol}, + rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate}, + rbac.ResourceUser.Type: {policy.ActionRead}, + rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop}, + rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop}, }), Org: map[string][]rbac.Permission{}, User: []rbac.Permission{}, diff --git a/coderd/notifications/notificationstest/fake_enqueuer.go b/coderd/notifications/notificationstest/fake_enqueuer.go new file mode 100644 index 0000000000000..023137720998d --- /dev/null +++ b/coderd/notifications/notificationstest/fake_enqueuer.go @@ -0,0 +1,99 @@ +package notificationstest + +import ( + "context" + "fmt" + "sync" + + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" + + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/policy" +) + +type FakeEnqueuer struct { + authorizer rbac.Authorizer + mu sync.Mutex + sent []*FakeNotification +} + +type FakeNotification struct { + UserID, TemplateID uuid.UUID + Labels map[string]string + Data map[string]any + CreatedBy string + Targets []uuid.UUID +} + +// TODO: replace this with actual calls to dbauthz. +// See: https://github.com/coder/coder/issues/15481 +func (f *FakeEnqueuer) assertRBACNoLock(ctx context.Context) { + if f.mu.TryLock() { + panic("Developer error: do not call assertRBACNoLock outside of a mutex lock!") + } + + // If we get here, we are locked. + if f.authorizer == nil { + f.authorizer = rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry()) + } + + act, ok := dbauthz.ActorFromContext(ctx) + if !ok { + panic("Developer error: no actor in context, you may need to use dbauthz.AsNotifier(ctx)") + } + + for _, a := range []policy.Action{policy.ActionCreate, policy.ActionRead} { + err := f.authorizer.Authorize(ctx, act, a, rbac.ResourceNotificationMessage) + if err == nil { + return + } + + if rbac.IsUnauthorizedError(err) { + panic(fmt.Sprintf("Developer error: not authorized to %s %s. "+ + "Ensure that you are using dbauthz.AsXXX with an actor that has "+ + "policy.ActionCreate on rbac.ResourceNotificationMessage", a, rbac.ResourceNotificationMessage.Type)) + } + panic("Developer error: failed to check auth:" + err.Error()) + } +} + +func (f *FakeEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { + return f.EnqueueWithData(ctx, userID, templateID, labels, nil, createdBy, targets...) +} + +func (f *FakeEnqueuer) EnqueueWithData(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { + return f.enqueueWithDataLock(ctx, userID, templateID, labels, data, createdBy, targets...) +} + +func (f *FakeEnqueuer) enqueueWithDataLock(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.assertRBACNoLock(ctx) + + f.sent = append(f.sent, &FakeNotification{ + UserID: userID, + TemplateID: templateID, + Labels: labels, + Data: data, + CreatedBy: createdBy, + Targets: targets, + }) + + id := uuid.New() + return &id, nil +} + +func (f *FakeEnqueuer) Clear() { + f.mu.Lock() + defer f.mu.Unlock() + + f.sent = nil +} + +func (f *FakeEnqueuer) Sent() []*FakeNotification { + f.mu.Lock() + defer f.mu.Unlock() + return append([]*FakeNotification{}, f.sent...) +} diff --git a/coderd/notifications/reports/generator_internal_test.go b/coderd/notifications/reports/generator_internal_test.go index fcf22d80d80f9..a4330493f0aed 100644 --- a/coderd/notifications/reports/generator_internal_test.go +++ b/coderd/notifications/reports/generator_internal_test.go @@ -21,8 +21,8 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/rbac" - "github.com/coder/coder/v2/testutil" ) const dayDuration = 24 * time.Hour @@ -49,7 +49,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then: no report should be generated require.NoError(t, err) - require.Empty(t, notifEnq.Sent) + require.Empty(t, notifEnq.Sent()) // Given: one week later and no jobs were executed clk.Advance(failedWorkspaceBuildsReportFrequency + time.Minute) @@ -60,7 +60,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then: report is still empty require.NoError(t, err) - require.Empty(t, notifEnq.Sent) + require.Empty(t, notifEnq.Sent()) }) t.Run("InitialState_NoBuilds_NoReport", func(t *testing.T) { @@ -101,7 +101,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then: failed builds should not be reported require.NoError(t, err) - require.Empty(t, notifEnq.Sent) + require.Empty(t, notifEnq.Sent()) // Given: one week later, but still no jobs clk.Advance(failedWorkspaceBuildsReportFrequency + time.Minute) @@ -112,13 +112,13 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then: report is still empty require.NoError(t, err) - require.Empty(t, notifEnq.Sent) + require.Empty(t, notifEnq.Sent()) }) t.Run("FailedBuilds_SecondRun_Report_ThirdRunTooEarly_NoReport_FourthRun_Report", func(t *testing.T) { t.Parallel() - verifyNotification := func(t *testing.T, recipient database.User, notif *testutil.Notification, tmpl database.Template, failedBuilds, totalBuilds int64, templateVersions []map[string]interface{}) { + verifyNotification := func(t *testing.T, recipient database.User, notif *notificationstest.FakeNotification, tmpl database.Template, failedBuilds, totalBuilds int64, templateVersions []map[string]interface{}) { t.Helper() require.Equal(t, recipient.ID, notif.UserID) @@ -175,7 +175,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then require.NoError(t, err) - require.Empty(t, notifEnq.Sent) // no notifications + require.Empty(t, notifEnq.Sent()) // no notifications // One week later... clk.Advance(failedWorkspaceBuildsReportFrequency + time.Minute) @@ -211,9 +211,10 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then require.NoError(t, err) - require.Len(t, notifEnq.Sent, 4) // 2 templates, 2 template admins + sent := notifEnq.Sent() + require.Len(t, sent, 4) // 2 templates, 2 template admins for i, templateAdmin := range []database.User{templateAdmin1, templateAdmin2} { - verifyNotification(t, templateAdmin, notifEnq.Sent[i], t1, 3, 4, []map[string]interface{}{ + verifyNotification(t, templateAdmin, sent[i], t1, 3, 4, []map[string]interface{}{ { "failed_builds": []map[string]interface{}{ {"build_number": int32(7), "workspace_name": w3.Name, "workspace_owner_username": user1.Username}, @@ -233,7 +234,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { } for i, templateAdmin := range []database.User{templateAdmin1, templateAdmin2} { - verifyNotification(t, templateAdmin, notifEnq.Sent[i+2], t2, 3, 5, []map[string]interface{}{ + verifyNotification(t, templateAdmin, sent[i+2], t2, 3, 5, []map[string]interface{}{ { "failed_builds": []map[string]interface{}{ {"build_number": int32(8), "workspace_name": w4.Name, "workspace_owner_username": user2.Username}, @@ -265,7 +266,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { require.NoError(t, err) // Then: no notifications as it is too early - require.Empty(t, notifEnq.Sent) + require.Empty(t, notifEnq.Sent()) // Given: 1 day 1 hour later clk.Advance(dayDuration + time.Hour).MustWait(context.Background()) @@ -276,9 +277,10 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { require.NoError(t, err) // Then: we should see the failed job in the report - require.Len(t, notifEnq.Sent, 2) // a new failed job should be reported + sent = notifEnq.Sent() + require.Len(t, sent, 2) // a new failed job should be reported for i, templateAdmin := range []database.User{templateAdmin1, templateAdmin2} { - verifyNotification(t, templateAdmin, notifEnq.Sent[i], t1, 1, 1, []map[string]interface{}{ + verifyNotification(t, templateAdmin, sent[i], t1, 1, 1, []map[string]interface{}{ { "failed_builds": []map[string]interface{}{ {"build_number": int32(77), "workspace_name": w1.Name, "workspace_owner_username": user1.Username}, @@ -293,7 +295,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { t.Run("TooManyFailedBuilds_SecondRun_Report", func(t *testing.T) { t.Parallel() - verifyNotification := func(t *testing.T, recipient database.User, notif *testutil.Notification, tmpl database.Template, failedBuilds, totalBuilds int64, templateVersions []map[string]interface{}) { + verifyNotification := func(t *testing.T, recipient database.User, notif *notificationstest.FakeNotification, tmpl database.Template, failedBuilds, totalBuilds int64, templateVersions []map[string]interface{}) { t.Helper() require.Equal(t, recipient.ID, notif.UserID) @@ -338,7 +340,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then require.NoError(t, err) - require.Empty(t, notifEnq.Sent) // no notifications + require.Empty(t, notifEnq.Sent()) // no notifications // One week later... clk.Advance(failedWorkspaceBuildsReportFrequency + time.Minute) @@ -365,8 +367,9 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then require.NoError(t, err) - require.Len(t, notifEnq.Sent, 1) // 1 template, 1 template admin - verifyNotification(t, templateAdmin1, notifEnq.Sent[0], t1, 46, 47, []map[string]interface{}{ + sent := notifEnq.Sent() + require.Len(t, sent, 1) // 1 template, 1 template admin + verifyNotification(t, templateAdmin1, sent[0], t1, 46, 47, []map[string]interface{}{ { "failed_builds": []map[string]interface{}{ {"build_number": int32(23), "workspace_name": w1.Name, "workspace_owner_username": user1.Username}, @@ -435,7 +438,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then: no notifications require.NoError(t, err) - require.Empty(t, notifEnq.Sent) + require.Empty(t, notifEnq.Sent()) // Given: one week later, and a successful few jobs being executed clk.Advance(failedWorkspaceBuildsReportFrequency + time.Minute) @@ -453,18 +456,18 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) { // Then: no failures? nothing to report require.NoError(t, err) - require.Len(t, notifEnq.Sent, 0) // all jobs succeeded so nothing to report + require.Len(t, notifEnq.Sent(), 0) // all jobs succeeded so nothing to report }) } -func setup(t *testing.T) (context.Context, slog.Logger, database.Store, pubsub.Pubsub, *testutil.FakeNotificationsEnqueuer, *quartz.Mock) { +func setup(t *testing.T) (context.Context, slog.Logger, database.Store, pubsub.Pubsub, *notificationstest.FakeEnqueuer, *quartz.Mock) { t.Helper() // nolint:gocritic // reportFailedWorkspaceBuilds is called by system. ctx := dbauthz.AsSystemRestricted(context.Background()) logger := slogtest.Make(t, &slogtest.Options{}) db, ps := dbtestutil.NewDB(t) - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} clk := quartz.NewMock(t) return ctx, logger, db, ps, notifyEnq, clk } diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 98ab07db3d0f7..0a51a6488f424 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -36,6 +36,7 @@ import ( "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/externalauth" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/schedule/cron" @@ -1634,7 +1635,7 @@ func TestNotifications(t *testing.T) { t.Parallel() ctx := context.Background() - notifEnq := &testutil.FakeNotificationsEnqueuer{} + notifEnq := ¬ificationstest.FakeEnqueuer{} srv, db, ps, pd := setup(t, false, &overrides{ notificationEnqueuer: notifEnq, @@ -1713,17 +1714,18 @@ func TestNotifications(t *testing.T) { if tc.shouldNotify { // Validate that the notification was sent and contained the expected values. - require.Len(t, notifEnq.Sent, 1) - require.Equal(t, notifEnq.Sent[0].UserID, user.ID) - require.Contains(t, notifEnq.Sent[0].Targets, template.ID) - require.Contains(t, notifEnq.Sent[0].Targets, workspace.ID) - require.Contains(t, notifEnq.Sent[0].Targets, workspace.OrganizationID) - require.Contains(t, notifEnq.Sent[0].Targets, user.ID) + sent := notifEnq.Sent() + require.Len(t, sent, 1) + require.Equal(t, sent[0].UserID, user.ID) + require.Contains(t, sent[0].Targets, template.ID) + require.Contains(t, sent[0].Targets, workspace.ID) + require.Contains(t, sent[0].Targets, workspace.OrganizationID) + require.Contains(t, sent[0].Targets, user.ID) if tc.deletionReason == database.BuildReasonInitiator { - require.Equal(t, initiator.Username, notifEnq.Sent[0].Labels["initiator"]) + require.Equal(t, initiator.Username, sent[0].Labels["initiator"]) } } else { - require.Len(t, notifEnq.Sent, 0) + require.Len(t, notifEnq.Sent(), 0) } }) } @@ -1755,7 +1757,7 @@ func TestNotifications(t *testing.T) { t.Parallel() ctx := context.Background() - notifEnq := &testutil.FakeNotificationsEnqueuer{} + notifEnq := ¬ificationstest.FakeEnqueuer{} // Otherwise `(*Server).FailJob` fails with: // audit log - get build {"error": "sql: no rows in result set"} @@ -1824,15 +1826,16 @@ func TestNotifications(t *testing.T) { if tc.shouldNotify { // Validate that the notification was sent and contained the expected values. - require.Len(t, notifEnq.Sent, 1) - require.Equal(t, notifEnq.Sent[0].UserID, user.ID) - require.Contains(t, notifEnq.Sent[0].Targets, template.ID) - require.Contains(t, notifEnq.Sent[0].Targets, workspace.ID) - require.Contains(t, notifEnq.Sent[0].Targets, workspace.OrganizationID) - require.Contains(t, notifEnq.Sent[0].Targets, user.ID) - require.Equal(t, string(tc.buildReason), notifEnq.Sent[0].Labels["reason"]) + sent := notifEnq.Sent() + require.Len(t, sent, 1) + require.Equal(t, sent[0].UserID, user.ID) + require.Contains(t, sent[0].Targets, template.ID) + require.Contains(t, sent[0].Targets, workspace.ID) + require.Contains(t, sent[0].Targets, workspace.OrganizationID) + require.Contains(t, sent[0].Targets, user.ID) + require.Equal(t, string(tc.buildReason), sent[0].Labels["reason"]) } else { - require.Len(t, notifEnq.Sent, 0) + require.Len(t, notifEnq.Sent(), 0) } }) } @@ -1844,7 +1847,7 @@ func TestNotifications(t *testing.T) { ctx := context.Background() // given - notifEnq := &testutil.FakeNotificationsEnqueuer{} + notifEnq := ¬ificationstest.FakeEnqueuer{} srv, db, ps, pd := setup(t, true /* ignoreLogErrors */, &overrides{notificationEnqueuer: notifEnq}) templateAdmin := dbgen.User(t, db, database.User{RBACRoles: []string{codersdk.RoleTemplateAdmin}}) @@ -1886,19 +1889,20 @@ func TestNotifications(t *testing.T) { require.NoError(t, err) // then - require.Len(t, notifEnq.Sent, 1) - assert.Equal(t, notifEnq.Sent[0].UserID, templateAdmin.ID) - assert.Equal(t, notifEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceManualBuildFailed) - assert.Contains(t, notifEnq.Sent[0].Targets, template.ID) - assert.Contains(t, notifEnq.Sent[0].Targets, workspace.ID) - assert.Contains(t, notifEnq.Sent[0].Targets, workspace.OrganizationID) - assert.Contains(t, notifEnq.Sent[0].Targets, user.ID) - assert.Equal(t, workspace.Name, notifEnq.Sent[0].Labels["name"]) - assert.Equal(t, template.DisplayName, notifEnq.Sent[0].Labels["template_name"]) - assert.Equal(t, version.Name, notifEnq.Sent[0].Labels["template_version_name"]) - assert.Equal(t, user.Username, notifEnq.Sent[0].Labels["initiator"]) - assert.Equal(t, user.Username, notifEnq.Sent[0].Labels["workspace_owner_username"]) - assert.Equal(t, strconv.Itoa(int(build.BuildNumber)), notifEnq.Sent[0].Labels["workspace_build_number"]) + sent := notifEnq.Sent() + require.Len(t, sent, 1) + assert.Equal(t, sent[0].UserID, templateAdmin.ID) + assert.Equal(t, sent[0].TemplateID, notifications.TemplateWorkspaceManualBuildFailed) + assert.Contains(t, sent[0].Targets, template.ID) + assert.Contains(t, sent[0].Targets, workspace.ID) + assert.Contains(t, sent[0].Targets, workspace.OrganizationID) + assert.Contains(t, sent[0].Targets, user.ID) + assert.Equal(t, workspace.Name, sent[0].Labels["name"]) + assert.Equal(t, template.DisplayName, sent[0].Labels["template_name"]) + assert.Equal(t, version.Name, sent[0].Labels["template_version_name"]) + assert.Equal(t, user.Username, sent[0].Labels["initiator"]) + assert.Equal(t, user.Username, sent[0].Labels["workspace_owner_username"]) + assert.Equal(t, strconv.Itoa(int(build.BuildNumber)), sent[0].Labels["workspace_build_number"]) }) } diff --git a/coderd/templates.go b/coderd/templates.go index 76534e2328f92..4280c25607ab7 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -140,7 +140,8 @@ func (api *API) notifyTemplateDeleted(ctx context.Context, template database.Tem templateNameLabel = template.Name } - if _, err := api.NotificationsEnqueuer.Enqueue(ctx, receiverID, notifications.TemplateTemplateDeleted, + // nolint:gocritic // Need notifier actor to enqueue notifications + if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), receiverID, notifications.TemplateTemplateDeleted, map[string]string{ "name": templateNameLabel, "initiator": initiator.Username, diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 3368aa582daf1..9bac24cdc248b 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -21,6 +21,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/util/ptr" @@ -1404,7 +1405,7 @@ func TestTemplateNotifications(t *testing.T) { // Given: an initiator var ( - notifyEnq = &testutil.FakeNotificationsEnqueuer{} + notifyEnq = ¬ificationstest.FakeEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, NotificationsEnqueuer: notifyEnq, @@ -1421,8 +1422,8 @@ func TestTemplateNotifications(t *testing.T) { require.NoError(t, err) // Then: the delete notification is not sent to the initiator. - deleteNotifications := make([]*testutil.Notification, 0) - for _, n := range notifyEnq.Sent { + deleteNotifications := make([]*notificationstest.FakeNotification, 0) + for _, n := range notifyEnq.Sent() { if n.TemplateID == notifications.TemplateTemplateDeleted { deleteNotifications = append(deleteNotifications, n) } @@ -1435,7 +1436,7 @@ func TestTemplateNotifications(t *testing.T) { // Given: multiple users with different roles var ( - notifyEnq = &testutil.FakeNotificationsEnqueuer{} + notifyEnq = ¬ificationstest.FakeEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, NotificationsEnqueuer: notifyEnq, @@ -1465,8 +1466,8 @@ func TestTemplateNotifications(t *testing.T) { // Then: only owners and template admins should receive the // notification. shouldBeNotified := []uuid.UUID{owner.ID, tmplAdmin.ID} - var deleteTemplateNotifications []*testutil.Notification - for _, n := range notifyEnq.Sent { + var deleteTemplateNotifications []*notificationstest.FakeNotification + for _, n := range notifyEnq.Sent() { if n.TemplateID == notifications.TemplateTemplateDeleted { deleteTemplateNotifications = append(deleteTemplateNotifications, n) } diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index 843f8ec753133..36713f6a6ae40 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -37,6 +37,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/jwtutils" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/promoauth" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" @@ -1805,7 +1806,7 @@ func TestUserForgotPassword(t *testing.T) { const oldPassword = "SomeSecurePassword!" const newPassword = "SomeNewSecurePassword!" - requireOneTimePasscodeNotification := func(t *testing.T, notif *testutil.Notification, userID uuid.UUID) { + requireOneTimePasscodeNotification := func(t *testing.T, notif *notificationstest.FakeNotification, userID uuid.UUID) { require.Equal(t, notifications.TemplateUserRequestedOneTimePasscode, notif.TemplateID) require.Equal(t, userID, notif.UserID) require.Equal(t, 1, len(notif.Targets)) @@ -1831,17 +1832,15 @@ func TestUserForgotPassword(t *testing.T) { require.Contains(t, apiErr.Message, "Incorrect email or password.") } - requireRequestOneTimePasscode := func(t *testing.T, ctx context.Context, client *codersdk.Client, notifyEnq *testutil.FakeNotificationsEnqueuer, email string, userID uuid.UUID) string { - notifsSent := len(notifyEnq.Sent) - + requireRequestOneTimePasscode := func(t *testing.T, ctx context.Context, client *codersdk.Client, notifyEnq *notificationstest.FakeEnqueuer, email string, userID uuid.UUID) string { + notifyEnq.Clear() err := client.RequestOneTimePasscode(ctx, codersdk.RequestOneTimePasscodeRequest{Email: email}) require.NoError(t, err) + sent := notifyEnq.Sent() + require.Len(t, sent, 1) - require.Equal(t, notifsSent+1, len(notifyEnq.Sent)) - - notif := notifyEnq.Sent[notifsSent] - requireOneTimePasscodeNotification(t, notif, userID) - return notif.Labels["one_time_passcode"] + requireOneTimePasscodeNotification(t, sent[0], userID) + return sent[0].Labels["one_time_passcode"] } requireChangePasswordWithOneTimePasscode := func(t *testing.T, ctx context.Context, client *codersdk.Client, email string, passcode string, password string) { @@ -1856,7 +1855,7 @@ func TestUserForgotPassword(t *testing.T) { t.Run("CanChangePassword", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, @@ -1897,7 +1896,7 @@ func TestUserForgotPassword(t *testing.T) { const oneTimePasscodeValidityPeriod = 1 * time.Millisecond - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, @@ -1934,7 +1933,7 @@ func TestUserForgotPassword(t *testing.T) { t.Run("CannotChangePasswordWithoutRequestingOneTimePasscode", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, @@ -1963,7 +1962,7 @@ func TestUserForgotPassword(t *testing.T) { t.Run("CannotChangePasswordWithInvalidOneTimePasscode", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, @@ -1994,7 +1993,7 @@ func TestUserForgotPassword(t *testing.T) { t.Run("CannotChangePasswordWithNoOneTimePasscode", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, @@ -2027,7 +2026,7 @@ func TestUserForgotPassword(t *testing.T) { t.Run("CannotChangePasswordWithWeakPassword", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, @@ -2060,7 +2059,7 @@ func TestUserForgotPassword(t *testing.T) { t.Run("CannotChangePasswordOfAnotherUser", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, @@ -2095,7 +2094,7 @@ func TestUserForgotPassword(t *testing.T) { t.Run("GivenOKResponseWithInvalidEmail", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, @@ -2112,10 +2111,9 @@ func TestUserForgotPassword(t *testing.T) { }) require.NoError(t, err) - require.Equal(t, 1, len(notifyEnq.Sent)) - - notif := notifyEnq.Sent[0] - require.NotEqual(t, notifications.TemplateUserRequestedOneTimePasscode, notif.TemplateID) + sent := notifyEnq.Sent() + require.Len(t, notifyEnq.Sent(), 1) + require.NotEqual(t, notifications.TemplateUserRequestedOneTimePasscode, sent[0].TemplateID) }) } diff --git a/coderd/users.go b/coderd/users.go index 445b44f334349..4978e12a788b9 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -604,7 +604,8 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) { } for _, u := range userAdmins { - if _, err := api.NotificationsEnqueuer.Enqueue(ctx, u.ID, notifications.TemplateUserAccountDeleted, + // nolint: gocritic // Need notifier actor to enqueue notifications + if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), u.ID, notifications.TemplateUserAccountDeleted, map[string]string{ "deleted_account_name": user.Username, "deleted_account_user_name": user.Name, @@ -946,14 +947,16 @@ func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName stri // Send notifications to user admins and affected user for _, u := range userAdmins { - if _, err := api.NotificationsEnqueuer.Enqueue(ctx, u.ID, adminTemplateID, + // nolint:gocritic // Need notifier actor to enqueue notifications + if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), u.ID, adminTemplateID, labels, "api-put-user-status", targetUser.ID, ); err != nil { api.Logger.Warn(ctx, "unable to notify about changed user's status", slog.F("affected_user", targetUser.Username), slog.Error(err)) } } - if _, err := api.NotificationsEnqueuer.Enqueue(ctx, targetUser.ID, personalTemplateID, + // nolint:gocritic // Need notifier actor to enqueue notifications + if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), targetUser.ID, personalTemplateID, labels, "api-put-user-status", targetUser.ID, ); err != nil { @@ -1420,7 +1423,8 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create } for _, u := range userAdmins { - if _, err := api.NotificationsEnqueuer.Enqueue(ctx, u.ID, notifications.TemplateUserAccountCreated, + // nolint:gocritic // Need notifier actor to enqueue notifications + if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), u.ID, notifications.TemplateUserAccountCreated, map[string]string{ "created_account_name": user.Username, "created_account_user_name": user.Name, diff --git a/coderd/users_test.go b/coderd/users_test.go index 375e4b3168066..c9038c7418034 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -11,6 +11,7 @@ import ( "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest/oidctest" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/serpent" @@ -383,13 +384,13 @@ func TestNotifyUserStatusChanged(t *testing.T) { UserID uuid.UUID } - verifyNotificationDispatched := func(notifyEnq *testutil.FakeNotificationsEnqueuer, expectedNotifications []expectedNotification, member codersdk.User, label string) { - require.Equal(t, len(expectedNotifications), len(notifyEnq.Sent)) + verifyNotificationDispatched := func(notifyEnq *notificationstest.FakeEnqueuer, expectedNotifications []expectedNotification, member codersdk.User, label string) { + require.Equal(t, len(expectedNotifications), len(notifyEnq.Sent())) - // Validate that each expected notification is present in notifyEnq.Sent + // Validate that each expected notification is present in notifyEnq.Sent() for _, expected := range expectedNotifications { found := false - for _, sent := range notifyEnq.Sent { + for _, sent := range notifyEnq.Sent() { if sent.TemplateID == expected.TemplateID && sent.UserID == expected.UserID && slices.Contains(sent.Targets, member.ID) && @@ -405,7 +406,7 @@ func TestNotifyUserStatusChanged(t *testing.T) { t.Run("Account suspended", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} adminClient := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, }) @@ -442,7 +443,7 @@ func TestNotifyUserStatusChanged(t *testing.T) { t.Parallel() // given - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} adminClient := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, }) @@ -486,7 +487,7 @@ func TestNotifyDeletedUser(t *testing.T) { t.Parallel() // given - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} adminClient := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, }) @@ -511,21 +512,21 @@ func TestNotifyDeletedUser(t *testing.T) { require.NoError(t, err) // then - require.Len(t, notifyEnq.Sent, 2) - // notifyEnq.Sent[0] is create account event - require.Equal(t, notifications.TemplateUserAccountDeleted, notifyEnq.Sent[1].TemplateID) - require.Equal(t, firstUser.ID, notifyEnq.Sent[1].UserID) - require.Contains(t, notifyEnq.Sent[1].Targets, user.ID) - require.Equal(t, user.Username, notifyEnq.Sent[1].Labels["deleted_account_name"]) - require.Equal(t, user.Name, notifyEnq.Sent[1].Labels["deleted_account_user_name"]) - require.Equal(t, firstUser.Name, notifyEnq.Sent[1].Labels["initiator"]) + require.Len(t, notifyEnq.Sent(), 2) + // notifyEnq.Sent()[0] is create account event + require.Equal(t, notifications.TemplateUserAccountDeleted, notifyEnq.Sent()[1].TemplateID) + require.Equal(t, firstUser.ID, notifyEnq.Sent()[1].UserID) + require.Contains(t, notifyEnq.Sent()[1].Targets, user.ID) + require.Equal(t, user.Username, notifyEnq.Sent()[1].Labels["deleted_account_name"]) + require.Equal(t, user.Name, notifyEnq.Sent()[1].Labels["deleted_account_user_name"]) + require.Equal(t, firstUser.Name, notifyEnq.Sent()[1].Labels["initiator"]) }) t.Run("UserAdminNotified", func(t *testing.T) { t.Parallel() // given - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} adminClient := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, }) @@ -549,22 +550,23 @@ func TestNotifyDeletedUser(t *testing.T) { require.NoError(t, err) // then - require.Len(t, notifyEnq.Sent, 5) - // notifyEnq.Sent[0]: "User admin" account created, "owner" notified - // notifyEnq.Sent[1]: "Member" account created, "owner" notified - // notifyEnq.Sent[2]: "Member" account created, "user admin" notified + sent := notifyEnq.Sent() + require.Len(t, sent, 5) + // sent[0]: "User admin" account created, "owner" notified + // sent[1]: "Member" account created, "owner" notified + // sent[2]: "Member" account created, "user admin" notified // "Member" account deleted, "owner" notified - require.Equal(t, notifications.TemplateUserAccountDeleted, notifyEnq.Sent[3].TemplateID) - require.Equal(t, firstUser.UserID, notifyEnq.Sent[3].UserID) - require.Contains(t, notifyEnq.Sent[3].Targets, member.ID) - require.Equal(t, member.Username, notifyEnq.Sent[3].Labels["deleted_account_name"]) + require.Equal(t, notifications.TemplateUserAccountDeleted, sent[3].TemplateID) + require.Equal(t, firstUser.UserID, sent[3].UserID) + require.Contains(t, sent[3].Targets, member.ID) + require.Equal(t, member.Username, sent[3].Labels["deleted_account_name"]) // "Member" account deleted, "user admin" notified - require.Equal(t, notifications.TemplateUserAccountDeleted, notifyEnq.Sent[4].TemplateID) - require.Equal(t, userAdmin.ID, notifyEnq.Sent[4].UserID) - require.Contains(t, notifyEnq.Sent[4].Targets, member.ID) - require.Equal(t, member.Username, notifyEnq.Sent[4].Labels["deleted_account_name"]) + require.Equal(t, notifications.TemplateUserAccountDeleted, sent[4].TemplateID) + require.Equal(t, userAdmin.ID, sent[4].UserID) + require.Contains(t, sent[4].Targets, member.ID) + require.Equal(t, member.Username, sent[4].Labels["deleted_account_name"]) }) } @@ -835,7 +837,7 @@ func TestNotifyCreatedUser(t *testing.T) { t.Parallel() // given - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} adminClient := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, }) @@ -854,18 +856,18 @@ func TestNotifyCreatedUser(t *testing.T) { require.NoError(t, err) // then - require.Len(t, notifyEnq.Sent, 1) - require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent[0].TemplateID) - require.Equal(t, firstUser.UserID, notifyEnq.Sent[0].UserID) - require.Contains(t, notifyEnq.Sent[0].Targets, user.ID) - require.Equal(t, user.Username, notifyEnq.Sent[0].Labels["created_account_name"]) + require.Len(t, notifyEnq.Sent(), 1) + require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent()[0].TemplateID) + require.Equal(t, firstUser.UserID, notifyEnq.Sent()[0].UserID) + require.Contains(t, notifyEnq.Sent()[0].Targets, user.ID) + require.Equal(t, user.Username, notifyEnq.Sent()[0].Labels["created_account_name"]) }) t.Run("UserAdminNotified", func(t *testing.T) { t.Parallel() // given - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} adminClient := coderdtest.New(t, &coderdtest.Options{ NotificationsEnqueuer: notifyEnq, }) @@ -899,25 +901,26 @@ func TestNotifyCreatedUser(t *testing.T) { require.NoError(t, err) // then - require.Len(t, notifyEnq.Sent, 3) + sent := notifyEnq.Sent() + require.Len(t, sent, 3) // "User admin" account created, "owner" notified - require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent[0].TemplateID) - require.Equal(t, firstUser.UserID, notifyEnq.Sent[0].UserID) - require.Contains(t, notifyEnq.Sent[0].Targets, userAdmin.ID) - require.Equal(t, userAdmin.Username, notifyEnq.Sent[0].Labels["created_account_name"]) + require.Equal(t, notifications.TemplateUserAccountCreated, sent[0].TemplateID) + require.Equal(t, firstUser.UserID, sent[0].UserID) + require.Contains(t, sent[0].Targets, userAdmin.ID) + require.Equal(t, userAdmin.Username, sent[0].Labels["created_account_name"]) // "Member" account created, "owner" notified - require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent[1].TemplateID) - require.Equal(t, firstUser.UserID, notifyEnq.Sent[1].UserID) - require.Contains(t, notifyEnq.Sent[1].Targets, member.ID) - require.Equal(t, member.Username, notifyEnq.Sent[1].Labels["created_account_name"]) + require.Equal(t, notifications.TemplateUserAccountCreated, sent[1].TemplateID) + require.Equal(t, firstUser.UserID, sent[1].UserID) + require.Contains(t, sent[1].Targets, member.ID) + require.Equal(t, member.Username, sent[1].Labels["created_account_name"]) // "Member" account created, "user admin" notified - require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent[1].TemplateID) - require.Equal(t, userAdmin.ID, notifyEnq.Sent[2].UserID) - require.Contains(t, notifyEnq.Sent[2].Targets, member.ID) - require.Equal(t, member.Username, notifyEnq.Sent[2].Labels["created_account_name"]) + require.Equal(t, notifications.TemplateUserAccountCreated, sent[1].TemplateID) + require.Equal(t, userAdmin.ID, sent[2].UserID) + require.Contains(t, sent[2].Targets, member.ID) + require.Equal(t, member.Username, sent[2].Labels["created_account_name"]) }) } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 4638596e66eae..ff8a55ded775a 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1056,7 +1056,8 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { if initiatorErr == nil && tmplErr == nil { dormantTime := dbtime.Now().Add(time.Duration(tmpl.TimeTilDormant)) _, err = api.NotificationsEnqueuer.Enqueue( - ctx, + // nolint:gocritic // Need notifier actor to enqueue notifications + dbauthz.AsNotifier(ctx), newWorkspace.OwnerID, notifications.TemplateWorkspaceDormant, map[string]string{ diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 0a4e10670132c..ac218bfb97f4b 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -31,6 +31,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/render" @@ -3485,7 +3486,7 @@ func TestWorkspaceNotifications(t *testing.T) { // Given var ( - notifyEnq = &testutil.FakeNotificationsEnqueuer{} + notifyEnq = ¬ificationstest.FakeEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, NotificationsEnqueuer: notifyEnq, @@ -3509,14 +3510,15 @@ func TestWorkspaceNotifications(t *testing.T) { // Then require.NoError(t, err, "mark workspace as dormant") - require.Len(t, notifyEnq.Sent, 2) + sent := notifyEnq.Sent() + require.Len(t, sent, 2) // notifyEnq.Sent[0] is an event for created user account - require.Equal(t, notifyEnq.Sent[1].TemplateID, notifications.TemplateWorkspaceDormant) - require.Equal(t, notifyEnq.Sent[1].UserID, workspace.OwnerID) - require.Contains(t, notifyEnq.Sent[1].Targets, template.ID) - require.Contains(t, notifyEnq.Sent[1].Targets, workspace.ID) - require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OrganizationID) - require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OwnerID) + require.Equal(t, sent[1].TemplateID, notifications.TemplateWorkspaceDormant) + require.Equal(t, sent[1].UserID, workspace.OwnerID) + require.Contains(t, sent[1].Targets, template.ID) + require.Contains(t, sent[1].Targets, workspace.ID) + require.Contains(t, sent[1].Targets, workspace.OrganizationID) + require.Contains(t, sent[1].Targets, workspace.OwnerID) }) t.Run("InitiatorIsOwner", func(t *testing.T) { @@ -3524,7 +3526,7 @@ func TestWorkspaceNotifications(t *testing.T) { // Given var ( - notifyEnq = &testutil.FakeNotificationsEnqueuer{} + notifyEnq = ¬ificationstest.FakeEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, NotificationsEnqueuer: notifyEnq, @@ -3547,7 +3549,7 @@ func TestWorkspaceNotifications(t *testing.T) { // Then require.NoError(t, err, "mark workspace as dormant") - require.Len(t, notifyEnq.Sent, 0) + require.Len(t, notifyEnq.Sent(), 0) }) t.Run("ActivateDormantWorkspace", func(t *testing.T) { @@ -3555,7 +3557,7 @@ func TestWorkspaceNotifications(t *testing.T) { // Given var ( - notifyEnq = &testutil.FakeNotificationsEnqueuer{} + notifyEnq = ¬ificationstest.FakeEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, NotificationsEnqueuer: notifyEnq, @@ -3585,7 +3587,7 @@ func TestWorkspaceNotifications(t *testing.T) { Dormant: false, }) require.NoError(t, err, "mark workspace as active") - require.Len(t, notifyEnq.Sent, 0) + require.Len(t, notifyEnq.Sent(), 0) }) }) } diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 626e296d6a3e8..a3f36e08dd218 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -208,7 +208,8 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S for _, ws := range markedForDeletion { dormantTime := dbtime.Now().Add(opts.TimeTilDormantAutoDelete) _, err = s.enqueuer.Enqueue( - ctx, + // nolint:gocritic // Need actor to enqueue notification + dbauthz.AsNotifier(ctx), ws.OwnerID, notifications.TemplateWorkspaceMarkedForDeletion, map[string]string{ diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index c85c2c6ea1b0e..37c99de610068 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -16,9 +16,11 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" agplschedule "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/cryptorand" @@ -673,7 +675,7 @@ func TestNotifications(t *testing.T) { } // Setup dependencies - notifyEnq := testutil.FakeNotificationsEnqueuer{} + notifyEnq := notificationstest.FakeEnqueuer{} logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) const userQuietHoursSchedule = "CRON_TZ=UTC 0 0 * * *" // midnight UTC userQuietHoursStore, err := schedule.NewEnterpriseUserQuietHoursScheduleStore(userQuietHoursSchedule, true) @@ -685,21 +687,23 @@ func TestNotifications(t *testing.T) { // Lower the dormancy TTL to ensure the schedule recalculates deadlines and // triggers notifications. - _, err = templateScheduleStore.Set(ctx, db, template, agplschedule.TemplateScheduleOptions{ + // nolint:gocritic // Need an actor in the context. + _, err = templateScheduleStore.Set(dbauthz.AsNotifier(ctx), db, template, agplschedule.TemplateScheduleOptions{ TimeTilDormant: timeTilDormant / 2, TimeTilDormantAutoDelete: timeTilDormant / 2, }) require.NoError(t, err) // We should expect a notification for each dormant workspace. - require.Len(t, notifyEnq.Sent, len(dormantWorkspaces)) + sent := notifyEnq.Sent() + require.Len(t, sent, len(dormantWorkspaces)) for i, dormantWs := range dormantWorkspaces { - require.Equal(t, notifyEnq.Sent[i].UserID, dormantWs.OwnerID) - require.Equal(t, notifyEnq.Sent[i].TemplateID, notifications.TemplateWorkspaceMarkedForDeletion) - require.Contains(t, notifyEnq.Sent[i].Targets, template.ID) - require.Contains(t, notifyEnq.Sent[i].Targets, dormantWs.ID) - require.Contains(t, notifyEnq.Sent[i].Targets, dormantWs.OrganizationID) - require.Contains(t, notifyEnq.Sent[i].Targets, dormantWs.OwnerID) + require.Equal(t, sent[i].UserID, dormantWs.OwnerID) + require.Equal(t, sent[i].TemplateID, notifications.TemplateWorkspaceMarkedForDeletion) + require.Contains(t, sent[i].Targets, template.ID) + require.Contains(t, sent[i].Targets, dormantWs.ID) + require.Contains(t, sent[i].Targets, dormantWs.OrganizationID) + require.Contains(t, sent[i].Targets, dormantWs.OwnerID) } }) } diff --git a/enterprise/coderd/scim_test.go b/enterprise/coderd/scim_test.go index 82355c3a3b9c0..3e5c22f7e9461 100644 --- a/enterprise/coderd/scim_test.go +++ b/enterprise/coderd/scim_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/coderdtest/oidctest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" "github.com/coder/coder/v2/enterprise/coderd" @@ -122,7 +123,7 @@ func TestScim(t *testing.T) { // given scimAPIKey := []byte("hi") mockAudit := audit.NewMock() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client, _ := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ Auditor: mockAudit, @@ -172,7 +173,7 @@ func TestScim(t *testing.T) { assert.Len(t, userRes.Users[0].OrganizationIDs, 1) // Expect zero notifications (SkipNotifications = true) - require.Empty(t, notifyEnq.Sent) + require.Empty(t, notifyEnq.Sent()) }) t.Run("OK_Bearer", func(t *testing.T) { @@ -184,7 +185,7 @@ func TestScim(t *testing.T) { // given scimAPIKey := []byte("hi") mockAudit := audit.NewMock() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} client, _ := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ Auditor: mockAudit, @@ -228,7 +229,7 @@ func TestScim(t *testing.T) { assert.Len(t, userRes.Users[0].OrganizationIDs, 1) // Expect zero notifications (SkipNotifications = true) - require.Empty(t, notifyEnq.Sent) + require.Empty(t, notifyEnq.Sent()) }) t.Run("OKNoDefault", func(t *testing.T) { @@ -240,7 +241,7 @@ func TestScim(t *testing.T) { // given scimAPIKey := []byte("hi") mockAudit := audit.NewMock() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} dv := coderdtest.DeploymentValues(t) dv.OIDC.OrganizationAssignDefault = false client, _ := coderdenttest.New(t, &coderdenttest.Options{ @@ -287,7 +288,7 @@ func TestScim(t *testing.T) { assert.Len(t, userRes.Users[0].OrganizationIDs, 0) // Expect zero notifications (SkipNotifications = true) - require.Empty(t, notifyEnq.Sent) + require.Empty(t, notifyEnq.Sent()) }) t.Run("Duplicate", func(t *testing.T) { diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 4321a5ed83fbd..2fc6bf9fda087 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -19,6 +19,7 @@ import ( "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/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" @@ -39,7 +40,7 @@ func TestTemplates(t *testing.T) { t.Run("Deprecated", func(t *testing.T) { t.Parallel() - notifyEnq := &testutil.FakeNotificationsEnqueuer{} + notifyEnq := ¬ificationstest.FakeEnqueuer{} owner, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, @@ -81,8 +82,8 @@ func TestTemplates(t *testing.T) { assert.True(t, updated.Deprecated) assert.NotEmpty(t, updated.DeprecationMessage) - notifs := []*testutil.Notification{} - for _, notif := range notifyEnq.Sent { + notifs := []*notificationstest.FakeNotification{} + for _, notif := range notifyEnq.Sent() { if notif.TemplateID == notifications.TemplateTemplateDeprecated { notifs = append(notifs, notif) } diff --git a/testutil/notifications.go b/testutil/notifications.go deleted file mode 100644 index 379218cd379e8..0000000000000 --- a/testutil/notifications.go +++ /dev/null @@ -1,49 +0,0 @@ -package testutil - -import ( - "context" - "sync" - - "github.com/google/uuid" -) - -type FakeNotificationsEnqueuer struct { - mu sync.Mutex - Sent []*Notification -} - -type Notification struct { - UserID, TemplateID uuid.UUID - Labels map[string]string - Data map[string]any - CreatedBy string - Targets []uuid.UUID -} - -func (f *FakeNotificationsEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { - return f.EnqueueWithData(ctx, userID, templateID, labels, nil, createdBy, targets...) -} - -func (f *FakeNotificationsEnqueuer) EnqueueWithData(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { - f.mu.Lock() - defer f.mu.Unlock() - - f.Sent = append(f.Sent, &Notification{ - UserID: userID, - TemplateID: templateID, - Labels: labels, - Data: data, - CreatedBy: createdBy, - Targets: targets, - }) - - id := uuid.New() - return &id, nil -} - -func (f *FakeNotificationsEnqueuer) Clear() { - f.mu.Lock() - defer f.mu.Unlock() - - f.Sent = nil -}