fix(hub): user alerts idor

fixes very unlikely scenario where user guesses another user's 15
character random system id and adds alerts for it

internal/alerts/alerts_api.go

@@ -9,6 +9,7 @@ import (
 	"slices"
 	"strings"
 
+	"github.com/henrygd/beszel/internal/hub/utils"
 	"github.com/pocketbase/dbx"
 	"github.com/pocketbase/pocketbase/core"
 )
@@ -37,6 +38,9 @@ func UpsertUserAlerts(e *core.RequestEvent) error {
 
 	err = e.App.RunInTransaction(func(txApp core.App) error {
 		for _, systemId := range reqData.Systems {
+			if !userHasSystem(txApp, userID, systemId) {
+				continue
+			}
 			// find existing matching alert
 			alertRecord, err := txApp.FindFirstRecordByFilter(alertsCollection,
 				"system={:system} && name={:name} && user={:user}",
@@ -94,6 +98,9 @@ func DeleteUserAlerts(e *core.RequestEvent) error {
 
 	err = e.App.RunInTransaction(func(txApp core.App) error {
 		for _, systemId := range reqData.Systems {
+			if !userHasSystem(txApp, userID, systemId) {
+				continue
+			}
 			// Find existing alert to delete
 			alertRecord, err := txApp.FindFirstRecordByFilter("alerts",
 				"system={:system} && name={:name} && user={:user}",
@@ -122,6 +129,15 @@ func DeleteUserAlerts(e *core.RequestEvent) error {
 	return e.JSON(http.StatusOK, map[string]any{"success": true, "count": numDeleted})
 }
 
+func userHasSystem(app core.App, userID, systemID string) bool {
+	system, err := app.FindRecordById("systems", systemID)
+	if err != nil {
+		return false
+	}
+	shareAll, _ := utils.GetEnv("SHARE_ALL_SYSTEMS")
+	return shareAll == "true" || slices.Contains(system.GetStringSlice("users"), userID)
+}
+
 // SendTestNotification handles API request to send a test notification to a specified Shoutrrr URL
 func (am *AlertManager) SendTestNotification(e *core.RequestEvent) error {
 	var data struct {

internal/alerts/alerts_api_test.go

@@ -190,6 +190,30 @@ func TestUserAlertsApi(t *testing.T) {
 				assert.EqualValues(t, 3, user1Alerts, "should have 3 alerts")
 			},
 		},
+		{
+			Name:   "POST ignores systems the user cannot access",
+			Method: http.MethodPost,
+			URL:    "/api/beszel/user-alerts",
+			Headers: map[string]string{
+				"Authorization": user2Token,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{"\"success\":true"},
+			TestAppFactory:  testAppFactory,
+			Body: jsonReader(map[string]any{
+				"name":    "CPU",
+				"systems": []string{system1.Id},
+				"value":   90,
+				"min":     10,
+			}),
+			BeforeTestFunc: func(t testing.TB, app *pbTests.TestApp, e *core.ServeEvent) {
+				beszelTests.ClearCollection(t, app, "alerts")
+			},
+			AfterTestFunc: func(t testing.TB, app *pbTests.TestApp, res *http.Response) {
+				alerts, _ := app.CountRecords("alerts")
+				assert.Zero(t, alerts)
+			},
+		},
 		{
 			Name:   "Overwrite: false, should not overwrite existing alert",
 			Method: http.MethodPost,
@@ -347,6 +371,31 @@ func TestUserAlertsApi(t *testing.T) {
 				assert.Zero(t, alerts, "should have 0 alerts")
 			},
 		},
+		{
+			Name:   "DELETE ignores systems the user cannot access",
+			Method: http.MethodDelete,
+			URL:    "/api/beszel/user-alerts",
+			Headers: map[string]string{
+				"Authorization": user2Token,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{"\"count\":0", "\"success\":true"},
+			TestAppFactory:  testAppFactory,
+			Body: jsonReader(map[string]any{
+				"name":    "CPU",
+				"systems": []string{system1.Id},
+			}),
+			BeforeTestFunc: func(t testing.TB, app *pbTests.TestApp, e *core.ServeEvent) {
+				beszelTests.ClearCollection(t, app, "alerts")
+				beszelTests.CreateRecord(app, "alerts", map[string]any{
+					"name": "CPU", "system": system1.Id, "user": user2.Id, "value": 80,
+				})
+			},
+			AfterTestFunc: func(t testing.TB, app *pbTests.TestApp, res *http.Response) {
+				alerts, _ := app.CountRecords("alerts")
+				assert.EqualValues(t, 1, alerts)
+			},
+		},
 		{
 			Name:   "User 2 should not be able to delete alert of user 1",
 			Method: http.MethodDelete,

internal/hub/api_test.go

@@ -11,6 +11,7 @@ import (
 	beszelTests "github.com/henrygd/beszel/internal/tests"
 
 	"github.com/henrygd/beszel/internal/migrations"
+	"github.com/pocketbase/dbx"
 	"github.com/pocketbase/pocketbase/core"
 	pbTests "github.com/pocketbase/pocketbase/tests"
 	"github.com/stretchr/testify/require"
@@ -55,7 +56,7 @@ func TestApiRoutesAuthentication(t *testing.T) {
 	// Create test system
 	system, err := beszelTests.CreateRecord(hub, "systems", map[string]any{
 		"name":  "test-system",
-		"users": []string{user.Id},
+		"users": []string{user.Id, readOnlyUser.Id},
 		"host":  "127.0.0.1",
 	})
 	require.NoError(t, err, "Failed to create test system")
@@ -277,6 +278,24 @@ func TestApiRoutesAuthentication(t *testing.T) {
 				"systems": []string{system.Id},
 			}),
 		},
+		{
+			Name:   "POST /user-alerts - readonly user can create own alert",
+			Method: http.MethodPost,
+			URL:    "/api/beszel/user-alerts",
+			Headers: map[string]string{
+				"Authorization": readOnlyUserToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{"\"success\":true"},
+			TestAppFactory:  testAppFactory,
+			Body: jsonReader(map[string]any{
+				"name": "CPU", "value": 80, "min": 10, "systems": []string{system.Id},
+			}),
+			AfterTestFunc: func(t testing.TB, app *pbTests.TestApp, res *http.Response) {
+				alerts, _ := app.CountRecords("alerts", dbx.HashExp{"user": readOnlyUser.Id})
+				require.EqualValues(t, 1, alerts)
+			},
+		},
 		{
 			Name:            "DELETE /user-alerts - no auth should fail",
 			Method:          http.MethodDelete,
@@ -314,6 +333,29 @@ func TestApiRoutesAuthentication(t *testing.T) {
 				})
 			},
 		},
+		{
+			Name:   "DELETE /user-alerts - readonly user can delete own alert",
+			Method: http.MethodDelete,
+			URL:    "/api/beszel/user-alerts",
+			Headers: map[string]string{
+				"Authorization": readOnlyUserToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{"\"count\":1", "\"success\":true"},
+			TestAppFactory:  testAppFactory,
+			Body: jsonReader(map[string]any{
+				"name": "CPU", "systems": []string{system.Id},
+			}),
+			BeforeTestFunc: func(t testing.TB, app *pbTests.TestApp, e *core.ServeEvent) {
+				beszelTests.CreateRecord(app, "alerts", map[string]any{
+					"name": "CPU", "system": system.Id, "user": readOnlyUser.Id, "value": 80,
+				})
+			},
+			AfterTestFunc: func(t testing.TB, app *pbTests.TestApp, res *http.Response) {
+				alerts, _ := app.CountRecords("alerts", dbx.HashExp{"user": readOnlyUser.Id})
+				require.Zero(t, alerts)
+			},
+		},
 		{
 			Name:            "GET /containers/logs - no auth should fail",
 			Method:          http.MethodGet,

internal/hub/collections_test.go

@@ -357,6 +357,13 @@ func TestApiCollectionsAuthRules(t *testing.T) {
 		"host":  "127.0.0.2",
 	})
 
+	userOneAlert, _ := beszelTests.CreateRecord(hub, "alerts", map[string]any{
+		"name": "CPU", "system": userOneSystem.Id, "user": user1.Id, "value": 80,
+	})
+	userTwoAlert, _ := beszelTests.CreateRecord(hub, "alerts", map[string]any{
+		"name": "CPU", "system": userTwoSystem.Id, "user": user2.Id, "value": 80,
+	})
+
 	userRecords, _ := hub.CountRecords("users")
 	assert.EqualValues(t, 3, userRecords, "all users should be created")
 
@@ -368,6 +375,30 @@ func TestApiCollectionsAuthRules(t *testing.T) {
 	}
 
 	scenarios := []beszelTests.ApiScenario{
+		{
+			Name:   "Users can only list their own alerts",
+			Method: http.MethodGet,
+			URL:    "/api/collections/alerts/records",
+			Headers: map[string]string{
+				"Authorization": user1Token,
+			},
+			ExpectedStatus:     200,
+			ExpectedContent:    []string{userOneAlert.Id},
+			NotExpectedContent: []string{userTwoAlert.Id},
+			TestAppFactory:     testAppFactory,
+		},
+		{
+			Name:   "Users cannot view another user's alert by id",
+			Method: http.MethodGet,
+			URL:    fmt.Sprintf("/api/collections/alerts/records/%s", userTwoAlert.Id),
+			Headers: map[string]string{
+				"Authorization": user1Token,
+			},
+			ExpectedStatus:     403,
+			ExpectedContent:    []string{"Only superusers"},
+			NotExpectedContent: []string{userTwoAlert.Id},
+			TestAppFactory:     testAppFactory,
+		},
 		{
 			Name:               "Unauthorized user cannot list systems",
 			Method:             http.MethodGet,