Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(auth): Uncaught Hosted UI cancellation #3686

Merged
merged 2 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions packages/amplify_core/lib/src/state_machine/event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ final class EventCompleter<Event extends StateMachineEvent,
/// match the Zone in which its future is listened to, otherwise the future
/// will never complete.
///
/// That is, running `_zone.run(completer.complete)` would still throw
/// the error in the Zone where the completer was instantiated. And due
/// to how Zone's work, a listener for a completer which completes in a
/// different error zone will never finish.
/// That is, running `_zone.run(() => completer.completeError(error))` would
/// still throw the error in the Zone where the completer was instantiated,
/// not `_zone`. And due to how Zone's work, a listener for a completer which
/// completes in a different error zone will never finish.
///
/// The following example illustrates the problem we're trying to solve
/// here:
Expand Down Expand Up @@ -168,5 +168,8 @@ final class EventCompleter<Event extends StateMachineEvent,
///
/// Since state machine methods are marked with `@useResult`, this allows
/// opting into fire-and-forget behavior explicitly.
void ignore() {}
void ignore() {
_acceptedCompleters.clear();
_completers.clear();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ abstract class StateMachine<

final resolution = resolveError(error, stackTrace);

logger.debug('Resolved error with state', resolution?.type);

// Add the error to the state stream if it cannot be resolved to a new
// state internally.
if (resolution == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import 'package:amplify_auth_cognito_dart/src/flows/hosted_ui/hosted_ui_platform
import 'package:amplify_auth_cognito_dart/src/model/hosted_ui/oauth_parameters.dart';
import 'package:amplify_auth_cognito_dart/src/state/cognito_state_machine.dart';
import 'package:amplify_auth_cognito_dart/src/state/state.dart';
import 'package:amplify_auth_cognito_test/amplify_auth_cognito_test.dart';
import 'package:amplify_flutter/amplify_flutter.dart';
import 'package:amplify_integration_test/amplify_integration_test.dart';
import 'package:flutter/material.dart';
Expand Down Expand Up @@ -56,21 +55,52 @@ void main() {
);
});

Future<void> signIn(WidgetTester tester) async {
Future<void> signIn(
WidgetTester tester, {
bool cancel = false,
}) async {
stateMachine.addInstance<HostedUiPlatform>(
HostedUiTestPlatform(
tester,
stateMachine,
username: username,
password: password,
cancel: cancel,
),
);
_logger.debug('Signing in with Web UI');
final result = await plugin.signInWithWebUI(
provider: AuthProvider.cognito,
);
_logger.debug('Signed in with Web UI');
expect(result.isSignedIn, isTrue);
if (cancel) {
final expectation = expectLater(
plugin.signInWithWebUI(
provider: AuthProvider.cognito,
),
throwsA(isA<UserCancelledException>()),
);
final hostedUiMachine =
stateMachine.expect(HostedUiStateMachine.type);
expect(
hostedUiMachine.stream,
emitsInOrder([
isA<HostedUiSigningIn>(),
isA<HostedUiFailure>().having(
(s) => s.exception,
'exception',
isA<UserCancelledException>(),
),
emitsDone,
]),
);
await expectation;
// Ensure queue is flushed and done event is emitted after
// signInWithWebUI completes.
await hostedUiMachine.close();
} else {
final result = await plugin.signInWithWebUI(
provider: AuthProvider.cognito,
);
_logger.debug('Signed in with Web UI');
expect(result.isSignedIn, isTrue);
}
}

Future<void> signOut({required bool globalSignOut}) async {
Expand Down Expand Up @@ -112,14 +142,17 @@ void main() {
await signOut(globalSignOut: false);
});

testWidgets('cancel sign-in', (tester) async {
await signIn(tester, cancel: true);
});

testWidgets('global sign out', (tester) async {
await signIn(tester);
await signOut(globalSignOut: true);
});
},
// Add remaining platforms as `webview_flutter` adds support.
// TODO(dnys1): Investigate Android failures in CI on Android 33+
skip: zIsWeb || !((Platform.isAndroid && !isCI) || Platform.isIOS),
skip: zIsWeb || !(Platform.isAndroid || Platform.isIOS),
);
}

Expand All @@ -129,11 +162,15 @@ class HostedUiTestPlatform extends HostedUiPlatformImpl {
DependencyManager manager, {
required this.username,
required this.password,
required this.cancel,
}) : super(manager);

final String username;
final String password;

/// Whether to cancel the sign-in flow once initiated.
final bool cancel;

final WidgetTester tester;
final Completer<WebViewController> _controller = Completer();
final Completer<OAuthParameters> _oauthParameters = Completer();
Expand All @@ -143,6 +180,9 @@ class HostedUiTestPlatform extends HostedUiPlatformImpl {
required CognitoSignInWithWebUIPluginOptions options,
AuthProvider? provider,
}) async {
if (cancel) {
throw const UserCancelledException('Cancelled');
}
final signInUri = await getSignInUri(provider: provider);
await tester.pumpWidget(
HostedUiApp(
Expand Down
1 change: 1 addition & 0 deletions packages/auth/amplify_auth_cognito/example/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ dev_dependencies:
io: ^1.0.0
otp: ^3.1.4
stack_trace: ^1.10.0
stream_transform: ^2.1.0
test: ^1.22.1
webdriver: ^3.0.0
webview_flutter: ^4.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import 'package:amplify_auth_cognito_dart/src/model/hosted_ui/oauth_parameters.d
// ignore: implementation_imports, invalid_use_of_internal_member
import 'package:amplify_auth_cognito_dart/src/state/state.dart';
import 'package:amplify_core/amplify_core.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';

/// {@template amplify_auth_cognito.hosted_ui_platform_flutter}
Expand All @@ -27,7 +28,17 @@ class HostedUiPlatformImpl extends io.HostedUiPlatformImpl {
/// {@macro amplify_auth_cognito.hosted_ui_platform_flutter}
HostedUiPlatformImpl(super.dependencyManager);

static final bool _isMobile = Platform.isAndroid || Platform.isIOS;
static bool get _isMobile {
if (Platform.isAndroid || Platform.isIOS) {
return true;
}
// Allow overrides for tests
if (zAssertsEnabled) {
return debugDefaultTargetPlatformOverride == TargetPlatform.android ||
debugDefaultTargetPlatformOverride == TargetPlatform.iOS;
}
return false;
}

NativeAuthBridge get _nativeAuthBridge => dependencyManager.expect();

Expand Down Expand Up @@ -80,39 +91,37 @@ class HostedUiPlatformImpl extends io.HostedUiPlatformImpl {
options.isPreferPrivateSession,
options.browserPackageName,
);
unawaited(
dispatcher.dispatchAndComplete(
HostedUiEvent.exchange(
OAuthParameters.fromJson(queryParameters.cast()),
),
),
);
dispatcher
.dispatch(
HostedUiEvent.exchange(
OAuthParameters.fromJson(queryParameters.cast()),
),
)
.ignore();
} on Exception catch (e) {
unawaited(
dispatcher.dispatchAndComplete(const HostedUiEvent.cancelSignIn()),
);
if (e is PlatformException) {
if (e.code == 'CANCELLED') {
switch (e) {
case PlatformException(code: 'CANCELLED'):
throw const UserCancelledException(
'The user cancelled the sign-in flow',
);
}
// Generated Android message is `CLASS_NAME: message`
var message = e.message;
if (message != null && message.contains(': ')) {
message = message.split(': ')[1];
}
String? recoverySuggestion;
if (e.code == 'NOBROWSER') {
recoverySuggestion = "Ensure you've added the <queries> tag to your "
'AndroidManifest.xml as outlined in the docs';
}
throw UrlLauncherException(
message ?? 'An unknown error occurred',
recoverySuggestion: recoverySuggestion,
);
case PlatformException(:final code, :var message):
// Generated Android message is `CLASS_NAME: message`
if (message != null && message.contains(': ')) {
message = message.split(': ')[1];
}
String? recoverySuggestion;
if (code == 'NOBROWSER') {
recoverySuggestion =
"Ensure you've added the <queries> tag to your "
'AndroidManifest.xml as outlined in the docs';
}
throw UrlLauncherException(
message ?? 'An unknown error occurred',
recoverySuggestion: recoverySuggestion,
);
default:
rethrow;
}
rethrow;
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/auth/amplify_auth_cognito/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dependencies:
plugin_platform_interface: ^2.0.0

dev_dependencies:
amplify_auth_cognito_test: any
amplify_lints: ">=3.0.0 <3.1.0"
flutter_test:
sdk: flutter
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

// ignore_for_file: invalid_use_of_internal_member, non_constant_identifier_names

import 'package:amplify_auth_cognito/amplify_auth_cognito.dart';
import 'package:amplify_auth_cognito/src/flows/hosted_ui/hosted_ui_platform_flutter.dart';
import 'package:amplify_auth_cognito/src/native_auth_plugin.g.dart';
import 'package:amplify_auth_cognito_dart/src/flows/hosted_ui/hosted_ui_platform.dart';
import 'package:amplify_auth_cognito_dart/src/state/cognito_state_machine.dart';
import 'package:amplify_auth_cognito_dart/src/state/state.dart';
import 'package:amplify_auth_cognito_test/amplify_auth_cognito_test.dart';
import 'package:amplify_flutter/amplify_flutter.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
AWSLogger().logLevel = LogLevel.verbose;

group('HostedUiPlatformFlutter', () {
late AmplifyAuthCognito plugin;
late SecureStorageInterface secureStorage;
late DependencyManager dependencyManager;

setUp(() async {
secureStorage = MockSecureStorage();
dependencyManager = DependencyManager()
..addInstance(hostedUiConfig)
..addInstance<SecureStorageInterface>(secureStorage)
..addInstance<NativeAuthBridge>(ThrowingNativeBridge());
plugin = AmplifyAuthCognito()
..stateMachine = CognitoAuthStateMachine(
dependencyManager: dependencyManager,
);
plugin.stateMachine.addBuilder<HostedUiPlatform>(
HostedUiPlatformImpl.new,
);
await plugin.stateMachine.acceptAndComplete(
ConfigurationEvent.configure(mockConfig),
);
});

tearDown(() => plugin.close());

test('can cancel flow', () async {
// Pretend to be iOS so that the `ThrowingNativeBridge` is called,
// mimicking a failure from the platform channel.
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
addTearDown(() => debugDefaultTargetPlatformOverride = null);

final expectation = expectLater(
plugin.signInWithWebUI(
provider: AuthProvider.cognito,
),
throwsA(isA<UserCancelledException>()),
);
final hostedUiMachine =
plugin.stateMachine.expect(HostedUiStateMachine.type);
expect(
hostedUiMachine.stream,
emitsInOrder([
isA<HostedUiSigningIn>(),
isA<HostedUiFailure>().having(
(s) => s.exception,
'exception',
isA<UserCancelledException>(),
),
emitsDone,
]),
);
await expectation;
// Ensure queue is flushed and done event is emitted after
// signInWithWebUI completes.
await hostedUiMachine.close();
});
});
}

final class ThrowingNativeBridge extends Fake implements NativeAuthBridge {
@override
Future<Map<String?, String?>> signInWithUrl(
String arg_url,
String arg_callbackUrlScheme,
bool arg_preferPrivateSession,
String? arg_browserPackageName,
) async {
throw PlatformException(code: 'CANCELLED');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,13 @@ class HostedUiPlatformImpl extends HostedUiPlatform {
);
continue;
}
unawaited(
dispatcher.dispatchAndComplete(
HostedUiEvent.exchange(
OAuthParameters.fromJson(queryParams),
),
),
);
dispatcher
.dispatch(
HostedUiEvent.exchange(
OAuthParameters.fromJson(queryParams),
),
)
.ignore();
await _respond(
request,
HttpStatus.ok,
Expand All @@ -268,7 +268,7 @@ class HostedUiPlatformImpl extends HostedUiPlatform {
break;
}
} finally {
unawaited(close());
close().ignore();
}
}

Expand Down Expand Up @@ -324,7 +324,7 @@ class HostedUiPlatformImpl extends HostedUiPlatform {
break;
}
} finally {
unawaited(close());
close().ignore();
}
}

Expand Down
Loading