From eeae84b3ff50dd626c973462d1b8c81725f3033f Mon Sep 17 00:00:00 2001 From: Dillon Nys <24740863+dnys1@users.noreply.github.com> Date: Wed, 6 Mar 2024 22:51:37 -0800 Subject: [PATCH] chore(core): Secure storage clean up (#61) - Separate memory implementation into its own class - Clean up Linux implementation --- .github/workflows/celest_core.yaml | 20 ++++++++++++++++ packages/celest_core/ffigen.glib.yaml | 7 ++++++ .../lib/src/native/linux/glib.ffi.dart | 9 +++---- .../src/secure_storage/secure_storage.dart | 22 +++++++++++++++++ .../secure_storage/secure_storage.linux.dart | 23 ++++++++---------- .../secure_storage/secure_storage.stub.dart | 24 ------------------- .../secure_storage_platform.web.dart | 14 ++++------- 7 files changed, 68 insertions(+), 51 deletions(-) delete mode 100644 packages/celest_core/lib/src/secure_storage/secure_storage.stub.dart diff --git a/.github/workflows/celest_core.yaml b/.github/workflows/celest_core.yaml index 1d748201..ca216f7c 100644 --- a/.github/workflows/celest_core.yaml +++ b/.github/workflows/celest_core.yaml @@ -151,3 +151,23 @@ jobs: # - name: Test (Windows) # working-directory: packages/celest_core/example # run: flutter test -d windows integration_test/secure_storage_test.dart + test_web: + needs: analyze_and_format + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Git Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # 4.1.1 + - name: Setup Flutter + uses: subosito/flutter-action@62f096cacda5168a3bd7b95793373be14fa4fbaf # 2.13.0 + with: + cache: true + - name: Get Packages + working-directory: packages/celest_core + run: dart pub get + - name: Test (Chrome, dart2js) + working-directory: packages/celest_core + run: dart test -p chrome + - name: Test (Chrome, dart2wasm) + working-directory: packages/celest_core + run: dart test -p chrome -c dart2wasm diff --git a/packages/celest_core/ffigen.glib.yaml b/packages/celest_core/ffigen.glib.yaml index 0af4a67b..c59218d5 100644 --- a/packages/celest_core/ffigen.glib.yaml +++ b/packages/celest_core/ffigen.glib.yaml @@ -24,6 +24,7 @@ typedefs: - gboolean - gint - gpointer + - gchar functions: include: - g_hash_table_new @@ -43,3 +44,9 @@ structs: "_GHashTable": GHashTable "_GCancellable": GCancellable "_GObject": GObject +type-map: + typedefs: + gchar: + lib: pkg_ffi + c-type: Utf8 + dart-type: Char diff --git a/packages/celest_core/lib/src/native/linux/glib.ffi.dart b/packages/celest_core/lib/src/native/linux/glib.ffi.dart index b516a959..e062d26e 100644 --- a/packages/celest_core/lib/src/native/linux/glib.ffi.dart +++ b/packages/celest_core/lib/src/native/linux/glib.ffi.dart @@ -6,6 +6,7 @@ // // Generated by `package:ffigen`. import 'dart:ffi' as ffi; +import 'package:ffi/ffi.dart' as pkg_ffi; /// Bindings for glib on Linux. /// @@ -108,7 +109,7 @@ class Glib { late final _g_hash_table_insert = _g_hash_table_insertPtr .asFunction, gpointer, gpointer)>(); - ffi.Pointer g_application_get_application_id( + ffi.Pointer g_application_get_application_id( ffi.Pointer<_GApplication> application, ) { return _g_application_get_application_id( @@ -118,11 +119,11 @@ class Glib { late final _g_application_get_application_idPtr = _lookup< ffi.NativeFunction< - ffi.Pointer Function( + ffi.Pointer Function( ffi.Pointer<_GApplication>)>>('g_application_get_application_id'); late final _g_application_get_application_id = _g_application_get_application_idPtr.asFunction< - ffi.Pointer Function(ffi.Pointer<_GApplication>)>(); + ffi.Pointer Function(ffi.Pointer<_GApplication>)>(); ffi.Pointer<_GApplication> g_application_get_default() { return _g_application_get_default(); @@ -142,7 +143,7 @@ final class GError extends ffi.Struct { @gint() external int code; - external ffi.Pointer message; + external ffi.Pointer message; } typedef gint = ffi.Int; diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage.dart b/packages/celest_core/lib/src/secure_storage/secure_storage.dart index e90ac9bb..c41def08 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.dart +++ b/packages/celest_core/lib/src/secure_storage/secure_storage.dart @@ -9,3 +9,25 @@ abstract interface class SecureStorage { 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/secure_storage/secure_storage.linux.dart b/packages/celest_core/lib/src/secure_storage/secure_storage.linux.dart index c50cc425..e764bc91 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.linux.dart +++ b/packages/celest_core/lib/src/secure_storage/secure_storage.linux.dart @@ -32,15 +32,11 @@ final class SecureStoragePlatformLinux extends SecureStoragePlatform { if (application == nullptr) { return File('/proc/self/exe').resolveSymbolicLinksSync(); } - return _gio - .g_application_get_application_id(application) - .cast() - .toDartString(); + return _gio.g_application_get_application_id(application).toDartString(); }(); - String _labelFor(String key) => '$scope/$key'; - Pointer _schemaFor(Arena arena) => arena() - ..ref.name = _appName.toNativeUtf8(allocator: arena) + Pointer _schema(Arena arena) => arena() + ..ref.name = '$_appName/$scope'.toNativeUtf8(allocator: arena) ..ref.flags = SecretSchemaFlags.SECRET_SCHEMA_NONE ..ref.attributes[0].name = 'key'.toNativeUtf8(allocator: arena) ..ref.attributes[0].type = @@ -64,7 +60,7 @@ final class SecureStoragePlatformLinux extends SecureStoragePlatform { @override void clear() => using((arena) { - final schema = _schemaFor(arena); + final schema = _schema(arena); final attributes = _attributes(arena: arena); _check( (err) => _libSecret.secret_password_clearv_sync( @@ -80,7 +76,7 @@ final class SecureStoragePlatformLinux extends SecureStoragePlatform { @override String? delete(String key) => using((arena) { final secret = read(key); - final schema = _schemaFor(arena); + final schema = _schema(arena); final attributes = _attributes(key: key, arena: arena); _check( (err) => _libSecret.secret_password_clearv_sync( @@ -97,7 +93,7 @@ final class SecureStoragePlatformLinux extends SecureStoragePlatform { @override String? read(String key) => using((arena) { final attributes = _attributes(key: key, arena: arena); - final schema = _schemaFor(arena); + final schema = _schema(arena); final result = _check( (err) => _libSecret.secret_password_lookupv_sync( schema, @@ -117,12 +113,13 @@ final class SecureStoragePlatformLinux extends SecureStoragePlatform { @override String write(String key, String value) { using((arena) { - final label = _labelFor(key).toNativeUtf8(allocator: arena); + final schema = _schema(arena); + final label = key.toNativeUtf8(allocator: arena); final secret = value.toNativeUtf8(allocator: arena); final attributes = _attributes(key: key, arena: arena); _check( (err) => _libSecret.secret_password_storev_sync( - _schemaFor(arena), + schema, attributes, nullptr, label, @@ -145,7 +142,7 @@ final class SecureStoragePlatformLinux extends SecureStoragePlatform { final error = err.value; if (error != nullptr) { arena.onReleaseAll(() => _glib.g_error_free(error)); - final message = error.ref.message.cast().toDartString(); + final message = error.ref.message.toDartString(); throw SecureStorageUnknownException(message); } return result; diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage.stub.dart b/packages/celest_core/lib/src/secure_storage/secure_storage.stub.dart deleted file mode 100644 index 856f05c9..00000000 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.stub.dart +++ /dev/null @@ -1,24 +0,0 @@ -import 'package:celest_core/src/secure_storage/secure_storage_platform.web.dart'; - -/// An in-memory implementation of [SecureStoragePlatform] for platforms which -/// do not support secure storage. -final class SecureStoragePlatformStub extends SecureStoragePlatform { - SecureStoragePlatformStub({ - required this.scope, - }) : super.base(); - - 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_platform.web.dart b/packages/celest_core/lib/src/secure_storage/secure_storage_platform.web.dart index a657e936..821de141 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage_platform.web.dart +++ b/packages/celest_core/lib/src/secure_storage/secure_storage_platform.web.dart @@ -1,15 +1,9 @@ import 'package:celest_core/src/secure_storage/secure_storage.dart'; -import 'package:celest_core/src/secure_storage/secure_storage.stub.dart'; -import 'package:meta/meta.dart'; -abstract base class SecureStoragePlatform implements SecureStorage { - factory SecureStoragePlatform({ - String? scope, - }) => - SecureStoragePlatformStub(scope: scope ?? _defaultScope); +extension type SecureStoragePlatform._(SecureStorage _impl) + implements SecureStorage { + SecureStoragePlatform({String? scope}) + : _impl = MemorySecureStorage(scope: scope ?? _defaultScope); static const _defaultScope = 'dev.celest.celest'; - - @protected - const SecureStoragePlatform.base(); }