From 5a372bc9cc5249c7553c4441bb4c303e2583f078 Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 06:34:13 +0100 Subject: [PATCH 01/10] chore: remove unused countries client provider file - Deleted the entire countries_client_provider.dart file - This file was likely an unused example of the individual provider pattern - Its content is now redundant, as the pattern it demonstrated is already explained in comments in other files --- .../providers/countries_client_provider.dart | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 lib/src/providers/countries_client_provider.dart diff --git a/lib/src/providers/countries_client_provider.dart b/lib/src/providers/countries_client_provider.dart deleted file mode 100644 index 36747ed..0000000 --- a/lib/src/providers/countries_client_provider.dart +++ /dev/null @@ -1,39 +0,0 @@ -// Dart Frog Dependency Injection Pattern: Individual Providers -// -// This directory (`lib/src/providers`) and files like this one demonstrate -// a common pattern in Dart Frog for providing dependencies using dedicated -// middleware for each specific dependency (e.g., a client or repository). -// -// Example (Conceptual - Code Removed): -// ```dart -// // Middleware countriesClientProvider() { -// // final HtCountriesClient client = HtCountriesInMemoryClient(); -// // return provider((_) => client); -// // } -// ``` -// This middleware would then be `.use()`d in a relevant `_middleware.dart` file. -// -// --- Why This Pattern Isn't Used for Core Data Models in THIS Project --- -// -// While the individual provider pattern is valid, this specific project uses a -// slightly different approach for its main data models (Headline, Category, etc.) -// to support the generic `/api/v1/data` endpoint. -// -// Instead of individual provider middleware files here: -// 1. Instances of the core data repositories (`DataRepository`, -// `DataRepository`, etc.) are created and provided directly -// within the top-level `routes/_middleware.dart` file. -// 2. A `modelRegistry` (`lib/src/registry/model_registry.dart`) is used in -// conjunction with middleware at `routes/api/v1/data/_middleware.dart` to -// dynamically determine which model and repository to use based on the -// `?model=` query parameter in requests to `/api/v1/data`. -// -// This centralized approach in `routes/_middleware.dart` and the use of the -// registry were chosen to facilitate the generic nature of the `/api/v1/data` -// endpoint. -// -// This `providers` directory is kept primarily as a reference to the standard -// individual provider pattern or for potential future use with dependencies -// ignore_for_file: lines_longer_than_80_chars - -// that don't fit the generic data model structure. From 543df42671207ff5a83aaa0fd6983758c7f0f8ce Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 07:00:14 +0100 Subject: [PATCH 02/10] feat(ModelActionPermission): add requiresAuthentication flag - Add requiresAuthentication parameter to ModelActionPermission class - Update modelRegistry to include requiresAuthentication for all actions - Set default value to true for most actions, allowing unauthenticated access only to specific endpoints like remote_config GET --- lib/src/registry/model_registry.dart | 63 ++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/lib/src/registry/model_registry.dart b/lib/src/registry/model_registry.dart index acf7e78..0e1e9fb 100644 --- a/lib/src/registry/model_registry.dart +++ b/lib/src/registry/model_registry.dart @@ -31,6 +31,7 @@ class ModelActionPermission { required this.type, this.permission, this.requiresOwnershipCheck = false, + this.requiresAuthentication = true, }) : assert( type != RequiredPermissionType.specificPermission || permission != null, @@ -48,6 +49,13 @@ class ModelActionPermission { /// is the owner of the specific data item being accessed (for item-specific /// methods like GET, PUT, DELETE on `/[id]`). final bool requiresOwnershipCheck; + + /// Whether this action requires an authenticated user. + /// + /// If `true` (default), the `authenticationProvider` middleware will ensure + /// a valid [User] is present in the context. If `false`, the action can + /// be performed by unauthenticated clients. + final bool requiresAuthentication; } /// {@template model_config} @@ -126,19 +134,24 @@ final modelRegistry = >{ getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.headlineRead, + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.headlineRead, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), ), 'topic': ModelConfig( @@ -148,19 +161,24 @@ final modelRegistry = >{ getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.topicRead, + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.topicRead, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), ), 'source': ModelConfig( @@ -170,19 +188,24 @@ final modelRegistry = >{ getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.sourceRead, + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.sourceRead, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), ), 'country': ModelConfig( @@ -194,19 +217,24 @@ final modelRegistry = >{ getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.countryRead, + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.countryRead, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), ), 'language': ModelConfig( @@ -218,19 +246,24 @@ final modelRegistry = >{ getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.languageRead, + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.languageRead, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), ), 'user': ModelConfig( @@ -240,25 +273,30 @@ final modelRegistry = >{ (item as User).id as String?, // User is the owner of their profile getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, // Only admin can list all users + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.userReadOwned, // User can read their own requiresOwnershipCheck: true, // Must be the owner + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType .unsupported, // User creation handled by auth routes + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.userUpdateOwned, // User can update their own requiresOwnershipCheck: true, // Must be the owner + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.userDeleteOwned, // User can delete their own requiresOwnershipCheck: true, // Must be the owner + requiresAuthentication: true, ), ), 'user_app_settings': ModelConfig( @@ -268,14 +306,17 @@ final modelRegistry = >{ (item as UserAppSettings).id as String?, // User ID is the owner ID getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, // Not accessible via collection + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.userAppSettingsReadOwned, requiresOwnershipCheck: true, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, // Creation of UserAppSettings is handled by the authentication service // during user creation, not via a direct POST to /api/v1/data. ), @@ -283,9 +324,11 @@ final modelRegistry = >{ type: RequiredPermissionType.specificPermission, permission: Permissions.userAppSettingsUpdateOwned, requiresOwnershipCheck: true, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, // Deletion of UserAppSettings is handled by the authentication service // during account deletion, not via a direct DELETE to /api/v1/data. ), @@ -298,14 +341,17 @@ final modelRegistry = >{ as String?, // User ID is the owner ID getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, // Not accessible via collection + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.userContentPreferencesReadOwned, requiresOwnershipCheck: true, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, // Creation of UserContentPreferences is handled by the authentication // service during user creation, not via a direct POST to /api/v1/data. ), @@ -313,9 +359,11 @@ final modelRegistry = >{ type: RequiredPermissionType.specificPermission, permission: Permissions.userContentPreferencesUpdateOwned, requiresOwnershipCheck: true, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, // Deletion of UserContentPreferences is handled by the authentication // service during account deletion, not via a direct DELETE to /api/v1/data. ), @@ -326,19 +374,24 @@ final modelRegistry = >{ getOwnerId: null, // RemoteConfig is a global resource, not user-owned getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, // Not accessible via collection + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.remoteConfigRead, + requiresAuthentication: false, // Make remote_config GET public ), postPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, // Only administrators can create + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, // Only administrators can update + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, // Only administrators can delete + requiresAuthentication: true, ), ), 'dashboard_summary': ModelConfig( @@ -348,18 +401,23 @@ final modelRegistry = >{ // Permissions: Read-only for admins, all other actions unsupported. getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.unsupported, + requiresAuthentication: true, ), ), 'local_ad': ModelConfig( @@ -369,22 +427,27 @@ final modelRegistry = >{ getCollectionPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.localAdRead, + requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.localAdRead, + requiresAuthentication: true, ), postPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, permission: Permissions.localAdCreate, + requiresAuthentication: true, ), putPermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, permission: Permissions.localAdUpdate, + requiresAuthentication: true, ), deletePermission: const ModelActionPermission( type: RequiredPermissionType.adminOnly, permission: Permissions.localAdDelete, + requiresAuthentication: true, ), ), }; From 14ba5d4a9fe567c696f902384b4eb381fde7b7ca Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 07:02:36 +0100 Subject: [PATCH 03/10] feat(data): implement conditional authentication middleware - Add _conditionalAuthenticationMiddleware to check ModelConfig for authentication requirements - Update middleware stack to use _conditionalAuthenticationMiddleware instead of requireAuthentication - Improve code documentation for data route middlewares --- routes/api/v1/data/_middleware.dart | 73 ++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 12 deletions(-) diff --git a/routes/api/v1/data/_middleware.dart b/routes/api/v1/data/_middleware.dart index c5b67f9..1ff24bc 100644 --- a/routes/api/v1/data/_middleware.dart +++ b/routes/api/v1/data/_middleware.dart @@ -36,7 +36,7 @@ Middleware _dataRateLimiterMiddleware() { }; } -// Helper middleware for model validation and context provision. +/// Helper middleware for model validation and context provision. Middleware _modelValidationAndProviderMiddleware() { return (handler) { // This 'handler' is the next handler in the chain, @@ -76,6 +76,56 @@ Middleware _modelValidationAndProviderMiddleware() { }; } +/// Helper middleware to conditionally apply authentication based on +/// `ModelConfig`. +/// +/// This middleware checks the `requiresAuthentication` flag on the +/// `ModelActionPermission` for the current model and HTTP method. +/// If authentication is required, it calls `requireAuthentication()`. +/// If not, it simply passes the request through, allowing public access. +Middleware _conditionalAuthenticationMiddleware() { + return (handler) { + return (context) { + final modelConfig = context.read>(); + final method = context.request.method; + + ModelActionPermission requiredPermissionConfig; + switch (method) { + case HttpMethod.get: + // Differentiate GET based on whether it's a collection or item request + final isCollectionRequest = + context.request.uri.path == '/api/v1/data'; + if (isCollectionRequest) { + requiredPermissionConfig = modelConfig.getCollectionPermission; + } else { + requiredPermissionConfig = modelConfig.getItemPermission; + } + case HttpMethod.post: + requiredPermissionConfig = modelConfig.postPermission; + case HttpMethod.put: + requiredPermissionConfig = modelConfig.putPermission; + case HttpMethod.delete: + requiredPermissionConfig = modelConfig.deletePermission; + default: + // For unsupported methods, assume authentication is required + // or let subsequent middleware/route handler deal with it. + requiredPermissionConfig = const ModelActionPermission( + type: RequiredPermissionType.unsupported, + requiresAuthentication: true, + ); + } + + if (requiredPermissionConfig.requiresAuthentication) { + // If authentication is required, apply the requireAuthentication middleware. + return requireAuthentication()(handler)(context); + } else { + // If authentication is not required, simply pass the request through. + return handler(context); + } + }; + }; +} + // Main middleware exported for the /api/v1/data route group. Handler middleware(Handler handler) { // This 'handler' is the actual route handler from index.dart or [id].dart. @@ -84,17 +134,15 @@ Handler middleware(Handler handler) { // the last .use() call in the chain represents the outermost middleware layer. // Therefore, the execution order for an incoming request is: // - // 1. `requireAuthentication()`: - // - This runs first. It relies on `authenticationProvider()` (from the - // parent `/api/v1/_middleware.dart`) having already attempted to - // authenticate the user and provide `User?` into the context. - // - If `User` is null (no valid authentication), `requireAuthentication()` - // throws an `UnauthorizedException`, and the request is aborted (usually - // resulting in a 401 response via the global `errorHandler`). - // - If `User` is present, the request proceeds to the next middleware. + // 1. `_conditionalAuthenticationMiddleware()`: + // - This runs first. It dynamically decides whether to apply + // `requireAuthentication()` based on the `ModelConfig` for the + // requested model and HTTP method. + // - If authentication is required and the user is not authenticated, + // it throws an `UnauthorizedException`. // // 2. `_dataRateLimiterMiddleware()`: - // - This runs if `requireAuthentication()` passes. + // - This runs if authentication (if required) passes. // - It checks if the user has a bypass permission. If not, it applies // the configured rate limit based on the user's ID. // - If the limit is exceeded, it throws a `ForbiddenException`. @@ -117,7 +165,8 @@ Handler middleware(Handler handler) { // // 5. Actual Route Handler (from `index.dart` or `[id].dart`): // - This runs last, only if all preceding middlewares pass. It will have - // access to a non-null `User`, `ModelConfig`, and `modelName` from the context. + // access to a non-null `User` (if authenticated), `ModelConfig`, and + // `modelName` from the context. // - It performs the data operation and any necessary handler-level // ownership checks (if flagged by `ModelActionPermission.requiresOwnershipCheck`). // @@ -125,5 +174,5 @@ Handler middleware(Handler handler) { .use(authorizationMiddleware()) // Applied fourth (inner-most) .use(_modelValidationAndProviderMiddleware()) // Applied third .use(_dataRateLimiterMiddleware()) // Applied second - .use(requireAuthentication()); // Applied first (outermost) + .use(_conditionalAuthenticationMiddleware()); // Applied first (outermost) } From 3edf699ff096364a96d4146970d5ff1f6020df2d Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 07:55:07 +0100 Subject: [PATCH 04/10] feat(data): implement IP-based rate limiting for public routes - Add IP-based rate limiting for unauthenticated users - Implement nullable User handling for consistent context - Update middleware execution order and comments --- routes/api/v1/data/_middleware.dart | 67 ++++++++++++++++++----------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/routes/api/v1/data/_middleware.dart b/routes/api/v1/data/_middleware.dart index 1ff24bc..c13ba99 100644 --- a/routes/api/v1/data/_middleware.dart +++ b/routes/api/v1/data/_middleware.dart @@ -11,24 +11,38 @@ import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_ // Helper middleware for applying rate limiting to the data routes. Middleware _dataRateLimiterMiddleware() { return (handler) { - return (context) { - final user = context.read(); + return (context) async { // Made async because ipKeyExtractor is async + final user = context.read(); // Read nullable User final permissionService = context.read(); // Users with the bypass permission are not rate-limited. - if (permissionService.hasPermission( + if (user != null && permissionService.hasPermission( user, Permissions.rateLimitingBypass, )) { return handler(context); } - // For all other users, apply the configured rate limit. - // The key is the user's ID, ensuring the limit is per-user. + String? key; + // If user is null, it means it's a public route (as per _conditionalAuthenticationMiddleware) + // In this case, use IP-based rate limiting. + if (user == null) { + key = await ipKeyExtractor(context); + } else { + // Authenticated user: use user ID for rate limiting. + key = user.id; + } + + // If a key cannot be extracted (e.g., no IP), bypass rate limiter. + if (key == null || key.isEmpty) { + return handler(context); + } + + // For all other users (or IPs for public routes), apply the configured rate limit. final rateLimitHandler = rateLimiter( limit: EnvironmentConfig.rateLimitDataApiLimit, window: EnvironmentConfig.rateLimitDataApiWindow, - keyExtractor: (context) async => context.read().id, + keyExtractor: (context) async => key, // Use the determined key )(handler); return rateLimitHandler(context); @@ -120,7 +134,9 @@ Middleware _conditionalAuthenticationMiddleware() { return requireAuthentication()(handler)(context); } else { // If authentication is not required, simply pass the request through. - return handler(context); + // Also provide a null User to the context for consistency, + // so downstream middleware can always read User? + return handler(context.provide(() => null)); } }; }; @@ -134,28 +150,31 @@ Handler middleware(Handler handler) { // the last .use() call in the chain represents the outermost middleware layer. // Therefore, the execution order for an incoming request is: // - // 1. `_conditionalAuthenticationMiddleware()`: - // - This runs first. It dynamically decides whether to apply + // 1. `_modelValidationAndProviderMiddleware()`: + // - This runs first. It validates the `?model=` query parameter and + // provides the `ModelConfig` and `modelName` into the context. + // - If model validation fails, it throws a `BadRequestException`. + // + // 2. `_conditionalAuthenticationMiddleware()`: + // - This runs second. It dynamically decides whether to apply // `requireAuthentication()` based on the `ModelConfig` for the // requested model and HTTP method. // - If authentication is required and the user is not authenticated, // it throws an `UnauthorizedException`. + // - If authentication is NOT required, it provides `User?` as `null` + // to the context. // - // 2. `_dataRateLimiterMiddleware()`: - // - This runs if authentication (if required) passes. - // - It checks if the user has a bypass permission. If not, it applies - // the configured rate limit based on the user's ID. + // 3. `_dataRateLimiterMiddleware()`: + // - This runs third. It checks if the user has a bypass permission. + // If not, it applies the configured rate limit based on the user's ID + // (if authenticated) or the client's IP address (if unauthenticated). // - If the limit is exceeded, it throws a `ForbiddenException`. // - // 3. `_modelValidationAndProviderMiddleware()`: - // - This runs if rate limiting passes. - // - It validates the `?model=` query parameter and provides the - // `ModelConfig` and `modelName` into the context. - // - If model validation fails, it throws a `BadRequestException`. - // // 4. `authorizationMiddleware()`: - // - This runs if `_modelValidationAndProviderMiddleware()` passes. - // - It reads the `User`, `modelName`, and `ModelConfig` from the context. + // - This runs fourth. It reads the `User` (which is guaranteed to be + // non-null if authentication was required and passed) or `User?` (if + // authentication was not required), `modelName`, and `ModelConfig` + // from the context. // - It checks if the user has permission to perform the requested HTTP // method on the specified model based on the `ModelConfig` metadata. // - If authorization fails, it throws a ForbiddenException, caught by @@ -172,7 +191,7 @@ Handler middleware(Handler handler) { // return handler .use(authorizationMiddleware()) // Applied fourth (inner-most) - .use(_modelValidationAndProviderMiddleware()) // Applied third - .use(_dataRateLimiterMiddleware()) // Applied second - .use(_conditionalAuthenticationMiddleware()); // Applied first (outermost) + .use(_dataRateLimiterMiddleware()) // Applied third + .use(_conditionalAuthenticationMiddleware()) // Applied second + .use(_modelValidationAndProviderMiddleware()); // Applied first (outermost) } From afbde1ae0267e3ee7badce328c1962ea3b2983d0 Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 08:21:30 +0100 Subject: [PATCH 05/10] 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 --- .../middlewares/authorization_middleware.dart | 54 ++++++++++++------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/lib/src/middlewares/authorization_middleware.dart b/lib/src/middlewares/authorization_middleware.dart index c89fb24..06f975d 100644 --- a/lib/src/middlewares/authorization_middleware.dart +++ b/lib/src/middlewares/authorization_middleware.dart @@ -9,14 +9,16 @@ final _log = Logger('AuthorizationMiddleware'); /// {@template authorization_middleware} /// Middleware to enforce role-based permissions and model-specific access rules. /// -/// This middleware reads the authenticated [User], the requested `modelName`, -/// the `HttpMethod`, and the `ModelConfig` from the request context. It then -/// determines the required permission based on the `ModelConfig` metadata for -/// the specific HTTP method and checks if the authenticated user has that -/// permission using the [PermissionService]. +/// This middleware reads the authenticated [User] (which may be null for +/// public routes), the requested `modelName`, the `HttpMethod`, and the +/// `ModelConfig` from the request context. It then determines the required +/// permission based on the `ModelConfig` metadata for the specific HTTP method +/// and checks if the authenticated user has that permission using the +/// [PermissionService]. /// -/// If the user does not have the required permission, it throws a -/// [ForbiddenException], which should be caught by the 'errorHandler' middleware. +/// If the user does not have the required permission, or if authentication is +/// required but no user is present, it throws a [ForbiddenException] or +/// [UnauthorizedException], which should be caught by the 'errorHandler' middleware. /// /// This middleware runs *after* authentication and model validation. /// It does NOT perform instance-level ownership checks; those are handled @@ -27,8 +29,9 @@ Middleware authorizationMiddleware() { return (handler) { return (context) async { // Read dependencies from the context. - // User is guaranteed non-null by requireAuthentication() middleware. - final user = context.read(); + // User is now read as nullable, as _conditionalAuthenticationMiddleware + // might provide null for public routes. + final user = context.read(); final permissionService = context.read(); final modelName = context.read(); // Provided by data/_middleware final modelConfig = context @@ -64,23 +67,33 @@ Middleware authorizationMiddleware() { ); } - // Perform the permission check based on the configuration type + // Handle authentication requirement based on ModelConfig --- + if (requiredPermissionConfig.requiresAuthentication && user == null) { + _log.warning( + 'Authorization required but no valid user found. Denying access.', + ); + throw const UnauthorizedException('Authentication required.'); + } + + // If authentication is not required, or if user is present, proceed with permission checks. + // All subsequent checks must now handle a potentially null 'user' if 'requiresAuthentication' is false. switch (requiredPermissionConfig.type) { case RequiredPermissionType.none: - // No specific permission required (beyond authentication if applicable) - // This case is primarily for documentation/completeness if a route - // group didn't require authentication, but the /data route does. - // For the /data route, 'none' effectively means 'authenticated users allowed'. + // No specific permission required. + // If user is null, it's a public route. If user is not null, they are authenticated. break; case RequiredPermissionType.adminOnly: - // Requires the user to be an admin - if (!permissionService.isAdmin(user)) { + // Requires the user to be an admin. + // If user is null here, it means requiresAuthentication was false, + // but adminOnly implies authentication. This is a misconfiguration + // or an attempt to access an admin-only resource publicly. + if (user == null || !permissionService.isAdmin(user)) { throw const ForbiddenException( 'Only administrators can perform this action.', ); } case RequiredPermissionType.specificPermission: - // Requires a specific permission string + // Requires a specific permission string. final permission = requiredPermissionConfig.permission; if (permission == null) { // This indicates a configuration error in ModelRegistry @@ -92,19 +105,20 @@ Middleware authorizationMiddleware() { 'Internal Server Error: Authorization configuration error.', ); } - if (!permissionService.hasPermission(user, permission)) { + // If user is null here, it means requiresAuthentication was false, + // but specificPermission implies authentication. This is a misconfiguration + // or an attempt to access a protected resource publicly. + if (user == null || !permissionService.hasPermission(user, permission)) { throw const ForbiddenException( 'You do not have permission to perform this action.', ); } case RequiredPermissionType.unsupported: // This action is explicitly marked as not supported via this generic route. - // Return Method Not Allowed. _log.warning( 'Action for model "$modelName", method "$method" is marked as ' 'unsupported via generic route.', ); - // Throw ForbiddenException to be caught by the errorHandler throw ForbiddenException( 'Method "$method" is not supported for model "$modelName" ' 'via this generic data endpoint.', From b46bfcab6cf889a8822d1381c1b99becb1c12650 Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 08:37:11 +0100 Subject: [PATCH 06/10] fix(middlewares): improve ownership check middleware - Add logging to track middleware execution flow - Handle null user scenario for ownership check - Throw exception for misconfigured public routes requiring ownership check - Improve error handling and messaging for configuration errors - Optimize ownership check logic for better readability and performance --- .../ownership_check_middleware.dart | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/lib/src/middlewares/ownership_check_middleware.dart b/lib/src/middlewares/ownership_check_middleware.dart index befab4a..b4cb21e 100644 --- a/lib/src/middlewares/ownership_check_middleware.dart +++ b/lib/src/middlewares/ownership_check_middleware.dart @@ -2,6 +2,9 @@ import 'package:core/core.dart'; import 'package:dart_frog/dart_frog.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/rbac/permission_service.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_registry.dart'; +import 'package:logging/logging.dart'; + +final _log = Logger('OwnershipCheckMiddleware'); /// A wrapper class to provide a fetched item into the request context. /// @@ -33,8 +36,9 @@ class FetchedItem { Middleware ownershipCheckMiddleware() { return (handler) { return (context) async { + _log.finer('Entering ownershipCheckMiddleware.'); final modelConfig = context.read>(); - final user = context.read(); + final user = context.read(); final permissionService = context.read(); final method = context.request.method; @@ -49,20 +53,45 @@ Middleware ownershipCheckMiddleware() { permission = modelConfig.deletePermission; default: // For any other methods, no ownership check is performed. + _log.finer('Method "$method" does not require ownership check. Skipping.'); return handler(context); } // If no ownership check is required for this action, or if the user is // an admin (who bypasses ownership checks), proceed immediately. if (!permission.requiresOwnershipCheck || - permissionService.isAdmin(user)) { + (user != null && permissionService.isAdmin(user))) { + _log.finer( + 'Ownership check not required or user is admin. Skipping ownership check.', + ); return handler(context); } // At this point, an ownership check is required for a non-admin user. + _log.finer( + 'Ownership check required for model "${context.read()}", ' + 'method "$method". User is not admin.', + ); + + // If an ownership check IS required, we must have an authenticated user. + // If user is null here, it means a public route is configured to require + // an ownership check, which is a misconfiguration. + if (user == null) { + _log.warning( + 'Ownership check required but no authenticated user found. ' + 'This indicates a configuration error.', + ); + throw const OperationFailedException( + 'Internal Server Error: Ownership check required for unauthenticated access.', + ); + } // Ensure the model is configured to support ownership checks. if (modelConfig.getOwnerId == null) { + _log.severe( + 'Configuration Error: Model "${context.read()}" requires ' + 'ownership check but getOwnerId is null.', + ); throw const OperationFailedException( 'Internal Server Error: Model configuration error for ownership check.', ); @@ -76,10 +105,15 @@ Middleware ownershipCheckMiddleware() { // Compare the item's owner ID with the authenticated user's ID. final itemOwnerId = modelConfig.getOwnerId!(item); if (itemOwnerId != user.id) { + _log.warning( + 'Ownership check failed for user ${user.id} on item with owner ID ' + '$itemOwnerId. Denying access.', + ); throw const ForbiddenException( 'You do not have permission to access this item.', ); } + _log.finer('Ownership check passed for user ${user.id}.'); // If the ownership check passes, proceed to the final route handler. return handler(context); From 2c42f938bf6bb43feab4f9a96b3a1c688f5d36be Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 08:37:25 +0100 Subject: [PATCH 07/10] feat(data): support unauthenticated requests and improve documentation - Update authenticatedUser to be nullable to support unauthenticated requests - Add comments to explain user ownership and authentication checks - Refactor user ID determination logic for repository calls --- routes/api/v1/data/index.dart | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/routes/api/v1/data/index.dart b/routes/api/v1/data/index.dart index 0b8a679..a4c708a 100644 --- a/routes/api/v1/data/index.dart +++ b/routes/api/v1/data/index.dart @@ -29,7 +29,8 @@ Future onRequest(RequestContext context) async { Future _handleGet(RequestContext context) async { final modelName = context.read(); final modelConfig = context.read>(); - final authenticatedUser = context.read(); + // Read authenticatedUser as nullable, as per configurable authentication. + final authenticatedUser = context.read(); final params = context.request.uri.queryParameters; _logger @@ -73,8 +74,12 @@ Future _handleGet(RequestContext context) async { ) : null; - final userIdForRepoCall = - (modelConfig.getOwnerId != null && + // Determine userId for repository call. + // If the model is user-owned and the user is authenticated and not an admin, + // then the operation should be scoped to the authenticated user's ID. + // Otherwise, it's a global operation or an admin bypass. + final userIdForRepoCall = (modelConfig.getOwnerId != null && + authenticatedUser != null && !context.read().isAdmin(authenticatedUser)) ? authenticatedUser.id : null; @@ -101,7 +106,8 @@ Future _handleGet(RequestContext context) async { Future _handlePost(RequestContext context) async { final modelName = context.read(); final modelConfig = context.read>(); - final authenticatedUser = context.read(); + // Read authenticatedUser as nullable, as per configurable authentication. + final authenticatedUser = context.read(); _logger.info('Handling POST request for model "$modelName".'); @@ -124,8 +130,12 @@ Future _handlePost(RequestContext context) async { ); } - final userIdForRepoCall = - (modelConfig.getOwnerId != null && + // Determine userId for repository call. + // If the model is user-owned and the user is authenticated and not an admin, + // then the operation should be scoped to the authenticated user's ID. + // Otherwise, it's a global operation or an admin bypass. + final userIdForRepoCall = (modelConfig.getOwnerId != null && + authenticatedUser != null && !context.read().isAdmin(authenticatedUser)) ? authenticatedUser.id : null; From dbc28dfbf6543046cb2618c35523d7c175da6dd8 Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 08:37:36 +0100 Subject: [PATCH 08/10] fix(api): handle unauthenticated user in data routes - Update authenticated user type from `User` to `User?` in GET and DELETE handlers - Add check for authentication when updating user content preferences - Adjust user ID retrieval logic for repository calls --- routes/api/v1/data/[id]/index.dart | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/routes/api/v1/data/[id]/index.dart b/routes/api/v1/data/[id]/index.dart index 9924660..4744c1d 100644 --- a/routes/api/v1/data/[id]/index.dart +++ b/routes/api/v1/data/[id]/index.dart @@ -55,7 +55,7 @@ Future _handleGet(RequestContext context, String id) async { Future _handlePut(RequestContext context, String id) async { final modelName = context.read(); final modelConfig = context.read>(); - final authenticatedUser = context.read(); + final authenticatedUser = context.read(); final permissionService = context.read(); final userPreferenceLimitService = context.read(); @@ -92,6 +92,12 @@ Future _handlePut(RequestContext context, String id) async { } if (modelName == 'user_content_preferences') { + // User content preferences can only be updated by an authenticated user. + if (authenticatedUser == null) { + throw const UnauthorizedException( + 'Authentication required to update user content preferences.', + ); + } if (itemToUpdate is UserContentPreferences) { await userPreferenceLimitService.checkUpdatePreferences( authenticatedUser, @@ -133,7 +139,7 @@ Future _handlePut(RequestContext context, String id) async { Future _handleDelete(RequestContext context, String id) async { final modelName = context.read(); final modelConfig = context.read>(); - final authenticatedUser = context.read(); + final authenticatedUser = context.read(); final permissionService = context.read(); _logger.info('Handling DELETE request for model "$modelName", id "$id".'); @@ -155,12 +161,20 @@ Future _handleDelete(RequestContext context, String id) async { /// Determines the `userId` to be used for a repository call based on user /// role and model configuration. +/// +/// If the model is user-owned and the authenticated user is not an admin, +/// the authenticated user's ID is returned. Otherwise, `null` is returned, +/// indicating a global operation or an admin-level bypass. String? _getUserIdForRepoCall({ required ModelConfig modelConfig, required PermissionService permissionService, - required User authenticatedUser, + required User? authenticatedUser, }) { + // If the model is user-owned and the user is authenticated and not an admin, + // then the operation should be scoped to the authenticated user's ID. + // Otherwise, it's a global operation or an admin bypass. return (modelConfig.getOwnerId != null && + authenticatedUser != null && !permissionService.isAdmin(authenticatedUser)) ? authenticatedUser.id : null; From f46c472f759080a6a89e38b5675df167799a2d74 Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 08:41:44 +0100 Subject: [PATCH 09/10] fix(remote_config): make GET request public Make the getItemPermission for remote_config GET request public by setting its type to RequiredPermissionType.none and requiresAuthentication to false. This change allows unauthenticated access to retrieve remote_config data. --- lib/src/registry/model_registry.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/registry/model_registry.dart b/lib/src/registry/model_registry.dart index 0e1e9fb..046559d 100644 --- a/lib/src/registry/model_registry.dart +++ b/lib/src/registry/model_registry.dart @@ -377,8 +377,7 @@ final modelRegistry = >{ requiresAuthentication: true, ), getItemPermission: const ModelActionPermission( - type: RequiredPermissionType.specificPermission, - permission: Permissions.remoteConfigRead, + type: RequiredPermissionType.none, requiresAuthentication: false, // Make remote_config GET public ), postPermission: const ModelActionPermission( From c8455e1775f8362893564a53b7ea374e2b632b4b Mon Sep 17 00:00:00 2001 From: fulleni Date: Mon, 15 Sep 2025 08:44:45 +0100 Subject: [PATCH 10/10] style: format misc --- .../middlewares/authorization_middleware.dart | 3 ++- .../middlewares/ownership_check_middleware.dart | 4 +++- routes/api/v1/data/_middleware.dart | 16 ++++++++++------ routes/api/v1/data/index.dart | 6 ++++-- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/lib/src/middlewares/authorization_middleware.dart b/lib/src/middlewares/authorization_middleware.dart index 06f975d..238ccec 100644 --- a/lib/src/middlewares/authorization_middleware.dart +++ b/lib/src/middlewares/authorization_middleware.dart @@ -108,7 +108,8 @@ Middleware authorizationMiddleware() { // If user is null here, it means requiresAuthentication was false, // but specificPermission implies authentication. This is a misconfiguration // or an attempt to access a protected resource publicly. - if (user == null || !permissionService.hasPermission(user, permission)) { + if (user == null || + !permissionService.hasPermission(user, permission)) { throw const ForbiddenException( 'You do not have permission to perform this action.', ); diff --git a/lib/src/middlewares/ownership_check_middleware.dart b/lib/src/middlewares/ownership_check_middleware.dart index b4cb21e..8c76a75 100644 --- a/lib/src/middlewares/ownership_check_middleware.dart +++ b/lib/src/middlewares/ownership_check_middleware.dart @@ -53,7 +53,9 @@ Middleware ownershipCheckMiddleware() { permission = modelConfig.deletePermission; default: // For any other methods, no ownership check is performed. - _log.finer('Method "$method" does not require ownership check. Skipping.'); + _log.finer( + 'Method "$method" does not require ownership check. Skipping.', + ); return handler(context); } diff --git a/routes/api/v1/data/_middleware.dart b/routes/api/v1/data/_middleware.dart index c13ba99..3dc6723 100644 --- a/routes/api/v1/data/_middleware.dart +++ b/routes/api/v1/data/_middleware.dart @@ -11,15 +11,17 @@ import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_ // Helper middleware for applying rate limiting to the data routes. Middleware _dataRateLimiterMiddleware() { return (handler) { - return (context) async { // Made async because ipKeyExtractor is async + return (context) async { + // Made async because ipKeyExtractor is async final user = context.read(); // Read nullable User final permissionService = context.read(); // Users with the bypass permission are not rate-limited. - if (user != null && permissionService.hasPermission( - user, - Permissions.rateLimitingBypass, - )) { + if (user != null && + permissionService.hasPermission( + user, + Permissions.rateLimitingBypass, + )) { return handler(context); } @@ -193,5 +195,7 @@ Handler middleware(Handler handler) { .use(authorizationMiddleware()) // Applied fourth (inner-most) .use(_dataRateLimiterMiddleware()) // Applied third .use(_conditionalAuthenticationMiddleware()) // Applied second - .use(_modelValidationAndProviderMiddleware()); // Applied first (outermost) + .use( + _modelValidationAndProviderMiddleware(), + ); // Applied first (outermost) } diff --git a/routes/api/v1/data/index.dart b/routes/api/v1/data/index.dart index a4c708a..ef00241 100644 --- a/routes/api/v1/data/index.dart +++ b/routes/api/v1/data/index.dart @@ -78,7 +78,8 @@ Future _handleGet(RequestContext context) async { // If the model is user-owned and the user is authenticated and not an admin, // then the operation should be scoped to the authenticated user's ID. // Otherwise, it's a global operation or an admin bypass. - final userIdForRepoCall = (modelConfig.getOwnerId != null && + final userIdForRepoCall = + (modelConfig.getOwnerId != null && authenticatedUser != null && !context.read().isAdmin(authenticatedUser)) ? authenticatedUser.id @@ -134,7 +135,8 @@ Future _handlePost(RequestContext context) async { // If the model is user-owned and the user is authenticated and not an admin, // then the operation should be scoped to the authenticated user's ID. // Otherwise, it's a global operation or an admin bypass. - final userIdForRepoCall = (modelConfig.getOwnerId != null && + final userIdForRepoCall = + (modelConfig.getOwnerId != null && authenticatedUser != null && !context.read().isAdmin(authenticatedUser)) ? authenticatedUser.id