From 37c0b104f267a8d37d695254d0575d72b01ce358 Mon Sep 17 00:00:00 2001 From: Dillon Nys Date: Wed, 6 Mar 2024 13:51:12 -0800 Subject: [PATCH] clean up --- .github/workflows/celest_core.yaml | 55 ++++++++++++++++--- .vscode/settings.json | 6 -- packages/celest_core/CHANGELOG.md | 2 +- .../celest/celest_core/CelestSecureStorage.kt | 2 +- .../secure_storage_shared.dart | 32 +++++++++-- .../secure_storage.android.dart | 9 ++- .../secure_storage_darwin_test.dart | 42 -------------- .../secure_storage/secure_storage_test.dart | 4 +- 8 files changed, 84 insertions(+), 68 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.github/workflows/celest_core.yaml b/.github/workflows/celest_core.yaml index f931c818..419ffaa5 100644 --- a/.github/workflows/celest_core.yaml +++ b/.github/workflows/celest_core.yaml @@ -31,10 +31,27 @@ jobs: - name: Format working-directory: packages/celest_core run: dart format --set-exit-if-changed . - test: + test_dart: + needs: analyze_and_format + runs-on: ubuntu-latest + timeout-minutes: 5 + 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 + working-directory: packages/celest_core + run: dart test + test_flutter_darwin: needs: analyze_and_format runs-on: macos-13-large - timeout-minutes: 15 + timeout-minutes: 10 steps: - name: Git Checkout uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # 4.1.1 @@ -51,13 +68,35 @@ jobs: - name: Get Packages (Example) working-directory: packages/celest_core/example run: flutter pub get - - name: Test (Example) + - name: Setup iOS Simulator + uses: aws-amplify/amplify-flutter/.github/composite_actions/launch_ios_simulator@main + - name: Test (iOS) + working-directory: packages/celest_core/example + run: flutter test -d iPhone integration_test/secure_storage_test.dart + - name: Test (macOS) + working-directory: packages/celest_core/example + run: flutter test -d macos integration_test/secure_storage_test.dart + test_flutter_android: + needs: analyze_and_format + runs-on: linux + 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/example + run: flutter pub get + - name: Test (Android) uses: aws-amplify/amplify-flutter/.github/composite_actions/launch_android_emulator@main with: - api-level: 34 + # Matches `package:jni` compileSdkVersion + # 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 - for OS in sdk iPhone macos - do - flutter test -d $OS integration_test/secure_storage_test.dart - done + flutter test -d sdk integration_test/secure_storage_test.dart diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 290fa83f..00000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "files.associations": { - "__locale": "cpp", - "locale": "cpp" - } -} \ No newline at end of file diff --git a/packages/celest_core/CHANGELOG.md b/packages/celest_core/CHANGELOG.md index f1399e1f..11aef13b 100644 --- a/packages/celest_core/CHANGELOG.md +++ b/packages/celest_core/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.2.2-wip -- Adds `Keychain` interface for secure storage of sensitive data. +- Adds `SecureStorage` interface for storage of sensitive data in the platform keychain. ## 0.2.1 diff --git a/packages/celest_core/android/src/main/kotlin/dev/celest/celest_core/CelestSecureStorage.kt b/packages/celest_core/android/src/main/kotlin/dev/celest/celest_core/CelestSecureStorage.kt index 1aa4a3f4..cb95e425 100644 --- a/packages/celest_core/android/src/main/kotlin/dev/celest/celest_core/CelestSecureStorage.kt +++ b/packages/celest_core/android/src/main/kotlin/dev/celest/celest_core/CelestSecureStorage.kt @@ -38,7 +38,7 @@ class CelestSecureStorage(private val mainActivity: Activity, private val scope: } } - fun read(dataKey: String): String? = sharedPreferences.getString(dataKey, null) + fun read(dataKey: String): String? = sharedPreferences. fun delete(dataKey: String): String? { val current = read(dataKey) diff --git a/packages/celest_core/example/integration_test/secure_storage_shared.dart b/packages/celest_core/example/integration_test/secure_storage_shared.dart index fe7e8a49..cdf4f1b1 100644 --- a/packages/celest_core/example/integration_test/secure_storage_shared.dart +++ b/packages/celest_core/example/integration_test/secure_storage_shared.dart @@ -51,14 +51,36 @@ void sharedTests() { }); test('can delete a non-existent key', () { + expect(storage.read(key), isNull); expect(() => storage.delete(key), returnsNormally); - expect(() => storage.delete(key), returnsNormally); - expect(() => storage.delete(key), returnsNormally); }); }); - group('read/write/delete can handle key value pairs of varying length', () { - for (final (length, s) in largeKeyValuePairs) { + group('clear', () { + const key1 = 'key1'; + const key2 = 'key2'; + const value1 = 'value1'; + const value2 = 'value2'; + + test('removes all keys from storage', () { + storage.write(key1, value1); + storage.write(key2, value2); + expect(storage.read(key1), value1); + expect(storage.read(key2), value2); + + storage.clear(); + + expect(storage.read(key1), isNull, reason: 'Storage was cleared'); + expect(storage.read(key2), isNull, reason: 'Storage was cleared'); + }); + + test('does not throw when no items present', () { + expect(storage.clear, returnsNormally); + }); + }); + + group('large values', () { + for (final (length, s) in _largeKeyValuePairs) { test('can store key/value with length $length', () { storage.write(s, s); expect(storage.read(s), s, reason: 'Value was written'); @@ -72,7 +94,7 @@ void sharedTests() { } final _random = Random(); -Iterable<(int, String)> get largeKeyValuePairs sync* { +Iterable<(int, String)> get _largeKeyValuePairs sync* { for (final length in const [100, 1000, 10000]) { final string = String.fromCharCodes( List.generate(length, (_) => _random.nextInt(255) + 1), diff --git a/packages/celest_core/lib/src/secure_storage/secure_storage.android.dart b/packages/celest_core/lib/src/secure_storage/secure_storage.android.dart index a9b0c3df..7db250a4 100644 --- a/packages/celest_core/lib/src/secure_storage/secure_storage.android.dart +++ b/packages/celest_core/lib/src/secure_storage/secure_storage.android.dart @@ -28,8 +28,13 @@ final class SecureStoragePlatformAndroid extends SecureStoragePlatform { } @override - String? read(String key) => - _secureStorage.read(key.toJString()).toDartString(); + String? read(String key) { + final value = _secureStorage.read(key.toJString()); + if (value.isNull) { + return null; + } + return value.toDartString(); + } @override String write(String key, String value) { diff --git a/packages/celest_core/test/secure_storage/secure_storage_darwin_test.dart b/packages/celest_core/test/secure_storage/secure_storage_darwin_test.dart index 4137b3ea..0a3e1197 100644 --- a/packages/celest_core/test/secure_storage/secure_storage_darwin_test.dart +++ b/packages/celest_core/test/secure_storage/secure_storage_darwin_test.dart @@ -1,52 +1,10 @@ @TestOn('mac-os') import 'package:celest_core/src/native/darwin/security.ffi.dart'; -import 'package:celest_core/src/secure_storage/secure_storage.dart'; import 'package:celest_core/src/secure_storage/secure_storage.darwin.dart'; import 'package:test/test.dart'; -const key1 = 'key1'; -const key2 = 'key2'; -const value1 = 'value1'; -const value2 = 'value2'; - void main() { - group('SecureStorage', () { - group('macos', () { - group('clear', () { - final storage = SecureStorage(scope: 'clear'); - - setUp(() { - storage.clear(); - }); - - tearDown(() { - storage.clear(); - }); - - test('removes all keys from storage', () { - // seed storage and confirm values are present - storage - ..write(key1, value1) - ..write(key2, value2); - expect(storage.read(key1), value1); - expect(storage.read(key2), value2); - - // remove all - storage.clear(); - - // assert all data was removed - expect(storage.read(key1), isNull); - expect(storage.read(key2), isNull); - }); - - test('does not throw when no items present', () { - expect(storage.clear, returnsNormally); - }); - }); - }); - }); - group('SecurityFrameworkException', () { test('errSecInvalidOwnerEdit', () { final error = SecurityFrameworkException.fromStatus( diff --git a/packages/celest_core/test/secure_storage/secure_storage_test.dart b/packages/celest_core/test/secure_storage/secure_storage_test.dart index 3fc45291..581b6be9 100644 --- a/packages/celest_core/test/secure_storage/secure_storage_test.dart +++ b/packages/celest_core/test/secure_storage/secure_storage_test.dart @@ -1,5 +1,3 @@ import '../../example/integration_test/secure_storage_shared.dart'; -void main() { - sharedTests(); -} +void main() => sharedTests();