Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: notify about created user account#14010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mtojek merged 26 commits intomainfrom17-account-notifs
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
26 commits
Select commitHold shift + click to select a range
c18ada0
feat: notify about created user account
mtojekJul 25, 2024
1cc3adb
Merge branch 'main' into 17-account-notifs
mtojekJul 26, 2024
ca2bdde
migrations
mtojekJul 26, 2024
d6e8964
Username
mtojekJul 26, 2024
2d74c4e
events
mtojekJul 26, 2024
3c3b5af
fix
mtojekJul 26, 2024
312d9fa
WIP
mtojekJul 26, 2024
82ec37f
fix version
mtojekJul 26, 2024
d7b2c73
simplify
mtojekJul 29, 2024
b81bb6a
WIP
mtojekJul 29, 2024
00275dd
fix: versions
mtojekJul 29, 2024
e356ba8
test
mtojekJul 29, 2024
6bc1d2d
fix test
mtojekJul 29, 2024
67a5137
users notified
mtojekJul 29, 2024
6431a0e
Merge branch 'main' into 17-account-notifs
mtojekJul 29, 2024
42d9ba1
post merge
mtojekJul 29, 2024
ecc7d30
fmt
mtojekJul 29, 2024
b2dcb3b
skip notif
mtojekJul 29, 2024
70e2d2c
Danny's feedback
mtojekJul 30, 2024
8696b70
given when then
mtojekJul 30, 2024
a92d059
Fix tests
mtojekJul 30, 2024
b5b0d90
Skip notifications
mtojekJul 30, 2024
2de1786
Merge branch 'main' into 17-account-notifs
mtojekJul 30, 2024
b843232
fix: created_account_name
mtojekJul 30, 2024
2a53a0b
api.Database -> tx
mtojekJul 30, 2024
77c0c28
pq issue
mtojekJul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletionscoderd/autobuild/lifecycle_executor_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1115,13 +1115,14 @@ func TestNotifications(t *testing.T) {
require.NotNil(t, workspace.DormantAt)

// Check that a notification was enqueued
require.Len(t, notifyEnq.Sent, 1)
require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID)
require.Equal(t, notifyEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceDormant)
require.Contains(t, notifyEnq.Sent[0].Targets, template.ID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID)
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)
})
}

Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
DELETE FROM notification_templates WHERE id = '4e19c0ac-94e1-4532-9515-d1801aa283b2';
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions)
VALUES ('4e19c0ac-94e1-4532-9515-d1801aa283b2', 'User account created', E'User account "{{.Labels.created_account_name}}" created',
E'Hi {{.UserName}},\n\New user account **{{.Labels.created_account_name}}** has been created.',
'Workspace Events', '[
{
"label": "View accounts",
"url": "{{ base_url }}/deployment/users?filter=status%3Aactive"
}
]'::jsonb);
5 changes: 5 additions & 0 deletionscoderd/notifications/events.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,3 +13,8 @@ var (
TemplateWorkspaceAutoUpdated = uuid.MustParse("c34a0c09-0704-4cac-bd1c-0c0146811c2b")
TemplateWorkspaceMarkedForDeletion = uuid.MustParse("51ce2fdf-c9ca-4be1-8d70-628674f9bc42")
)

// Account-related events.
var (
TemplateUserAccountCreated = uuid.MustParse("4e19c0ac-94e1-4532-9515-d1801aa283b2")
)
39 changes: 37 additions & 2 deletionscoderd/users.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,8 @@ import (
"github.com/google/uuid"
"golang.org/x/xerrors"

"cdr.dev/slog"

"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
Expand All@@ -20,6 +22,7 @@ import (
"github.com/coder/coder/v2/coderd/gitsshkey"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/httpmw"
"github.com/coder/coder/v2/coderd/notifications"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/rbac/policy"
"github.com/coder/coder/v2/coderd/searchquery"
Expand DownExpand Up@@ -1200,7 +1203,8 @@ func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques

type CreateUserRequest struct {
codersdk.CreateUserRequest
LoginType database.LoginType
LoginType database.LoginType
SkipNotifications bool
}

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

var user database.User
return user, req.OrganizationID, store.InTx(func(tx database.Store) error {
err := store.InTx(func(tx database.Store) error {
orgRoles := make([]string, 0)
// Organization is required to know where to allocate the user.
if req.OrganizationID == uuid.Nil {
Expand DownExpand Up@@ -1272,6 +1276,37 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
}
return nil
}, nil)
if err != nil || req.SkipNotifications {
return user, req.OrganizationID, err
}

// Notify all users with user admin permission including owners
// Notice: we can't scrape the user information in parallel as pq
// fails with: unexpected describe rows response: 'D'
owners, err := store.GetUsers(ctx, database.GetUsersParams{
RbacRole: []string{codersdk.RoleOwner},
})
if err != nil {
return user, req.OrganizationID, xerrors.Errorf("get owners: %w", err)
}
userAdmins, err := store.GetUsers(ctx, database.GetUsersParams{
RbacRole: []string{codersdk.RoleUserAdmin},
})
if err != nil {
return user, req.OrganizationID, xerrors.Errorf("get user admins: %w", err)
}

for _, u := range append(owners, userAdmins...) {
if _, err := api.NotificationsEnqueuer.Enqueue(ctx, u.ID, notifications.TemplateUserAccountCreated,
map[string]string{
"created_account_name": user.Username,
}, "api-users-create",
user.ID,
); err != nil {
api.Logger.Warn(ctx, "unable to notify about created user", slog.F("created_user", user.Username), slog.Error(err))
}
}
return user, req.OrganizationID, err
}

func convertUsers(users []database.User, organizationIDsByUserID map[uuid.UUID][]uuid.UUID) []codersdk.User {
Expand Down
94 changes: 94 additions & 0 deletionscoderd/users_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,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/rbac/policy"
"github.com/coder/serpent"

Expand DownExpand Up@@ -598,6 +599,99 @@ func TestPostUsers(t *testing.T) {
})
}

func TestNotifyCreatedUser(t *testing.T) {
t.Parallel()

t.Run("OwnerNotified", func(t *testing.T) {
t.Parallel()

// given
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
adminClient := coderdtest.New(t, &coderdtest.Options{
NotificationsEnqueuer: notifyEnq,
})
firstUser := coderdtest.CreateFirstUser(t, adminClient)

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

// when
user, err := adminClient.CreateUser(ctx, codersdk.CreateUserRequest{
OrganizationID: firstUser.OrganizationID,
Email: "another@user.org",
Username: "someone-else",
Password: "SomeSecurePassword!",
})
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"])
})

t.Run("UserAdminNotified", func(t *testing.T) {
t.Parallel()

// given
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
adminClient := coderdtest.New(t, &coderdtest.Options{
NotificationsEnqueuer: notifyEnq,
})
firstUser := coderdtest.CreateFirstUser(t, adminClient)

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

userAdmin, err := adminClient.CreateUser(ctx, codersdk.CreateUserRequest{
OrganizationID: firstUser.OrganizationID,
Email: "user-admin@user.org",
Username: "mr-user-admin",
Password: "SomeSecurePassword!",
})
require.NoError(t, err)

_, err = adminClient.UpdateUserRoles(ctx, userAdmin.Username, codersdk.UpdateRoles{
Roles: []string{
rbac.RoleUserAdmin().String(),
},
})
require.NoError(t, err)

// when
member, err := adminClient.CreateUser(ctx, codersdk.CreateUserRequest{
OrganizationID: firstUser.OrganizationID,
Email: "another@user.org",
Username: "someone-else",
Password: "SomeSecurePassword!",
})
require.NoError(t, err)

// then
require.Len(t, notifyEnq.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"])

// "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"])

// "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"])
})
}

func TestUpdateUserProfile(t *testing.T) {
t.Parallel()
t.Run("UserNotFound", func(t *testing.T) {
Expand Down
15 changes: 8 additions & 7 deletionscoderd/workspaces_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3476,13 +3476,14 @@ func TestNotifications(t *testing.T) {

// Then
require.NoError(t, err, "mark workspace as dormant")
require.Len(t, notifyEnq.Sent, 1)
require.Equal(t, notifyEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceDormant)
require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID)
require.Contains(t, notifyEnq.Sent[0].Targets, template.ID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID)
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID)
require.Len(t, notifyEnq.Sent, 2)
// notifyEnq.Sent[0] is an event for created user account
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Nit: would be safer to validate this

Copy link
MemberAuthor

@mtojekmtojekJul 30, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I left it like this on purpose, this is not the aim of this test. Less chance that it will require refining if we alter something in Notifications Enqueuer logic

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)
})

t.Run("InitiatorIsOwner", func(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletionsenterprise/coderd/scim.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,6 +239,8 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) {
OrganizationID: defaultOrganization.ID,
},
LoginType: database.LoginTypeOIDC,
// Do not send notifications to user admins as SCIM endpoint might be called sequentially to all users.
SkipNotifications: true,
})
if err != nil {
_ = handlerutil.WriteError(rw, err)
Expand Down
15 changes: 13 additions & 2 deletionsenterprise/coderd/scim_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -113,10 +113,15 @@ func TestScim(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

// given
scimAPIKey := []byte("hi")
mockAudit := audit.NewMock()
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
client, _ := coderdenttest.New(t, &coderdenttest.Options{
Options: &coderdtest.Options{Auditor: mockAudit},
Options: &coderdtest.Options{
Auditor: mockAudit,
NotificationsEnqueuer: notifyEnq,
},
SCIMAPIKey: scimAPIKey,
AuditLogging: true,
LicenseOptions: &coderdenttest.LicenseOptions{
Expand All@@ -129,12 +134,15 @@ func TestScim(t *testing.T) {
})
mockAudit.ResetLogs()

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

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

// Expect users exposed over API
userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value})
require.NoError(t, err)
require.Len(t, userRes.Users, 1)

assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email)
assert.Equal(t, sUser.UserName, userRes.Users[0].Username)

// Expect zero notifications (SkipNotifications = true)
require.Empty(t, notifyEnq.Sent)
})

t.Run("Duplicate", func(t *testing.T) {
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp