Skip to content

Commit

Permalink
[local_auth] Fix enum return on Android (flutter#3780)
Browse files Browse the repository at this point in the history
The Pigeon conversion in flutter#3748 used a `List<enum>` return value, which Pigeon doesn't support correctly (see flutter/flutter#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 flutter/flutter#125214
  • Loading branch information
stuartmorgan authored and nploi committed Jul 16, 2023
1 parent ca57ded commit d7b44d3
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 40 deletions.
4 changes: 4 additions & 0 deletions packages/local_auth/local_auth_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.0.24

* Fixes `getEnrolledBiometrics` return value handling.

## 1.0.23

* Switches internals to Pigeon and fixes Java warnings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -98,19 +99,23 @@ public LocalAuthPlugin() {}
return hasBiometricHardware();
}

public @NonNull List<AuthClassification> getEnrolledBiometrics() {
ArrayList<AuthClassification> biometrics = new ArrayList<>();
public @NonNull List<AuthClassificationWrapper> getEnrolledBiometrics() {
ArrayList<AuthClassificationWrapper> 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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,55 @@ ArrayList<Object> 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<Object> toList() {
ArrayList<Object> toListResult = new ArrayList<Object>(1);
toListResult.add(value == null ? null : value.index);
return toListResult;
}

static @NonNull AuthClassificationWrapper fromList(@NonNull ArrayList<Object> list) {
AuthClassificationWrapper pigeonResult = new AuthClassificationWrapper();
Object value = list.get(0);
pigeonResult.setValue(value == null ? null : AuthClassification.values()[(int) value]);
return pigeonResult;
}
}

public interface Result<T> {
@SuppressWarnings("UnknownNullness")
void success(T result);
Expand All @@ -554,10 +603,12 @@ private LocalAuthApiCodec() {}
protected Object readValueOfType(byte type, @NonNull ByteBuffer buffer) {
switch (type) {
case (byte) 128:
return AuthOptions.fromList((ArrayList<Object>) readValue(buffer));
return AuthClassificationWrapper.fromList((ArrayList<Object>) readValue(buffer));
case (byte) 129:
return AuthResultWrapper.fromList((ArrayList<Object>) readValue(buffer));
return AuthOptions.fromList((ArrayList<Object>) readValue(buffer));
case (byte) 130:
return AuthResultWrapper.fromList((ArrayList<Object>) readValue(buffer));
case (byte) 131:
return AuthStrings.fromList((ArrayList<Object>) readValue(buffer));
default:
return super.readValueOfType(type, buffer);
Expand All @@ -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);
Expand Down Expand Up @@ -603,7 +657,7 @@ public interface LocalAuthApi {
* Returns the biometric types that are enrolled, and can thus be used without additional setup.
*/
@NonNull
List<AuthClassification> getEnrolledBiometrics();
List<AuthClassificationWrapper> getEnrolledBiometrics();
/**
* Attempts to authenticate the user with the provided [options], and using [strings] for any
* UI.
Expand Down Expand Up @@ -695,7 +749,7 @@ static void setup(@NonNull BinaryMessenger binaryMessenger, @Nullable LocalAuthA
(message, reply) -> {
ArrayList<Object> wrapped = new ArrayList<Object>();
try {
List<AuthClassification> output = api.getEnrolledBiometrics();
List<AuthClassificationWrapper> output = api.getEnrolledBiometrics();
wrapped.add(0, output);
} catch (Throwable exception) {
ArrayList<Object> wrappedError = wrapError(exception);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -291,7 +292,7 @@ public void getEnrolledBiometrics_shouldReturnEmptyList_withoutHardwarePresent()
.thenReturn(BiometricManager.BIOMETRIC_ERROR_NO_HARDWARE);
plugin.setBiometricManager(mockBiometricManager);

final List<AuthClassification> enrolled = plugin.getEnrolledBiometrics();
final List<AuthClassificationWrapper> enrolled = plugin.getEnrolledBiometrics();
assertTrue(enrolled.isEmpty());
}

Expand All @@ -304,7 +305,7 @@ public void getEnrolledBiometrics_shouldReturnEmptyList_withNoMethodsEnrolled()
.thenReturn(BiometricManager.BIOMETRIC_ERROR_NONE_ENROLLED);
plugin.setBiometricManager(mockBiometricManager);

final List<AuthClassification> enrolled = plugin.getEnrolledBiometrics();
final List<AuthClassificationWrapper> enrolled = plugin.getEnrolledBiometrics();
assertTrue(enrolled.isEmpty());
}

Expand All @@ -319,9 +320,9 @@ public void getEnrolledBiometrics_shouldOnlyAddEnrolledBiometrics() {
.thenReturn(BiometricManager.BIOMETRIC_ERROR_NONE_ENROLLED);
plugin.setBiometricManager(mockBiometricManager);

final List<AuthClassification> enrolled = plugin.getEnrolledBiometrics();
final List<AuthClassificationWrapper> enrolled = plugin.getEnrolledBiometrics();
assertEquals(1, enrolled.size());
assertEquals(AuthClassification.WEAK, enrolled.get(0));
assertEquals(AuthClassification.WEAK, enrolled.get(0).getValue());
}

@Test
Expand All @@ -335,10 +336,10 @@ public void getEnrolledBiometrics_shouldAddStrongBiometrics() {
.thenReturn(BiometricManager.BIOMETRIC_SUCCESS);
plugin.setBiometricManager(mockBiometricManager);

final List<AuthClassification> enrolled = plugin.getEnrolledBiometrics();
final List<AuthClassificationWrapper> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,12 @@ class LocalAuthAndroid extends LocalAuthPlatform {

@override
Future<List<BiometricType>> getEnrolledBiometrics() async {
final List<AuthClassification?> result = await _api.getEnrolledBiometrics();
return result.cast<AuthClassification>().map((AuthClassification entry) {
switch (entry) {
final List<AuthClassificationWrapper?> result =
await _api.getEnrolledBiometrics();
return result
.cast<AuthClassificationWrapper>()
.map((AuthClassificationWrapper entry) {
switch (entry.value) {
case AuthClassification.weak:
return BiometricType.weak;
case AuthClassification.strong:
Expand Down
41 changes: 34 additions & 7 deletions packages/local_auth/local_auth_android/lib/src/messages.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,19 +174,43 @@ class AuthResultWrapper {
}
}

class AuthClassificationWrapper {
AuthClassificationWrapper({
required this.value,
});

AuthClassification value;

Object encode() {
return <Object?>[
value.index,
];
}

static AuthClassificationWrapper decode(Object result) {
result as List<Object?>;
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);
}
Expand All @@ -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);
Expand Down Expand Up @@ -304,7 +330,7 @@ class LocalAuthApi {

/// Returns the biometric types that are enrolled, and can thus be used
/// without additional setup.
Future<List<AuthClassification?>> getEnrolledBiometrics() async {
Future<List<AuthClassificationWrapper?>> getEnrolledBiometrics() async {
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.LocalAuthApi.getEnrolledBiometrics', codec,
binaryMessenger: _binaryMessenger);
Expand All @@ -326,7 +352,8 @@ class LocalAuthApi {
message: 'Host platform returned null value for non-null return value.',
);
} else {
return (replyList[0] as List<Object?>?)!.cast<AuthClassification?>();
return (replyList[0] as List<Object?>?)!
.cast<AuthClassificationWrapper?>();
}
}

Expand Down
9 changes: 8 additions & 1 deletion packages/local_auth/local_auth_android/pigeons/messages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -111,7 +118,7 @@ abstract class LocalAuthApi {

/// Returns the biometric types that are enrolled, and can thus be used
/// without additional setup.
List<AuthClassification> getEnrolledBiometrics();
List<AuthClassificationWrapper> getEnrolledBiometrics();

/// Attempts to authenticate the user with the provided [options], and using
/// [strings] for any UI.
Expand Down
2 changes: 1 addition & 1 deletion packages/local_auth/local_auth_android/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 6 additions & 6 deletions packages/local_auth/local_auth_android/test/local_auth_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ void main() {

group('getEnrolledBiometrics', () {
test('translates values', () async {
when(api.getEnrolledBiometrics()).thenAnswer((_) async =>
<AuthClassification>[
AuthClassification.weak,
AuthClassification.strong
]);
when(api.getEnrolledBiometrics())
.thenAnswer((_) async => <AuthClassificationWrapper>[
AuthClassificationWrapper(value: AuthClassification.weak),
AuthClassificationWrapper(value: AuthClassification.strong),
]);

final List<BiometricType> result = await plugin.getEnrolledBiometrics();

Expand All @@ -81,7 +81,7 @@ void main() {

test('handles emtpy', () async {
when(api.getEnrolledBiometrics())
.thenAnswer((_) async => <AuthClassification>[]);
.thenAnswer((_) async => <AuthClassificationWrapper>[]);

final List<BiometricType> result = await plugin.getEnrolledBiometrics();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ class MockLocalAuthApi extends _i1.Mock implements _i2.LocalAuthApi {
returnValue: _i3.Future<bool>.value(false),
) as _i3.Future<bool>);
@override
_i3.Future<List<_i2.AuthClassification?>> getEnrolledBiometrics() =>
_i3.Future<List<_i2.AuthClassificationWrapper?>> getEnrolledBiometrics() =>
(super.noSuchMethod(
Invocation.method(
#getEnrolledBiometrics,
[],
),
returnValue: _i3.Future<List<_i2.AuthClassification?>>.value(
<_i2.AuthClassification?>[]),
) as _i3.Future<List<_i2.AuthClassification?>>);
returnValue: _i3.Future<List<_i2.AuthClassificationWrapper?>>.value(
<_i2.AuthClassificationWrapper?>[]),
) as _i3.Future<List<_i2.AuthClassificationWrapper?>>);
@override
_i3.Future<_i2.AuthResultWrapper> authenticate(
_i2.AuthOptions? arg_options,
Expand Down

0 comments on commit d7b44d3

Please sign in to comment.