From d7b44d3fe4dc435926a51a1242dc14da9b980fe7 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Fri, 21 Apr 2023 14:54:39 -0700 Subject: [PATCH] [local_auth] Fix enum return on Android (#3780) The Pigeon conversion in https://github.com/flutter/packages/pull/3748 used a `List` return value, which Pigeon doesn't support correctly (see https://github.com/flutter/flutter/issues/125230 for catching this earlier next time). Because the method doesn't return any values in integration test environments it wasn't caught (since the issue is in Pigeon message handling, and thus doesn't show up in unit tests of the plugin). This adds a wrapper class, since enum fields do work. Fixes https://github.com/flutter/flutter/issues/125214 --- .../local_auth_android/CHANGELOG.md | 4 ++ .../plugins/localauth/LocalAuthPlugin.java | 13 ++-- .../flutter/plugins/localauth/Messages.java | 68 +++++++++++++++++-- .../plugins/localauth/LocalAuthTest.java | 15 ++-- .../lib/local_auth_android.dart | 9 ++- .../lib/src/messages.g.dart | 41 +++++++++-- .../local_auth_android/pigeons/messages.dart | 9 ++- .../local_auth_android/pubspec.yaml | 2 +- .../test/local_auth_test.dart | 12 ++-- .../test/local_auth_test.mocks.dart | 8 +-- 10 files changed, 141 insertions(+), 40 deletions(-) diff --git a/packages/local_auth/local_auth_android/CHANGELOG.md b/packages/local_auth/local_auth_android/CHANGELOG.md index 7452cd4d043f..aef9fe308ba6 100644 --- a/packages/local_auth/local_auth_android/CHANGELOG.md +++ b/packages/local_auth/local_auth_android/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.0.24 + +* Fixes `getEnrolledBiometrics` return value handling. + ## 1.0.23 * Switches internals to Pigeon and fixes Java warnings. diff --git a/packages/local_auth/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/LocalAuthPlugin.java b/packages/local_auth/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/LocalAuthPlugin.java index f955f7821ec9..ef492f5445cc 100644 --- a/packages/local_auth/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/LocalAuthPlugin.java +++ b/packages/local_auth/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/LocalAuthPlugin.java @@ -24,6 +24,7 @@ import io.flutter.plugin.common.PluginRegistry; import io.flutter.plugins.localauth.AuthenticationHelper.AuthCompletionHandler; import io.flutter.plugins.localauth.Messages.AuthClassification; +import io.flutter.plugins.localauth.Messages.AuthClassificationWrapper; import io.flutter.plugins.localauth.Messages.AuthOptions; import io.flutter.plugins.localauth.Messages.AuthResult; import io.flutter.plugins.localauth.Messages.AuthResultWrapper; @@ -98,19 +99,23 @@ public LocalAuthPlugin() {} return hasBiometricHardware(); } - public @NonNull List getEnrolledBiometrics() { - ArrayList biometrics = new ArrayList<>(); + public @NonNull List getEnrolledBiometrics() { + ArrayList biometrics = new ArrayList<>(); if (biometricManager.canAuthenticate(BiometricManager.Authenticators.BIOMETRIC_WEAK) == BiometricManager.BIOMETRIC_SUCCESS) { - biometrics.add(AuthClassification.WEAK); + biometrics.add(wrappedBiometric(AuthClassification.WEAK)); } if (biometricManager.canAuthenticate(BiometricManager.Authenticators.BIOMETRIC_STRONG) == BiometricManager.BIOMETRIC_SUCCESS) { - biometrics.add(AuthClassification.STRONG); + biometrics.add(wrappedBiometric(AuthClassification.STRONG)); } return biometrics; } + private @NonNull AuthClassificationWrapper wrappedBiometric(AuthClassification value) { + return new AuthClassificationWrapper.Builder().setValue(value).build(); + } + public @NonNull Boolean stopAuthentication() { try { if (authHelper != null && authInProgress.get()) { diff --git a/packages/local_auth/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/Messages.java b/packages/local_auth/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/Messages.java index a8b6738bd753..8b5bc41e750d 100644 --- a/packages/local_auth/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/Messages.java +++ b/packages/local_auth/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/Messages.java @@ -538,6 +538,55 @@ ArrayList toList() { } } + /** Generated class from Pigeon that represents data sent in messages. */ + public static final class AuthClassificationWrapper { + private @NonNull AuthClassification value; + + public @NonNull AuthClassification getValue() { + return value; + } + + public void setValue(@NonNull AuthClassification setterArg) { + if (setterArg == null) { + throw new IllegalStateException("Nonnull field \"value\" is null."); + } + this.value = setterArg; + } + + /** Constructor is non-public to enforce null safety; use Builder. */ + AuthClassificationWrapper() {} + + public static final class Builder { + + private @Nullable AuthClassification value; + + public @NonNull Builder setValue(@NonNull AuthClassification setterArg) { + this.value = setterArg; + return this; + } + + public @NonNull AuthClassificationWrapper build() { + AuthClassificationWrapper pigeonReturn = new AuthClassificationWrapper(); + pigeonReturn.setValue(value); + return pigeonReturn; + } + } + + @NonNull + ArrayList toList() { + ArrayList toListResult = new ArrayList(1); + toListResult.add(value == null ? null : value.index); + return toListResult; + } + + static @NonNull AuthClassificationWrapper fromList(@NonNull ArrayList list) { + AuthClassificationWrapper pigeonResult = new AuthClassificationWrapper(); + Object value = list.get(0); + pigeonResult.setValue(value == null ? null : AuthClassification.values()[(int) value]); + return pigeonResult; + } + } + public interface Result { @SuppressWarnings("UnknownNullness") void success(T result); @@ -554,10 +603,12 @@ private LocalAuthApiCodec() {} protected Object readValueOfType(byte type, @NonNull ByteBuffer buffer) { switch (type) { case (byte) 128: - return AuthOptions.fromList((ArrayList) readValue(buffer)); + return AuthClassificationWrapper.fromList((ArrayList) readValue(buffer)); case (byte) 129: - return AuthResultWrapper.fromList((ArrayList) readValue(buffer)); + return AuthOptions.fromList((ArrayList) readValue(buffer)); case (byte) 130: + return AuthResultWrapper.fromList((ArrayList) readValue(buffer)); + case (byte) 131: return AuthStrings.fromList((ArrayList) readValue(buffer)); default: return super.readValueOfType(type, buffer); @@ -566,14 +617,17 @@ protected Object readValueOfType(byte type, @NonNull ByteBuffer buffer) { @Override protected void writeValue(@NonNull ByteArrayOutputStream stream, Object value) { - if (value instanceof AuthOptions) { + if (value instanceof AuthClassificationWrapper) { stream.write(128); + writeValue(stream, ((AuthClassificationWrapper) value).toList()); + } else if (value instanceof AuthOptions) { + stream.write(129); writeValue(stream, ((AuthOptions) value).toList()); } else if (value instanceof AuthResultWrapper) { - stream.write(129); + stream.write(130); writeValue(stream, ((AuthResultWrapper) value).toList()); } else if (value instanceof AuthStrings) { - stream.write(130); + stream.write(131); writeValue(stream, ((AuthStrings) value).toList()); } else { super.writeValue(stream, value); @@ -603,7 +657,7 @@ public interface LocalAuthApi { * Returns the biometric types that are enrolled, and can thus be used without additional setup. */ @NonNull - List getEnrolledBiometrics(); + List getEnrolledBiometrics(); /** * Attempts to authenticate the user with the provided [options], and using [strings] for any * UI. @@ -695,7 +749,7 @@ static void setup(@NonNull BinaryMessenger binaryMessenger, @Nullable LocalAuthA (message, reply) -> { ArrayList wrapped = new ArrayList(); try { - List output = api.getEnrolledBiometrics(); + List output = api.getEnrolledBiometrics(); wrapped.add(0, output); } catch (Throwable exception) { ArrayList wrappedError = wrapError(exception); diff --git a/packages/local_auth/local_auth_android/android/src/test/java/io/flutter/plugins/localauth/LocalAuthTest.java b/packages/local_auth/local_auth_android/android/src/test/java/io/flutter/plugins/localauth/LocalAuthTest.java index 4d2c6423b214..62b03f900597 100644 --- a/packages/local_auth/local_auth_android/android/src/test/java/io/flutter/plugins/localauth/LocalAuthTest.java +++ b/packages/local_auth/local_auth_android/android/src/test/java/io/flutter/plugins/localauth/LocalAuthTest.java @@ -30,6 +30,7 @@ import io.flutter.plugin.common.BinaryMessenger; import io.flutter.plugins.localauth.AuthenticationHelper.AuthCompletionHandler; import io.flutter.plugins.localauth.Messages.AuthClassification; +import io.flutter.plugins.localauth.Messages.AuthClassificationWrapper; import io.flutter.plugins.localauth.Messages.AuthOptions; import io.flutter.plugins.localauth.Messages.AuthResult; import io.flutter.plugins.localauth.Messages.AuthResultWrapper; @@ -291,7 +292,7 @@ public void getEnrolledBiometrics_shouldReturnEmptyList_withoutHardwarePresent() .thenReturn(BiometricManager.BIOMETRIC_ERROR_NO_HARDWARE); plugin.setBiometricManager(mockBiometricManager); - final List enrolled = plugin.getEnrolledBiometrics(); + final List enrolled = plugin.getEnrolledBiometrics(); assertTrue(enrolled.isEmpty()); } @@ -304,7 +305,7 @@ public void getEnrolledBiometrics_shouldReturnEmptyList_withNoMethodsEnrolled() .thenReturn(BiometricManager.BIOMETRIC_ERROR_NONE_ENROLLED); plugin.setBiometricManager(mockBiometricManager); - final List enrolled = plugin.getEnrolledBiometrics(); + final List enrolled = plugin.getEnrolledBiometrics(); assertTrue(enrolled.isEmpty()); } @@ -319,9 +320,9 @@ public void getEnrolledBiometrics_shouldOnlyAddEnrolledBiometrics() { .thenReturn(BiometricManager.BIOMETRIC_ERROR_NONE_ENROLLED); plugin.setBiometricManager(mockBiometricManager); - final List enrolled = plugin.getEnrolledBiometrics(); + final List enrolled = plugin.getEnrolledBiometrics(); assertEquals(1, enrolled.size()); - assertEquals(AuthClassification.WEAK, enrolled.get(0)); + assertEquals(AuthClassification.WEAK, enrolled.get(0).getValue()); } @Test @@ -335,10 +336,10 @@ public void getEnrolledBiometrics_shouldAddStrongBiometrics() { .thenReturn(BiometricManager.BIOMETRIC_SUCCESS); plugin.setBiometricManager(mockBiometricManager); - final List enrolled = plugin.getEnrolledBiometrics(); + final List enrolled = plugin.getEnrolledBiometrics(); assertEquals(2, enrolled.size()); - assertEquals(AuthClassification.WEAK, enrolled.get(0)); - assertEquals(AuthClassification.STRONG, enrolled.get(1)); + assertEquals(AuthClassification.WEAK, enrolled.get(0).getValue()); + assertEquals(AuthClassification.STRONG, enrolled.get(1).getValue()); } @Test diff --git a/packages/local_auth/local_auth_android/lib/local_auth_android.dart b/packages/local_auth/local_auth_android/lib/local_auth_android.dart index 4791e2c7a0e2..145c4681f013 100644 --- a/packages/local_auth/local_auth_android/lib/local_auth_android.dart +++ b/packages/local_auth/local_auth_android/lib/local_auth_android.dart @@ -96,9 +96,12 @@ class LocalAuthAndroid extends LocalAuthPlatform { @override Future> getEnrolledBiometrics() async { - final List result = await _api.getEnrolledBiometrics(); - return result.cast().map((AuthClassification entry) { - switch (entry) { + final List result = + await _api.getEnrolledBiometrics(); + return result + .cast() + .map((AuthClassificationWrapper entry) { + switch (entry.value) { case AuthClassification.weak: return BiometricType.weak; case AuthClassification.strong: diff --git a/packages/local_auth/local_auth_android/lib/src/messages.g.dart b/packages/local_auth/local_auth_android/lib/src/messages.g.dart index 9905652fa9a9..3dead4f69fc2 100644 --- a/packages/local_auth/local_auth_android/lib/src/messages.g.dart +++ b/packages/local_auth/local_auth_android/lib/src/messages.g.dart @@ -174,19 +174,43 @@ class AuthResultWrapper { } } +class AuthClassificationWrapper { + AuthClassificationWrapper({ + required this.value, + }); + + AuthClassification value; + + Object encode() { + return [ + value.index, + ]; + } + + static AuthClassificationWrapper decode(Object result) { + result as List; + return AuthClassificationWrapper( + value: AuthClassification.values[result[0]! as int], + ); + } +} + class _LocalAuthApiCodec extends StandardMessageCodec { const _LocalAuthApiCodec(); @override void writeValue(WriteBuffer buffer, Object? value) { - if (value is AuthOptions) { + if (value is AuthClassificationWrapper) { buffer.putUint8(128); writeValue(buffer, value.encode()); - } else if (value is AuthResultWrapper) { + } else if (value is AuthOptions) { buffer.putUint8(129); writeValue(buffer, value.encode()); - } else if (value is AuthStrings) { + } else if (value is AuthResultWrapper) { buffer.putUint8(130); writeValue(buffer, value.encode()); + } else if (value is AuthStrings) { + buffer.putUint8(131); + writeValue(buffer, value.encode()); } else { super.writeValue(buffer, value); } @@ -196,10 +220,12 @@ class _LocalAuthApiCodec extends StandardMessageCodec { Object? readValueOfType(int type, ReadBuffer buffer) { switch (type) { case 128: - return AuthOptions.decode(readValue(buffer)!); + return AuthClassificationWrapper.decode(readValue(buffer)!); case 129: - return AuthResultWrapper.decode(readValue(buffer)!); + return AuthOptions.decode(readValue(buffer)!); case 130: + return AuthResultWrapper.decode(readValue(buffer)!); + case 131: return AuthStrings.decode(readValue(buffer)!); default: return super.readValueOfType(type, buffer); @@ -304,7 +330,7 @@ class LocalAuthApi { /// Returns the biometric types that are enrolled, and can thus be used /// without additional setup. - Future> getEnrolledBiometrics() async { + Future> getEnrolledBiometrics() async { final BasicMessageChannel channel = BasicMessageChannel( 'dev.flutter.pigeon.LocalAuthApi.getEnrolledBiometrics', codec, binaryMessenger: _binaryMessenger); @@ -326,7 +352,8 @@ class LocalAuthApi { message: 'Host platform returned null value for non-null return value.', ); } else { - return (replyList[0] as List?)!.cast(); + return (replyList[0] as List?)! + .cast(); } } diff --git a/packages/local_auth/local_auth_android/pigeons/messages.dart b/packages/local_auth/local_auth_android/pigeons/messages.dart index 262b5934f5f4..69d0157a34be 100644 --- a/packages/local_auth/local_auth_android/pigeons/messages.dart +++ b/packages/local_auth/local_auth_android/pigeons/messages.dart @@ -94,6 +94,13 @@ class AuthResultWrapper { /// Pigeon equivalent of the subset of BiometricType used by Android. enum AuthClassification { weak, strong } +// TODO(stuartmorgan): Remove this when +// https://github.com/flutter/flutter/issues/87307 is implemented. +class AuthClassificationWrapper { + AuthClassificationWrapper({required this.value}); + final AuthClassification value; +} + @HostApi() abstract class LocalAuthApi { /// Returns true if this device supports authentication. @@ -111,7 +118,7 @@ abstract class LocalAuthApi { /// Returns the biometric types that are enrolled, and can thus be used /// without additional setup. - List getEnrolledBiometrics(); + List getEnrolledBiometrics(); /// Attempts to authenticate the user with the provided [options], and using /// [strings] for any UI. diff --git a/packages/local_auth/local_auth_android/pubspec.yaml b/packages/local_auth/local_auth_android/pubspec.yaml index 79886bd482d1..0d1dd4308dad 100644 --- a/packages/local_auth/local_auth_android/pubspec.yaml +++ b/packages/local_auth/local_auth_android/pubspec.yaml @@ -2,7 +2,7 @@ name: local_auth_android description: Android implementation of the local_auth plugin. repository: https://github.com/flutter/packages/tree/main/packages/local_auth/local_auth_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+local_auth%22 -version: 1.0.23 +version: 1.0.24 environment: sdk: ">=2.17.0 <4.0.0" diff --git a/packages/local_auth/local_auth_android/test/local_auth_test.dart b/packages/local_auth/local_auth_android/test/local_auth_test.dart index cdebe8583349..4e416fb72bc2 100644 --- a/packages/local_auth/local_auth_android/test/local_auth_test.dart +++ b/packages/local_auth/local_auth_android/test/local_auth_test.dart @@ -65,11 +65,11 @@ void main() { group('getEnrolledBiometrics', () { test('translates values', () async { - when(api.getEnrolledBiometrics()).thenAnswer((_) async => - [ - AuthClassification.weak, - AuthClassification.strong - ]); + when(api.getEnrolledBiometrics()) + .thenAnswer((_) async => [ + AuthClassificationWrapper(value: AuthClassification.weak), + AuthClassificationWrapper(value: AuthClassification.strong), + ]); final List result = await plugin.getEnrolledBiometrics(); @@ -81,7 +81,7 @@ void main() { test('handles emtpy', () async { when(api.getEnrolledBiometrics()) - .thenAnswer((_) async => []); + .thenAnswer((_) async => []); final List result = await plugin.getEnrolledBiometrics(); diff --git a/packages/local_auth/local_auth_android/test/local_auth_test.mocks.dart b/packages/local_auth/local_auth_android/test/local_auth_test.mocks.dart index 5e5386210466..7a4325ef6431 100644 --- a/packages/local_auth/local_auth_android/test/local_auth_test.mocks.dart +++ b/packages/local_auth/local_auth_android/test/local_auth_test.mocks.dart @@ -63,15 +63,15 @@ class MockLocalAuthApi extends _i1.Mock implements _i2.LocalAuthApi { returnValue: _i3.Future.value(false), ) as _i3.Future); @override - _i3.Future> getEnrolledBiometrics() => + _i3.Future> getEnrolledBiometrics() => (super.noSuchMethod( Invocation.method( #getEnrolledBiometrics, [], ), - returnValue: _i3.Future>.value( - <_i2.AuthClassification?>[]), - ) as _i3.Future>); + returnValue: _i3.Future>.value( + <_i2.AuthClassificationWrapper?>[]), + ) as _i3.Future>); @override _i3.Future<_i2.AuthResultWrapper> authenticate( _i2.AuthOptions? arg_options,