fix: implement proper auth check for editing paired devices

See: BEDS-863
gobitfly · Oct 24, 2024 · a5bc428 · a5bc428
1 parent 103de90
commit a5bc428
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 20 deletions.
diff --git a/backend/pkg/api/data_access/dummy.go b/backend/pkg/api/data_access/dummy.go
@@ -582,10 +582,10 @@ func (d *DummyService) UpdateNotificationSettingsGeneral(ctx context.Context, us
 func (d *DummyService) UpdateNotificationSettingsNetworks(ctx context.Context, userId uint64, chainId uint64, settings t.NotificationSettingsNetwork) error {
 	return nil
 }
-func (d *DummyService) UpdateNotificationSettingsPairedDevice(ctx context.Context, userId uint64, pairedDeviceId uint64, name string, IsNotificationsEnabled bool) error {
+func (d *DummyService) UpdateNotificationSettingsPairedDevice(ctx context.Context, pairedDeviceId uint64, name string, IsNotificationsEnabled bool) error {
 	return nil
 }
-func (d *DummyService) DeleteNotificationSettingsPairedDevice(ctx context.Context, userId uint64, pairedDeviceId uint64) error {
+func (d *DummyService) DeleteNotificationSettingsPairedDevice(ctx context.Context, pairedDeviceId uint64) error {
 	return nil
 }
 
@@ -796,3 +796,7 @@ func (d *DummyService) QueueTestPushNotification(ctx context.Context, userId uin
 func (d *DummyService) QueueTestWebhookNotification(ctx context.Context, userId uint64, webhookUrl string, isDiscordWebhook bool) error {
 	return nil
 }
+
+func (d *DummyService) GetPairedDeviceUserId(ctx context.Context, pairedDeviceId uint64) (uint64, error) {
+	return getDummyData[uint64](ctx)
+}
diff --git a/backend/pkg/api/data_access/notifications.go b/backend/pkg/api/data_access/notifications.go
@@ -51,8 +51,9 @@ type NotificationsRepository interface {
 	GetNotificationSettingsDefaultValues(ctx context.Context) (*t.NotificationSettingsDefaultValues, error)
 	UpdateNotificationSettingsGeneral(ctx context.Context, userId uint64, settings t.NotificationSettingsGeneral) error
 	UpdateNotificationSettingsNetworks(ctx context.Context, userId uint64, chainId uint64, settings t.NotificationSettingsNetwork) error
-	UpdateNotificationSettingsPairedDevice(ctx context.Context, userId uint64, pairedDeviceId uint64, name string, IsNotificationsEnabled bool) error
-	DeleteNotificationSettingsPairedDevice(ctx context.Context, userId uint64, pairedDeviceId uint64) error
+	GetPairedDeviceUserId(ctx context.Context, pairedDeviceId uint64) (uint64, error)
+	UpdateNotificationSettingsPairedDevice(ctx context.Context, pairedDeviceId uint64, name string, IsNotificationsEnabled bool) error
+	DeleteNotificationSettingsPairedDevice(ctx context.Context, pairedDeviceId uint64) error
 	UpdateNotificationSettingsClients(ctx context.Context, userId uint64, clientId uint64, IsSubscribed bool) (*t.NotificationSettingsClient, error)
 	GetNotificationSettingsDashboards(ctx context.Context, userId uint64, cursor string, colSort t.Sort[enums.NotificationSettingsDashboardColumn], search string, limit uint64) ([]t.NotificationSettingsDashboardsTableRow, *t.Paging, error)
 	UpdateNotificationSettingsValidatorDashboard(ctx context.Context, userId uint64, dashboardId t.VDBIdPrimary, groupId uint64, settings t.NotificationSettingsValidatorDashboard) error
@@ -1637,47 +1638,61 @@ func (d *DataAccessService) UpdateNotificationSettingsNetworks(ctx context.Conte
 	}
 	return nil
 }
-func (d *DataAccessService) UpdateNotificationSettingsPairedDevice(ctx context.Context, userId uint64, pairedDeviceId uint64, name string, IsNotificationsEnabled bool) error {
+
+func (d *DataAccessService) GetPairedDeviceUserId(ctx context.Context, pairedDeviceId uint64) (uint64, error) {
+	var userId uint64
+	err := d.userReader.GetContext(context.Background(), &userId, `
+		SELECT user_id
+		FROM users_devices
+		WHERE id = $1`, pairedDeviceId)
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return 0, fmt.Errorf("%w, paired device with id %v not found", ErrNotFound, pairedDeviceId)
+		}
+		return 0, err
+	}
+	return userId, nil
+}
+
+func (d *DataAccessService) UpdateNotificationSettingsPairedDevice(ctx context.Context, pairedDeviceId uint64, name string, IsNotificationsEnabled bool) error {
 	result, err := d.userWriter.ExecContext(ctx, `
 		UPDATE users_devices 
 		SET 
 			device_name = $1,
 			notify_enabled = $2
-		WHERE user_id = $3 AND id = $4`,
-		name, IsNotificationsEnabled, userId, pairedDeviceId)
+		WHERE id = $3`,
+		name, IsNotificationsEnabled, pairedDeviceId)
 	if err != nil {
 		return err
 	}
-
-	// TODO: This can be deleted when the API layer has an improved check for the device id
 	rowsAffected, err := result.RowsAffected()
 	if err != nil {
 		return err
 	}
 	if rowsAffected == 0 {
-		return fmt.Errorf("device with id %v to update notification settings not found", pairedDeviceId)
+		return fmt.Errorf("%w, paired device with id %v not found", ErrNotFound, pairedDeviceId)
 	}
 	return nil
 }
-func (d *DataAccessService) DeleteNotificationSettingsPairedDevice(ctx context.Context, userId uint64, pairedDeviceId uint64) error {
+
+func (d *DataAccessService) DeleteNotificationSettingsPairedDevice(ctx context.Context, pairedDeviceId uint64) error {
 	result, err := d.userWriter.ExecContext(ctx, `
-		DELETE FROM users_devices 
-		WHERE user_id = $1 AND id = $2`,
-		userId, pairedDeviceId)
+		DELETE FROM users_devices
+		WHERE id = $1`,
+		pairedDeviceId)
 	if err != nil {
 		return err
 	}
-
-	// TODO: This can be deleted when the API layer has an improved check for the device id
 	rowsAffected, err := result.RowsAffected()
 	if err != nil {
 		return err
 	}
 	if rowsAffected == 0 {
-		return fmt.Errorf("device with id %v to delete not found", pairedDeviceId)
+		return fmt.Errorf("%w, paired device with id %v not found", ErrNotFound, pairedDeviceId)
 	}
 	return nil
 }
+
 func (d *DataAccessService) UpdateNotificationSettingsClients(ctx context.Context, userId uint64, clientId uint64, IsSubscribed bool) (*t.NotificationSettingsClient, error) {
 	result := &t.NotificationSettingsClient{Id: clientId, IsSubscribed: IsSubscribed}
 

diff --git a/backend/pkg/api/handlers/public.go b/backend/pkg/api/handlers/public.go
@@ -2353,7 +2353,16 @@ func (h *HandlerService) PublicPutUserNotificationSettingsPairedDevices(w http.R
 		handleErr(w, r, v)
 		return
 	}
-	err = h.getDataAccessor(r).UpdateNotificationSettingsPairedDevice(r.Context(), userId, pairedDeviceId, name, req.IsNotificationsEnabled)
+	pairedDeviceUserId, err := h.getDataAccessor(r).GetPairedDeviceUserId(r.Context(), pairedDeviceId)
+	if err != nil {
+		handleErr(w, r, err)
+		return
+	}
+	if userId != pairedDeviceUserId {
+		returnNotFound(w, r, fmt.Errorf("not found: paired device with id %d not found", pairedDeviceId)) // return 404 to not leak information
+		return
+	}
+	err = h.getDataAccessor(r).UpdateNotificationSettingsPairedDevice(r.Context(), pairedDeviceId, name, req.IsNotificationsEnabled)
 	if err != nil {
 		handleErr(w, r, err)
 		return
@@ -2387,13 +2396,21 @@ func (h *HandlerService) PublicDeleteUserNotificationSettingsPairedDevices(w htt
 		handleErr(w, r, err)
 		return
 	}
-	// TODO use a better way to validate the paired device id
 	pairedDeviceId := v.checkUint(mux.Vars(r)["paired_device_id"], "paired_device_id")
 	if v.hasErrors() {
 		handleErr(w, r, v)
 		return
 	}
-	err = h.getDataAccessor(r).DeleteNotificationSettingsPairedDevice(r.Context(), userId, pairedDeviceId)
+	pairedDeviceUserId, err := h.getDataAccessor(r).GetPairedDeviceUserId(r.Context(), pairedDeviceId)
+	if err != nil {
+		handleErr(w, r, err)
+		return
+	}
+	if userId != pairedDeviceUserId {
+		returnNotFound(w, r, fmt.Errorf("not found: paired device with id %d not found", pairedDeviceId)) // return 404 to not leak information
+		return
+	}
+	err = h.getDataAccessor(r).DeleteNotificationSettingsPairedDevice(r.Context(), pairedDeviceId)
 	if err != nil {
 		handleErr(w, r, err)
 		return