Skip to content

Commit 553a00a

Browse files
committed
fix(api): correct critical bug in preference limit validation
This commit fixes a critical bug in the DefaultUserPreferenceLimitService where interest-specific limits (pinned filters and notification subscriptions) were not being checked correctly. Previously, the logic only ran if it detected a single added or updated interest, which meant the limits were completely bypassed if a user updated their preferences without changing an interest, or if they changed multiple interests at once. The logic has been corrected to be stateless. It now validates the entire proposed updatedPreferences state on every request, ensuring that the total counts for pinned filters and all notification subscription types are always checked against the user's role-based limits. This closes the loophole that could have allowed users to exceed their configured limits.
1 parent 8a45a96 commit 553a00a

File tree

1 file changed

+36
-63
lines changed

1 file changed

+36
-63
lines changed

lib/src/services/default_user_preference_limit_service.dart

Lines changed: 36 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -108,78 +108,51 @@ class DefaultUserPreferenceLimitService implements UserPreferenceLimitService {
108108
);
109109
}
110110

111-
// Find the interest that was added or updated to check its specific limits.
112-
// This logic assumes only one interest is added or updated per request.
113-
final currentInterestIds = currentPreferences.interests
114-
.map((i) => i.id)
115-
.toSet();
116-
Interest? changedInterest;
117-
118-
for (final updatedInterest in updatedPreferences.interests) {
119-
if (!currentInterestIds.contains(updatedInterest.id)) {
120-
// This is a newly added interest.
121-
changedInterest = updatedInterest;
122-
break;
123-
} else {
124-
// This is a potentially updated interest. Find the original.
125-
final originalInterest = currentPreferences.interests.firstWhere(
126-
(i) => i.id == updatedInterest.id,
127-
);
128-
if (updatedInterest != originalInterest) {
129-
changedInterest = updatedInterest;
130-
break;
131-
}
132-
}
111+
// Check total number of pinned feed filters.
112+
final pinnedCount = updatedPreferences.interests
113+
.where((i) => i.isPinnedFeedFilter)
114+
.length;
115+
if (pinnedCount > interestLimits.pinnedFeedFilters) {
116+
_log.warning(
117+
'User ${user.id} exceeded pinned feed filter limit: '
118+
'${interestLimits.pinnedFeedFilters} (attempted $pinnedCount).',
119+
);
120+
throw ForbiddenException(
121+
'You have reached your limit of ${interestLimits.pinnedFeedFilters} '
122+
'pinned feed filters.',
123+
);
133124
}
134125

135-
// If an interest was added or updated, check its specific limits.
136-
if (changedInterest != null) {
137-
_log.info('Checking limits for changed interest: ${changedInterest.id}');
126+
// Check notification subscription limits for each possible delivery type.
127+
for (final deliveryType
128+
in PushNotificationSubscriptionDeliveryType.values) {
129+
final notificationLimit = interestLimits.notifications[deliveryType];
130+
if (notificationLimit == null) {
131+
_log.severe(
132+
'Notification limit for type ${deliveryType.name} not found for '
133+
'role ${user.appRole}. Denying request by default.',
134+
);
135+
throw ForbiddenException(
136+
'Notification limits for ${deliveryType.name} are not configured.',
137+
);
138+
}
138139

139-
// Check total number of pinned feed filters.
140-
final pinnedCount = updatedPreferences.interests
141-
.where((i) => i.isPinnedFeedFilter)
140+
// Count how many of the user's proposed interests include this type.
141+
final subscriptionCount = updatedPreferences.interests
142+
.where((i) => i.deliveryTypes.contains(deliveryType))
142143
.length;
143-
if (pinnedCount > interestLimits.pinnedFeedFilters) {
144+
145+
if (subscriptionCount > notificationLimit) {
144146
_log.warning(
145-
'User ${user.id} exceeded pinned feed filter limit: '
146-
'${interestLimits.pinnedFeedFilters} (attempted $pinnedCount).',
147+
'User ${user.id} exceeded notification limit for '
148+
'${deliveryType.name}: $notificationLimit '
149+
'(attempted $subscriptionCount).',
147150
);
148151
throw ForbiddenException(
149-
'You have reached your limit of ${interestLimits.pinnedFeedFilters} '
150-
'pinned feed filters.',
152+
'You have reached your limit of $notificationLimit '
153+
'${deliveryType.name} notification subscriptions.',
151154
);
152155
}
153-
154-
// Check notification subscription limits for each type.
155-
for (final deliveryType in changedInterest.deliveryTypes) {
156-
final notificationLimit = interestLimits.notifications[deliveryType];
157-
if (notificationLimit == null) {
158-
_log.severe(
159-
'Notification limit for type ${deliveryType.name} not found for '
160-
'role ${user.appRole}. Denying request by default.',
161-
);
162-
throw ForbiddenException(
163-
'Notification limits for ${deliveryType.name} are not configured.',
164-
);
165-
}
166-
167-
final subscriptionCount = updatedPreferences.interests
168-
.where((i) => i.deliveryTypes.contains(deliveryType))
169-
.length;
170-
171-
if (subscriptionCount > notificationLimit) {
172-
_log.warning(
173-
'User ${user.id} exceeded notification limit for '
174-
'${deliveryType.name}: $notificationLimit '
175-
'(attempted $subscriptionCount).',
176-
);
177-
throw ForbiddenException(
178-
'You have reached your limit of $notificationLimit '
179-
'${deliveryType.name} notification subscriptions.',
180-
);
181-
}
182-
}
183156
}
184157

185158
_log.info(

0 commit comments

Comments
 (0)