Skip to content

Commit afbde1a

Browse files
committed
feat(authorization): enhance middleware for public routes and clarify documentation
- Update handling of null users in authorization checks - Improve documentation of middleware behavior for public and authenticated routes - Refine error handling for unauthenticated access attempts - Clarify implications of different RequiredPermissionType values
1 parent 3edf699 commit afbde1a

File tree

1 file changed

+34
-20
lines changed

1 file changed

+34
-20
lines changed

lib/src/middlewares/authorization_middleware.dart

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,16 @@ final _log = Logger('AuthorizationMiddleware');
99
/// {@template authorization_middleware}
1010
/// Middleware to enforce role-based permissions and model-specific access rules.
1111
///
12-
/// This middleware reads the authenticated [User], the requested `modelName`,
13-
/// the `HttpMethod`, and the `ModelConfig` from the request context. It then
14-
/// determines the required permission based on the `ModelConfig` metadata for
15-
/// the specific HTTP method and checks if the authenticated user has that
16-
/// permission using the [PermissionService].
12+
/// This middleware reads the authenticated [User] (which may be null for
13+
/// public routes), the requested `modelName`, the `HttpMethod`, and the
14+
/// `ModelConfig` from the request context. It then determines the required
15+
/// permission based on the `ModelConfig` metadata for the specific HTTP method
16+
/// and checks if the authenticated user has that permission using the
17+
/// [PermissionService].
1718
///
18-
/// If the user does not have the required permission, it throws a
19-
/// [ForbiddenException], which should be caught by the 'errorHandler' middleware.
19+
/// If the user does not have the required permission, or if authentication is
20+
/// required but no user is present, it throws a [ForbiddenException] or
21+
/// [UnauthorizedException], which should be caught by the 'errorHandler' middleware.
2022
///
2123
/// This middleware runs *after* authentication and model validation.
2224
/// It does NOT perform instance-level ownership checks; those are handled
@@ -27,8 +29,9 @@ Middleware authorizationMiddleware() {
2729
return (handler) {
2830
return (context) async {
2931
// Read dependencies from the context.
30-
// User is guaranteed non-null by requireAuthentication() middleware.
31-
final user = context.read<User>();
32+
// User is now read as nullable, as _conditionalAuthenticationMiddleware
33+
// might provide null for public routes.
34+
final user = context.read<User?>();
3235
final permissionService = context.read<PermissionService>();
3336
final modelName = context.read<String>(); // Provided by data/_middleware
3437
final modelConfig = context
@@ -64,23 +67,33 @@ Middleware authorizationMiddleware() {
6467
);
6568
}
6669

67-
// Perform the permission check based on the configuration type
70+
// Handle authentication requirement based on ModelConfig ---
71+
if (requiredPermissionConfig.requiresAuthentication && user == null) {
72+
_log.warning(
73+
'Authorization required but no valid user found. Denying access.',
74+
);
75+
throw const UnauthorizedException('Authentication required.');
76+
}
77+
78+
// If authentication is not required, or if user is present, proceed with permission checks.
79+
// All subsequent checks must now handle a potentially null 'user' if 'requiresAuthentication' is false.
6880
switch (requiredPermissionConfig.type) {
6981
case RequiredPermissionType.none:
70-
// No specific permission required (beyond authentication if applicable)
71-
// This case is primarily for documentation/completeness if a route
72-
// group didn't require authentication, but the /data route does.
73-
// For the /data route, 'none' effectively means 'authenticated users allowed'.
82+
// No specific permission required.
83+
// If user is null, it's a public route. If user is not null, they are authenticated.
7484
break;
7585
case RequiredPermissionType.adminOnly:
76-
// Requires the user to be an admin
77-
if (!permissionService.isAdmin(user)) {
86+
// Requires the user to be an admin.
87+
// If user is null here, it means requiresAuthentication was false,
88+
// but adminOnly implies authentication. This is a misconfiguration
89+
// or an attempt to access an admin-only resource publicly.
90+
if (user == null || !permissionService.isAdmin(user)) {
7891
throw const ForbiddenException(
7992
'Only administrators can perform this action.',
8093
);
8194
}
8295
case RequiredPermissionType.specificPermission:
83-
// Requires a specific permission string
96+
// Requires a specific permission string.
8497
final permission = requiredPermissionConfig.permission;
8598
if (permission == null) {
8699
// This indicates a configuration error in ModelRegistry
@@ -92,19 +105,20 @@ Middleware authorizationMiddleware() {
92105
'Internal Server Error: Authorization configuration error.',
93106
);
94107
}
95-
if (!permissionService.hasPermission(user, permission)) {
108+
// If user is null here, it means requiresAuthentication was false,
109+
// but specificPermission implies authentication. This is a misconfiguration
110+
// or an attempt to access a protected resource publicly.
111+
if (user == null || !permissionService.hasPermission(user, permission)) {
96112
throw const ForbiddenException(
97113
'You do not have permission to perform this action.',
98114
);
99115
}
100116
case RequiredPermissionType.unsupported:
101117
// This action is explicitly marked as not supported via this generic route.
102-
// Return Method Not Allowed.
103118
_log.warning(
104119
'Action for model "$modelName", method "$method" is marked as '
105120
'unsupported via generic route.',
106121
);
107-
// Throw ForbiddenException to be caught by the errorHandler
108122
throw ForbiddenException(
109123
'Method "$method" is not supported for model "$modelName" '
110124
'via this generic data endpoint.',

0 commit comments

Comments
 (0)