chore(auth): Disambiguate native storage scoping (#148)

Use consistent absolute scoping when referring to sections of the native storage to avoid collisions/incorrect storage.
celest-dev · Jun 8, 2024 · e815d9a · e815d9a
1 parent a31e037
commit e815d9a
Show file tree

Hide file tree

Showing 9 changed files with 184 additions and 11 deletions.
diff --git a/.github/workflows/celest_auth.yaml b/.github/workflows/celest_auth.yaml
@@ -33,6 +33,9 @@ jobs:
       - name: Format
         working-directory: packages/celest_auth
         run: dart format --set-exit-if-changed .
+      - name: Test
+        working-directory: packages/celest_auth
+        run: dart test
       - name: Test (Example)
         working-directory: packages/celest_auth/example/celest
         run: dart test

diff --git a/packages/celest_auth/CHANGELOG.md b/packages/celest_auth/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.4.2-wip
+
+- chore: Disambiguate native storage scoping
+
 ## 0.4.1
 
 - fix: Pana issues ([dart-lang/pana#1351](https://github.com/dart-lang/pana/issues/1351))

diff --git a/packages/celest_auth/lib/src/auth_impl.dart b/packages/celest_auth/lib/src/auth_impl.dart
@@ -13,7 +13,7 @@ final class AuthImpl implements Auth {
   AuthImpl(
     this.celest, {
     NativeStorage? storage,
-  }) : _storage = storage ?? NativeStorage(scope: 'celest/auth');
+  }) : _storage = (storage ?? celest.nativeStorage).scoped('/celest/auth');
 
   AuthState? _authState;
 
@@ -29,7 +29,7 @@ final class AuthImpl implements Auth {
   final StreamController<AuthState> _authStateController =
       StreamController.broadcast();
 
-  late final StreamSubscription<AuthState> _authStateSubscription;
+  StreamSubscription<AuthState>? _authStateSubscription;
   StreamSubscription<AuthState>? _authFlowSubscription;
 
   @override
@@ -87,7 +87,9 @@ final class AuthImpl implements Auth {
     try {
       await protocol.signOut();
     } finally {
-      _authStateController.add(const Unauthenticated());
+      if (!_authStateController.isClosed) {
+        _authStateController.add(const Unauthenticated());
+      }
     }
   }
 
@@ -100,7 +102,7 @@ final class AuthImpl implements Auth {
   late final AuthClient protocol = AuthClient(celest);
 
   Future<void> close() async {
-    await _authStateSubscription.cancel();
+    await _authStateSubscription?.cancel();
     await _authFlowSubscription?.cancel();
     await _authStateController.close();
     localStorage.close();

diff --git a/packages/celest_auth/pubspec.yaml b/packages/celest_auth/pubspec.yaml
@@ -1,6 +1,6 @@
 name: celest_auth
 description: The Auth runtime and client library for Celest, the Flutter cloud platform.
-version: 0.4.1
+version: 0.4.2-wip
 homepage: https://celest.dev
 repository: https://github.com/celest-dev/celest/tree/main/packages/celest_auth
 
@@ -13,7 +13,7 @@ dependencies:
   built_collection: ^5.1.1
   built_value: ^8.9.1
   cedar: ^0.1.0
-  celest_core: ^0.4.0
+  celest_core: ^0.4.3-wip
   chunked_stream: ^1.4.2
   corks_cedar: ^0.1.0
   ffi: ^2.1.2

diff --git a/packages/celest_auth/test/auth_impl_test.dart b/packages/celest_auth/test/auth_impl_test.dart
@@ -0,0 +1,161 @@
+import 'dart:convert';
+
+import 'package:celest_auth/celest_auth.dart';
+import 'package:celest_auth/src/auth_impl.dart';
+import 'package:celest_core/_internal.dart';
+import 'package:http/http.dart';
+import 'package:http/testing.dart';
+import 'package:test/test.dart';
+
+class Celest extends CelestBase {
+  Celest({
+    required Client httpClient,
+  }) {
+    this.httpClient = CelestHttpClient(
+      secureStorage: nativeStorage,
+      baseClient: httpClient,
+    );
+  }
+
+  @override
+  Uri get baseUri => Uri.parse('');
+
+  @override
+  late final Client httpClient;
+
+  @override
+  final NativeMemoryStorage nativeStorage = NativeMemoryStorage();
+}
+
+final unauthorized = Response(
+  jsonEncode({
+    'error': {
+      'code': 'UnauthorizedException',
+      'details': {'message': 'Unauthorized'},
+    },
+  }),
+  401,
+);
+final ok = Response('{}', 200);
+
+final class MockAuth extends BaseClient {
+  MockAuth({
+    this.signOut = _default,
+    this.userInfo = _default,
+  });
+
+  static Future<Response> _default(Request request) async => Response('', 404);
+
+  final MockClientHandler signOut;
+  final MockClientHandler userInfo;
+
+  @override
+  Future<StreamedResponse> send(BaseRequest baseRequest) async {
+    final bodyBytes = await baseRequest.finalize().toBytes();
+    var request = Request(baseRequest.method, baseRequest.url)
+      ..persistentConnection = baseRequest.persistentConnection
+      ..followRedirects = baseRequest.followRedirects
+      ..maxRedirects = baseRequest.maxRedirects
+      ..headers.addAll(baseRequest.headers)
+      ..bodyBytes = bodyBytes
+      ..finalize();
+    final handler = switch (request.url.path) {
+      '/_auth/sign-out' => signOut,
+      '/_auth/userinfo' => userInfo,
+      _ => _default,
+    };
+    final response = await handler(request);
+    return StreamedResponse(
+      ByteStream.fromBytes(response.bodyBytes),
+      response.statusCode,
+      contentLength: response.contentLength,
+      request: response.request,
+      headers: response.headers,
+      isRedirect: response.isRedirect,
+      persistentConnection: response.persistentConnection,
+      reasonPhrase: response.reasonPhrase,
+    );
+  }
+}
+
+void main() {
+  group('AuthImpl', () {
+    tearDown(() {
+      NativeStorage.instances.clear();
+    });
+
+    group('init', () {
+      test('no session', () async {
+        final celest = Celest(
+          httpClient: MockAuth(
+            userInfo: expectAsync1((request) async {
+              expect(request.headers, isNot(contains('authorization')));
+              return unauthorized;
+            }),
+            signOut: expectAsync1((request) async {
+              expect(request.headers, isNot(contains('authorization')));
+              return ok;
+            }),
+          ),
+        );
+        final auth = AuthImpl(celest);
+        addTearDown(auth.close);
+
+        final state = await auth.init();
+        expect(state, isA<Unauthenticated>());
+      });
+
+      test('invalid session', () async {
+        final celest = Celest(
+          httpClient: MockAuth(
+            userInfo: expectAsync1((request) async {
+              expect(
+                request.headers,
+                containsPair('authorization', 'Bearer invalid'),
+              );
+              return unauthorized;
+            }),
+            signOut: expectAsync1((request) async {
+              expect(request.headers, isNot(contains('authorization')));
+              return ok;
+            }),
+          ),
+        );
+        final auth = AuthImpl(celest);
+        addTearDown(auth.close);
+
+        await auth.secureStorage.write('cork', 'invalid');
+
+        final state = await auth.init();
+        expect(state, isA<Unauthenticated>());
+      });
+    });
+
+    group('close', () {
+      test('without init', () {
+        final celest = Celest(
+          httpClient: MockAuth(),
+        );
+        final auth = AuthImpl(celest);
+        expect(auth.close(), completes);
+      });
+
+      test('called multiple times', () async {
+        final celest = Celest(
+          httpClient: MockAuth(),
+        );
+        final auth = AuthImpl(celest);
+        await expectLater(auth.close(), completes);
+        await expectLater(auth.close(), completes);
+      });
+
+      test('called multiple times synchronously', () {
+        final celest = Celest(
+          httpClient: MockAuth(),
+        );
+        final auth = AuthImpl(celest);
+        expect(Future.wait([auth.close(), auth.close()]), completes);
+      });
+    });
+  });
+}
diff --git a/packages/celest_core/CHANGELOG.md b/packages/celest_core/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.4.3-wip
+
+- chore: Update dependencies
+
 ## 0.4.2
 
 - feat: Add support for event streaming via SSE/WebSockets

diff --git a/packages/celest_core/lib/src/auth/authenticator.dart b/packages/celest_core/lib/src/auth/authenticator.dart
@@ -3,7 +3,7 @@ import 'package:celest_core/_internal.dart';
 final class Authenticator {
   Authenticator({
     required NativeSecureStorage secureStorage,
-  }) : _secureStorage = secureStorage;
+  }) : _secureStorage = secureStorage.scoped('/celest/auth');
 
   final NativeSecureStorage _secureStorage;
 

diff --git a/packages/celest_core/lib/src/http/celest_http_client.dart b/packages/celest_core/lib/src/http/celest_http_client.dart
@@ -9,8 +9,7 @@ final class CelestHttpClient extends http.BaseClient {
     NativeSecureStorage? secureStorage,
     http.Client? baseClient,
   })  : _authenticator = Authenticator(
-          secureStorage:
-              secureStorage ?? NativeSecureStorage(scope: 'celest/auth'),
+          secureStorage: secureStorage ?? NativeSecureStorage(),
         ),
         _ownsInner = baseClient == null,
         _inner = baseClient ?? createHttpClient();

diff --git a/packages/celest_core/pubspec.yaml b/packages/celest_core/pubspec.yaml
@@ -1,6 +1,6 @@
 name: celest_core
 description: Celest types and utilities shared between the client and the cloud.
-version: 0.4.2
+version: 0.4.3-wip
 homepage: https://celest.dev
 repository: https://github.com/celest-dev/celest/tree/main/packages/celest_core
 
@@ -15,7 +15,7 @@ dependencies:
   http_parser: ^4.0.0
   logging: ^1.2.0
   meta: ^1.10.0
-  native_storage: ^0.1.0
+  native_storage: ^0.1.7
   os_detect: ^2.0.1
   path: ^1.9.0
   stream_channel: ^2.1.2