Skip to content

Commit 3820e66

Browse files
committed
fix(registry): Correct user update logic to support full object payload
Refactored the custom user updater to correctly handle a full `User` object in the request body, aligning with the `DataRepository` contract and fixing the `500` error seen by the client. The new logic performs a state comparison: - It deserializes the incoming request body into a `User` object. - It compares this object against the pre-fetched user from the database to identify which fields have changed. - It verifies that only permitted fields (`appRole`/`dashboardRole` for admins, `feedDecoratorStatus` for users) have been modified. - If validation passes, it proceeds with the update using the full `User` object from the request. This fixes the bug while maintaining security and architectural consistency.
1 parent a77e7c0 commit 3820e66

File tree

1 file changed

+45
-45
lines changed

1 file changed

+45
-45
lines changed

lib/src/registry/data_operation_registry.dart

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -231,82 +231,82 @@ class DataOperationRegistry {
231231
// It enforces the following rules:
232232
// 1. Admins can ONLY update a user's `appRole` and `dashboardRole`.
233233
// 2. Regular users can ONLY update their own `feedDecoratorStatus`.
234-
// 3. Email updates are handled by the `AuthService`, not this generic
235-
// endpoint.
236234
//
237-
// The updater receives a raw `Map<String, dynamic>` from the request
238-
// body to prevent mass assignment vulnerabilities. It then reads the
239-
// pre-fetched user object (guaranteed by `dataFetchMiddleware`) and
240-
// selectively applies only the allowed fields using `copyWith`. This
241-
// creates a complete, valid `User` object that is then passed to the
242-
// repository's `update` method, satisfying the `DataRepository<T>`
243-
// contract.
235+
// This logic correctly handles a full `User` object in the request body,
236+
// aligning with the DataRepository contract. It works by comparing the
237+
// incoming `User` object from the request (`requestedUpdateUser`) with
238+
// the current state of the user in the database (`userToUpdate`), which
239+
// is pre-fetched by middleware. It then verifies that the *only* fields
240+
// that have changed are ones the authenticated user is permitted to
241+
// modify.
244242
'user': (context, id, item, uid) async {
245243
_log.info('Executing custom updater for user ID: $id.');
246244
final permissionService = context.read<PermissionService>();
247245
final authenticatedUser = context.read<User>();
248246
final userToUpdate = context.read<FetchedItem<dynamic>>().data as User;
249247
final requestBody = item as Map<String, dynamic>;
248+
final requestedUpdateUser = User.fromJson(requestBody);
250249

251-
var userWithUpdates = userToUpdate;
252-
250+
// --- State Comparison Logic ---
253251
if (permissionService.isAdmin(authenticatedUser)) {
254252
_log.finer(
255253
'Admin user ${authenticatedUser.id} is updating user $id.',
256254
);
257-
// Admin is only allowed to update roles.
258-
if (requestBody.keys.any(
259-
(k) => k != 'appRole' && k != 'dashboardRole',
260-
)) {
255+
256+
// Create a version of the original user with only the fields an
257+
// admin is allowed to change applied from the request.
258+
final permissibleUpdate = userToUpdate.copyWith(
259+
appRole: requestedUpdateUser.appRole,
260+
dashboardRole: requestedUpdateUser.dashboardRole,
261+
);
262+
263+
// If the user from the request is not identical to the one with
264+
// only permissible changes, it means an unauthorized field was
265+
// modified.
266+
if (requestedUpdateUser != permissibleUpdate) {
261267
_log.warning(
262-
'Admin ${authenticatedUser.id} attempted to update invalid fields for user $id.',
268+
'Admin ${authenticatedUser.id} attempted to update unauthorized fields for user $id.',
263269
);
264270
throw const ForbiddenException(
265271
'Administrators can only update "appRole" and "dashboardRole" via this endpoint.',
266272
);
267273
}
268-
269-
final newAppRole = requestBody.containsKey('appRole')
270-
? AppUserRole.values.byName(requestBody['appRole'] as String)
271-
: null;
272-
final newDashboardRole = requestBody.containsKey('dashboardRole')
273-
? DashboardUserRole.values.byName(
274-
requestBody['dashboardRole'] as String,
275-
)
276-
: null;
277-
278-
userWithUpdates = userWithUpdates.copyWith(
279-
appRole: newAppRole,
280-
dashboardRole: newDashboardRole,
281-
);
274+
_log.finer('Admin update for user $id validation passed.');
282275
} else {
283276
_log.finer(
284277
'Regular user ${authenticatedUser.id} is updating their own profile.',
285278
);
286-
// Regular user is only allowed to update feed decorator status.
287-
if (requestBody.keys.any((k) => k != 'feedDecoratorStatus')) {
279+
280+
// Create a version of the original user with only the fields a
281+
// regular user is allowed to change applied from the request.
282+
final permissibleUpdate = userToUpdate.copyWith(
283+
feedDecoratorStatus: requestedUpdateUser.feedDecoratorStatus,
284+
);
285+
286+
// If the user from the request is not identical to the one with
287+
// only permissible changes, it means an unauthorized field was
288+
// modified.
289+
if (requestedUpdateUser != permissibleUpdate) {
288290
_log.warning(
289-
'User ${authenticatedUser.id} attempted to update invalid fields.',
291+
'User ${authenticatedUser.id} attempted to update unauthorized fields.',
290292
);
291293
throw const ForbiddenException(
292294
'You can only update "feedDecoratorStatus" via this endpoint.',
293295
);
294296
}
295-
296-
if (requestBody.containsKey('feedDecoratorStatus')) {
297-
final newStatus = User.fromJson(
298-
{'feedDecoratorStatus': requestBody['feedDecoratorStatus']},
299-
).feedDecoratorStatus;
300-
userWithUpdates = userWithUpdates.copyWith(
301-
feedDecoratorStatus: newStatus,
302-
);
303-
}
297+
_log.finer(
298+
'Regular user update for user $id validation passed.',
299+
);
304300
}
305301

306-
_log.info('User update validation passed. Calling repository.');
307-
return context.read<DataRepository<User>>().update(
302+
_log.info(
303+
'User update validation passed. Calling repository with full object.',
304+
);
305+
// The validation passed, so we can now safely pass the full User
306+
// object from the request to the repository, honoring the contract.
307+
return await context.read<DataRepository<User>>().update(
308308
id: id,
309-
item: userWithUpdates,
309+
item: requestedUpdateUser,
310310
userId: uid,
311311
);
312312
},

0 commit comments

Comments
 (0)