Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit f61fbf3

Browse files
committed
refactor(registry): Overhaul user data operations
Removed the 'user' model from the item creators and deleters maps, enforcing that user creation and deletion are handled exclusively by the authentication service. Completely rewrote the custom updater for the 'user' model to be secure and architecturally sound. The new logic: - Enforces that administrators can only update `appRole` and `dashboardRole`. - Enforces that regular users can only update their own `feedDecoratorStatus`. - Rejects attempts to update any other fields via this endpoint. - Reads the pre-fetched user object and uses `copyWith` to apply only the allowed changes, creating a full, valid `User` object for the repository's `update` method. This resolves the previous partial update issue and aligns with the `DataRepository<T>` contract.
1 parent 35f3d5e commit f61fbf3

File tree

1 file changed

+76
-44
lines changed

1 file changed

+76
-44
lines changed

‎lib/src/registry/data_operation_registry.dart‎

Lines changed: 76 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import 'package:core/core.dart';
22
import 'package:dart_frog/dart_frog.dart';
33
import 'package:data_repository/data_repository.dart';
44
import 'package:flutter_news_app_api_server_full_source_code/src/middlewares/ownership_check_middleware.dart';
5+
import 'package:flutter_news_app_api_server_full_source_code/src/rbac/permission_service.dart';
56
import 'package:flutter_news_app_api_server_full_source_code/src/services/country_query_service.dart';
67
import 'package:flutter_news_app_api_server_full_source_code/src/services/dashboard_summary_service.dart';
8+
import 'package:logging/logging.dart';
79

810
// --- Typedefs for Data Operations ---
911

@@ -54,6 +56,9 @@ typedef ItemDeleter =
5456
/// data operations are performed for each model, improving consistency across
5557
/// the API.
5658
/// {@endtemplate}
59+
60+
final _log = Logger('DataOperationRegistry');
61+
5762
class DataOperationRegistry {
5863
/// {@macro data_operation_registry}
5964
DataOperationRegistry() {
@@ -188,11 +193,6 @@ class DataOperationRegistry {
188193
item: item as Language,
189194
userId: uid,
190195
),
191-
// Handler for creating a new user.
192-
'user': (c, item, uid) => c.read<DataRepository<User>>().create(
193-
item: item as User,
194-
userId: uid,
195-
),
196196
'remote_config': (c, item, uid) => c
197197
.read<DataRepository<RemoteConfig>>()
198198
.create(item: item as RemoteConfig, userId: uid),
@@ -225,56 +225,90 @@ class DataOperationRegistry {
225225
'language': (c, id, item, uid) => c
226226
.read<DataRepository<Language>>()
227227
.update(id: id, item: item as Language, userId: uid),
228-
// Custom updater for the 'user' model.
229-
// This updater handles two distinct use cases:
230-
// 1. Admins updating user roles (`appRole`, `dashboardRole`).
231-
// 2. Regular users updating their own `feedDecoratorStatus`.
232-
// It accepts a raw Map<String, dynamic> as the `item` to prevent
233-
// mass assignment vulnerabilities, only applying allowed fields.
234-
'user': (c, id, item, uid) {
235-
final repo = c.read<DataRepository<User>>();
236-
final existingUser = c.read<FetchedItem<dynamic>>().data as User;
228+
// Custom updater for the 'user' model. This logic is critical for
229+
// security and architectural consistency.
230+
//
231+
// It enforces the following rules:
232+
// 1. Admins can ONLY update a user's `appRole` and `dashboardRole`.
233+
// 2. Regular users can ONLY update their own `feedDecoratorStatus`.
234+
// 3. Email updates are handled by the `AuthService`, not this generic
235+
// endpoint.
236+
//
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.
244+
'user': (context, id, item, uid) async {
245+
_log.info('Executing custom updater for user ID: $id.');
246+
final permissionService = context.read<PermissionService>();
247+
final authenticatedUser = context.read<User>();
248+
final userToUpdate = context.read<FetchedItem<dynamic>>().data as User;
237249
final requestBody = item as Map<String, dynamic>;
238250

239-
AppUserRole? newAppRole;
240-
if (requestBody.containsKey('appRole')) {
241-
try {
242-
newAppRole = AppUserRole.values.byName(
243-
requestBody['appRole'] as String,
251+
var userWithUpdates = userToUpdate;
252+
253+
if (permissionService.isAdmin(authenticatedUser)) {
254+
_log.finer(
255+
'Admin user ${authenticatedUser.id} is updating user $id.',
256+
);
257+
// Admin is only allowed to update roles.
258+
if (requestBody.keys.any(
259+
(k) => k != 'appRole' && k != 'dashboardRole',
260+
)) {
261+
_log.warning(
262+
'Admin ${authenticatedUser.id} attempted to update invalid fields for user $id.',
244263
);
245-
} on ArgumentError {
246-
throw BadRequestException(
247-
'Invalid value for "appRole": "${requestBody['appRole']}".',
264+
throw const ForbiddenException(
265+
'Administrators can only update "appRole" and "dashboardRole" via this endpoint.',
248266
);
249267
}
250-
}
251268

252-
DashboardUserRole? newDashboardRole;
253-
if (requestBody.containsKey('dashboardRole')) {
254-
try {
255-
newDashboardRole = DashboardUserRole.values.byName(
256-
requestBody['dashboardRole'] as String,
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+
);
282+
} else {
283+
_log.finer(
284+
'Regular user ${authenticatedUser.id} is updating their own profile.',
285+
);
286+
// Regular user is only allowed to update feed decorator status.
287+
if (requestBody.keys.any((k) => k != 'feedDecoratorStatus')) {
288+
_log.warning(
289+
'User ${authenticatedUser.id} attempted to update invalid fields.',
257290
);
258-
} on ArgumentError {
259-
throw BadRequestException(
260-
'Invalid value for "dashboardRole": "${requestBody['dashboardRole']}".',
291+
throw const ForbiddenException(
292+
'You can only update "feedDecoratorStatus" via this endpoint.',
261293
);
262294
}
263-
}
264295

265-
Map<FeedDecoratorType, UserFeedDecoratorStatus>? newStatus;
266-
if (requestBody.containsKey('feedDecoratorStatus')) {
267-
newStatus = User.fromJson(
268-
{'feedDecoratorStatus': requestBody['feedDecoratorStatus']},
269-
).feedDecoratorStatus;
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+
}
270304
}
271305

272-
final userWithUpdates = existingUser.copyWith(
273-
appRole: newAppRole,
274-
dashboardRole: newDashboardRole,
275-
feedDecoratorStatus: newStatus,
306+
_log.info('User update validation passed. Calling repository.');
307+
return context.read<DataRepository<User>>().update(
308+
id: id,
309+
item: userWithUpdates,
310+
userId: uid,
276311
);
277-
return repo.update(id: id, item: userWithUpdates, userId: uid);
278312
},
279313
'user_app_settings': (c, id, item, uid) => c
280314
.read<DataRepository<UserAppSettings>>()
@@ -302,8 +336,6 @@ class DataOperationRegistry {
302336
c.read<DataRepository<Country>>().delete(id: id, userId: uid),
303337
'language': (c, id, uid) =>
304338
c.read<DataRepository<Language>>().delete(id: id, userId: uid),
305-
'user': (c, id, uid) =>
306-
c.read<DataRepository<User>>().delete(id: id, userId: uid),
307339
'user_app_settings': (c, id, uid) =>
308340
c.read<DataRepository<UserAppSettings>>().delete(id: id, userId: uid),
309341
'user_content_preferences': (c, id, uid) => c

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /