Skip to content

Commit 327ec0d

Browse files
ssncferreirassncferreira
committed
fix: block system users from sending custom notifications
1 parent 2ab1c4d commit 327ec0d

File tree

7 files changed

+127
-11
lines changed

7 files changed

+127
-11
lines changed

‎cli/notifications_test.go

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import (
1010
"github.com/stretchr/testify/assert"
1111
"github.com/stretchr/testify/require"
1212

13+
"github.com/coder/coder/v2/coderd/database"
14+
"github.com/coder/coder/v2/coderd/database/dbgen"
15+
1316
"github.com/coder/coder/v2/cli/clitest"
1417
"github.com/coder/coder/v2/coderd/coderdtest"
1518
"github.com/coder/coder/v2/coderd/notifications"
@@ -175,11 +178,12 @@ func TestCustomNotifications(t *testing.T) {
175178

176179
notifyEnq := &notificationstest.FakeEnqueuer{}
177180

178-
// Given: A member user
179181
ownerClient := coderdtest.New(t, &coderdtest.Options{
180182
DeploymentValues: coderdtest.DeploymentValues(t),
181183
NotificationsEnqueuer: notifyEnq,
182184
})
185+
186+
// Given: A member user
183187
ownerUser := coderdtest.CreateFirstUser(t, ownerClient)
184188
memberClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, ownerUser.OrganizationID)
185189

@@ -192,23 +196,58 @@ func TestCustomNotifications(t *testing.T) {
192196
var sdkError *codersdk.Error
193197
require.Error(t, err)
194198
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
195-
assert.Equal(t, http.StatusBadRequest, sdkError.StatusCode())
199+
require.Equal(t, http.StatusBadRequest, sdkError.StatusCode())
196200
require.Equal(t, "Invalid request body", sdkError.Message)
197201

198202
sent := notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateTestNotification))
199203
require.Len(t, sent, 0)
200204
})
201205

206+
t.Run("SystemUserNotAllowed", func(t *testing.T) {
207+
t.Parallel()
208+
209+
notifyEnq := &notificationstest.FakeEnqueuer{}
210+
211+
ownerClient, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{
212+
DeploymentValues: coderdtest.DeploymentValues(t),
213+
NotificationsEnqueuer: notifyEnq,
214+
})
215+
216+
// Given: A system user (prebuilds system user)
217+
_, token := dbgen.APIKey(t, db, database.APIKey{
218+
UserID: database.PrebuildsSystemUserID,
219+
LoginType: database.LoginTypeNone,
220+
})
221+
systemUserClient := codersdk.New(ownerClient.URL)
222+
systemUserClient.SetSessionToken(token)
223+
224+
// When: The system user attempts to send a custom notification
225+
inv, root := clitest.New(t, "notifications", "custom", "Custom Title", "Custom Message")
226+
clitest.SetupConfig(t, systemUserClient, root)
227+
228+
// Then: an error is expected with no notifications sent
229+
err := inv.Run()
230+
var sdkError *codersdk.Error
231+
require.Error(t, err)
232+
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
233+
require.Equal(t, http.StatusForbidden, sdkError.StatusCode())
234+
require.Equal(t, "Forbidden", sdkError.Message)
235+
236+
sent := notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateTestNotification))
237+
require.Len(t, sent, 0)
238+
})
239+
202240
t.Run("Success", func(t *testing.T) {
203241
t.Parallel()
204242

205243
notifyEnq := &notificationstest.FakeEnqueuer{}
206244

207-
// Given: A member user
208245
ownerClient := coderdtest.New(t, &coderdtest.Options{
209246
DeploymentValues: coderdtest.DeploymentValues(t),
210247
NotificationsEnqueuer: notifyEnq,
211248
})
249+
250+
// Given: A member user
212251
ownerUser := coderdtest.CreateFirstUser(t, ownerClient)
213252
memberClient, memberUser := coderdtest.CreateAnotherUser(t, ownerClient, ownerUser.OrganizationID)
214253

‎coderd/apidoc/docs.go

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

‎coderd/apidoc/swagger.json

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

‎coderd/database/modelmethods.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,10 @@ func (u User) RBACObject() rbac.Object {
376376
return rbac.ResourceUserObject(u.ID)
377377
}
378378

379+
func (u User) IsSystemUser() bool {
380+
return u.IsSystem
381+
}
382+
379383
func (u GetUsersRow) RBACObject() rbac.Object {
380384
return rbac.ResourceUserObject(u.ID)
381385
}

‎coderd/notifications.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ func (api *API) putUserNotificationPreferences(rw http.ResponseWriter, r *http.R
333333
// @Param request body codersdk.CustomNotification true "Provide a non-empty title or message"
334334
// @Success 204 "No Content"
335335
// @Failure 400 {object} codersdk.Response "Invalid request body"
336+
// @Failure 403 {object} codersdk.Response "System users cannot send custom notifications"
336337
// @Failure 500 {object} codersdk.Response "Failed to send custom notification"
337338
// @Router /notifications/custom [post]
338339
func (api *API) postCustomNotification(rw http.ResponseWriter, r *http.Request) {
@@ -357,17 +358,36 @@ func (api *API) postCustomNotification(rw http.ResponseWriter, r *http.Request)
357358
return
358359
}
359360

360-
userID := apiKey.UserID
361+
// Block system users from sending custom notifications
362+
user, err := api.Database.GetUserByID(ctx, apiKey.UserID)
363+
if err != nil {
364+
api.Logger.Error(ctx, "send custom notification", slog.Error(err))
365+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
366+
Message: "Failed to send custom notification",
367+
Detail: err.Error(),
368+
})
369+
return
370+
}
371+
if user.IsSystemUser() {
372+
api.Logger.Error(ctx, "send custom notification: system user is not allowed",
373+
slog.F("id", user.ID.String()), slog.F("name", user.Name))
374+
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
375+
Message: "Forbidden",
376+
Detail: "System users cannot send custom notifications.",
377+
})
378+
return
379+
}
380+
361381
if _, err := api.NotificationsEnqueuer.Enqueue(
362382
//nolint:gocritic // We need to be notifier to send the notification.
363383
dbauthz.AsNotifier(ctx),
364-
userID,
384+
user.ID,
365385
notifications.TemplateCustomNotification,
366386
map[string]string{
367387
"custom_title": req.Title,
368388
"custom_message": req.Message,
369389
},
370-
userID.String(),
390+
user.ID.String(),
371391
); err != nil {
372392
api.Logger.Error(ctx, "send custom notification", slog.Error(err))
373393
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{

‎coderd/notifications_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/coder/coder/v2/coderd/coderdtest"
1313
"github.com/coder/coder/v2/coderd/database"
14+
"github.com/coder/coder/v2/coderd/database/dbgen"
1415
"github.com/coder/coder/v2/coderd/notifications"
1516
"github.com/coder/coder/v2/coderd/notifications/notificationstest"
1617
"github.com/coder/coder/v2/codersdk"
@@ -407,6 +408,45 @@ func TestCustomNotification(t *testing.T) {
407408
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
408409
require.Equal(t, http.StatusBadRequest, sdkError.StatusCode())
409410
require.Equal(t, "Invalid request body", sdkError.Message)
411+
412+
sent := notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateTestNotification))
413+
require.Len(t, sent, 0)
414+
})
415+
416+
t.Run("SystemUserNotAllowed", func(t *testing.T) {
417+
t.Parallel()
418+
419+
ctx := testutil.Context(t, testutil.WaitShort)
420+
421+
notifyEnq := &notificationstest.FakeEnqueuer{}
422+
ownerClient, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{
423+
DeploymentValues: coderdtest.DeploymentValues(t),
424+
NotificationsEnqueuer: notifyEnq,
425+
})
426+
427+
// Given: A system user (prebuilds system user)
428+
_, token := dbgen.APIKey(t, db, database.APIKey{
429+
UserID: database.PrebuildsSystemUserID,
430+
LoginType: database.LoginTypeNone,
431+
})
432+
systemUserClient := codersdk.New(ownerClient.URL)
433+
systemUserClient.SetSessionToken(token)
434+
435+
// When: The system user attempts to send a custom notification
436+
err := systemUserClient.PostCustomNotification(ctx, codersdk.CustomNotification{
437+
Title: "Custom Title",
438+
Message: "Custom Message",
439+
})
440+
441+
// Then: a forbidden error is expected with no notifications sent
442+
var sdkError *codersdk.Error
443+
require.Error(t, err)
444+
require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
445+
require.Equal(t, http.StatusForbidden, sdkError.StatusCode())
446+
require.Equal(t, "Forbidden", sdkError.Message)
447+
448+
sent := notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateTestNotification))
449+
require.Len(t, sent, 0)
410450
})
411451

412452
t.Run("Success", func(t *testing.T) {

‎docs/reference/api/notifications.md

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

0 commit comments

Comments
 (0)