From ce97ec351117fd3a6a00e4fd73be36ecf9c03f7c Mon Sep 17 00:00:00 2001 From: Dillon Nys <24740863+dnys1@users.noreply.github.com> Date: Thu, 7 Mar 2024 08:37:44 -0800 Subject: [PATCH] chore(core): Split storage interfaces (#62) Create two variations of a new `Storage` interface: `LocalStorage` and `SecureStorage`. This is necessary in Auth to properly store information, since platform keychains have different persistence characteristics (e.g. iOS/macOS persist after uninstall), and some platforms cannot support secure storage (e.g. Web). --- .github/workflows/celest_core.yaml | 10 ++--- .../integration_test/secure_storage_test.dart | 8 ---- ...torage_shared.dart => storage_shared.dart} | 6 +-- .../integration_test/storage_test.dart | 11 +++++ .../src/secure_storage/secure_storage.dart | 33 --------------- .../lib/src/storage/local/local_storage.dart | 7 ++++ .../local/local_storage_platform.vm.dart | 10 +++++ .../local/local_storage_platform.web.dart | 42 +++++++++++++++++++ .../lib/src/storage/memory_storage.dart | 24 +++++++++++ .../secure}/secure_storage.android.dart | 2 +- .../src/storage/secure/secure_storage.dart | 7 ++++ .../secure}/secure_storage.darwin.dart | 4 +- .../secure}/secure_storage.linux.dart | 4 +- .../secure}/secure_storage.windows.dart | 6 +-- .../secure}/secure_storage_exception.dart | 0 .../secure}/secure_storage_platform.vm.dart | 14 ++++--- .../secure}/secure_storage_platform.web.dart | 5 ++- .../celest_core/lib/src/storage/storage.dart | 12 ++++++ packages/celest_core/pubspec.yaml | 1 + .../secure_storage/secure_storage_test.dart | 3 -- .../secure}/secure_storage_darwin_test.dart | 2 +- .../test/storage/storage_test.dart | 9 ++++ 22 files changed, 151 insertions(+), 69 deletions(-) delete mode 100644 packages/celest_core/example/integration_test/secure_storage_test.dart rename packages/celest_core/example/integration_test/{secure_storage_shared.dart => storage_shared.dart} (94%) create mode 100644 packages/celest_core/example/integration_test/storage_test.dart delete mode 100644 packages/celest_core/lib/src/secure_storage/secure_storage.dart create mode 100644 packages/celest_core/lib/src/storage/local/local_storage.dart create mode 100644 packages/celest_core/lib/src/storage/local/local_storage_platform.vm.dart create mode 100644 packages/celest_core/lib/src/storage/local/local_storage_platform.web.dart create mode 100644 packages/celest_core/lib/src/storage/memory_storage.dart rename packages/celest_core/lib/src/{secure_storage => storage/secure}/secure_storage.android.dart (94%) create mode 100644 packages/celest_core/lib/src/storage/secure/secure_storage.dart rename packages/celest_core/lib/src/{secure_storage => storage/secure}/secure_storage.darwin.dart (97%) rename packages/celest_core/lib/src/{secure_storage => storage/secure}/secure_storage.linux.dart (97%) rename packages/celest_core/lib/src/{secure_storage => storage/secure}/secure_storage.windows.dart (96%) rename packages/celest_core/lib/src/{secure_storage => storage/secure}/secure_storage_exception.dart (100%) rename packages/celest_core/lib/src/{secure_storage => storage/secure}/secure_storage_platform.vm.dart (62%) rename packages/celest_core/lib/src/{secure_storage => storage/secure}/secure_storage_platform.web.dart (50%) create mode 100644 packages/celest_core/lib/src/storage/storage.dart delete mode 100644 packages/celest_core/test/secure_storage/secure_storage_test.dart rename packages/celest_core/test/{secure_storage => storage/secure}/secure_storage_darwin_test.dart (92%) create mode 100644 packages/celest_core/test/storage/storage_test.dart diff --git a/.github/workflows/celest_core.yaml b/.github/workflows/celest_core.yaml index ca216f7c..7541d60e 100644 --- a/.github/workflows/celest_core.yaml +++ b/.github/workflows/celest_core.yaml @@ -61,10 +61,10 @@ jobs: echo "Booted simulator" - name: Test (iOS) working-directory: packages/celest_core/example - run: flutter test -d ios integration_test/secure_storage_test.dart + run: flutter test -d ios integration_test/storage_test.dart - name: Test (macOS) working-directory: packages/celest_core/example - run: flutter test -d macos integration_test/secure_storage_test.dart + run: flutter test -d macos integration_test/storage_test.dart test_android: needs: analyze_and_format runs-on: @@ -93,7 +93,7 @@ jobs: # https://github.com/dart-lang/native/blob/001910c9f40d637cb25c19bb500fb89cebdf7450/pkgs/jni/android/build.gradle#L57C23-L57C25 api-level: 31 arch: x86_64 - script: cd packages/celest_core/example && flutter test -d emulator integration_test/secure_storage_test.dart + script: cd packages/celest_core/example && flutter test -d emulator integration_test/storage_test.dart test_linux: needs: analyze_and_format runs-on: ubuntu-latest @@ -125,7 +125,7 @@ jobs: # Headless tests require virtual display for the linux tests to run. export DISPLAY=:99 sudo Xvfb -ac :99 -screen 0 1280x1024x24 > /dev/null 2>&1 & - flutter test -d linux integration_test/secure_storage_test.dart + flutter test -d linux integration_test/storage_test.dart # TODO: Re-enable # Need to fix this: Git error. Command: `git clone --mirror https://github.com/dart-lang/native /c/Users/runneradmin/.pub-cache\git\cache\native-647c69ed8027da6d6def6bc40efa87cf1a2f76aa` # test_windows: @@ -150,7 +150,7 @@ jobs: # run: flutter pub get # - name: Test (Windows) # working-directory: packages/celest_core/example - # run: flutter test -d windows integration_test/secure_storage_test.dart + # run: flutter test -d windows integration_test/storage_test.dart test_web: needs: analyze_and_format runs-on: ubuntu-latest diff --git a/packages/celest_core/example/integration_test/secure_storage_test.dart b/packages/celest_core/example/integration_test/secure_storage_test.dart deleted file mode 100644 index 5e4eff5e..00000000 --- a/packages/celest_core/example/integration_test/secure_storage_test.dart +++ /dev/null @@ -1,8 +0,0 @@ -import 'package:integration_test/integration_test.dart'; - -import 'secure_storage_shared.dart'; - -void main() { - IntegrationTestWidgetsFlutterBinding.ensureInitialized(); - sharedTests(); -} diff --git a/packages/celest_core/example/integration_test/secure_storage_shared.dart b/packages/celest_core/example/integration_test/storage_shared.dart similarity index 94% rename from packages/celest_core/example/integration_test/secure_storage_shared.dart rename to packages/celest_core/example/integration_test/storage_shared.dart index 4561692c..4e5defe0 100644 --- a/packages/celest_core/example/integration_test/secure_storage_shared.dart +++ b/packages/celest_core/example/integration_test/storage_shared.dart @@ -1,12 +1,12 @@ import 'dart:math'; -import 'package:celest_core/src/secure_storage/secure_storage.dart'; +import 'package:celest_core/src/storage/storage.dart'; import 'package:test/test.dart'; -void sharedTests() { +void sharedTests(String name, Storage Function({String? scope}) factory) { group('SecureStorage', () { late String key; - final storage = SecureStorage(scope: 'test'); + final storage = factory(scope: 'test'); setUp(() { storage.clear(); diff --git a/packages/celest_core/example/integration_test/storage_test.dart b/packages/celest_core/example/integration_test/storage_test.dart new file mode 100644 index 00000000..bfbd1a76 --- /dev/null +++ b/packages/celest_core/example/integration_test/storage_test.dart @@ -0,0 +1,11 @@ +import 'package:celest_core/src/storage/local/local_storage.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.dart'; +import 'package:integration_test/integration_test.dart'; + +import 'storage_shared.dart'; + +void main() { + IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + sharedTests('SecureStorage', SecureStorage.new); + sharedTests('LocalStorage', LocalStorage.new); +} diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage.dart b/packages/celest_core/lib/src/secure_storage/secure_storage.dart deleted file mode 100644 index c41def08..00000000 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.dart +++ /dev/null @@ -1,33 +0,0 @@ -import 'package:celest_core/src/secure_storage/secure_storage_platform.vm.dart' - if (dart.library.js_interop) 'package:celest_core/src/secure_storage/secure_storage_platform.web.dart'; - -abstract interface class SecureStorage { - factory SecureStorage({String? scope}) = SecureStoragePlatform; - - String? read(String key); - String write(String key, String value); - String? delete(String key); - void clear(); -} - -/// An in-memory implementation of [SecureStorage]. -final class MemorySecureStorage implements SecureStorage { - MemorySecureStorage({ - required this.scope, - }); - - final _storage = {}; - final String scope; - - @override - void clear() => _storage.removeWhere((key, _) => key.startsWith('$scope/')); - - @override - String? delete(String key) => _storage.remove('$scope/$key'); - - @override - String? read(String key) => _storage['$scope/$key']; - - @override - String write(String key, String value) => _storage['$scope/$key'] = value; -} diff --git a/packages/celest_core/lib/src/storage/local/local_storage.dart b/packages/celest_core/lib/src/storage/local/local_storage.dart new file mode 100644 index 00000000..f380d1a7 --- /dev/null +++ b/packages/celest_core/lib/src/storage/local/local_storage.dart @@ -0,0 +1,7 @@ +import 'package:celest_core/src/storage/local/local_storage_platform.vm.dart' + if (dart.library.js_interop) 'package:celest_core/src/storage/local/local_storage_platform.web.dart'; +import 'package:celest_core/src/storage/storage.dart'; + +abstract interface class LocalStorage implements Storage { + factory LocalStorage({String? scope}) = LocalStoragePlatform; +} diff --git a/packages/celest_core/lib/src/storage/local/local_storage_platform.vm.dart b/packages/celest_core/lib/src/storage/local/local_storage_platform.vm.dart new file mode 100644 index 00000000..dceec240 --- /dev/null +++ b/packages/celest_core/lib/src/storage/local/local_storage_platform.vm.dart @@ -0,0 +1,10 @@ +import 'package:celest_core/src/storage/local/local_storage.dart'; +import 'package:celest_core/src/storage/secure/secure_storage_platform.vm.dart'; + +extension type LocalStoragePlatform._(LocalStorage _impl) + implements LocalStorage { + LocalStoragePlatform({String? scope}) + : _impl = SecureStoragePlatform(scope: scope ?? _defaultScope); + + static const _defaultScope = 'dev.celest.celest'; +} diff --git a/packages/celest_core/lib/src/storage/local/local_storage_platform.web.dart b/packages/celest_core/lib/src/storage/local/local_storage_platform.web.dart new file mode 100644 index 00000000..fb830cb3 --- /dev/null +++ b/packages/celest_core/lib/src/storage/local/local_storage_platform.web.dart @@ -0,0 +1,42 @@ +import 'package:celest_core/src/storage/local/local_storage.dart'; +import 'package:web/web.dart' as web; + +final class LocalStoragePlatform implements LocalStorage { + LocalStoragePlatform({String? scope}) : scope = scope ?? _defaultScope; + + static const _defaultScope = 'dev.celest.celest'; + + final String scope; + final web.Storage _storage = web.window.localStorage; + + @override + void clear() { + for (final key in _storage.keys) { + if (key.startsWith('$scope/')) { + _storage.removeItem(key); + } + } + } + + @override + String? delete(String key) { + final value = read(key); + if (value != null) { + _storage.removeItem('$scope/$key'); + } + return null; + } + + @override + String? read(String key) => _storage['$scope/$key']; + + @override + String write(String key, String value) { + _storage.setItem('$scope/$key', value); + return value; + } +} + +extension on web.Storage { + List get keys => [for (var i = 0; i < length; i++) key(i)!]; +} diff --git a/packages/celest_core/lib/src/storage/memory_storage.dart b/packages/celest_core/lib/src/storage/memory_storage.dart new file mode 100644 index 00000000..ece77fb6 --- /dev/null +++ b/packages/celest_core/lib/src/storage/memory_storage.dart @@ -0,0 +1,24 @@ +import 'package:celest_core/src/storage/secure/secure_storage.dart'; +import 'package:celest_core/src/storage/storage.dart'; + +/// An in-memory implementation of [Storage] and [SecureStorage]. +final class MemoryStorage implements Storage, SecureStorage { + MemoryStorage({ + required this.scope, + }); + + final _storage = {}; + final String scope; + + @override + void clear() => _storage.removeWhere((key, _) => key.startsWith('$scope/')); + + @override + String? delete(String key) => _storage.remove('$scope/$key'); + + @override + String? read(String key) => _storage['$scope/$key']; + + @override + String write(String key, String value) => _storage['$scope/$key'] = value; +} diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage.android.dart b/packages/celest_core/lib/src/storage/secure/secure_storage.android.dart similarity index 94% rename from packages/celest_core/lib/src/secure_storage/secure_storage.android.dart rename to packages/celest_core/lib/src/storage/secure/secure_storage.android.dart index 7db250a4..c859b77c 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.android.dart +++ b/packages/celest_core/lib/src/storage/secure/secure_storage.android.dart @@ -1,5 +1,5 @@ import 'package:celest_core/src/native/android/jni_bindings.ffi.dart'; -import 'package:celest_core/src/secure_storage/secure_storage_platform.vm.dart'; +import 'package:celest_core/src/storage/secure/secure_storage_platform.vm.dart'; import 'package:jni/jni.dart'; final class SecureStoragePlatformAndroid extends SecureStoragePlatform { diff --git a/packages/celest_core/lib/src/storage/secure/secure_storage.dart b/packages/celest_core/lib/src/storage/secure/secure_storage.dart new file mode 100644 index 00000000..d0afd1ba --- /dev/null +++ b/packages/celest_core/lib/src/storage/secure/secure_storage.dart @@ -0,0 +1,7 @@ +import 'package:celest_core/src/storage/secure/secure_storage_platform.vm.dart' + if (dart.library.js_interop) 'package:celest_core/src/storage/secure/secure_storage_platform.web.dart'; +import 'package:celest_core/src/storage/storage.dart'; + +abstract interface class SecureStorage implements Storage { + factory SecureStorage({String? scope}) = SecureStoragePlatform; +} diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage.darwin.dart b/packages/celest_core/lib/src/storage/secure/secure_storage.darwin.dart similarity index 97% rename from packages/celest_core/lib/src/secure_storage/secure_storage.darwin.dart rename to packages/celest_core/lib/src/storage/secure/secure_storage.darwin.dart index e73ee466..699663d7 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.darwin.dart +++ b/packages/celest_core/lib/src/storage/secure/secure_storage.darwin.dart @@ -4,8 +4,8 @@ import 'dart:io'; import 'package:celest_core/src/native/darwin/core_foundation.ffi.dart'; import 'package:celest_core/src/native/darwin/darwin_ffi_helpers.dart'; import 'package:celest_core/src/native/darwin/security.ffi.dart'; -import 'package:celest_core/src/secure_storage/secure_storage_exception.dart'; -import 'package:celest_core/src/secure_storage/secure_storage_platform.vm.dart'; +import 'package:celest_core/src/storage/secure/secure_storage_exception.dart'; +import 'package:celest_core/src/storage/secure/secure_storage_platform.vm.dart'; import 'package:celest_core/src/util/globals.dart'; import 'package:ffi/ffi.dart'; diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage.linux.dart b/packages/celest_core/lib/src/storage/secure/secure_storage.linux.dart similarity index 97% rename from packages/celest_core/lib/src/secure_storage/secure_storage.linux.dart rename to packages/celest_core/lib/src/storage/secure/secure_storage.linux.dart index e764bc91..b29d5972 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.linux.dart +++ b/packages/celest_core/lib/src/storage/secure/secure_storage.linux.dart @@ -3,8 +3,8 @@ import 'dart:io'; import 'package:celest_core/src/native/linux/glib.ffi.dart'; import 'package:celest_core/src/native/linux/libsecret.ffi.dart'; -import 'package:celest_core/src/secure_storage/secure_storage_exception.dart'; -import 'package:celest_core/src/secure_storage/secure_storage_platform.vm.dart'; +import 'package:celest_core/src/storage/secure/secure_storage_exception.dart'; +import 'package:celest_core/src/storage/secure/secure_storage_platform.vm.dart'; import 'package:ffi/ffi.dart'; final class SecureStoragePlatformLinux extends SecureStoragePlatform { diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage.windows.dart b/packages/celest_core/lib/src/storage/secure/secure_storage.windows.dart similarity index 96% rename from packages/celest_core/lib/src/secure_storage/secure_storage.windows.dart rename to packages/celest_core/lib/src/storage/secure/secure_storage.windows.dart index b2171d0b..6815fda2 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.windows.dart +++ b/packages/celest_core/lib/src/storage/secure/secure_storage.windows.dart @@ -4,10 +4,10 @@ import 'dart:io'; import 'dart:typed_data'; import 'package:celest_core/src/native/windows/windows_paths.dart'; -import 'package:celest_core/src/secure_storage/secure_storage_exception.dart'; -import 'package:celest_core/src/secure_storage/secure_storage_platform.vm.dart'; -import 'package:path/path.dart' as p; +import 'package:celest_core/src/storage/secure/secure_storage_exception.dart'; +import 'package:celest_core/src/storage/secure/secure_storage_platform.vm.dart'; import 'package:ffi/ffi.dart'; +import 'package:path/path.dart' as p; import 'package:win32/win32.dart'; final class SecureStoragePlatformWindows extends SecureStoragePlatform { diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage_exception.dart b/packages/celest_core/lib/src/storage/secure/secure_storage_exception.dart similarity index 100% rename from packages/celest_core/lib/src/secure_storage/secure_storage_exception.dart rename to packages/celest_core/lib/src/storage/secure/secure_storage_exception.dart diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage_platform.vm.dart b/packages/celest_core/lib/src/storage/secure/secure_storage_platform.vm.dart similarity index 62% rename from packages/celest_core/lib/src/secure_storage/secure_storage_platform.vm.dart rename to packages/celest_core/lib/src/storage/secure/secure_storage_platform.vm.dart index a4ddb92a..7fd73088 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage_platform.vm.dart +++ b/packages/celest_core/lib/src/storage/secure/secure_storage_platform.vm.dart @@ -1,13 +1,15 @@ import 'dart:io'; -import 'package:celest_core/src/secure_storage/secure_storage.android.dart'; -import 'package:celest_core/src/secure_storage/secure_storage.dart'; -import 'package:celest_core/src/secure_storage/secure_storage.darwin.dart'; -import 'package:celest_core/src/secure_storage/secure_storage.linux.dart'; -import 'package:celest_core/src/secure_storage/secure_storage.windows.dart'; +import 'package:celest_core/src/storage/local/local_storage.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.android.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.darwin.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.linux.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.windows.dart'; import 'package:meta/meta.dart'; -abstract base class SecureStoragePlatform implements SecureStorage { +abstract base class SecureStoragePlatform + implements SecureStorage, LocalStorage { factory SecureStoragePlatform({ String? scope, }) { diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage_platform.web.dart b/packages/celest_core/lib/src/storage/secure/secure_storage_platform.web.dart similarity index 50% rename from packages/celest_core/lib/src/secure_storage/secure_storage_platform.web.dart rename to packages/celest_core/lib/src/storage/secure/secure_storage_platform.web.dart index 821de141..95307c82 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage_platform.web.dart +++ b/packages/celest_core/lib/src/storage/secure/secure_storage_platform.web.dart @@ -1,9 +1,10 @@ -import 'package:celest_core/src/secure_storage/secure_storage.dart'; +import 'package:celest_core/src/storage/memory_storage.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.dart'; extension type SecureStoragePlatform._(SecureStorage _impl) implements SecureStorage { SecureStoragePlatform({String? scope}) - : _impl = MemorySecureStorage(scope: scope ?? _defaultScope); + : _impl = MemoryStorage(scope: scope ?? _defaultScope); static const _defaultScope = 'dev.celest.celest'; } diff --git a/packages/celest_core/lib/src/storage/storage.dart b/packages/celest_core/lib/src/storage/storage.dart new file mode 100644 index 00000000..7bc64447 --- /dev/null +++ b/packages/celest_core/lib/src/storage/storage.dart @@ -0,0 +1,12 @@ +import 'package:celest_core/src/storage/local/local_storage.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.dart'; + +abstract interface class Storage { + factory Storage.local({String? scope}) = LocalStorage; + factory Storage.secure({String? scope}) = SecureStorage; + + String? read(String key); + String write(String key, String value); + String? delete(String key); + void clear(); +} diff --git a/packages/celest_core/pubspec.yaml b/packages/celest_core/pubspec.yaml index 009693fe..a9db7617 100644 --- a/packages/celest_core/pubspec.yaml +++ b/packages/celest_core/pubspec.yaml @@ -14,6 +14,7 @@ dependencies: meta: ^1.10.0 os_detect: ^2.0.1 path: ^1.9.0 + web: ^0.5.1 win32: ^5.2.0 dev_dependencies: diff --git a/packages/celest_core/test/secure_storage/secure_storage_test.dart b/packages/celest_core/test/secure_storage/secure_storage_test.dart deleted file mode 100644 index 581b6be9..00000000 --- a/packages/celest_core/test/secure_storage/secure_storage_test.dart +++ /dev/null @@ -1,3 +0,0 @@ -import '../../example/integration_test/secure_storage_shared.dart'; - -void main() => sharedTests(); diff --git a/packages/celest_core/test/secure_storage/secure_storage_darwin_test.dart b/packages/celest_core/test/storage/secure/secure_storage_darwin_test.dart similarity index 92% rename from packages/celest_core/test/secure_storage/secure_storage_darwin_test.dart rename to packages/celest_core/test/storage/secure/secure_storage_darwin_test.dart index 0a3e1197..67efa3c3 100644 --- a/packages/celest_core/test/secure_storage/secure_storage_darwin_test.dart +++ b/packages/celest_core/test/storage/secure/secure_storage_darwin_test.dart @@ -1,7 +1,7 @@ @TestOn('mac-os') import 'package:celest_core/src/native/darwin/security.ffi.dart'; -import 'package:celest_core/src/secure_storage/secure_storage.darwin.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.darwin.dart'; import 'package:test/test.dart'; void main() { diff --git a/packages/celest_core/test/storage/storage_test.dart b/packages/celest_core/test/storage/storage_test.dart new file mode 100644 index 00000000..b2d21f3f --- /dev/null +++ b/packages/celest_core/test/storage/storage_test.dart @@ -0,0 +1,9 @@ +import 'package:celest_core/src/storage/local/local_storage.dart'; +import 'package:celest_core/src/storage/secure/secure_storage.dart'; + +import '../../example/integration_test/storage_shared.dart'; + +void main() { + sharedTests('SecureStorage', SecureStorage.new); + sharedTests('LocalStorage', LocalStorage.new); +}