fix: send workspace create/update notifications to template admins only (#16071)

Relates to https://github.com/coder/coder/issues/15845

Rather than sending the notification to the user, we send it to the
template admins. We also do not send it to the person that created the
request.
This commit is contained in:
Danielle Maywood
2025-01-15 17:43:11 +00:00
committed by GitHub
parent cd62e3934a
commit 3e3de0588a
4 changed files with 103 additions and 33 deletions

View File

@ -446,7 +446,20 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
// If this workspace build has a different template version ID to the previous build // If this workspace build has a different template version ID to the previous build
// we can assume it has just been updated. // we can assume it has just been updated.
if createBuild.TemplateVersionID != uuid.Nil && createBuild.TemplateVersionID != previousWorkspaceBuild.TemplateVersionID { if createBuild.TemplateVersionID != uuid.Nil && createBuild.TemplateVersionID != previousWorkspaceBuild.TemplateVersionID {
api.notifyWorkspaceUpdated(ctx, apiKey.UserID, workspace, createBuild.RichParameterValues) // nolint:gocritic // Need system context to fetch admins
admins, err := findTemplateAdmins(dbauthz.AsSystemRestricted(ctx), api.Database)
if err != nil {
api.Logger.Error(ctx, "find template admins", slog.Error(err))
} else {
for _, admin := range admins {
// Don't send notifications to user which initiated the event.
if admin.ID == apiKey.UserID {
continue
}
api.notifyWorkspaceUpdated(ctx, apiKey.UserID, admin.ID, workspace, createBuild.RichParameterValues)
}
}
} }
api.publishWorkspaceUpdate(ctx, workspace.OwnerID, wspubsub.WorkspaceEvent{ api.publishWorkspaceUpdate(ctx, workspace.OwnerID, wspubsub.WorkspaceEvent{
@ -460,6 +473,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
func (api *API) notifyWorkspaceUpdated( func (api *API) notifyWorkspaceUpdated(
ctx context.Context, ctx context.Context,
initiatorID uuid.UUID, initiatorID uuid.UUID,
receiverID uuid.UUID,
workspace database.Workspace, workspace database.Workspace,
parameters []codersdk.WorkspaceBuildParameter, parameters []codersdk.WorkspaceBuildParameter,
) { ) {
@ -500,7 +514,7 @@ func (api *API) notifyWorkspaceUpdated(
if _, err := api.NotificationsEnqueuer.EnqueueWithData( if _, err := api.NotificationsEnqueuer.EnqueueWithData(
// nolint:gocritic // Need notifier actor to enqueue notifications // nolint:gocritic // Need notifier actor to enqueue notifications
dbauthz.AsNotifier(ctx), dbauthz.AsNotifier(ctx),
workspace.OwnerID, receiverID,
notifications.TemplateWorkspaceManuallyUpdated, notifications.TemplateWorkspaceManuallyUpdated,
map[string]string{ map[string]string{
"organization": template.OrganizationName, "organization": template.OrganizationName,

View File

@ -565,19 +565,20 @@ func TestWorkspaceBuildResources(t *testing.T) {
func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T) { func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T) {
t.Parallel() t.Parallel()
t.Run("OnlyOneNotification", func(t *testing.T) { t.Run("NoRepeatedNotifications", func(t *testing.T) {
t.Parallel() t.Parallel()
notify := &notificationstest.FakeEnqueuer{} notify := &notificationstest.FakeEnqueuer{}
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: notify}) client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: notify})
first := coderdtest.CreateFirstUser(t, client) first := coderdtest.CreateFirstUser(t, client)
templateAdminClient, templateAdmin := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleTemplateAdmin())
userClient, user := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) userClient, user := coderdtest.CreateAnotherUser(t, client, first.OrganizationID)
// Create a template with an initial version // Create a template with an initial version
version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil) version := coderdtest.CreateTemplateVersion(t, templateAdminClient, first.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdminClient, version.ID)
template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) template := coderdtest.CreateTemplate(t, templateAdminClient, first.OrganizationID, version.ID)
// Create a workspace using this template // Create a workspace using this template
workspace := coderdtest.CreateWorkspace(t, userClient, template.ID) workspace := coderdtest.CreateWorkspace(t, userClient, template.ID)
@ -585,10 +586,10 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
// Create a new version of the template // Create a new version of the template
newVersion := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) { newVersion := coderdtest.CreateTemplateVersion(t, templateAdminClient, first.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) {
ctvr.TemplateID = template.ID ctvr.TemplateID = template.ID
}) })
coderdtest.AwaitTemplateVersionJobCompleted(t, client, newVersion.ID) coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdminClient, newVersion.ID)
// Create a workspace build using this new template version // Create a workspace build using this new template version
build := coderdtest.CreateWorkspaceBuild(t, userClient, workspace, database.WorkspaceTransitionStart, func(cwbr *codersdk.CreateWorkspaceBuildRequest) { build := coderdtest.CreateWorkspaceBuild(t, userClient, workspace, database.WorkspaceTransitionStart, func(cwbr *codersdk.CreateWorkspaceBuildRequest) {
@ -597,21 +598,45 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, build.ID) coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, build.ID)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
// Create the workspace build _again_. We are doing this to ensure we only create 1 notification. // Create the workspace build _again_. We are doing this to
// ensure we do not create _another_ notification. This is
// separate to the notifications subsystem dedupe mechanism
// as this build shouldn't create a notification. It shouldn't
// create another notification as this new build isn't changing
// the template version.
build = coderdtest.CreateWorkspaceBuild(t, userClient, workspace, database.WorkspaceTransitionStart, func(cwbr *codersdk.CreateWorkspaceBuildRequest) { build = coderdtest.CreateWorkspaceBuild(t, userClient, workspace, database.WorkspaceTransitionStart, func(cwbr *codersdk.CreateWorkspaceBuildRequest) {
cwbr.TemplateVersionID = newVersion.ID cwbr.TemplateVersionID = newVersion.ID
}) })
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, build.ID) coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, build.ID)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
// Ensure we receive only 1 workspace manually updated notification // We're going to have two notifications (one for the first user and one for the template admin)
// By ensuring we only have these two, we are sure the second build didn't trigger more
// notifications.
sent := notify.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceManuallyUpdated)) sent := notify.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceManuallyUpdated))
require.Len(t, sent, 1) require.Len(t, sent, 2)
require.Equal(t, user.ID, sent[0].UserID)
receivers := make([]uuid.UUID, len(sent))
for idx, notif := range sent {
receivers[idx] = notif.UserID
}
// Check the notification was sent to the first user and template admin
// (both of whom have the "template admin" role), and explicitly not the
// workspace owner (since they initiated the workspace build).
require.Contains(t, receivers, templateAdmin.ID)
require.Contains(t, receivers, first.UserID)
require.NotContains(t, receivers, user.ID)
require.Contains(t, sent[0].Targets, template.ID) require.Contains(t, sent[0].Targets, template.ID)
require.Contains(t, sent[0].Targets, workspace.ID) require.Contains(t, sent[0].Targets, workspace.ID)
require.Contains(t, sent[0].Targets, workspace.OrganizationID) require.Contains(t, sent[0].Targets, workspace.OrganizationID)
require.Contains(t, sent[0].Targets, workspace.OwnerID) require.Contains(t, sent[0].Targets, 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("ToCorrectUser", func(t *testing.T) { t.Run("ToCorrectUser", func(t *testing.T) {
@ -621,12 +646,13 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: notify}) client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: notify})
first := coderdtest.CreateFirstUser(t, client) first := coderdtest.CreateFirstUser(t, client)
userClient, user := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) templateAdminClient, templateAdmin := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleTemplateAdmin())
userClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID)
// Create a template with an initial version // Create a template with an initial version
version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil) version := coderdtest.CreateTemplateVersion(t, templateAdminClient, first.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdminClient, version.ID)
template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) template := coderdtest.CreateTemplate(t, templateAdminClient, first.OrganizationID, version.ID)
// Create a workspace using this template // Create a workspace using this template
workspace := coderdtest.CreateWorkspace(t, userClient, template.ID) workspace := coderdtest.CreateWorkspace(t, userClient, template.ID)
@ -634,10 +660,10 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
// Create a new version of the template // Create a new version of the template
newVersion := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) { newVersion := coderdtest.CreateTemplateVersion(t, templateAdminClient, first.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) {
ctvr.TemplateID = template.ID ctvr.TemplateID = template.ID
}) })
coderdtest.AwaitTemplateVersionJobCompleted(t, client, newVersion.ID) coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdminClient, newVersion.ID)
// Create a workspace build using this new template version from a different user // Create a workspace build using this new template version from a different user
ctx := testutil.Context(t, testutil.WaitShort) ctx := testutil.Context(t, testutil.WaitShort)
@ -652,7 +678,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
// Ensure we receive only 1 workspace manually updated notification and to the right user // Ensure we receive only 1 workspace manually updated notification and to the right user
sent := notify.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceManuallyUpdated)) sent := notify.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceManuallyUpdated))
require.Len(t, sent, 1) require.Len(t, sent, 1)
require.Equal(t, user.ID, sent[0].UserID) require.Equal(t, templateAdmin.ID, sent[0].UserID)
require.Contains(t, sent[0].Targets, template.ID) require.Contains(t, sent[0].Targets, template.ID)
require.Contains(t, sent[0].Targets, workspace.ID) require.Contains(t, sent[0].Targets, workspace.ID)
require.Contains(t, sent[0].Targets, workspace.OrganizationID) require.Contains(t, sent[0].Targets, workspace.OrganizationID)

View File

@ -665,9 +665,6 @@ func createWorkspace(
) )
return err return err
}, nil) }, nil)
api.notifyWorkspaceCreated(ctx, workspace, req.RichParameterValues)
var bldErr wsbuilder.BuildError var bldErr wsbuilder.BuildError
if xerrors.As(err, &bldErr) { if xerrors.As(err, &bldErr) {
httpapi.Write(ctx, rw, bldErr.Status, codersdk.Response{ httpapi.Write(ctx, rw, bldErr.Status, codersdk.Response{
@ -689,6 +686,21 @@ func createWorkspace(
api.Logger.Error(ctx, "failed to post provisioner job to pubsub", slog.Error(err)) api.Logger.Error(ctx, "failed to post provisioner job to pubsub", slog.Error(err))
} }
// nolint:gocritic // Need system context to fetch admins
admins, err := findTemplateAdmins(dbauthz.AsSystemRestricted(ctx), api.Database)
if err != nil {
api.Logger.Error(ctx, "find template admins", slog.Error(err))
} else {
for _, admin := range admins {
// Don't send notifications to user which initiated the event.
if admin.ID == initiatorID {
continue
}
api.notifyWorkspaceCreated(ctx, admin.ID, workspace, req.RichParameterValues)
}
}
auditReq.New = workspace.WorkspaceTable() auditReq.New = workspace.WorkspaceTable()
api.Telemetry.Report(&telemetry.Snapshot{ api.Telemetry.Report(&telemetry.Snapshot{
@ -739,6 +751,7 @@ func createWorkspace(
func (api *API) notifyWorkspaceCreated( func (api *API) notifyWorkspaceCreated(
ctx context.Context, ctx context.Context,
receiverID uuid.UUID,
workspace database.Workspace, workspace database.Workspace,
parameters []codersdk.WorkspaceBuildParameter, parameters []codersdk.WorkspaceBuildParameter,
) { ) {
@ -773,7 +786,7 @@ func (api *API) notifyWorkspaceCreated(
if _, err := api.NotificationsEnqueuer.EnqueueWithData( if _, err := api.NotificationsEnqueuer.EnqueueWithData(
// nolint:gocritic // Need notifier actor to enqueue notifications // nolint:gocritic // Need notifier actor to enqueue notifications
dbauthz.AsNotifier(ctx), dbauthz.AsNotifier(ctx),
workspace.OwnerID, receiverID,
notifications.TemplateWorkspaceCreated, notifications.TemplateWorkspaceCreated,
map[string]string{ map[string]string{
"workspace": workspace.Name, "workspace": workspace.Name,

View File

@ -577,22 +577,38 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
enqueuer := notificationstest.FakeEnqueuer{} enqueuer := notificationstest.FakeEnqueuer{}
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: &enqueuer}) client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: &enqueuer})
user := coderdtest.CreateFirstUser(t, client) user := coderdtest.CreateFirstUser(t, client)
templateAdminClient, templateAdmin := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleTemplateAdmin())
memberClient, memberUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) memberClient, memberUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) version := coderdtest.CreateTemplateVersion(t, templateAdminClient, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) template := coderdtest.CreateTemplate(t, templateAdminClient, user.OrganizationID, version.ID)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdminClient, version.ID)
workspace := coderdtest.CreateWorkspace(t, memberClient, template.ID) workspace := coderdtest.CreateWorkspace(t, memberClient, template.ID)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, memberClient, workspace.LatestBuild.ID) coderdtest.AwaitWorkspaceBuildJobCompleted(t, memberClient, workspace.LatestBuild.ID)
sent := enqueuer.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceCreated)) sent := enqueuer.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceCreated))
require.Len(t, sent, 1) require.Len(t, sent, 2)
require.Equal(t, memberUser.ID, sent[0].UserID)
receivers := make([]uuid.UUID, len(sent))
for idx, notif := range sent {
receivers[idx] = notif.UserID
}
// Check the notification was sent to the first user and template admin
require.Contains(t, receivers, templateAdmin.ID)
require.Contains(t, receivers, user.UserID)
require.NotContains(t, receivers, memberUser.ID)
require.Contains(t, sent[0].Targets, template.ID) require.Contains(t, sent[0].Targets, template.ID)
require.Contains(t, sent[0].Targets, workspace.ID) require.Contains(t, sent[0].Targets, workspace.ID)
require.Contains(t, sent[0].Targets, workspace.OrganizationID) require.Contains(t, sent[0].Targets, workspace.OrganizationID)
require.Contains(t, sent[0].Targets, workspace.OwnerID) require.Contains(t, sent[0].Targets, 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("CreateSendsNotificationToCorrectUser", func(t *testing.T) { t.Run("CreateSendsNotificationToCorrectUser", func(t *testing.T) {
@ -601,14 +617,15 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
enqueuer := notificationstest.FakeEnqueuer{} enqueuer := notificationstest.FakeEnqueuer{}
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: &enqueuer}) client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: &enqueuer})
user := coderdtest.CreateFirstUser(t, client) user := coderdtest.CreateFirstUser(t, client)
templateAdminClient, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleTemplateAdmin(), rbac.RoleOwner())
_, memberUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) _, memberUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) version := coderdtest.CreateTemplateVersion(t, templateAdminClient, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) template := coderdtest.CreateTemplate(t, templateAdminClient, user.OrganizationID, version.ID)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdminClient, version.ID)
ctx := testutil.Context(t, testutil.WaitShort) ctx := testutil.Context(t, testutil.WaitShort)
workspace, err := client.CreateUserWorkspace(ctx, memberUser.Username, codersdk.CreateWorkspaceRequest{ workspace, err := templateAdminClient.CreateUserWorkspace(ctx, memberUser.Username, codersdk.CreateWorkspaceRequest{
TemplateID: template.ID, TemplateID: template.ID,
Name: coderdtest.RandomUsername(t), Name: coderdtest.RandomUsername(t),
}) })
@ -617,7 +634,7 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
sent := enqueuer.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceCreated)) sent := enqueuer.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceCreated))
require.Len(t, sent, 1) require.Len(t, sent, 1)
require.Equal(t, memberUser.ID, sent[0].UserID) require.Equal(t, user.UserID, sent[0].UserID)
require.Contains(t, sent[0].Targets, template.ID) require.Contains(t, sent[0].Targets, template.ID)
require.Contains(t, sent[0].Targets, workspace.ID) require.Contains(t, sent[0].Targets, workspace.ID)
require.Contains(t, sent[0].Targets, workspace.OrganizationID) require.Contains(t, sent[0].Targets, workspace.OrganizationID)