Skip to content

Commit 8a45a96

Browse files
committed
fix(api): harden migration script and correctly implement admin limit bypass
This commit applies two important fixes based on code review feedback: Robust Migration Script: The UnifyInterestsAndRemoteConfig migration script has been hardened. It now wraps the parsing of deliveryTypes in a try-catch block. This prevents the entire server from crashing on startup if it encounters malformed data (an invalid enum string) in a user's legacy notificationSubscriptions, making the migration process more resilient. Correct Admin Bypass: The administrator bypass for user preference limits has been re-implemented in the architecturally correct location. Instead of coupling the UserPreferenceLimitService with the PermissionService, the check is now performed in the user_content_preferences custom updater within data_operation_registry.dart. This ensures that the limit service is only called if the user does not have the userPreferenceBypassLimits permission, maintaining a clean separation of concerns.
1 parent aaae462 commit 8a45a96

File tree

2 files changed

+31
-12
lines changed

2 files changed

+31
-12
lines changed

lib/src/database/migrations/20251111000000_unify_interests_and_remote_config.dart

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,18 @@ class UnifyInterestsAndRemoteConfig extends Migration {
104104
final key = _generateCriteriaKey(criteria);
105105
final deliveryTypes =
106106
(subscription['deliveryTypes'] as List<dynamic>? ?? [])
107-
.map(
108-
(e) => PushNotificationSubscriptionDeliveryType.values.byName(
109-
e as String,
110-
),
111-
)
107+
.map((e) {
108+
try {
109+
return PushNotificationSubscriptionDeliveryType.values
110+
.byName(e as String);
111+
} catch (_) {
112+
log.warning(
113+
'User $userId has a notificationSubscription with an invalid deliveryType: "$e". Skipping this type.',
114+
);
115+
return null;
116+
}
117+
})
118+
.whereType<PushNotificationSubscriptionDeliveryType>()
112119
.toSet();
113120

114121
interestMap.update(

lib/src/registry/data_operation_registry.dart

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'package:core/core.dart';
44
import 'package:dart_frog/dart_frog.dart';
55
import 'package:data_repository/data_repository.dart';
66
import 'package:flutter_news_app_api_server_full_source_code/src/middlewares/ownership_check_middleware.dart';
7+
import 'package:flutter_news_app_api_server_full_source_code/src/rbac/permissions.dart';
78
import 'package:flutter_news_app_api_server_full_source_code/src/rbac/permission_service.dart';
89
import 'package:flutter_news_app_api_server_full_source_code/src/services/country_query_service.dart';
910
import 'package:flutter_news_app_api_server_full_source_code/src/services/dashboard_summary_service.dart';
@@ -386,6 +387,7 @@ class DataOperationRegistry {
386387
'Executing custom updater for user_content_preferences ID: $id.',
387388
);
388389
final authenticatedUser = context.read<User>();
390+
final permissionService = context.read<PermissionService>();
389391
final userPreferenceLimitService = context
390392
.read<UserPreferenceLimitService>();
391393
final userContentPreferencesRepository = context
@@ -400,13 +402,23 @@ class DataOperationRegistry {
400402

401403
// 2. Validate all limits using the consolidated service method.
402404
// The service now contains all logic to compare the updated and
403-
// current preferences and check all relevant limits (interests,
404-
// followed items, etc.) in one go.
405-
await userPreferenceLimitService.checkUserContentPreferencesLimits(
406-
user: authenticatedUser,
407-
updatedPreferences: preferencesToUpdate,
408-
currentPreferences: currentPreferences,
409-
);
405+
// current preferences and check all relevant limits.
406+
//
407+
// We first check if the user has permission to bypass these limits.
408+
if (permissionService.hasPermission(
409+
authenticatedUser,
410+
Permissions.userPreferenceBypassLimits,
411+
)) {
412+
_log.info(
413+
'User ${authenticatedUser.id} has bypass permission. Skipping limit checks.',
414+
);
415+
} else {
416+
await userPreferenceLimitService.checkUserContentPreferencesLimits(
417+
user: authenticatedUser,
418+
updatedPreferences: preferencesToUpdate,
419+
currentPreferences: currentPreferences,
420+
);
421+
}
410422

411423
// 3. If all checks pass, proceed with the update.
412424
_log.info(

0 commit comments

Comments
 (0)