Skip to content

Commit cf1fcab

Browse files
mtojekmtojek
authored
feat: notify about created user account (#14010)
1 parent c6fb779 commit cf1fcab

File tree

9 files changed

+177
-18
lines changed

9 files changed

+177
-18
lines changed

‎coderd/autobuild/lifecycle_executor_test.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -1115,13 +1115,14 @@ func TestNotifications(t *testing.T) {
11151115
require.NotNil(t, workspace.DormantAt)
11161116

11171117
// Check that a notification was enqueued
1118-
require.Len(t, notifyEnq.Sent, 1)
1119-
require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID)
1120-
require.Equal(t, notifyEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceDormant)
1121-
require.Contains(t, notifyEnq.Sent[0].Targets, template.ID)
1122-
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID)
1123-
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID)
1124-
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID)
1118+
require.Len(t, notifyEnq.Sent, 2)
1119+
// notifyEnq.Sent[0] is an event for created user account
1120+
require.Equal(t, notifyEnq.Sent[1].UserID, workspace.OwnerID)
1121+
require.Equal(t, notifyEnq.Sent[1].TemplateID, notifications.TemplateWorkspaceDormant)
1122+
require.Contains(t, notifyEnq.Sent[1].Targets, template.ID)
1123+
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.ID)
1124+
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OrganizationID)
1125+
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OwnerID)
11251126
})
11261127
}
11271128

‎coderd/database/migrations/000233_notifications_user_created.down.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
DELETE FROM notification_templates WHERE id = '4e19c0ac-94e1-4532-9515-d1801aa283b2';

‎coderd/database/migrations/000233_notifications_user_created.up.sql

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions)
2+
VALUES ('4e19c0ac-94e1-4532-9515-d1801aa283b2', 'User account created', E'User account "{{.Labels.created_account_name}}" created',
3+
E'Hi {{.UserName}},\n\New user account **{{.Labels.created_account_name}}** has been created.',
4+
'Workspace Events', '[
5+
{
6+
"label": "View accounts",
7+
"url": "{{ base_url }}/deployment/users?filter=status%3Aactive"
8+
}
9+
]'::jsonb);

‎coderd/notifications/events.go

+5
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,8 @@ var (
1313
TemplateWorkspaceAutoUpdated = uuid.MustParse("c34a0c09-0704-4cac-bd1c-0c0146811c2b")
1414
TemplateWorkspaceMarkedForDeletion = uuid.MustParse("51ce2fdf-c9ca-4be1-8d70-628674f9bc42")
1515
)
16+
17+
// Account-related events.
18+
var (
19+
TemplateUserAccountCreated = uuid.MustParse("4e19c0ac-94e1-4532-9515-d1801aa283b2")
20+
)

‎coderd/users.go

+37-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"github.com/google/uuid"
1313
"golang.org/x/xerrors"
1414

15+
"cdr.dev/slog"
16+
1517
"github.com/coder/coder/v2/coderd/audit"
1618
"github.com/coder/coder/v2/coderd/database"
1719
"github.com/coder/coder/v2/coderd/database/db2sdk"
@@ -20,6 +22,7 @@ import (
2022
"github.com/coder/coder/v2/coderd/gitsshkey"
2123
"github.com/coder/coder/v2/coderd/httpapi"
2224
"github.com/coder/coder/v2/coderd/httpmw"
25+
"github.com/coder/coder/v2/coderd/notifications"
2326
"github.com/coder/coder/v2/coderd/rbac"
2427
"github.com/coder/coder/v2/coderd/rbac/policy"
2528
"github.com/coder/coder/v2/coderd/searchquery"
@@ -1200,7 +1203,8 @@ func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques
12001203

12011204
type CreateUserRequest struct {
12021205
codersdk.CreateUserRequest
1203-
LoginType database.LoginType
1206+
LoginType database.LoginType
1207+
SkipNotifications bool
12041208
}
12051209

12061210
func (api *API) CreateUser(ctx context.Context, store database.Store, req CreateUserRequest) (database.User, uuid.UUID, error) {
@@ -1211,7 +1215,7 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
12111215
}
12121216

12131217
var user database.User
1214-
return user, req.OrganizationID, store.InTx(func(tx database.Store) error {
1218+
err := store.InTx(func(tx database.Store) error {
12151219
orgRoles := make([]string, 0)
12161220
// Organization is required to know where to allocate the user.
12171221
if req.OrganizationID == uuid.Nil {
@@ -1272,6 +1276,37 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
12721276
}
12731277
return nil
12741278
}, nil)
1279+
if err != nil || req.SkipNotifications {
1280+
return user, req.OrganizationID, err
1281+
}
1282+
1283+
// Notify all users with user admin permission including owners
1284+
// Notice: we can't scrape the user information in parallel as pq
1285+
// fails with: unexpected describe rows response: 'D'
1286+
owners, err := store.GetUsers(ctx, database.GetUsersParams{
1287+
RbacRole: []string{codersdk.RoleOwner},
1288+
})
1289+
if err != nil {
1290+
return user, req.OrganizationID, xerrors.Errorf("get owners: %w", err)
1291+
}
1292+
userAdmins, err := store.GetUsers(ctx, database.GetUsersParams{
1293+
RbacRole: []string{codersdk.RoleUserAdmin},
1294+
})
1295+
if err != nil {
1296+
return user, req.OrganizationID, xerrors.Errorf("get user admins: %w", err)
1297+
}
1298+
1299+
for _, u := range append(owners, userAdmins...) {
1300+
if _, err := api.NotificationsEnqueuer.Enqueue(ctx, u.ID, notifications.TemplateUserAccountCreated,
1301+
map[string]string{
1302+
"created_account_name": user.Username,
1303+
}, "api-users-create",
1304+
user.ID,
1305+
); err != nil {
1306+
api.Logger.Warn(ctx, "unable to notify about created user", slog.F("created_user", user.Username), slog.Error(err))
1307+
}
1308+
}
1309+
return user, req.OrganizationID, err
12751310
}
12761311

12771312
func convertUsers(users []database.User, organizationIDsByUserID map[uuid.UUID][]uuid.UUID) []codersdk.User {

‎coderd/users_test.go

+94
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/coder/coder/v2/coderd"
1212
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
13+
"github.com/coder/coder/v2/coderd/notifications"
1314
"github.com/coder/coder/v2/coderd/rbac/policy"
1415
"github.com/coder/serpent"
1516

@@ -598,6 +599,99 @@ func TestPostUsers(t *testing.T) {
598599
})
599600
}
600601

602+
func TestNotifyCreatedUser(t *testing.T) {
603+
t.Parallel()
604+
605+
t.Run("OwnerNotified", func(t *testing.T) {
606+
t.Parallel()
607+
608+
// given
609+
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
610+
adminClient := coderdtest.New(t, &coderdtest.Options{
611+
NotificationsEnqueuer: notifyEnq,
612+
})
613+
firstUser := coderdtest.CreateFirstUser(t, adminClient)
614+
615+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
616+
defer cancel()
617+
618+
// when
619+
user, err := adminClient.CreateUser(ctx, codersdk.CreateUserRequest{
620+
OrganizationID: firstUser.OrganizationID,
621+
Email: "another@user.org",
622+
Username: "someone-else",
623+
Password: "SomeSecurePassword!",
624+
})
625+
require.NoError(t, err)
626+
627+
// then
628+
require.Len(t, notifyEnq.Sent, 1)
629+
require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent[0].TemplateID)
630+
require.Equal(t, firstUser.UserID, notifyEnq.Sent[0].UserID)
631+
require.Contains(t, notifyEnq.Sent[0].Targets, user.ID)
632+
require.Equal(t, user.Username, notifyEnq.Sent[0].Labels["created_account_name"])
633+
})
634+
635+
t.Run("UserAdminNotified", func(t *testing.T) {
636+
t.Parallel()
637+
638+
// given
639+
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
640+
adminClient := coderdtest.New(t, &coderdtest.Options{
641+
NotificationsEnqueuer: notifyEnq,
642+
})
643+
firstUser := coderdtest.CreateFirstUser(t, adminClient)
644+
645+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
646+
defer cancel()
647+
648+
userAdmin, err := adminClient.CreateUser(ctx, codersdk.CreateUserRequest{
649+
OrganizationID: firstUser.OrganizationID,
650+
Email: "user-admin@user.org",
651+
Username: "mr-user-admin",
652+
Password: "SomeSecurePassword!",
653+
})
654+
require.NoError(t, err)
655+
656+
_, err = adminClient.UpdateUserRoles(ctx, userAdmin.Username, codersdk.UpdateRoles{
657+
Roles: []string{
658+
rbac.RoleUserAdmin().String(),
659+
},
660+
})
661+
require.NoError(t, err)
662+
663+
// when
664+
member, err := adminClient.CreateUser(ctx, codersdk.CreateUserRequest{
665+
OrganizationID: firstUser.OrganizationID,
666+
Email: "another@user.org",
667+
Username: "someone-else",
668+
Password: "SomeSecurePassword!",
669+
})
670+
require.NoError(t, err)
671+
672+
// then
673+
require.Len(t, notifyEnq.Sent, 3)
674+
675+
// "User admin" account created, "owner" notified
676+
require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent[0].TemplateID)
677+
require.Equal(t, firstUser.UserID, notifyEnq.Sent[0].UserID)
678+
require.Contains(t, notifyEnq.Sent[0].Targets, userAdmin.ID)
679+
require.Equal(t, userAdmin.Username, notifyEnq.Sent[0].Labels["created_account_name"])
680+
681+
// "Member" account created, "owner" notified
682+
require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent[1].TemplateID)
683+
require.Equal(t, firstUser.UserID, notifyEnq.Sent[1].UserID)
684+
require.Contains(t, notifyEnq.Sent[1].Targets, member.ID)
685+
require.Equal(t, member.Username, notifyEnq.Sent[1].Labels["created_account_name"])
686+
687+
// "Member" account created, "user admin" notified
688+
require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent[1].TemplateID)
689+
require.Equal(t, userAdmin.ID, notifyEnq.Sent[2].UserID)
690+
require.Contains(t, notifyEnq.Sent[2].Targets, member.ID)
691+
require.Equal(t, member.Username, notifyEnq.Sent[2].Labels["created_account_name"])
692+
})
693+
}
694+
601695
func TestUpdateUserProfile(t *testing.T) {
602696
t.Parallel()
603697
t.Run("UserNotFound", func(t *testing.T) {

‎coderd/workspaces_test.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -3476,13 +3476,14 @@ func TestNotifications(t *testing.T) {
34763476

34773477
// Then
34783478
require.NoError(t, err, "mark workspace as dormant")
3479-
require.Len(t, notifyEnq.Sent, 1)
3480-
require.Equal(t, notifyEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceDormant)
3481-
require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID)
3482-
require.Contains(t, notifyEnq.Sent[0].Targets, template.ID)
3483-
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID)
3484-
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID)
3485-
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID)
3479+
require.Len(t, notifyEnq.Sent, 2)
3480+
// notifyEnq.Sent[0] is an event for created user account
3481+
require.Equal(t, notifyEnq.Sent[1].TemplateID, notifications.TemplateWorkspaceDormant)
3482+
require.Equal(t, notifyEnq.Sent[1].UserID, workspace.OwnerID)
3483+
require.Contains(t, notifyEnq.Sent[1].Targets, template.ID)
3484+
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.ID)
3485+
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OrganizationID)
3486+
require.Contains(t, notifyEnq.Sent[1].Targets, workspace.OwnerID)
34863487
})
34873488

34883489
t.Run("InitiatorIsOwner", func(t *testing.T) {

‎enterprise/coderd/scim.go

+2
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,8 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) {
239239
OrganizationID: defaultOrganization.ID,
240240
},
241241
LoginType: database.LoginTypeOIDC,
242+
// Do not send notifications to user admins as SCIM endpoint might be called sequentially to all users.
243+
SkipNotifications: true,
242244
})
243245
if err != nil {
244246
_ = handlerutil.WriteError(rw, err)

‎enterprise/coderd/scim_test.go

+13-2
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,15 @@ func TestScim(t *testing.T) {
113113
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
114114
defer cancel()
115115

116+
// given
116117
scimAPIKey := []byte("hi")
117118
mockAudit := audit.NewMock()
119+
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
118120
client, _ := coderdenttest.New(t, &coderdenttest.Options{
119-
Options: &coderdtest.Options{Auditor: mockAudit},
121+
Options: &coderdtest.Options{
122+
Auditor: mockAudit,
123+
NotificationsEnqueuer: notifyEnq,
124+
},
120125
SCIMAPIKey: scimAPIKey,
121126
AuditLogging: true,
122127
LicenseOptions: &coderdenttest.LicenseOptions{
@@ -129,12 +134,15 @@ func TestScim(t *testing.T) {
129134
})
130135
mockAudit.ResetLogs()
131136

137+
// when
132138
sUser := makeScimUser(t)
133139
res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey))
134140
require.NoError(t, err)
135141
defer res.Body.Close()
136142
require.Equal(t, http.StatusOK, res.StatusCode)
137143

144+
// then
145+
// Expect audit logs
138146
aLogs := mockAudit.AuditLogs()
139147
require.Len(t, aLogs, 1)
140148
af := map[string]string{}
@@ -143,12 +151,15 @@ func TestScim(t *testing.T) {
143151
assert.Equal(t, coderd.SCIMAuditAdditionalFields, af)
144152
assert.Equal(t, database.AuditActionCreate, aLogs[0].Action)
145153

154+
// Expect users exposed over API
146155
userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value})
147156
require.NoError(t, err)
148157
require.Len(t, userRes.Users, 1)
149-
150158
assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email)
151159
assert.Equal(t, sUser.UserName, userRes.Users[0].Username)
160+
161+
// Expect zero notifications (SkipNotifications = true)
162+
require.Empty(t, notifyEnq.Sent)
152163
})
153164

154165
t.Run("Duplicate", func(t *testing.T) {

0 commit comments

Comments
 (0)