dataconnect: Fixed occasional NullPointerException leading to erroneous UNAUTHENTICATED exceptions (#7001)

Author: dconeybe

firebase-dataconnect/CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased
-
+* [fixed] Fixed occasional `NullPointerException` when registering with
+  FirebaseAuth, leading to erroneous UNAUTHENTICATED exceptions.
+  ([#7001](https://github.com/firebase/firebase-android-sdk/pull/7001))
 
 # 16.0.2
 * [changed] Improved code robustness related to state management in

firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt

@@ -48,13 +48,14 @@ import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.filter
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.getAndUpdate
+import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 
 /** Base class that shares logic for managing the Auth token and AppCheck token. */
 internal sealed class DataConnectCredentialsTokenManager<T : Any>(
   private val deferredProvider: com.google.firebase.inject.Deferred<T>,
   parentCoroutineScope: CoroutineScope,
-  blockingDispatcher: CoroutineDispatcher,
+  private val blockingDispatcher: CoroutineDispatcher,
   protected val logger: Logger,
 ) {
   val instanceId: String
@@ -74,17 +75,23 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
         }
     )
 
-  init {
-    // Call `whenAvailable()` on a non-main thread because it accesses SharedPreferences, which
-    // performs disk i/o, violating the StrictMode policy android.os.strictmode.DiskReadViolation.
-    val coroutineName = CoroutineName("k6rwgqg9gh $instanceId whenAvailable")
-    coroutineScope.launch(coroutineName + blockingDispatcher) {
-      deferredProvider.whenAvailable(DeferredProviderHandlerImpl(weakThis))
-    }
-  }
-
   private sealed interface State<out T> {
 
+    /**
+     * State indicating that the object has just been created and [initialize] has not yet been
+     * called.
+     */
+    object New : State<Nothing>
+
+    /**
+     * State indicating that [initialize] has been invoked but the token provider is not (yet?)
+     * available.
+     */
+    data class Initialized(override val forceTokenRefresh: Boolean) :
+      StateWithForceTokenRefresh<Nothing> {
+      constructor() : this(false)
+    }
+
     /** State indicating that [close] has been invoked. */
     object Closed : State<Nothing>
 
@@ -93,9 +100,6 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
       val forceTokenRefresh: Boolean
     }
 
-    /** State indicating that the token provider is not (yet?) available. */
-    data class New(override val forceTokenRefresh: Boolean) : StateWithForceTokenRefresh<Nothing>
-
     sealed interface StateWithProvider<out T> : State<T> {
       /** The token provider, [InternalAuthProvider] or [InteropAppCheckTokenProvider] */
       val provider: T
@@ -115,7 +119,7 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
   }
 
   /** The current state of this object. */
-  private val state = MutableStateFlow<State<T>>(State.New(forceTokenRefresh = false))
+  private val state = MutableStateFlow<State<T>>(State.New)
 
   /**
    * Adds the token listener to the given provider.
@@ -137,6 +141,34 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
    */
   protected abstract suspend fun getToken(provider: T, forceRefresh: Boolean): GetTokenResult
 
+  /**
+   * Initializes this object.
+   *
+   * Before calling this method, the _only_ other methods that are allowed to be called on this
+   * object are [awaitTokenProvider] and [close].
+   *
+   * This method may only be called once; subsequent calls result in an exception.
+   */
+  fun initialize() {
+    logger.debug { "initialize()" }
+
+    state.update { currentState ->
+      when (currentState) {
+        is State.New -> State.Initialized()
+        is State.Closed ->
+          throw IllegalStateException("initialize() cannot be called after close()")
+        else -> throw IllegalStateException("initialize() has already been called")
+      }
+    }
+
+    // Call `whenAvailable()` on a non-main thread because it accesses SharedPreferences, which
+    // performs disk i/o, violating the StrictMode policy android.os.strictmode.DiskReadViolation.
+    val coroutineName = CoroutineName("k6rwgqg9gh $instanceId whenAvailable")
+    coroutineScope.launch(coroutineName + blockingDispatcher) {
+      deferredProvider.whenAvailable(DeferredProviderHandlerImpl(weakThis))
+    }
+  }
+
   /**
    * Closes this object, releasing its resources, unregistering any registered listeners, and
    * cancelling any in-flight token requests.
@@ -155,8 +187,9 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
 
     val oldState = state.getAndUpdate { State.Closed }
     when (oldState) {
-      is State.Closed -> {}
       is State.New -> {}
+      is State.Initialized -> {}
+      is State.Closed -> {}
       is State.StateWithProvider -> {
         removeTokenListener(oldState.provider)
       }
@@ -166,6 +199,9 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
   /**
    * Suspends until the token provider becomes available to this object.
    *
+   * This method _may_ be called before [initialize], which is the method that asynchronously gets
+   * the token provider.
+   *
    * If [close] has been invoked, or is invoked _before_ a token provider becomes available, then
    * this method returns normally, as if a token provider _had_ become available.
    */
@@ -177,6 +213,7 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
           when (it) {
             State.Closed -> true
             is State.New -> false
+            is State.Initialized -> false
             is State.Idle -> true
             is State.Active -> true
           }
@@ -197,25 +234,34 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
         val newState =
           when (currentState) {
             is State.Closed -> State.Closed
-            is State.New -> currentState.copy(forceTokenRefresh = true)
+            is State.New -> currentState
+            is State.Initialized -> currentState.copy(forceTokenRefresh = true)
             is State.Idle -> currentState.copy(forceTokenRefresh = true)
             is State.Active -> State.Idle(currentState.provider, forceTokenRefresh = true)
           }
 
-        check(newState is State.Closed || newState is State.StateWithForceTokenRefresh<T>) {
+        check(
+          newState is State.New ||
+            newState is State.Closed ||
+            newState is State.StateWithForceTokenRefresh<T>
+        ) {
           "internal error gbazc7qr66: newState should have been Closed or " +
             "StateWithForceTokenRefresh, but got: $newState"
         }
-        check((newState as? State.StateWithForceTokenRefresh<T>)?.forceTokenRefresh !== false) {
-          "internal error fnzwyrsez2: newState.forceTokenRefresh should have been true"
+        if (newState is State.StateWithForceTokenRefresh<T>) {
+          check(newState.forceTokenRefresh) {
+            "internal error fnzwyrsez2: newState.forceTokenRefresh should have been true"
+          }
         }
 
         newState
       }
 
     when (oldState) {
       is State.Closed -> {}
-      is State.New -> {}
+      is State.New ->
+        throw IllegalStateException("initialize() must be called before forceRefresh()")
+      is State.Initialized -> {}
       is State.Idle -> {}
       is State.Active -> {
         val message = "needs token refresh (wgrwbrvjxt)"
@@ -259,14 +305,16 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
 
       val newState: State.Active<T> =
         when (oldState) {
+          is State.New ->
+            throw IllegalStateException("initialize() must be called before getToken()")
           is State.Closed -> {
             logger.debug {
               "$invocationId getToken() throws CredentialsTokenManagerClosedException" +
                 " because the DataConnectCredentialsTokenManager instance has been closed"
             }
             throw CredentialsTokenManagerClosedException(this)
           }
-          is State.New -> {
+          is State.Initialized -> {
             logger.debug {
               "$invocationId getToken() returns null (token provider is not (yet?) available)"
             }
@@ -353,22 +401,28 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any>(
     val oldState =
       state.getAndUpdate { currentState ->
         when (currentState) {
+          is State.New -> currentState
           is State.Closed -> State.Closed
-          is State.New -> State.Idle(newProvider, currentState.forceTokenRefresh)
+          is State.Initialized -> State.Idle(newProvider, currentState.forceTokenRefresh)
           is State.Idle -> State.Idle(newProvider, currentState.forceTokenRefresh)
           is State.Active -> State.Idle(newProvider, forceTokenRefresh = false)
         }
       }
 
     when (oldState) {
+      is State.New ->
+        throw IllegalStateException(
+          "internal error sdpzwhmhd3: " +
+            "initialize() should have been called before onProviderAvailable()"
+        )
       is State.Closed -> {
         logger.debug {
           "onProviderAvailable(newProvider=$newProvider)" +
             " unregistering token listener that was just added"
         }
         removeTokenListener(newProvider)
       }
-      is State.New -> {}
+      is State.Initialized -> {}
       is State.Idle -> {}
       is State.Active -> {
         val newProviderClassName = newProvider::class.qualifiedName

firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt

@@ -118,23 +118,25 @@ internal class FirebaseDataConnectImpl(
 
   private val dataConnectAuth: DataConnectAuth =
     DataConnectAuth(
-      deferredAuthProvider = deferredAuthProvider,
-      parentCoroutineScope = coroutineScope,
-      blockingDispatcher = blockingDispatcher,
-      logger = Logger("DataConnectAuth").apply { debug { "created by $instanceId" } },
-    )
+        deferredAuthProvider = deferredAuthProvider,
+        parentCoroutineScope = coroutineScope,
+        blockingDispatcher = blockingDispatcher,
+        logger = Logger("DataConnectAuth").apply { debug { "created by $instanceId" } },
+      )
+      .apply { initialize() }
 
   override suspend fun awaitAuthReady() {
     dataConnectAuth.awaitTokenProvider()
   }
 
   private val dataConnectAppCheck: DataConnectAppCheck =
     DataConnectAppCheck(
-      deferredAppCheckTokenProvider = deferredAppCheckProvider,
-      parentCoroutineScope = coroutineScope,
-      blockingDispatcher = blockingDispatcher,
-      logger = Logger("DataConnectAppCheck").apply { debug { "created by $instanceId" } },
-    )
+        deferredAppCheckTokenProvider = deferredAppCheckProvider,
+        parentCoroutineScope = coroutineScope,
+        blockingDispatcher = blockingDispatcher,
+        logger = Logger("DataConnectAppCheck").apply { debug { "created by $instanceId" } },
+      )
+      .apply { initialize() }
 
   override suspend fun awaitAppCheckReady() {
     dataConnectAppCheck.awaitTokenProvider()