Fix log redaction, device registration race, and incomplete reset

lib/src/pushfire_sdk_impl.dart

@@ -1,5 +1,6 @@
 import 'dart:async';
-import 'package:flutter/foundation.dart' show debugPrint, kIsWeb;
+import 'package:flutter/foundation.dart'
+    show debugPrint, kIsWeb, visibleForTesting;
 import 'package:flutter/widgets.dart';
 import 'package:firebase_auth/firebase_auth.dart' show FirebaseAuth, User;
 import 'package:firebase_core/firebase_core.dart';
@@ -47,8 +48,9 @@ class PushFireSDKImpl with WidgetsBindingObserver {
   StreamSubscription<sp.AuthState>? _supabaseAuthSubscription;
   StreamSubscription<User?>? _firebaseAuthSubscription;
 
-  // Flag to prevent overlapping permission checks
-  bool _isCheckingPermission = false;
+  // The permission check currently in flight, if any. Concurrent callers join
+  // it rather than starting a second one. See _guardedPermissionCheck.
+  Future<Device?>? _permissionCheck;
 
   PushFireSDKImpl._();
 
@@ -232,8 +234,13 @@ class PushFireSDKImpl with WidgetsBindingObserver {
         PushFireLogger.info('FCM token refreshed');
         PushFireLogger.logFcmToken(newToken);
 
-        // Check for permission status changes before re-registering
-        await _deviceService.checkAndHandlePermissionStatusChange();
+        // Check for permission status changes before re-registering, through
+        // the same guard the resume path uses: a token refresh arriving during
+        // a foreground resume would otherwise run two overlapping checks.
+        // The result is deliberately ignored — the registerDevice() below
+        // already emits onDeviceRegistered for this refresh, and emitting here
+        // too would produce two events for one logical change.
+        await _guardedPermissionCheck();
 
         // Re-register device with new token
         _currentDevice = await _deviceService.registerDevice();
@@ -269,22 +276,45 @@ class PushFireSDKImpl with WidgetsBindingObserver {
     }
   }
 
+  /// Run the permission-change check under the overlap guard shared by every
+  /// caller — the foreground resume, the FCM token-refresh handler and
+  /// [syncNotificationPermission].
+  ///
+  /// A caller that arrives while a check is running joins it rather than
+  /// starting a second one, and receives null: the owner of the check reports
+  /// the change, so one permission change produces exactly one event no matter
+  /// how many callers are waiting.
+  ///
+  /// Returns the re-registered device when this caller's check synced a
+  /// change, null otherwise.
+  Future<Device?> _guardedPermissionCheck() {
+    final existing = _permissionCheck;
+    if (existing != null) {
+      PushFireLogger.info('Permission check already in progress - joining it');
+      return existing.then((_) => null);
+    }
+
+    final check = _runPermissionCheck();
+    _permissionCheck = check;
+    return check;
+  }
+
+  /// Wraps the check so the guard is released on both success and failure.
+  Future<Device?> _runPermissionCheck() async {
+    try {
+      return await _deviceService.checkAndHandlePermissionStatusChange();
+    } finally {
+      _permissionCheck = null;
+    }
+  }
+
   /// Check permission status when app resumes
   /// This method is safe to call multiple times - it guards against overlapping executions
   Future<void> _checkPermissionStatusOnResume() async {
-    // Prevent overlapping calls - if a check is already in progress, skip this one
-    if (_isCheckingPermission) {
-      PushFireLogger.info(
-          'Permission check already in progress - skipping duplicate call');
-      return;
-    }
-
-    _isCheckingPermission = true;
     try {
       PushFireLogger.info(
           'App resumed - checking notification permission status');
-      final updatedDevice =
-          await _deviceService.checkAndHandlePermissionStatusChange();
+      final updatedDevice = await _guardedPermissionCheck();
 
       if (updatedDevice != null) {
         // Device was already re-registered by checkAndHandlePermissionStatusChange
@@ -296,8 +326,6 @@ class PushFireSDKImpl with WidgetsBindingObserver {
     } catch (e) {
       PushFireLogger.warning(
           'Failed to check permission status on app resume', e);
-    } finally {
-      _isCheckingPermission = false;
     }
   }
 
@@ -548,7 +576,7 @@ class PushFireSDKImpl with WidgetsBindingObserver {
   /// Returns the current [NotificationStatus] after syncing.
   Future<NotificationStatus> syncNotificationPermission() async {
     _ensureInitialized();
-    final device = await _deviceService.checkAndHandlePermissionStatusChange();
+    final device = await _guardedPermissionCheck();
     if (device != null) {
       _currentDevice = device;
       _deviceRegisteredController.add(device);
@@ -590,13 +618,12 @@ class PushFireSDKImpl with WidgetsBindingObserver {
 
     PushFireLogger.info('Resetting SDK');
 
-    // Logout subscriber if logged in
-    if (await isSubscriberLoggedIn()) {
-      await logoutSubscriber();
-    }
-
-    // Clear device data
-    await _deviceService.clearDeviceData();
+    await clearAllLocalState(
+      subscriberService: _subscriberService,
+      deviceService: _deviceService,
+      isSubscriberLoggedIn: isSubscriberLoggedIn,
+      logoutSubscriber: logoutSubscriber,
+    );
 
     // Reset current state
     _currentDevice = null;
@@ -605,6 +632,39 @@ class PushFireSDKImpl with WidgetsBindingObserver {
     PushFireLogger.info('SDK reset completed');
   }
 
+  /// The local-state teardown performed by [reset].
+  ///
+  /// Extracted so it can be tested without a live SDK singleton, which needs
+  /// Firebase.
+  ///
+  /// Both clears are unconditional and run after a logout that cannot escape:
+  ///
+  /// - A stored subscriber blob whose `id` is null does not count as logged in,
+  ///   so the gated logout skips it and the blob — name, email, phone — would
+  ///   survive a call that documents itself as clearing all local state.
+  /// - `logoutSubscriber` clears locally and then rethrows when the server call
+  ///   fails. Letting that escape would skip [DeviceService.clearDeviceData],
+  ///   leaving the device id, FCM token and permission state behind.
+  @visibleForTesting
+  static Future<void> clearAllLocalState({
+    required SubscriberService subscriberService,
+    required DeviceService deviceService,
+    required Future<bool> Function() isSubscriberLoggedIn,
+    required Future<void> Function() logoutSubscriber,
+  }) async {
+    try {
+      if (await isSubscriberLoggedIn()) {
+        await logoutSubscriber();
+      }
+    } catch (e) {
+      PushFireLogger.warning(
+          'Logout during reset failed - clearing local state anyway', e);
+    }
+
+    await subscriberService.clearSubscriberData();
+    await deviceService.clearDeviceData();
+  }
+
   /// Dispose SDK resources
   void dispose() {
     if (!_isInitialized) return;
@@ -618,8 +678,8 @@ class PushFireSDKImpl with WidgetsBindingObserver {
       // Ignore if observer wasn't added or WidgetsBinding is not available
     }
 
-    // Reset flags
-    _isCheckingPermission = false;
+    // Drop the in-flight permission check, if any
+    _permissionCheck = null;
 
     // Cancel stream subscriptions to prevent memory leaks
     _fcmTokenRefreshSubscription?.cancel();

lib/src/services/device_service.dart

@@ -42,8 +42,45 @@ class DeviceService {
     this.openAppSettingsOverride,
   });
 
-  /// Register or update device automatically
-  Future<Device> registerDevice() async {
+  /// The registration currently in flight, if any. See [registerDevice].
+  Future<Device>? _inFlightRegistration;
+
+  /// Register or update device automatically.
+  ///
+  /// Single-flight: registration reads the stored device id, makes a network
+  /// round trip, then writes the id back. Two callers entering before either
+  /// writes would both see no stored id, both POST, and create two device rows
+  /// for one device — the second write wins locally, orphaning the first row
+  /// server-side while it still holds the same FCM token. Concurrent callers
+  /// therefore join the in-flight registration instead of starting a second.
+  ///
+  /// Reachable concurrently from auto-registration at init, the FCM
+  /// token-refresh handler, the foreground permission check, and
+  /// [requestNotificationPermission].
+  Future<Device> registerDevice() {
+    final existing = _inFlightRegistration;
+    if (existing != null) {
+      PushFireLogger.info(
+          'Device registration already in progress - joining it');
+      return existing;
+    }
+
+    final registration = _runRegistration();
+    _inFlightRegistration = registration;
+    return registration;
+  }
+
+  /// Wraps [_performRegistration] so the guard is released on both success and
+  /// failure — a failed attempt must not wedge every later caller.
+  Future<Device> _runRegistration() async {
+    try {
+      return await _performRegistration();
+    } finally {
+      _inFlightRegistration = null;
+    }
+  }
+
+  Future<Device> _performRegistration() async {
     try {
       PushFireLogger.info('Starting device registration');
 

lib/src/services/subscriber_service.dart

@@ -47,7 +47,8 @@ class SubscriberService {
         },
       };
 
-      PushFireLogger.info('Logging in subscriber with data: $subscriberData');
+      PushFireLogger.info(
+          'Logging in subscriber with data: ${PushFireLogger.redact(subscriberData)}');
 
       // Make API call
       final response =
@@ -147,7 +148,7 @@ class SubscriberService {
 
       if (subscriber?.id == null || deviceId == null) {
         PushFireLogger.warning('No subscriber or device found for logout');
-        await _clearSubscriberData();
+        await clearSubscriberData();
         return;
       }
 
@@ -162,15 +163,15 @@ class SubscriberService {
       await _apiClient.post('logout-subscriber', logoutData);
 
       // Clear local data
-      await _clearSubscriberData();
+      await clearSubscriberData();
 
       PushFireLogger.info('Subscriber logout completed');
     } catch (e) {
       // Single catch (not `on PushFireException { rethrow; }` like the other
       // methods) is deliberate: local data must be cleared on BOTH expected
       // and unexpected failures. Clear first, then rethrow PushFireExceptions
       // (already logged downstream) or wrap-and-log genuinely unexpected ones.
-      await _clearSubscriberData();
+      await clearSubscriberData();
 
       if (e is PushFireException) {
         rethrow;
@@ -221,7 +222,7 @@ class SubscriberService {
   }
 
   /// Clear subscriber data from local storage
-  Future<void> _clearSubscriberData() async {
+  Future<void> clearSubscriberData() async {
     final prefs = await SharedPreferences.getInstance();
     await prefs.remove(_subscriberIdKey);
     await prefs.remove(_subscriberDataKey);

lib/src/services/tag_service.dart

@@ -14,7 +14,8 @@ class TagService {
   /// Add a tag to the current subscriber
   Future<SubscriberTag> addTag(String tagId, String value) async {
     try {
-      PushFireLogger.info('Adding tag: $tagId = $value');
+      // Value omitted: tag values routinely hold an email, plan or region.
+      PushFireLogger.info('Adding tag: $tagId');
 
       // Get current subscriber ID
       final subscriberId = await _subscriberService.getSubscriberId();
@@ -59,7 +60,7 @@ class TagService {
   /// Update a tag value for the current subscriber
   Future<SubscriberTag> updateTag(String tagId, String value) async {
     try {
-      PushFireLogger.info('Updating tag: $tagId = $value');
+      PushFireLogger.info('Updating tag: $tagId');
 
       // Get current subscriber ID
       final subscriberId = await _subscriberService.getSubscriberId();