Fixed issue where auth wasn't properly refreshing id token

firebase · maneesht · Oct 17, 2024 · Oct 16, 2024 · Oct 16, 2024 · Oct 16, 2024
commit dde91c0cf0dcdb52ca308b423db98474cfa903c6
diff --git a/packages/firebase_data_connect/firebase_data_connect/example/.firebaserc b/packages/firebase_data_connect/firebase_data_connect/example/.firebaserc
@@ -3,10 +3,5 @@
     "default": "flutterfire-e2e-tests"
   },
   "targets": {},
-  "etags": {},
-  "dataconnectEmulatorConfig": {
-    "postgres": {
-      "localConnectionString": "postgresql://postgres:postgres@localhost:5432/postgres?sslmode=disable"
-    }
-  }
+  "etags": {}
 }
diff --git a/packages/firebase_data_connect/firebase_data_connect/lib/src/common/common_library.dart b/packages/firebase_data_connect/firebase_data_connect/lib/src/common/common_library.dart
@@ -59,9 +59,6 @@ abstract class DataConnectTransport {
   /// DataConnect backend configuration.
   DataConnectOptions options;
 
-  /// FirebaseAuth to use to get auth token.
-  FirebaseAuth? auth;
-
   /// FirebaseAppCheck to use to get app check token.
   FirebaseAppCheck? appCheck;
 
@@ -76,12 +73,14 @@ abstract class DataConnectTransport {
       String queryName,
       Deserializer<Data> deserializer,
       Serializer<Variables> serializer,
-      Variables? vars);
+      Variables? vars,
+      String? token);
 
   /// Invokes corresponding mutation endpoint.
   Future<Data> invokeMutation<Data, Variables>(
       String queryName,
       Deserializer<Data> deserializer,
       Serializer<Variables> serializer,
-      Variables? vars);
+      Variables? vars,
+      String? token);
 }
diff --git a/packages/firebase_data_connect/firebase_data_connect/lib/src/core/ref.dart b/packages/firebase_data_connect/firebase_data_connect/lib/src/core/ref.dart
@@ -45,7 +45,26 @@ abstract class OperationRef<Data, Variables> {
 
   FirebaseDataConnect dataConnect;
 
-  Future<OperationResult<Data, Variables>> execute();
+  Future<OperationResult<Data, Variables>> execute() {
+    return this._executeWithRetry(true);
+  }
+
+  Future<OperationResult<Data, Variables>> _executeOperation(bool forceRefresh);
+
+  Future<OperationResult<Data, Variables>> _executeWithRetry(bool retry) async {
+    try {
+      OperationResult<Data, Variables> r = await this._executeOperation(!retry);
+      return r;
+    } on DataConnectError catch (e) {
+      if (retry && e.code == DataConnectErrorCode.unauthorized.toString()) {
+        return this._executeWithRetry(false);
+      } else {
+        throw e;
+      }
+    } catch (e) {
+      throw e;
+    }
+  }
 }
 
 /// Tracks currently active queries, and emits events when a new query is executed.
@@ -112,11 +131,14 @@ class QueryRef<Data, Variables> extends OperationRef<Data, Variables> {
             variables);
 
   QueryManager _queryManager;
-  @override
-  Future<QueryResult<Data, Variables>> execute() async {
+
+  Future<QueryResult<Data, Variables>> _executeOperation(
+      bool forceRefresh) async {
+    String? token =
+        await this.dataConnect.auth?.currentUser?.getIdToken(forceRefresh);
     try {
       Data data = await _transport.invokeQuery<Data, Variables>(
-          operationName, deserializer, serializer, variables);
+          operationName, deserializer, serializer, variables, token);
       QueryResult<Data, Variables> res = QueryResult(dataConnect, data, this);
       await _queryManager.triggerCallback<Data, Variables>(operationName,
           serializer(variables as Variables), this, res.data, null);
@@ -135,7 +157,7 @@ class QueryRef<Data, Variables> extends OperationRef<Data, Variables> {
         .cast<QueryResult<Data, Variables>>();
     if (_queryManager.containsQuery(operationName, variables, varsSerialized)) {
       try {
-        this.execute();
+        this._executeWithRetry(true);
       } catch (_) {
         // Call to `execute` should properly pass the error to the Stream.
         log("Error thrown by execute. The error will propagate via onError.");
@@ -156,9 +178,12 @@ class MutationRef<Data, Variables> extends OperationRef<Data, Variables> {
   ) : super(dataConnect, operationName, transport, deserializer, serializer,
             variables);
   @override
-  Future<OperationResult<Data, Variables>> execute() async {
+  Future<OperationResult<Data, Variables>> _executeOperation(
+      bool forceRefresh) async {
+    String? token =
+        await this.dataConnect.auth?.currentUser?.getIdToken(forceRefresh);
     Data data = await _transport.invokeMutation<Data, Variables>(
-        operationName, deserializer, serializer, variables);
+        operationName, deserializer, serializer, variables, token);
     return OperationResult(dataConnect, data, this);
   }
 }
diff --git a/packages/firebase_data_connect/firebase_data_connect/lib/src/firebase_data_connect.dart b/packages/firebase_data_connect/firebase_data_connect/lib/src/firebase_data_connect.dart
@@ -43,6 +43,7 @@ class FirebaseDataConnect extends FirebasePluginPlatform {
     if (sdkType != null) {
       this._sdkType = sdkType;
     }
+    QueryRef qf;
   }
 
   /// QueryManager manages ongoing queries, and their subscriptions.

diff --git a/packages/firebase_data_connect/firebase_data_connect/lib/src/network/grpc_transport.dart b/packages/firebase_data_connect/firebase_data_connect/lib/src/network/grpc_transport.dart
@@ -22,7 +22,6 @@ class GRPCTransport implements DataConnectTransport {
     this.options,
     this.appId,
     this.sdkType,
-    this.auth,
     this.appCheck,
   ) {
     bool isSecure =
@@ -38,10 +37,6 @@ class GRPCTransport implements DataConnectTransport {
         'projects/${options.projectId}/locations/${options.location}/services/${options.serviceId}/connectors/${options.connector}';
   }
 
-  /// FirebaseAuth
-  @override
-  FirebaseAuth? auth;
-
   /// FirebaseAppCheck
   @override
   FirebaseAppCheck? appCheck;
@@ -70,13 +65,7 @@ class GRPCTransport implements DataConnectTransport {
   @override
   String appId;
 
-  Future<Map<String, String>> getMetadata() async {
-    String? authToken;
-    try {
-      authToken = await auth?.currentUser?.getIdToken();
-    } catch (e) {
-      log('Unable to get auth token: $e');
-    }
+  Future<Map<String, String>> getMetadata(String? authToken) async {
     String? appCheckToken;
     try {
       appCheckToken = await appCheck?.getToken();
@@ -101,11 +90,11 @@ class GRPCTransport implements DataConnectTransport {
   /// Invokes GPRC query endpoint.
   @override
   Future<Data> invokeQuery<Data, Variables>(
-    String queryName,
-    Deserializer<Data> deserializer,
-    Serializer<Variables>? serializer,
-    Variables? vars,
-  ) async {
+      String queryName,
+      Deserializer<Data> deserializer,
+      Serializer<Variables>? serializer,
+      Variables? vars,
+      String? authToken) async {
     ExecuteQueryResponse response;
 
     ExecuteQueryRequest request =
@@ -115,9 +104,13 @@ class GRPCTransport implements DataConnectTransport {
     }
     try {
       response = await stub.executeQuery(request,
-          options: CallOptions(metadata: await getMetadata()));
+          options: CallOptions(metadata: await getMetadata(authToken)));
       return deserializer(jsonEncode(response.data.toProto3Json()));
     } on Exception catch (e) {
+      if (e.toString().contains("invalid Firebase Auth Credentials")) {
+        throw DataConnectError(DataConnectErrorCode.unauthorized,
+            'Failed to invoke operation: ${e.toString()}');
+      }
       throw DataConnectError(DataConnectErrorCode.other,
           'Failed to invoke operation: ${e.toString()}');
     }
@@ -137,7 +130,8 @@ class GRPCTransport implements DataConnectTransport {
       String queryName,
       Deserializer<Data> deserializer,
       Serializer<Variables>? serializer,
-      Variables? vars) async {
+      Variables? vars,
+      String? authToken) async {
     ExecuteMutationResponse response;
     ExecuteMutationRequest request =
         ExecuteMutationRequest(name: name, operationName: queryName);
@@ -146,7 +140,7 @@ class GRPCTransport implements DataConnectTransport {
     }
     try {
       response = await stub.executeMutation(request,
-          options: CallOptions(metadata: await getMetadata()));
+          options: CallOptions(metadata: await getMetadata(authToken)));
       if (response.errors.isNotEmpty) {
         throw Exception(response.errors);
       }
@@ -166,4 +160,4 @@ DataConnectTransport getTransport(
         CallerSDKType sdkType,
         FirebaseAuth? auth,
         FirebaseAppCheck? appCheck) =>
-    GRPCTransport(transportOptions, options, appId, sdkType, auth, appCheck);
+    GRPCTransport(transportOptions, options, appId, sdkType, appCheck);
diff --git a/packages/firebase_data_connect/firebase_data_connect/lib/src/network/rest_transport.dart b/packages/firebase_data_connect/firebase_data_connect/lib/src/network/rest_transport.dart
@@ -18,7 +18,7 @@ part of firebase_data_connect_rest;
 class RestTransport implements DataConnectTransport {
   /// Initializes necessary protocol and port.
   RestTransport(this.transportOptions, this.options, this.appId, this.sdkType,
-      this.auth, this.appCheck) {
+      this.appCheck) {
     String protocol = 'http';
     if (transportOptions.isSecure == null ||
         transportOptions.isSecure == true) {
@@ -34,9 +34,6 @@ class RestTransport implements DataConnectTransport {
         '$protocol://$host:$port/v1beta/projects/$project/locations/$location/services/$service/connectors/$connector';
   }
 
-  @override
-  FirebaseAuth? auth;
-
   @override
   FirebaseAppCheck? appCheck;
 
@@ -71,6 +68,7 @@ class RestTransport implements DataConnectTransport {
       Deserializer<Data> deserializer,
       Serializer<Variables>? serializer,
       Variables? vars,
+      String? authToken,
       String endpoint) async {
     String project = options.projectId;
     String location = options.location;
@@ -81,12 +79,6 @@ class RestTransport implements DataConnectTransport {
       'Accept': 'application/json',
       'x-goog-api-client': getGoogApiVal(sdkType, packageVersion)
     };
-    String? authToken;
-    try {
-      authToken = await auth?.currentUser?.getIdToken();
-    } catch (e) {
-      log('Unable to get auth token: $e');
-    }
     String? appCheckToken;
     try {
       appCheckToken = await appCheck?.getToken();
@@ -148,25 +140,25 @@ class RestTransport implements DataConnectTransport {
   /// Invokes query REST endpoint.
   @override
   Future<Data> invokeQuery<Data, Variables>(
-    String queryName,
-    Deserializer<Data> deserializer,
-    Serializer<Variables>? serializer,
-    Variables? vars,
-  ) async {
+      String queryName,
+      Deserializer<Data> deserializer,
+      Serializer<Variables>? serializer,
+      Variables? vars,
+      String? token) async {
     return invokeOperation(
-        queryName, deserializer, serializer, vars, 'executeQuery');
+        queryName, deserializer, serializer, vars, token, 'executeQuery');
   }
 
   /// Invokes mutation REST endpoint.
   @override
   Future<Data> invokeMutation<Data, Variables>(
-    String queryName,
-    Deserializer<Data> deserializer,
-    Serializer<Variables>? serializer,
-    Variables? vars,
-  ) async {
+      String queryName,
+      Deserializer<Data> deserializer,
+      Serializer<Variables>? serializer,
+      Variables? vars,
+      String? token) async {
     return invokeOperation(
-        queryName, deserializer, serializer, vars, 'executeMutation');
+        queryName, deserializer, serializer, vars, token, 'executeMutation');
   }
 }
 
@@ -176,6 +168,5 @@ DataConnectTransport getTransport(
         DataConnectOptions options,
         String appId,
         CallerSDKType sdkType,
-        FirebaseAuth? auth,
         FirebaseAppCheck? appCheck) =>
-    RestTransport(transportOptions, options, appId, sdkType, auth, appCheck);
+    RestTransport(transportOptions, options, appId, sdkType, appCheck);
diff --git a/packages/firebase_data_connect/firebase_data_connect/lib/src/network/transport_stub.dart b/packages/firebase_data_connect/firebase_data_connect/lib/src/network/transport_stub.dart
@@ -22,13 +22,11 @@ class TransportStub implements DataConnectTransport {
     this.options,
     this.appId,
     this.sdkType,
-    this.auth,
     this.appCheck,
   );
 
   /// FirebaseAuth
   @override
-  FirebaseAuth? auth;
 
   /// FirebaseAppCheck
   @override
@@ -55,7 +53,8 @@ class TransportStub implements DataConnectTransport {
       String queryName,
       Deserializer<Data> deserializer,
       Serializer<Variables>? serializer,
-      Variables? vars) async {
+      Variables? vars,
+      String? token) async {
     // TODO: implement invokeMutation
     throw UnimplementedError();
   }
@@ -66,7 +65,8 @@ class TransportStub implements DataConnectTransport {
       String queryName,
       Deserializer<Data> deserializer,
       Serializer<Variables>? serialize,
-      Variables? vars) async {
+      Variables? vars,
+      String? token) async {
     // TODO: implement invokeQuery
     throw UnimplementedError();
   }
@@ -80,4 +80,4 @@ DataConnectTransport getTransport(
   FirebaseAuth? auth,
   FirebaseAppCheck? appCheck,
 ) =>
-    TransportStub(transportOptions, options, appId, sdkType, auth, appCheck);
+    TransportStub(transportOptions, options, appId, sdkType, appCheck);
diff --git a/...ages/firebase_data_connect/firebase_data_connect/test/src/common/common_library_test.dart b/...ages/firebase_data_connect/firebase_data_connect/test/src/common/common_library_test.dart
@@ -105,7 +105,7 @@ void main() {
       final queryName = 'testQuery';
       final deserializer = (json) => json;
       final result = await transport.invokeQuery(
-          queryName, deserializer, emptySerializer, null);
+          queryName, deserializer, emptySerializer, null, null);
 
       expect(result, isNotNull);
     });
@@ -114,7 +114,7 @@ void main() {
       final queryName = 'testMutation';
       final deserializer = (json) => json;
       final result = await transport.invokeMutation(
-          queryName, deserializer, emptySerializer, null);
+          queryName, deserializer, emptySerializer, null, null);
 
       expect(result, isNotNull);
     });
@@ -127,28 +127,27 @@ class TestDataConnectTransport extends DataConnectTransport {
       DataConnectOptions options, String appId, CallerSDKType sdkType,
       {FirebaseAuth? auth, FirebaseAppCheck? appCheck})
       : super(transportOptions, options, appId, sdkType) {
-    this.auth = auth;
     this.appCheck = appCheck;
   }
 
   @override
   Future<Data> invokeQuery<Data, Variables>(
-    String queryName,
-    Deserializer<Data> deserializer,
-    Serializer<Variables>? serializer,
-    Variables? vars,
-  ) async {
+      String queryName,
+      Deserializer<Data> deserializer,
+      Serializer<Variables>? serializer,
+      Variables? vars,
+      String? authToken) async {
     // Simulate query invocation logic here
     return deserializer('{}');
   }
 
   @override
   Future<Data> invokeMutation<Data, Variables>(
-    String queryName,
-    Deserializer<Data> deserializer,
-    Serializer<Variables>? serializer,
-    Variables? vars,
-  ) async {
+      String queryName,
+      Deserializer<Data> deserializer,
+      Serializer<Variables>? serializer,
+      Variables? vars,
+      String? authToken) async {
     // Simulate mutation invocation logic here
     return deserializer('{}');
   }