Merge pull request #89 from flutter-news-app-full-source-code/refactor/user-management-capabilities-within-the-generic-data-API

fulleni · web-flow · commit 5eae9a0b13ca · 2025-11-01T08:11:05.000+01:00
Refactor/user management capabilities within the generic data api
diff --git a/lib/src/rbac/permissions.dart b/lib/src/rbac/permissions.dart
@@ -44,12 +44,10 @@ 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';
diff --git a/lib/src/rbac/role_permissions.dart b/lib/src/rbac/role_permissions.dart
@@ -68,11 +68,12 @@ final Set<String> _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,
diff --git 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<DataRepository<User>>().create(
-        item: item as User,
-        userId: uid,
-      ),
       'remote_config': (c, item, uid) => c
           .read<DataRepository<RemoteConfig>>()
           .create(item: item as RemoteConfig, userId: uid),
@@ -225,56 +225,90 @@ class DataOperationRegistry {
       'language': (c, id, item, uid) => c
           .read<DataRepository<Language>>()
           .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<String, dynamic> as the `item` to prevent
-      // mass assignment vulnerabilities, only applying allowed fields.
-      'user': (c, id, item, uid) {
-        final repo = c.read<DataRepository<User>>();
-        final existingUser = c.read<FetchedItem<dynamic>>().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`.
+      //
+      // 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<PermissionService>();
+        final authenticatedUser = context.read<User>();
+        final userToUpdate = context.read<FetchedItem<dynamic>>().data as User;
         final requestBody = item as Map<String, dynamic>;
+        final requestedUpdateUser = User.fromJson(requestBody);
 
-        AppUserRole? newAppRole;
-        if (requestBody.containsKey('appRole')) {
-          try {
-            newAppRole = AppUserRole.values.byName(
-              requestBody['appRole'] as String,
+        // --- State Comparison Logic ---
+        if (permissionService.isAdmin(authenticatedUser)) {
+          _log.finer(
+            'Admin user ${authenticatedUser.id} is updating user $id.',
+          );
+
+          // 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 unauthorized 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.',
             );
           }
-        }
+          _log.finer('Admin update for user $id validation passed.');
+        } else {
+          _log.finer(
+            'Regular user ${authenticatedUser.id} is updating their own profile.',
+          );
+
+          // 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,
+          );
 
-        DashboardUserRole? newDashboardRole;
-        if (requestBody.containsKey('dashboardRole')) {
-          try {
-            newDashboardRole = DashboardUserRole.values.byName(
-              requestBody['dashboardRole'] as String,
+          // 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 unauthorized fields.',
             );
-          } on ArgumentError {
-            throw BadRequestException(
-              'Invalid value for "dashboardRole": "${requestBody['dashboardRole']}".',
+            throw const ForbiddenException(
+              'You can only update "feedDecoratorStatus" via this endpoint.',
             );
           }
+          _log.finer(
+            'Regular user update for user $id validation passed.',
+          );
         }
 
-        Map<FeedDecoratorType, UserFeedDecoratorStatus>? newStatus;
-        if (requestBody.containsKey('feedDecoratorStatus')) {
-          newStatus = User.fromJson(
-            {'feedDecoratorStatus': requestBody['feedDecoratorStatus']},
-          ).feedDecoratorStatus;
-        }
-
-        final userWithUpdates = existingUser.copyWith(
-          appRole: newAppRole,
-          dashboardRole: newDashboardRole,
-          feedDecoratorStatus: newStatus,
+        _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 context.read<DataRepository<User>>().update(
+          id: id,
+          item: requestedUpdateUser,
+          userId: uid,
         );
-        return repo.update(id: id, item: userWithUpdates, userId: uid);
       },
       'user_app_settings': (c, id, item, uid) => c
           .read<DataRepository<UserAppSettings>>()
@@ -302,8 +336,6 @@ class DataOperationRegistry {
           c.read<DataRepository<Country>>().delete(id: id, userId: uid),
       'language': (c, id, uid) =>
           c.read<DataRepository<Language>>().delete(id: id, userId: uid),
-      'user': (c, id, uid) =>
-          c.read<DataRepository<User>>().delete(id: id, userId: uid),
       'user_app_settings': (c, id, uid) =>
           c.read<DataRepository<UserAppSettings>>().delete(id: id, userId: uid),
       'user_content_preferences': (c, id, uid) => c
diff --git a/lib/src/registry/model_registry.dart b/lib/src/registry/model_registry.dart
@@ -281,33 +281,28 @@ final modelRegistry = <String, ModelConfig<dynamic>>{
       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<UserAppSettings>(
diff --git 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<void> 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<User> 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;
+  }
 }
