From b9e957bc38ba974e0fbb583454043c78ebe70c6a Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 1 Nov 2025 07:05:54 +0100 Subject: [PATCH 1/7] docs(permissions): Clarify userUpdate permission scope Updated the documentation for the `user.update` permission to explicitly state it is an administrator-level permission, distinguishing it from `user.update_owned`. --- lib/src/rbac/permissions.dart | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/src/rbac/permissions.dart b/lib/src/rbac/permissions.dart index 31c6342..cdbbc55 100644 --- a/lib/src/rbac/permissions.dart +++ b/lib/src/rbac/permissions.dart @@ -44,12 +44,11 @@ abstract class Permissions { // Allows deleting the authenticated user's own account static const String userDeleteOwned = 'user.delete_owned'; - // Allows creating a new user (admin-only). - static const String userCreate = 'user.create'; + // Allows updating any user's profile (admin-only). + // This is distinct from `userUpdateOwned`, which allows a user to update + // their own record. static const String userUpdate = 'user.update'; - // Allows deleting any user's account (admin-only). - static const String userDelete = 'user.delete'; // User App Settings Permissions (User-owned) static const String userAppSettingsReadOwned = 'user_app_settings.read_owned'; From f2849cae4d0936a8c44845e60f06eb0aaa8a230f Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 1 Nov 2025 07:06:23 +0100 Subject: [PATCH 2/7] refactor(rbac): Remove user create/delete permissions from admin role Removed `Permissions.userCreate` and `Permissions.userDelete` from the `_dashboardAdminPermissions` set. This change enforces the rule that administrators can only update users through the generic data API, while creation and deletion are handled exclusively by the authentication service. --- lib/src/rbac/role_permissions.dart | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/src/rbac/role_permissions.dart b/lib/src/rbac/role_permissions.dart index f0cc81f..303f069 100644 --- a/lib/src/rbac/role_permissions.dart +++ b/lib/src/rbac/role_permissions.dart @@ -68,11 +68,12 @@ final Set _dashboardAdminPermissions = { Permissions.languageCreate, Permissions.languageUpdate, Permissions.languageDelete, - Permissions.userRead, // Allows reading any user's profile. - // Allow full user account management for admins. - Permissions.userCreate, + // Allows reading any user's profile. + Permissions.userRead, + // Allows updating any user's profile (e.g., changing their roles). + // User creation and deletion are handled by the auth service, not the + // generic data API. Permissions.userUpdate, - Permissions.userDelete, Permissions.remoteConfigCreate, Permissions.remoteConfigUpdate, Permissions.remoteConfigDelete, From 35f3d5eb12882aea5758f12d05439e7b80b4c7a8 Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 1 Nov 2025 07:06:49 +0100 Subject: [PATCH 3/7] refactor(registry): Disable user creation/deletion via data API Updated the `ModelConfig` for the `user` model to mark `POST` (create) and `DELETE` operations as `RequiredPermissionType.unsupported`. This change enforces that user lifecycle management is handled exclusively by the authentication service, not the generic data endpoint. Added comments to clarify the update logic flow. --- lib/src/registry/model_registry.dart | 31 ++++++++++++---------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/lib/src/registry/model_registry.dart b/lib/src/registry/model_registry.dart index bbe9ce3..b32e96c 100644 --- a/lib/src/registry/model_registry.dart +++ b/lib/src/registry/model_registry.dart @@ -281,33 +281,28 @@ final modelRegistry = >{ requiresOwnershipCheck: true, // Must be the owner requiresAuthentication: true, ), - // Admins can create users via the data endpoint. - // User creation via auth routes (e.g., sign-up) is separate. + // User creation is handled exclusively by the authentication service + // (e.g., during sign-up) and is not supported via the generic data API. postPermission: const ModelActionPermission( - type: RequiredPermissionType.specificPermission, - permission: Permissions.userCreate, - requiresAuthentication: true, + type: RequiredPermissionType.unsupported, ), - // An admin can update any user's roles. - // A regular user can update specific fields on their own profile - // (e.g., feedDecoratorStatus), which is handled by the updater logic - // in DataOperationRegistry. The ownership check ensures they can only - // access their own user object to begin with. + // User updates are handled by a custom updater in DataOperationRegistry. + // - Admins can update roles (`appRole`, `dashboardRole`). + // - Users can update their own `feedDecoratorStatus` and `email`. + // The `userUpdateOwned` permission, combined with the ownership check, + // provides the entry point for both admins (who bypass ownership checks) + // and users to target a user object for an update. putPermission: const ModelActionPermission( type: RequiredPermissionType.specificPermission, permission: Permissions.userUpdateOwned, // User can update their own requiresOwnershipCheck: true, // Must be the owner requiresAuthentication: true, ), - // An admin can delete any user. - // A regular user can delete their own account. - // The ownership check middleware is bypassed for admins, so this single - // config works for both roles. + // User deletion is handled exclusively by the authentication service + // (e.g., via a dedicated "delete account" endpoint) and is not + // supported via the generic data API. deletePermission: const ModelActionPermission( - type: RequiredPermissionType.specificPermission, - permission: Permissions.userDeleteOwned, // User can delete their own - requiresOwnershipCheck: true, // Must be the owner - requiresAuthentication: true, + type: RequiredPermissionType.unsupported, ), ), 'user_app_settings': ModelConfig( From f61fbf303854d3bd8505377ebafad7ee5b49a658 Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 1 Nov 2025 07:08:14 +0100 Subject: [PATCH 4/7] 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` contract. --- lib/src/registry/data_operation_registry.dart | 120 +++++++++++------- 1 file changed, 76 insertions(+), 44 deletions(-) diff --git a/lib/src/registry/data_operation_registry.dart b/lib/src/registry/data_operation_registry.dart index b51146d..be74574 100644 --- a/lib/src/registry/data_operation_registry.dart +++ b/lib/src/registry/data_operation_registry.dart @@ -2,8 +2,10 @@ import 'package:core/core.dart'; import 'package:dart_frog/dart_frog.dart'; import 'package:data_repository/data_repository.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/middlewares/ownership_check_middleware.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/services/country_query_service.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/services/dashboard_summary_service.dart'; +import 'package:logging/logging.dart'; // --- Typedefs for Data Operations --- @@ -54,6 +56,9 @@ typedef ItemDeleter = /// data operations are performed for each model, improving consistency across /// the API. /// {@endtemplate} + +final _log = Logger('DataOperationRegistry'); + class DataOperationRegistry { /// {@macro data_operation_registry} DataOperationRegistry() { @@ -188,11 +193,6 @@ class DataOperationRegistry { item: item as Language, userId: uid, ), - // Handler for creating a new user. - 'user': (c, item, uid) => c.read>().create( - item: item as User, - userId: uid, - ), 'remote_config': (c, item, uid) => c .read>() .create(item: item as RemoteConfig, userId: uid), @@ -225,56 +225,90 @@ class DataOperationRegistry { 'language': (c, id, item, uid) => c .read>() .update(id: id, item: item as Language, userId: uid), - // Custom updater for the 'user' model. - // This updater handles two distinct use cases: - // 1. Admins updating user roles (`appRole`, `dashboardRole`). - // 2. Regular users updating their own `feedDecoratorStatus`. - // It accepts a raw Map as the `item` to prevent - // mass assignment vulnerabilities, only applying allowed fields. - 'user': (c, id, item, uid) { - final repo = c.read>(); - final existingUser = c.read>().data as User; + // Custom updater for the 'user' model. This logic is critical for + // security and architectural consistency. + // + // It enforces the following rules: + // 1. Admins can ONLY update a user's `appRole` and `dashboardRole`. + // 2. Regular users can ONLY update their own `feedDecoratorStatus`. + // 3. Email updates are handled by the `AuthService`, not this generic + // endpoint. + // + // The updater receives a raw `Map` from the request + // body to prevent mass assignment vulnerabilities. It then reads the + // pre-fetched user object (guaranteed by `dataFetchMiddleware`) and + // selectively applies only the allowed fields using `copyWith`. This + // creates a complete, valid `User` object that is then passed to the + // repository's `update` method, satisfying the `DataRepository` + // contract. + 'user': (context, id, item, uid) async { + _log.info('Executing custom updater for user ID: $id.'); + final permissionService = context.read(); + final authenticatedUser = context.read(); + final userToUpdate = context.read>().data as User; final requestBody = item as Map; - AppUserRole? newAppRole; - if (requestBody.containsKey('appRole')) { - try { - newAppRole = AppUserRole.values.byName( - requestBody['appRole'] as String, + var userWithUpdates = userToUpdate; + + if (permissionService.isAdmin(authenticatedUser)) { + _log.finer( + 'Admin user ${authenticatedUser.id} is updating user $id.', + ); + // Admin is only allowed to update roles. + if (requestBody.keys.any( + (k) => k != 'appRole' && k != 'dashboardRole', + )) { + _log.warning( + 'Admin ${authenticatedUser.id} attempted to update invalid fields for user $id.', ); - } on ArgumentError { - throw BadRequestException( - 'Invalid value for "appRole": "${requestBody['appRole']}".', + throw const ForbiddenException( + 'Administrators can only update "appRole" and "dashboardRole" via this endpoint.', ); } - } - DashboardUserRole? newDashboardRole; - if (requestBody.containsKey('dashboardRole')) { - try { - newDashboardRole = DashboardUserRole.values.byName( - requestBody['dashboardRole'] as String, + final newAppRole = requestBody.containsKey('appRole') + ? AppUserRole.values.byName(requestBody['appRole'] as String) + : null; + final newDashboardRole = requestBody.containsKey('dashboardRole') + ? DashboardUserRole.values.byName( + requestBody['dashboardRole'] as String, + ) + : null; + + userWithUpdates = userWithUpdates.copyWith( + appRole: newAppRole, + dashboardRole: newDashboardRole, + ); + } else { + _log.finer( + 'Regular user ${authenticatedUser.id} is updating their own profile.', + ); + // Regular user is only allowed to update feed decorator status. + if (requestBody.keys.any((k) => k != 'feedDecoratorStatus')) { + _log.warning( + 'User ${authenticatedUser.id} attempted to update invalid fields.', ); - } on ArgumentError { - throw BadRequestException( - 'Invalid value for "dashboardRole": "${requestBody['dashboardRole']}".', + throw const ForbiddenException( + 'You can only update "feedDecoratorStatus" via this endpoint.', ); } - } - Map? newStatus; - if (requestBody.containsKey('feedDecoratorStatus')) { - newStatus = User.fromJson( - {'feedDecoratorStatus': requestBody['feedDecoratorStatus']}, - ).feedDecoratorStatus; + if (requestBody.containsKey('feedDecoratorStatus')) { + final newStatus = User.fromJson( + {'feedDecoratorStatus': requestBody['feedDecoratorStatus']}, + ).feedDecoratorStatus; + userWithUpdates = userWithUpdates.copyWith( + feedDecoratorStatus: newStatus, + ); + } } - final userWithUpdates = existingUser.copyWith( - appRole: newAppRole, - dashboardRole: newDashboardRole, - feedDecoratorStatus: newStatus, + _log.info('User update validation passed. Calling repository.'); + return context.read>().update( + id: id, + item: userWithUpdates, + userId: uid, ); - return repo.update(id: id, item: userWithUpdates, userId: uid); }, 'user_app_settings': (c, id, item, uid) => c .read>() @@ -302,8 +336,6 @@ class DataOperationRegistry { c.read>().delete(id: id, userId: uid), 'language': (c, id, uid) => c.read>().delete(id: id, userId: uid), - 'user': (c, id, uid) => - c.read>().delete(id: id, userId: uid), 'user_app_settings': (c, id, uid) => c.read>().delete(id: id, userId: uid), 'user_content_preferences': (c, id, uid) => c From a77e7c03f3518b947ea3e234769b64f844bb774d Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 1 Nov 2025 07:11:00 +0100 Subject: [PATCH 5/7] feat(auth): Implement secure two-step email update flow Added two new methods to `AuthService` to handle user email changes securely: - `initiateEmailUpdate`: Checks if the new email is available, then generates and sends a verification code to the new address. - `completeEmailUpdate`: Verifies the provided code and, upon success, updates the user's email in the database. This ensures that a user must prove ownership of the new email address before the change is finalized, enhancing account security. --- lib/src/services/auth_service.dart | 107 +++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/lib/src/services/auth_service.dart b/lib/src/services/auth_service.dart index eb0a751..8e1280c 100644 --- a/lib/src/services/auth_service.dart +++ b/lib/src/services/auth_service.dart @@ -611,4 +611,111 @@ class AuthService { return (user: permanentUser, token: newToken); } + + /// Initiates the process of updating a user's email address. + /// + /// This is the first step in a two-step verification process. It checks if + /// the new email is already in use, then generates and sends a verification + /// code to that new email address. + /// + /// - [user]: The currently authenticated user initiating the change. + /// - [newEmail]: The desired new email address. + /// + /// Throws [ConflictException] if the `newEmail` is already taken by another + /// user. + /// Throws [OperationFailedException] for other unexpected errors. + Future initiateEmailUpdate({ + required User user, + required String newEmail, + }) async { + _log.info( + 'User ${user.id} is initiating an email update to "$newEmail".', + ); + + try { + // 1. Check if the new email address is already in use. + final existingUser = await _findUserByEmail(newEmail); + if (existingUser != null) { + _log.warning( + 'Email update failed for user ${user.id}: new email "$newEmail" is already in use by user ${existingUser.id}.', + ); + throw const ConflictException( + 'This email address is already registered.', + ); + } + _log.finer('New email "$newEmail" is available.'); + + // 2. Generate and send a verification code to the new email. + // We reuse the sign-in code mechanism for this verification step. + final code = await _verificationCodeStorageService + .generateAndStoreSignInCode(newEmail); + _log.finer('Generated verification code for "$newEmail".'); + + await _emailRepository.sendOtpEmail( + senderEmail: EnvironmentConfig.defaultSenderEmail, + recipientEmail: newEmail, + templateId: EnvironmentConfig.otpTemplateId, + subject: 'Verify your new email address', + otpCode: code, + ); + _log.info('Sent email update verification code to "$newEmail".'); + } on HttpException { + // Propagate known exceptions (like ConflictException). + rethrow; + } catch (e, s) { + _log.severe( + 'Unexpected error during initiateEmailUpdate for user ${user.id}.', + e, + s, + ); + throw const OperationFailedException( + 'Failed to initiate email update process.', + ); + } + } + + /// Completes the email update process by verifying the code and updating + /// the user's record. + /// + /// - [user]: The currently authenticated user. + /// - [newEmail]: The new email address being verified. + /// - [code]: The verification code sent to the new email address. + /// + /// Returns the updated [User] object upon success. + /// + /// Throws [InvalidInputException] if the verification code is invalid. + /// Throws [OperationFailedException] for other unexpected errors. + Future completeEmailUpdate({ + required User user, + required String newEmail, + required String code, + }) async { + _log.info('User ${user.id} is completing email update to "$newEmail".'); + + // 1. Validate the verification code for the new email. + final isValid = await _verificationCodeStorageService.validateSignInCode( + newEmail, + code, + ); + if (!isValid) { + _log.warning('Invalid verification code provided for "$newEmail".'); + throw const InvalidInputException( + 'Invalid or expired verification code.', + ); + } + _log.finer('Verification code for "$newEmail" is valid.'); + + // 2. Clear the used code from storage. + await _verificationCodeStorageService.clearSignInCode(newEmail); + + // 3. Update the user's email in the repository. + final updatedUser = user.copyWith(email: newEmail); + final finalUser = await _userRepository.update( + id: user.id, + item: updatedUser, + ); + _log.info('Successfully updated email for user ${user.id} to "$newEmail".'); + + return finalUser; + } } From 3820e661f914fd4086fb8a7c01947e3d0d9dd03d Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 1 Nov 2025 07:48:36 +0100 Subject: [PATCH 6/7] 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. --- lib/src/registry/data_operation_registry.dart | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/lib/src/registry/data_operation_registry.dart b/lib/src/registry/data_operation_registry.dart index be74574..57371d4 100644 --- a/lib/src/registry/data_operation_registry.dart +++ b/lib/src/registry/data_operation_registry.dart @@ -231,82 +231,82 @@ class DataOperationRegistry { // It enforces the following rules: // 1. Admins can ONLY update a user's `appRole` and `dashboardRole`. // 2. Regular users can ONLY update their own `feedDecoratorStatus`. - // 3. Email updates are handled by the `AuthService`, not this generic - // endpoint. // - // The updater receives a raw `Map` from the request - // body to prevent mass assignment vulnerabilities. It then reads the - // pre-fetched user object (guaranteed by `dataFetchMiddleware`) and - // selectively applies only the allowed fields using `copyWith`. This - // creates a complete, valid `User` object that is then passed to the - // repository's `update` method, satisfying the `DataRepository` - // contract. + // This logic correctly handles a full `User` object in the request body, + // aligning with the DataRepository contract. It works by comparing the + // incoming `User` object from the request (`requestedUpdateUser`) with + // the current state of the user in the database (`userToUpdate`), which + // is pre-fetched by middleware. It then verifies that the *only* fields + // that have changed are ones the authenticated user is permitted to + // modify. 'user': (context, id, item, uid) async { _log.info('Executing custom updater for user ID: $id.'); final permissionService = context.read(); final authenticatedUser = context.read(); final userToUpdate = context.read>().data as User; final requestBody = item as Map; + final requestedUpdateUser = User.fromJson(requestBody); - var userWithUpdates = userToUpdate; - + // --- State Comparison Logic --- if (permissionService.isAdmin(authenticatedUser)) { _log.finer( 'Admin user ${authenticatedUser.id} is updating user $id.', ); - // Admin is only allowed to update roles. - if (requestBody.keys.any( - (k) => k != 'appRole' && k != 'dashboardRole', - )) { + + // Create a version of the original user with only the fields an + // admin is allowed to change applied from the request. + final permissibleUpdate = userToUpdate.copyWith( + appRole: requestedUpdateUser.appRole, + dashboardRole: requestedUpdateUser.dashboardRole, + ); + + // If the user from the request is not identical to the one with + // only permissible changes, it means an unauthorized field was + // modified. + if (requestedUpdateUser != permissibleUpdate) { _log.warning( - 'Admin ${authenticatedUser.id} attempted to update invalid fields for user $id.', + 'Admin ${authenticatedUser.id} attempted to update unauthorized fields for user $id.', ); throw const ForbiddenException( 'Administrators can only update "appRole" and "dashboardRole" via this endpoint.', ); } - - final newAppRole = requestBody.containsKey('appRole') - ? AppUserRole.values.byName(requestBody['appRole'] as String) - : null; - final newDashboardRole = requestBody.containsKey('dashboardRole') - ? DashboardUserRole.values.byName( - requestBody['dashboardRole'] as String, - ) - : null; - - userWithUpdates = userWithUpdates.copyWith( - appRole: newAppRole, - dashboardRole: newDashboardRole, - ); + _log.finer('Admin update for user $id validation passed.'); } else { _log.finer( 'Regular user ${authenticatedUser.id} is updating their own profile.', ); - // Regular user is only allowed to update feed decorator status. - if (requestBody.keys.any((k) => k != 'feedDecoratorStatus')) { + + // Create a version of the original user with only the fields a + // regular user is allowed to change applied from the request. + final permissibleUpdate = userToUpdate.copyWith( + feedDecoratorStatus: requestedUpdateUser.feedDecoratorStatus, + ); + + // If the user from the request is not identical to the one with + // only permissible changes, it means an unauthorized field was + // modified. + if (requestedUpdateUser != permissibleUpdate) { _log.warning( - 'User ${authenticatedUser.id} attempted to update invalid fields.', + 'User ${authenticatedUser.id} attempted to update unauthorized fields.', ); throw const ForbiddenException( 'You can only update "feedDecoratorStatus" via this endpoint.', ); } - - if (requestBody.containsKey('feedDecoratorStatus')) { - final newStatus = User.fromJson( - {'feedDecoratorStatus': requestBody['feedDecoratorStatus']}, - ).feedDecoratorStatus; - userWithUpdates = userWithUpdates.copyWith( - feedDecoratorStatus: newStatus, - ); - } + _log.finer( + 'Regular user update for user $id validation passed.', + ); } - _log.info('User update validation passed. Calling repository.'); - return context.read>().update( + _log.info( + 'User update validation passed. Calling repository with full object.', + ); + // The validation passed, so we can now safely pass the full User + // object from the request to the repository, honoring the contract. + return await context.read>().update( id: id, - item: userWithUpdates, + item: requestedUpdateUser, userId: uid, ); }, From 8af7a002260131b2a64de60eec913fe8abf73919 Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 1 Nov 2025 07:56:05 +0100 Subject: [PATCH 7/7] style: format --- lib/src/rbac/permissions.dart | 1 - lib/src/registry/data_operation_registry.dart | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/rbac/permissions.dart b/lib/src/rbac/permissions.dart index cdbbc55..0f66f9f 100644 --- a/lib/src/rbac/permissions.dart +++ b/lib/src/rbac/permissions.dart @@ -44,7 +44,6 @@ abstract class Permissions { // Allows deleting the authenticated user's own account static const String userDeleteOwned = 'user.delete_owned'; - // Allows updating any user's profile (admin-only). // This is distinct from `userUpdateOwned`, which allows a user to update // their own record. diff --git a/lib/src/registry/data_operation_registry.dart b/lib/src/registry/data_operation_registry.dart index 57371d4..89f7a83 100644 --- a/lib/src/registry/data_operation_registry.dart +++ b/lib/src/registry/data_operation_registry.dart @@ -304,7 +304,7 @@ class DataOperationRegistry { ); // The validation passed, so we can now safely pass the full User // object from the request to the repository, honoring the contract. - return await context.read>().update( + return context.read>().update( id: id, item: requestedUpdateUser, userId: uid,