Skip to content

Commit c1048a2

Browse files
committed
refactor(ownershipCheckMiddleware): improve and clarify middleware logic
- Remove unused import of data_repository - Adjust middleware to run after dataFetchMiddleware, assuming item is already fetched - Simplify ownership check logic and error handling - Update comments and documentation for clearer understanding
1 parent e63fbbf commit c1048a2

File tree

1 file changed

+25
-40
lines changed

1 file changed

+25
-40
lines changed
Lines changed: 25 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import 'package:core/core.dart';
22
import 'package:dart_frog/dart_frog.dart';
3-
import 'package:data_repository/data_repository.dart';
43
import 'package:flutter_news_app_api_server_full_source_code/src/rbac/permission_service.dart';
54
import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_registry.dart';
65

@@ -19,28 +18,27 @@ class FetchedItem<T> {
1918
/// Middleware to check if the authenticated user is the owner of the requested
2019
/// item.
2120
///
22-
/// This middleware is designed to run on item-specific routes (e.g., `/[id]`).
23-
/// It performs the following steps:
21+
/// This middleware runs *after* the `dataFetchMiddleware`, which means it can
22+
/// safely assume that the requested item has already been fetched and is
23+
/// available in the context.
2424
///
25-
/// 1. Determines if an ownership check is required for the current action
26-
/// (GET, PUT, DELETE) based on the `ModelConfig`.
27-
/// 2. If a check is required and the user is not an admin, it fetches the
28-
/// item from the database.
29-
/// 3. It then compares the item's owner ID with the authenticated user's ID.
30-
/// 4. If the check fails, it throws a [ForbiddenException].
31-
/// 5. If the check passes, it provides the fetched item into the request
32-
/// context via `context.provide<FetchedItem<dynamic>>`. This prevents the
33-
/// downstream route handler from needing to fetch the item again.
25+
/// It performs the following steps:
26+
/// 1. Determines if an ownership check is required for the current action
27+
/// based on the `ModelConfig`.
28+
/// 2. If a check is required and the user is not an admin, it reads the
29+
/// pre-fetched item from the context.
30+
/// 3. It then compares the item's owner ID with the authenticated user's ID.
31+
/// 4. If the IDs do not match, it throws a [ForbiddenException].
32+
/// 5. If the check is not required or passes, it calls the next handler.
3433
Middleware ownershipCheckMiddleware() {
3534
return (handler) {
3635
return (context) async {
37-
final modelName = context.read<String>();
3836
final modelConfig = context.read<ModelConfig<dynamic>>();
3937
final user = context.read<User>();
4038
final permissionService = context.read<PermissionService>();
4139
final method = context.request.method;
42-
final id = context.request.uri.pathSegments.last;
4340

41+
// Determine the required permission configuration for the current method.
4442
ModelActionPermission permission;
4543
switch (method) {
4644
case HttpMethod.get:
@@ -50,54 +48,41 @@ Middleware ownershipCheckMiddleware() {
5048
case HttpMethod.delete:
5149
permission = modelConfig.deletePermission;
5250
default:
53-
// For other methods, no ownership check is performed here.
51+
// For any other methods, no ownership check is performed.
5452
return handler(context);
5553
}
5654

57-
// If no ownership check is required or if the user is an admin,
58-
// proceed to the next handler without fetching the item.
55+
// If no ownership check is required for this action, or if the user is
56+
// an admin (who bypasses ownership checks), proceed immediately.
5957
if (!permission.requiresOwnershipCheck ||
6058
permissionService.isAdmin(user)) {
6159
return handler(context);
6260
}
6361

62+
// At this point, an ownership check is required for a non-admin user.
63+
64+
// Ensure the model is configured to support ownership checks.
6465
if (modelConfig.getOwnerId == null) {
6566
throw const OperationFailedException(
6667
'Internal Server Error: Model configuration error for ownership check.',
6768
);
6869
}
6970

70-
final userIdForRepoCall = user.id;
71-
dynamic item;
72-
73-
switch (modelName) {
74-
case 'user':
75-
final repo = context.read<DataRepository<User>>();
76-
item = await repo.read(id: id, userId: userIdForRepoCall);
77-
case 'user_app_settings':
78-
final repo = context.read<DataRepository<UserAppSettings>>();
79-
item = await repo.read(id: id, userId: userIdForRepoCall);
80-
case 'user_content_preferences':
81-
final repo = context.read<DataRepository<UserContentPreferences>>();
82-
item = await repo.read(id: id, userId: userIdForRepoCall);
83-
default:
84-
throw OperationFailedException(
85-
'Ownership check not implemented for model "$modelName".',
86-
);
87-
}
71+
// Read the item that was pre-fetched by the dataFetchMiddleware.
72+
// This is guaranteed to exist because dataFetchMiddleware would have
73+
// thrown a NotFoundException if the item did not exist.
74+
final item = context.read<FetchedItem<dynamic>>().data;
8875

76+
// Compare the item's owner ID with the authenticated user's ID.
8977
final itemOwnerId = modelConfig.getOwnerId!(item);
9078
if (itemOwnerId != user.id) {
9179
throw const ForbiddenException(
9280
'You do not have permission to access this item.',
9381
);
9482
}
9583

96-
final updatedContext = context.provide<FetchedItem<dynamic>>(
97-
() => FetchedItem(item),
98-
);
99-
100-
return handler(updatedContext);
84+
// If the ownership check passes, proceed to the final route handler.
85+
return handler(context);
10186
};
10287
};
10388
}

0 commit comments

Comments
 (0)