From fb8bb4e4aa36a79ef7693385d94b1356105a8c67 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Mar 2024 16:19:53 -0700 Subject: [PATCH 01/14] ZulipApp [nfc]: Convert to a stateful widget --- lib/widgets/app.dart | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index e32d39dd10..617e33efd9 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -17,7 +17,7 @@ import 'store.dart'; import 'subscription_list.dart'; import 'text.dart'; -class ZulipApp extends StatelessWidget { +class ZulipApp extends StatefulWidget { const ZulipApp({super.key, this.navigatorObservers}); /// Whether the app's widget tree is ready. @@ -79,6 +79,11 @@ class ZulipApp extends StatelessWidget { _ready.value = true; } + @override + State createState() => _ZulipAppState(); +} + +class _ZulipAppState extends State { @override Widget build(BuildContext context) { final theme = ThemeData( @@ -123,12 +128,12 @@ class ZulipApp extends StatelessWidget { supportedLocales: ZulipLocalizations.supportedLocales, theme: theme, - navigatorKey: navigatorKey, - navigatorObservers: navigatorObservers ?? const [], + navigatorKey: ZulipApp.navigatorKey, + navigatorObservers: widget.navigatorObservers ?? const [], builder: (BuildContext context, Widget? child) { - if (!ready.value) { + if (!ZulipApp.ready.value) { SchedulerBinding.instance.addPostFrameCallback( - (_) => _declareReady()); + (_) => widget._declareReady()); } GlobalLocalizations.zulipLocalizations = ZulipLocalizations.of(context); return child!; From 07955863fb621e823006d2aa6c4a19a4a958c4bd Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Mar 2024 17:53:54 -0700 Subject: [PATCH 02/14] login [nfc]: s/PasswordLoginPage/LoginPage/ We're about to add some more login options. --- lib/widgets/login.dart | 14 +++++++------- test/widgets/login_test.dart | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart index 2c316d08e1..ff7043d7e2 100644 --- a/lib/widgets/login.dart +++ b/lib/widgets/login.dart @@ -178,7 +178,7 @@ class _AddAccountPageState extends State { // TODO(#36): support login methods beyond username/password Navigator.push(context, - PasswordLoginPage.buildRoute(serverSettings: serverSettings)); + LoginPage.buildRoute(serverSettings: serverSettings)); } finally { setState(() { _inProgress = false; @@ -235,21 +235,21 @@ class _AddAccountPageState extends State { } } -class PasswordLoginPage extends StatefulWidget { - const PasswordLoginPage({super.key, required this.serverSettings}); +class LoginPage extends StatefulWidget { + const LoginPage({super.key, required this.serverSettings}); final GetServerSettingsResult serverSettings; static Route buildRoute({required GetServerSettingsResult serverSettings}) { return _LoginSequenceRoute( - page: PasswordLoginPage(serverSettings: serverSettings)); + page: LoginPage(serverSettings: serverSettings)); } @override - State createState() => _PasswordLoginPageState(); + State createState() => _LoginPageState(); } -class _PasswordLoginPageState extends State { +class _LoginPageState extends State { bool _inProgress = false; Future _tryInsertAccountAndNavigate({ @@ -330,7 +330,7 @@ class _PasswordLoginPageState extends State { class _UsernamePasswordForm extends StatefulWidget { const _UsernamePasswordForm({required this.loginPageState}); - final _PasswordLoginPageState loginPageState; + final _LoginPageState loginPageState; @override State<_UsernamePasswordForm> createState() => _UsernamePasswordFormState(); diff --git a/test/widgets/login_test.dart b/test/widgets/login_test.dart index f53af3544b..8a99dbbcf5 100644 --- a/test/widgets/login_test.dart +++ b/test/widgets/login_test.dart @@ -56,7 +56,7 @@ void main() { // TODO test AddAccountPage - group('PasswordLoginPage', () { + group('LoginPage', () { late FakeApiConnection connection; Future prepare(WidgetTester tester, @@ -72,7 +72,7 @@ void main() { localizationsDelegates: ZulipLocalizations.localizationsDelegates, supportedLocales: ZulipLocalizations.supportedLocales, home: GlobalStoreWidget( - child: PasswordLoginPage(serverSettings: serverSettings)))); + child: LoginPage(serverSettings: serverSettings)))); await tester.pump(); // load global store } From 70233ef0f9d27b17e5acddd7254bf96086c9e269 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 22 Mar 2024 16:31:53 -0700 Subject: [PATCH 03/14] login: Make content scrollable --- lib/widgets/login.dart | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart index ff7043d7e2..0d018bcf61 100644 --- a/lib/widgets/login.dart +++ b/lib/widgets/login.dart @@ -319,11 +319,18 @@ class _LoginPageState extends State { child: LinearProgressIndicator(minHeight: 4)) // 4 restates default : null), body: SafeArea( - minimum: const EdgeInsets.all(8), + minimum: const EdgeInsets.symmetric(horizontal: 8), + top: false, + bottom: false, child: Center( - child: ConstrainedBox( - constraints: const BoxConstraints(maxWidth: 400), - child: _UsernamePasswordForm(loginPageState: this))))); + child: SingleChildScrollView( + child: SafeArea( + minimum: const EdgeInsets.symmetric(vertical: 8), + // TODO also detect vertical scroll gestures that start on the + // left or the right of this box + child: ConstrainedBox( + constraints: const BoxConstraints(maxWidth: 400), + child: _UsernamePasswordForm(loginPageState: this))))))); } } From a88135a28c8ec93750f92e8266f1220799b73990 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 25 Mar 2024 15:50:36 -0700 Subject: [PATCH 04/14] binding: Add canLaunchUrl from url_launcher To sit alongside the existing launchUrl. --- lib/model/binding.dart | 8 ++++++++ test/model/binding.dart | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/lib/model/binding.dart b/lib/model/binding.dart index bb9a54ac11..77aa367fcc 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -75,6 +75,11 @@ abstract class ZulipBinding { /// to a [GlobalStore]. Future loadGlobalStore(); + /// Checks whether the platform can launch [url], via package:url_launcher. + /// + /// This wraps [url_launcher.canLaunchUrl]. + Future canLaunchUrl(Uri url); + /// Pass [url] to the underlying platform, via package:url_launcher. /// /// This wraps [url_launcher.launchUrl]. @@ -159,6 +164,9 @@ class LiveZulipBinding extends ZulipBinding { return LiveGlobalStore.load(); } + @override + Future canLaunchUrl(Uri url) => url_launcher.canLaunchUrl(url); + @override Future launchUrl( Uri url, { diff --git a/test/model/binding.dart b/test/model/binding.dart index 394d89d988..25eb3295a8 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -66,6 +66,7 @@ class TestZulipBinding extends ZulipBinding { void reset() { ZulipApp.debugReset(); _resetStore(); + _resetCanLaunchUrl(); _resetLaunchUrl(); _resetDeviceInfo(); _resetFirebase(); @@ -119,6 +120,36 @@ class TestZulipBinding extends ZulipBinding { return Future.value(globalStore); } + /// The value that `ZulipBinding.instance.canLaunchUrl()` should return. + /// + /// See also [takeCanLaunchUrlCalls]. + bool canLaunchUrlResult = true; + + void _resetCanLaunchUrl() { + canLaunchUrlResult = true; + _canLaunchUrlCalls = null; + } + + /// Consume the log of calls made to `ZulipBinding.instance.canLaunchUrl()`. + /// + /// This returns a list of the arguments to all calls made + /// to `ZulipBinding.instance.canLaunchUrl()` since the last call to + /// either this method or [reset]. + /// + /// See also [canLaunchUrlResult]. + List takeCanLaunchUrlCalls() { + final result = _canLaunchUrlCalls; + _canLaunchUrlCalls = null; + return result ?? []; + } + List? _canLaunchUrlCalls; + + @override + Future canLaunchUrl(Uri url) async { + (_canLaunchUrlCalls ??= []).add(url); + return canLaunchUrlResult; + } + /// The value that `ZulipBinding.instance.launchUrl()` should return. /// /// See also [takeLaunchUrlCalls]. From a2ed21b21395f76cbbdd0675da76e55afa91ca7c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 28 Mar 2024 12:14:56 -0700 Subject: [PATCH 05/14] binding: supportsCloseForLaunchMode, closeInAppWebView from url_launcher The binding classes are getting a bit crowded with all the stuff from url_launcher; perhaps we can pull it all out to a helper as a followup. --- lib/model/binding.dart | 20 ++++++++++++++++++++ test/model/binding.dart | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/model/binding.dart b/lib/model/binding.dart index 77aa367fcc..31ed514973 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -88,6 +88,16 @@ abstract class ZulipBinding { url_launcher.LaunchMode mode = url_launcher.LaunchMode.platformDefault, }); + /// Checks whether [closeInAppWebView] is supported, via package:url_launcher. + /// + /// This wraps [url_launcher.supportsCloseForLaunchMode]. + Future supportsCloseForLaunchMode(url_launcher.LaunchMode mode); + + /// Closes the current in-app web view, via package:url_launcher. + /// + /// This wraps [url_launcher.closeInAppWebView]. + Future closeInAppWebView(); + /// Provides device and operating system information, /// via package:device_info_plus. /// @@ -175,6 +185,16 @@ class LiveZulipBinding extends ZulipBinding { return url_launcher.launchUrl(url, mode: mode); } + @override + Future supportsCloseForLaunchMode(url_launcher.LaunchMode mode) async { + return url_launcher.supportsCloseForLaunchMode(mode); + } + + @override + Future closeInAppWebView() async { + return url_launcher.closeInAppWebView(); + } + @override Future deviceInfo() async { final deviceInfo = await device_info_plus.DeviceInfoPlugin().deviceInfo; diff --git a/test/model/binding.dart b/test/model/binding.dart index 25eb3295a8..1a561da92d 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -68,6 +68,7 @@ class TestZulipBinding extends ZulipBinding { _resetStore(); _resetCanLaunchUrl(); _resetLaunchUrl(); + _resetCloseInAppWebView(); _resetDeviceInfo(); _resetFirebase(); _resetNotifications(); @@ -183,6 +184,26 @@ class TestZulipBinding extends ZulipBinding { return launchUrlResult; } + @override + Future supportsCloseForLaunchMode(url_launcher.LaunchMode mode) async => true; + + void _resetCloseInAppWebView() { + _closeInAppWebViewCallCount = 0; + } + + /// Read and reset the count of calls to `ZulipBinding.instance.closeInAppWebView()`. + int takeCloseInAppWebViewCallCount() { + final result = _closeInAppWebViewCallCount; + _closeInAppWebViewCallCount = 0; + return result; + } + int _closeInAppWebViewCallCount = 0; + + @override + Future closeInAppWebView() async { + _closeInAppWebViewCallCount++; + } + /// The value that `ZulipBinding.instance.deviceInfo()` should return. /// /// See also [takeDeviceInfoCalls]. From 84ff624d3df6257b040c8a13c671ccce7b0a25f9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 25 Mar 2024 16:10:35 -0700 Subject: [PATCH 06/14] login test: Use checkErrorDialog And also go a bit further than what the test was doing: simulate a tap on the error dialog's dismiss button. --- test/widgets/login_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/widgets/login_test.dart b/test/widgets/login_test.dart index 8a99dbbcf5..f93160fd5e 100644 --- a/test/widgets/login_test.dart +++ b/test/widgets/login_test.dart @@ -13,6 +13,7 @@ import '../api/fake_api.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; import '../stdlib_checks.dart'; +import 'dialog_checks.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -131,9 +132,8 @@ void main() { await tester.pumpAndSettle(); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - final findAlertDialogWithExistsMessage = find.widgetWithText( - AlertDialog, zulipLocalizations.errorAccountLoggedInTitle); - check(findAlertDialogWithExistsMessage.evaluate()).isNotEmpty(); + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: zulipLocalizations.errorAccountLoggedInTitle))); }); // TODO test validators on the TextFormField widgets From aa8b91ba947214928dcb562b4f324f304ea9c9e0 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 25 Mar 2024 17:23:55 -0700 Subject: [PATCH 07/14] login test [nfc]: Group username/password login tests together We're about to add the web-auth feature, and the tests for that feature will want their own separate group. --- test/widgets/login_test.dart | 108 ++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/test/widgets/login_test.dart b/test/widgets/login_test.dart index f93160fd5e..d85a86b64d 100644 --- a/test/widgets/login_test.dart +++ b/test/widgets/login_test.dart @@ -85,61 +85,63 @@ void main() { && (widget.autofillHints ?? []).contains(AutofillHints.password)); final findSubmitButton = find.widgetWithText(ElevatedButton, 'Log in'); - void checkFetchApiKey({required String username, required String password}) { - check(connection.lastRequest).isA() - ..method.equals('POST') - ..url.path.equals('/api/v1/fetch_api_key') - ..bodyFields.deepEquals({ - 'username': username, - 'password': password, - }); - } + group('username/password login', () { + void checkFetchApiKey({required String username, required String password}) { + check(connection.lastRequest).isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/fetch_api_key') + ..bodyFields.deepEquals({ + 'username': username, + 'password': password, + }); + } + + testWidgets('basic happy case', (tester) async { + final serverSettings = eg.serverSettings(); + await prepare(tester, serverSettings); + check(testBinding.globalStore.accounts).isEmpty(); + + await tester.enterText(findUsernameInput, eg.selfAccount.email); + await tester.enterText(findPasswordInput, 'p455w0rd'); + connection.prepare(json: FetchApiKeyResult( + apiKey: eg.selfAccount.apiKey, + email: eg.selfAccount.email, + userId: eg.selfAccount.userId, + ).toJson()); + await tester.tap(findSubmitButton); + checkFetchApiKey(username: eg.selfAccount.email, password: 'p455w0rd'); + await tester.idle(); + check(testBinding.globalStore.accounts).single + .equals(eg.selfAccount.copyWith( + id: testBinding.globalStore.accounts.single.id)); + }); - testWidgets('basic happy case', (tester) async { - final serverSettings = eg.serverSettings(); - await prepare(tester, serverSettings); - check(testBinding.globalStore.accounts).isEmpty(); - - await tester.enterText(findUsernameInput, eg.selfAccount.email); - await tester.enterText(findPasswordInput, 'p455w0rd'); - connection.prepare(json: FetchApiKeyResult( - apiKey: eg.selfAccount.apiKey, - email: eg.selfAccount.email, - userId: eg.selfAccount.userId, - ).toJson()); - await tester.tap(findSubmitButton); - checkFetchApiKey(username: eg.selfAccount.email, password: 'p455w0rd'); - await tester.idle(); - check(testBinding.globalStore.accounts).single - .equals(eg.selfAccount.copyWith( - id: testBinding.globalStore.accounts.single.id)); - }); + testWidgets('account already exists', (tester) async { + final serverSettings = eg.serverSettings(); + await prepare(tester, serverSettings); + check(testBinding.globalStore.accounts).isEmpty(); + testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + + await tester.enterText(findUsernameInput, eg.selfAccount.email); + await tester.enterText(findPasswordInput, 'p455w0rd'); + connection.prepare(json: FetchApiKeyResult( + apiKey: eg.selfAccount.apiKey, + email: eg.selfAccount.email, + userId: eg.selfAccount.userId, + ).toJson()); + await tester.tap(findSubmitButton); + await tester.pumpAndSettle(); + + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: zulipLocalizations.errorAccountLoggedInTitle))); + }); - testWidgets('account already exists', (tester) async { - final serverSettings = eg.serverSettings(); - await prepare(tester, serverSettings); - check(testBinding.globalStore.accounts).isEmpty(); - testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - - await tester.enterText(findUsernameInput, eg.selfAccount.email); - await tester.enterText(findPasswordInput, 'p455w0rd'); - connection.prepare(json: FetchApiKeyResult( - apiKey: eg.selfAccount.apiKey, - email: eg.selfAccount.email, - userId: eg.selfAccount.userId, - ).toJson()); - await tester.tap(findSubmitButton); - await tester.pumpAndSettle(); - - final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - await tester.tap(find.byWidget(checkErrorDialog(tester, - expectedTitle: zulipLocalizations.errorAccountLoggedInTitle))); + // TODO test validators on the TextFormField widgets + // TODO test navigation, i.e. the call to pushAndRemoveUntil + // TODO test _getUserId case + // TODO test handling failure in fetchApiKey request + // TODO test _inProgress logic }); - - // TODO test validators on the TextFormField widgets - // TODO test navigation, i.e. the call to pushAndRemoveUntil - // TODO test _getUserId case - // TODO test handling failure in fetchApiKey request - // TODO test _inProgress logic }); } From a8d488872d16049451a3d039604360b362f4cc7b Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 26 Mar 2024 20:18:50 -0700 Subject: [PATCH 08/14] login test: Use real ZulipApp instead in tests' setup Soon we'll reuse this setup to test the upcoming web-auth feature, and that feature exercises functionality on _ZulipAppState. --- test/widgets/login_test.dart | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/test/widgets/login_test.dart b/test/widgets/login_test.dart index d85a86b64d..6fb883010e 100644 --- a/test/widgets/login_test.dart +++ b/test/widgets/login_test.dart @@ -1,13 +1,12 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:zulip/api/route/account.dart'; import 'package:zulip/api/route/realm.dart'; import 'package:zulip/model/localizations.dart'; +import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/login.dart'; -import 'package:zulip/widgets/store.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -68,13 +67,11 @@ void main() { realmUrl: serverSettings.realmUrl, zulipFeatureLevel: serverSettings.zulipFeatureLevel); - await tester.pumpWidget( - MaterialApp( - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: GlobalStoreWidget( - child: LoginPage(serverSettings: serverSettings)))); - await tester.pump(); // load global store + await tester.pumpWidget(const ZulipApp()); + await tester.pump(); + final navigator = await ZulipApp.navigator; + navigator.push(LoginPage.buildRoute(serverSettings: serverSettings)); + await tester.pumpAndSettle(); } final findUsernameInput = find.byWidgetPredicate((widget) => From e4cf95503f406965c0f3c276df68a2b0bb2c854a Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 26 Mar 2024 20:34:26 -0700 Subject: [PATCH 09/14] test [nfc]: Allow customizing eg.serverSettings --- test/example_data.dart | 45 ++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index e542d1e3c4..4b0ffc77a4 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -21,22 +21,37 @@ const String recentZulipVersion = '8.0'; const int recentZulipFeatureLevel = 185; const int futureZulipFeatureLevel = 9999; -GetServerSettingsResult serverSettings() { +GetServerSettingsResult serverSettings({ + Map? authenticationMethods, + List? externalAuthenticationMethods, + int? zulipFeatureLevel, + String? zulipVersion, + String? zulipMergeBase, + bool? pushNotificationsEnabled, + bool? isIncompatible, + bool? emailAuthEnabled, + bool? requireEmailFormatUsernames, + Uri? realmUrl, + String? realmName, + String? realmIcon, + String? realmDescription, + bool? realmWebPublicAccessEnabled, +}) { return GetServerSettingsResult( - authenticationMethods: {}, - externalAuthenticationMethods: [], - zulipFeatureLevel: recentZulipFeatureLevel, - zulipVersion: recentZulipVersion, - zulipMergeBase: recentZulipVersion, - pushNotificationsEnabled: true, - isIncompatible: false, - emailAuthEnabled: true, - requireEmailFormatUsernames: true, - realmUrl: realmUrl, - realmName: 'Example Zulip organization', - realmIcon: '$realmUrl/icon.png', - realmDescription: 'An example Zulip organization', - realmWebPublicAccessEnabled: false, + authenticationMethods: authenticationMethods ?? {}, + externalAuthenticationMethods: externalAuthenticationMethods ?? [], + zulipFeatureLevel: zulipFeatureLevel ?? recentZulipFeatureLevel, + zulipVersion: zulipVersion ?? recentZulipVersion, + zulipMergeBase: zulipMergeBase ?? recentZulipVersion, + pushNotificationsEnabled: pushNotificationsEnabled ?? true, + isIncompatible: isIncompatible ?? false, + emailAuthEnabled: emailAuthEnabled ?? true, + requireEmailFormatUsernames: requireEmailFormatUsernames ?? true, + realmUrl: realmUrl ?? _realmUrl, + realmName: realmName ?? 'Example Zulip organization', + realmIcon: realmIcon ?? '$realmUrl/icon.png', + realmDescription: realmDescription ?? 'An example Zulip organization', + realmWebPublicAccessEnabled: realmWebPublicAccessEnabled ?? false, ); } From d6d31997e5f07679feee94290e4f6ce6c0c3deb8 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 28 Mar 2024 12:17:44 -0700 Subject: [PATCH 10/14] test: Use right-shaped API keys for selfAccount and otherAccount --- test/example_data.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index 4b0ffc77a4..e6608a3f9b 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -123,14 +123,14 @@ final User selfUser = user(fullName: 'Self User', email: 'self@example'); final Account selfAccount = account( id: 1001, user: selfUser, - apiKey: 'asdfqwer', + apiKey: 'dQcEJWTq3LczosDkJnRTwf31zniGvMrO', // A Zulip API key is 32 digits of base64. ); final User otherUser = user(fullName: 'Other User', email: 'other@example'); final Account otherAccount = account( id: 1002, user: otherUser, - apiKey: 'sdfgwert', + apiKey: '6dxT4b73BYpCTU+i4BB9LAKC5h/CufqY', // A Zulip API key is 32 digits of base64. ); final User thirdUser = user(fullName: 'Third User', email: 'third@example'); From 1bd1fd7f57fa256a7bc5e5ef1514a92fb798de42 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 26 Mar 2024 20:56:46 -0700 Subject: [PATCH 11/14] test [nfc]: Move prepareBoringImageHttpClient to test_images.dart This was already being used for tests outside the test file it was defined in, and we'd like to start using it in an additional different test file. So, define it centrally in this place that makes sense. --- test/test_images.dart | 18 ++++++++++++++++++ test/widgets/autocomplete_test.dart | 2 +- test/widgets/content_test.dart | 19 ------------------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/test/test_images.dart b/test/test_images.dart index 9ec4b32e16..c7a04c264f 100644 --- a/test/test_images.dart +++ b/test/test_images.dart @@ -1,8 +1,26 @@ import 'dart:async'; import 'dart:io'; +import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +/// Set [debugNetworkImageHttpClientProvider] to return a constant image. +/// +/// Returns the [FakeImageHttpClient] that handles the requests. +/// +/// The caller must set [debugNetworkImageHttpClientProvider] back to null +/// before the end of the test. +// TODO(upstream) simplify callers by using addTearDown: https://github.com/flutter/flutter/issues/123189 +// See also: https://github.com/flutter/flutter/issues/121917 +FakeImageHttpClient prepareBoringImageHttpClient() { + final httpClient = FakeImageHttpClient(); + debugNetworkImageHttpClientProvider = () => httpClient; + httpClient.request.response + ..statusCode = HttpStatus.ok + ..content = kSolidBlueAvatar; + return httpClient; +} + class FakeImageHttpClient extends Fake implements HttpClient { final FakeImageHttpClientRequest request = FakeImageHttpClientRequest(); diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart index c5028bb0ff..f1ac188291 100644 --- a/test/widgets/autocomplete_test.dart +++ b/test/widgets/autocomplete_test.dart @@ -14,7 +14,7 @@ import '../api/fake_api.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; import '../model/test_store.dart'; -import 'content_test.dart'; +import '../test_images.dart'; /// Simulates loading a [MessageListPage] and tapping to focus the compose input. /// diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index e6b0bc553b..5298ee09c3 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -1,5 +1,3 @@ -import 'dart:io'; - import 'package:checks/checks.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; @@ -27,23 +25,6 @@ import 'dialog_checks.dart'; import 'message_list_checks.dart'; import 'page_checks.dart'; -/// Set [debugNetworkImageHttpClientProvider] to return a constant image. -/// -/// Returns the [FakeImageHttpClient] that handles the requests. -/// -/// The caller must set [debugNetworkImageHttpClientProvider] back to null -/// before the end of the test. -// TODO(upstream) simplify callers by using addTearDown: https://github.com/flutter/flutter/issues/123189 -// See also: https://github.com/flutter/flutter/issues/121917 -FakeImageHttpClient prepareBoringImageHttpClient() { - final httpClient = FakeImageHttpClient(); - debugNetworkImageHttpClientProvider = () => httpClient; - httpClient.request.response - ..statusCode = HttpStatus.ok - ..content = kSolidBlueAvatar; - return httpClient; -} - void main() { // For testing a new content feature: // From 60c5245d32ff5e06bc4e4a1b22c87cabf593255d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 1 Apr 2024 13:48:55 -0700 Subject: [PATCH 12/14] login [nfc]: Reorder members of LoginPage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greg points out, referring to the Flutter style guide at https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#order-other-class-members-in-a-way-that-makes-sense : https://github.com/zulip/zulip-flutter/pull/600#discussion_r1546825813 > I think it's likely helpful to treat [`buildRoute`] for the > ordering as if it does return the class's type, though. It plays > pretty much the role of a factory constructor — it's the thing > that application code, at least, should always use instead of > calling the actual constructor directly. (Possibly we should even > make the constructor private when we have a `buildRoute` method > like this; but that might be annoying in tests.) --- lib/widgets/login.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart index 0d018bcf61..b301b12c40 100644 --- a/lib/widgets/login.dart +++ b/lib/widgets/login.dart @@ -238,13 +238,13 @@ class _AddAccountPageState extends State { class LoginPage extends StatefulWidget { const LoginPage({super.key, required this.serverSettings}); - final GetServerSettingsResult serverSettings; - static Route buildRoute({required GetServerSettingsResult serverSettings}) { return _LoginSequenceRoute( page: LoginPage(serverSettings: serverSettings)); } + final GetServerSettingsResult serverSettings; + @override State createState() => _LoginPageState(); } From bd2fc3d6b2242edb3aed5928f1febe300f13ea1c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Mar 2024 17:34:51 -0700 Subject: [PATCH 13/14] login: Support web-based auth methods Fixes: #36 --- android/app/src/main/AndroidManifest.xml | 7 ++ assets/l10n/app_en.arb | 19 +++ ios/Runner/Info.plist | 13 ++ lib/api/model/web_auth.dart | 87 +++++++++++++ lib/widgets/app.dart | 25 +++- lib/widgets/login.dart | 150 ++++++++++++++++++++++- test/api/model/web_auth_test.dart | 98 +++++++++++++++ test/widgets/login_test.dart | 79 +++++++++++- 8 files changed, 473 insertions(+), 5 deletions(-) create mode 100644 lib/api/model/web_auth.dart create mode 100644 test/api/model/web_auth_test.dart diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 6052f6804d..936cbd039e 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -25,6 +25,13 @@ + + + + + + + diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index fb7040ed8f..f2686f9bd9 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -63,6 +63,14 @@ "@actionSheetOptionUnstarMessage": { "description": "Label for unstar button on action sheet." }, + "errorWebAuthOperationalErrorTitle": "Something went wrong", + "@errorWebAuthOperationalErrorTitle": { + "description": "Error title when third-party authentication has an operational error (not necessarily caused by invalid credentials)." + }, + "errorWebAuthOperationalError": "An unexpected error occurred.", + "@errorWebAuthOperationalError": { + "description": "Error message when third-party authentication has an operational error (not necessarily caused by invalid credentials)." + }, "errorAccountLoggedInTitle": "Account already logged in", "@errorAccountLoggedInTitle": { "description": "Error title on attempting to log into an account that's already logged in." @@ -281,6 +289,17 @@ "@loginFormSubmitLabel": { "description": "Button text to submit login credentials." }, + "loginMethodDivider": "OR", + "@loginMethodDivider": { + "description": "Text on the divider between the username/password form and the third-party login options. Uppercase (for languages with letter case)." + }, + "signInWithFoo": "Sign in with {method}", + "@signInWithFoo": { + "description": "Button to use {method} to sign in to the app.", + "placeholders": { + "method": {"type": "String", "example": "Google"} + } + }, "loginAddAnAccountPageTitle": "Add an account", "@loginAddAnAccountPageTitle": { "description": "Page title for screen to add a Zulip account." diff --git a/ios/Runner/Info.plist b/ios/Runner/Info.plist index 8f08f0cdda..5f5d9c5ea2 100644 --- a/ios/Runner/Info.plist +++ b/ios/Runner/Info.plist @@ -22,8 +22,21 @@ $(FLUTTER_BUILD_NAME) CFBundleSignature ???? + CFBundleURLTypes + + + CFBundleURLName + com.zulip.flutter + CFBundleURLSchemes + + zulip + + + CFBundleVersion $(FLUTTER_BUILD_NUMBER) + FlutterDeepLinkingEnabled + ITSAppUsesNonExemptEncryption LSRequiresIPhoneOS diff --git a/lib/api/model/web_auth.dart b/lib/api/model/web_auth.dart new file mode 100644 index 0000000000..490c4b79db --- /dev/null +++ b/lib/api/model/web_auth.dart @@ -0,0 +1,87 @@ +import 'dart:math'; + +import 'package:convert/convert.dart'; +import 'package:flutter/foundation.dart'; + +/// The authentication information contained in the zulip:// redirect URL. +class WebAuthPayload { + final Uri realm; + final String email; + final int? userId; // TODO(server-5) new in FL 108 + final String otpEncryptedApiKey; + + WebAuthPayload._({ + required this.realm, + required this.email, + required this.userId, + required this.otpEncryptedApiKey, + }); + + factory WebAuthPayload.parse(Uri url) { + if ( + url case Uri( + scheme: 'zulip', + host: 'login', + queryParameters: { + 'realm': String realmStr, + 'email': String email, + // 'user_id' handled below + 'otp_encrypted_api_key': String otpEncryptedApiKey, + }, + ) + ) { + final Uri? realm = Uri.tryParse(realmStr); + if (realm == null) throw const FormatException(); + + // TODO(server-5) require in queryParameters (new in FL 108) + final userIdStr = url.queryParameters['user_id']; + int? userId; + if (userIdStr != null) { + userId = int.tryParse(userIdStr, radix: 10); + if (userId == null) throw const FormatException(); + } + + if (!RegExp(r'^[0-9a-fA-F]{64}$').hasMatch(otpEncryptedApiKey)) { + throw const FormatException(); + } + + return WebAuthPayload._( + otpEncryptedApiKey: otpEncryptedApiKey, + email: email, + userId: userId, + realm: realm, + ); + } else { + // TODO(dart): simplify after https://github.com/dart-lang/language/issues/2537 + throw const FormatException(); + } + } + + String decodeApiKey(String otp) { + final otpBytes = hex.decode(otp); + final otpEncryptedApiKeyBytes = hex.decode(otpEncryptedApiKey); + if (otpBytes.length != otpEncryptedApiKeyBytes.length) { + throw const FormatException(); + } + return String.fromCharCodes(Iterable.generate(otpBytes.length, + (i) => otpBytes[i] ^ otpEncryptedApiKeyBytes[i])); + } +} + +String generateOtp() { + final rand = Random.secure(); + final Uint8List bytes = Uint8List.fromList( + List.generate(32, (_) => rand.nextInt(256))); + return hex.encode(bytes); +} + +/// For tests, create an OTP-encrypted API key. +@visibleForTesting +String debugEncodeApiKey(String apiKey, String otp) { + final apiKeyBytes = apiKey.codeUnits; + assert(apiKeyBytes.every((byte) => byte <= 0xff)); + final otpBytes = hex.decode(otp); + assert(apiKeyBytes.length == otpBytes.length); + return hex.encode(List.generate(otpBytes.length, + (i) => apiKeyBytes[i] ^ otpBytes[i])); +} diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 617e33efd9..dce9a1a64a 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -83,7 +83,30 @@ class ZulipApp extends StatefulWidget { State createState() => _ZulipAppState(); } -class _ZulipAppState extends State { +class _ZulipAppState extends State with WidgetsBindingObserver { + @override + Future didPushRouteInformation(routeInformation) async { + if (routeInformation case RouteInformation( + uri: Uri(scheme: 'zulip', host: 'login') && var url) + ) { + await LoginPage.handleWebAuthUrl(url); + return true; + } + return super.didPushRouteInformation(routeInformation); + } + + @override + void initState() { + super.initState(); + WidgetsBinding.instance.addObserver(this); + } + + @override + void dispose() { + WidgetsBinding.instance.removeObserver(this); + super.dispose(); + } + @override Widget build(BuildContext context) { final theme = ThemeData( diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart index b301b12c40..6dd80e0959 100644 --- a/lib/widgets/login.dart +++ b/lib/widgets/login.dart @@ -1,16 +1,23 @@ +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; +import 'package:url_launcher/url_launcher.dart'; import '../api/exception.dart'; +import '../api/model/web_auth.dart'; import '../api/route/account.dart'; import '../api/route/realm.dart'; import '../api/route/users.dart'; +import '../log.dart'; +import '../model/binding.dart'; import '../model/store.dart'; import 'app.dart'; import 'dialog.dart'; import 'input.dart'; import 'page.dart'; import 'store.dart'; +import 'text.dart'; class _LoginSequenceRoute extends MaterialWidgetRoute { _LoginSequenceRoute({ @@ -176,7 +183,6 @@ class _AddAccountPageState extends State { return; } - // TODO(#36): support login methods beyond username/password Navigator.push(context, LoginPage.buildRoute(serverSettings: serverSettings)); } finally { @@ -240,11 +246,23 @@ class LoginPage extends StatefulWidget { static Route buildRoute({required GetServerSettingsResult serverSettings}) { return _LoginSequenceRoute( - page: LoginPage(serverSettings: serverSettings)); + page: LoginPage(serverSettings: serverSettings, key: _lastBuiltKey)); } final GetServerSettingsResult serverSettings; + /// Log in using the payload of a web-auth URL like zulip://login?… + static Future handleWebAuthUrl(Uri url) async { + return _lastBuiltKey.currentState?.handleWebAuthUrl(url); + } + + /// A key for the page from the last [buildRoute] call. + static final _lastBuiltKey = GlobalKey<_LoginPageState>(); + + /// The OTP to use, instead of an app-generated one, for testing. + @visibleForTesting + static String? debugOtpOverride; + @override State createState() => _LoginPageState(); } @@ -252,6 +270,84 @@ class LoginPage extends StatefulWidget { class _LoginPageState extends State { bool _inProgress = false; + String? get _otp { + String? result; + assert(() { + result = LoginPage.debugOtpOverride; + return true; + }()); + return result ?? __otp; + } + String? __otp; + + Future handleWebAuthUrl(Uri url) async { + setState(() { + _inProgress = true; + }); + try { + await ZulipBinding.instance.closeInAppWebView(); + + if (_otp == null) throw Error(); + final payload = WebAuthPayload.parse(url); + if (payload.realm.origin != widget.serverSettings.realmUrl.origin) throw Error(); + final apiKey = payload.decodeApiKey(_otp!); + await _tryInsertAccountAndNavigate( + // TODO(server-5): Rely on userId from payload. + userId: payload.userId ?? await _getUserId(payload.email, apiKey), + email: payload.email, + apiKey: apiKey, + ); + } catch (e) { + assert(debugLog(e.toString())); + if (!mounted) return; + final zulipLocalizations = ZulipLocalizations.of(context); + // Could show different error messages for different failure modes. + await showErrorDialog(context: context, + title: zulipLocalizations.errorWebAuthOperationalErrorTitle, + message: zulipLocalizations.errorWebAuthOperationalError); + } finally { + setState(() { + _inProgress = false; + __otp = null; + }); + } + } + + Future _beginWebAuth(ExternalAuthenticationMethod method) async { + __otp = generateOtp(); + try { + final url = widget.serverSettings.realmUrl.resolve(method.loginUrl) + .replace(queryParameters: {'mobile_flow_otp': _otp!}); + + // Could set [_inProgress]… but we'd need to unset it if the web-auth + // attempt is aborted (by the user closing the browser, for example), + // and I don't think we can reliably know when that happens. + await ZulipBinding.instance.launchUrl(url, mode: LaunchMode.inAppBrowserView); + } catch (e) { + assert(debugLog(e.toString())); + + if (e is PlatformException + && defaultTargetPlatform == TargetPlatform.iOS + && e.message != null && e.message!.startsWith('Error while launching')) { + // Ignore; I've seen this on my iPhone even when auth succeeds. + // Specifically, Apple web auth…which on iOS should be replaced by + // Apple native auth; that's #462. + // Possibly related: + // https://github.com/flutter/flutter/issues/91660 + // but in that issue, people report authentication not succeeding. + // TODO(#462) remove this? + return; + } + + if (!mounted) return; + final zulipLocalizations = ZulipLocalizations.of(context); + // Could show different error messages for different failure modes. + await showErrorDialog(context: context, + title: zulipLocalizations.errorWebAuthOperationalErrorTitle, + message: zulipLocalizations.errorWebAuthOperationalError); + } + } + Future _tryInsertAccountAndNavigate({ required String email, required String apiKey, @@ -312,6 +408,26 @@ class _LoginPageState extends State { assert(!PerAccountStoreWidget.debugExistsOf(context)); final zulipLocalizations = ZulipLocalizations.of(context); + final externalAuthenticationMethods = widget.serverSettings.externalAuthenticationMethods; + + final loginForm = Column(mainAxisAlignment: MainAxisAlignment.center, children: [ + _UsernamePasswordForm(loginPageState: this), + if (externalAuthenticationMethods.isNotEmpty) ...[ + const OrDivider(), + ...externalAuthenticationMethods.map((method) { + final icon = method.displayIcon; + return OutlinedButton.icon( + icon: icon != null + ? Image.network(icon, width: 24, height: 24) + : null, + onPressed: !_inProgress + ? () => _beginWebAuth(method) + : null, + label: Text(zulipLocalizations.signInWithFoo(method.displayName))); + }), + ], + ]); + return Scaffold( appBar: AppBar(title: Text(zulipLocalizations.loginPageTitle), bottom: _inProgress @@ -330,7 +446,7 @@ class _LoginPageState extends State { // left or the right of this box child: ConstrainedBox( constraints: const BoxConstraints(maxWidth: 400), - child: _UsernamePasswordForm(loginPageState: this))))))); + child: loginForm)))))); } } @@ -495,3 +611,31 @@ class _UsernamePasswordFormState extends State<_UsernamePasswordForm> { ]))); } } + +// Loosely based on the corresponding element in the web app. +class OrDivider extends StatelessWidget { + const OrDivider({super.key}); + + @override + Widget build(BuildContext context) { + final zulipLocalizations = ZulipLocalizations.of(context); + + const divider = Expanded( + child: Divider(color: Color(0xffdedede), thickness: 2)); + + return Padding( + padding: const EdgeInsets.symmetric(vertical: 10), + child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [ + divider, + Padding( + padding: const EdgeInsets.symmetric(horizontal: 5), + child: Text(zulipLocalizations.loginMethodDivider, + textAlign: TextAlign.center, + style: const TextStyle( + color: Color(0xff575757), + height: 1.5, + ).merge(weightVariableTextStyle(context, wght: 600)))), + divider, + ])); + } +} diff --git a/test/api/model/web_auth_test.dart b/test/api/model/web_auth_test.dart new file mode 100644 index 0000000000..f836b1f962 --- /dev/null +++ b/test/api/model/web_auth_test.dart @@ -0,0 +1,98 @@ +import 'package:checks/checks.dart'; +import 'package:convert/convert.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/web_auth.dart'; + +import '../../example_data.dart' as eg; + +void main() { + group('WebAuthPayload', () { + const otp = '186f6d085a5621ebaf1ccfc05033e8acba57dae03f061705ac1e58c402c30a31'; + final encryptedApiKey = debugEncodeApiKey(eg.selfAccount.apiKey, otp); + final wellFormed = Uri.parse( + 'zulip://login?otp_encrypted_api_key=$encryptedApiKey' + '&email=self%40example&user_id=1&realm=https%3A%2F%2Fchat.example%2F'); + + test('basic happy case', () { + final payload = WebAuthPayload.parse(wellFormed); + check(payload) + ..otpEncryptedApiKey.equals(encryptedApiKey) + ..email.equals('self@example') + ..userId.equals(1) + ..realm.equals(Uri.parse('https://chat.example/')); + check(payload.decodeApiKey(otp)).equals(eg.selfAccount.apiKey); + }); + + // TODO(server-5) remove + test('legacy: no userId', () { + final queryParams = {...wellFormed.queryParameters}..remove('user_id'); + final payload = WebAuthPayload.parse( + wellFormed.replace(queryParameters: queryParams)); + check(payload) + ..otpEncryptedApiKey.equals(encryptedApiKey) + ..email.equals('self@example') + ..userId.isNull() + ..realm.equals(Uri.parse('https://chat.example/')); + check(payload.decodeApiKey(otp)).equals(eg.selfAccount.apiKey); + }); + + test('parse fails when an expected field is missing', () { + final queryParams = {...wellFormed.queryParameters}..remove('email'); + final input = wellFormed.replace(queryParameters: queryParams); + check(() => WebAuthPayload.parse(input)).throws(); + }); + + test('parse fails when otp_encrypted_api_key is wrong length', () { + final queryParams = {...wellFormed.queryParameters} + ..['otp_encrypted_api_key'] = 'asdf'; + final input = wellFormed.replace(queryParameters: queryParams); + check(() => WebAuthPayload.parse(input)).throws(); + }); + + test('parse fails when host is not "login"', () { + final input = wellFormed.replace(host: 'foo'); + check(() => WebAuthPayload.parse(input)).throws(); + }); + + test('parse fails when scheme is not "zulip"', () { + final input = wellFormed.replace(scheme: 'https'); + check(() => WebAuthPayload.parse(input)).throws(); + }); + + test('decodeApiKey fails when otp is wrong length', () { + final payload = WebAuthPayload.parse(wellFormed); + check(() => payload.decodeApiKey('asdf')).throws(); + }); + }); + + group('generateOtp', () { + test('smoke, and check all 256 byte values are used', () { + // This is a probabilistic test. We've chosen `n` so that when the test + // should pass, the probability it fails is < 1e-9. See analysis below. + const n = 216; + final manyOtps = List.generate(n, (_) => generateOtp()); + + final bytesThatAppear = {}; + for (final otp in manyOtps) { + final bytes = hex.decode(otp); + check(bytes).length.equals(32); + bytesThatAppear.addAll(bytes); + } + + // Each possible value gets n * 32 opportunities to show up, + // each with probability 1/256; so the probability of missing all of those + // is exp(- n * 32 / 256) < 2e-12, and there are 256 such possible + // byte values so the probability that any of them gets missed is < 1e-9. + for (final byteValue in Iterable.generate(256)) { + check(bytesThatAppear).contains(byteValue); + } + }); + }); +} + +extension WebAuthPayloadChecks on Subject { + Subject get otpEncryptedApiKey => has((x) => x.otpEncryptedApiKey, 'otpEncryptedApiKey'); + Subject get email => has((x) => x.email, 'email'); + Subject get userId => has((x) => x.userId, 'userId'); + Subject get realm => has((x) => x.realm, 'realm'); +} diff --git a/test/widgets/login_test.dart b/test/widgets/login_test.dart index 6fb883010e..8684a66c0d 100644 --- a/test/widgets/login_test.dart +++ b/test/widgets/login_test.dart @@ -1,18 +1,25 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; +import 'package:zulip/api/model/web_auth.dart'; import 'package:zulip/api/route/account.dart'; import 'package:zulip/api/route/realm.dart'; +import 'package:zulip/model/binding.dart'; import 'package:zulip/model/localizations.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/login.dart'; +import 'package:zulip/widgets/page.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; import '../stdlib_checks.dart'; +import '../test_images.dart'; +import '../test_navigation.dart'; import 'dialog_checks.dart'; +import 'page_checks.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -58,6 +65,16 @@ void main() { group('LoginPage', () { late FakeApiConnection connection; + late List> pushedRoutes; + + void takeStartingRoutes() { + final expected = [ + (Subject it) => it.isA().page.isA(), + (Subject it) => it.isA().page.isA(), + ]; + check(pushedRoutes.take(expected.length)).deepEquals(expected); + pushedRoutes.removeRange(0, expected.length); + } Future prepare(WidgetTester tester, GetServerSettingsResult serverSettings) async { @@ -67,11 +84,16 @@ void main() { realmUrl: serverSettings.realmUrl, zulipFeatureLevel: serverSettings.zulipFeatureLevel); - await tester.pumpWidget(const ZulipApp()); + pushedRoutes = []; + final testNavObserver = TestNavigatorObserver() + ..onPushed = (route, prevRoute) => pushedRoutes.add(route); + await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); await tester.pump(); final navigator = await ZulipApp.navigator; navigator.push(LoginPage.buildRoute(serverSettings: serverSettings)); await tester.pumpAndSettle(); + takeStartingRoutes(); + check(pushedRoutes).isEmpty(); } final findUsernameInput = find.byWidgetPredicate((widget) => @@ -140,5 +162,60 @@ void main() { // TODO test handling failure in fetchApiKey request // TODO test _inProgress logic }); + + group('web auth', () { + testWidgets('basic happy case', (tester) async { + final method = ExternalAuthenticationMethod( + name: 'google', + displayName: 'Google', + displayIcon: eg.realmUrl.resolve('/static/images/authentication_backends/googl_e-icon.png').toString(), + loginUrl: '/accounts/login/social/google', + signupUrl: '/accounts/register/social/google', + ); + final serverSettings = eg.serverSettings( + externalAuthenticationMethods: [method]); + prepareBoringImageHttpClient(); // icon on social-auth button + await prepare(tester, serverSettings); + check(testBinding.globalStore.accounts).isEmpty(); + + const otp = '186f6d085a5621ebaf1ccfc05033e8acba57dae03f061705ac1e58c402c30a31'; + LoginPage.debugOtpOverride = otp; + await tester.tap(find.textContaining('Google')); + + final expectedUrl = eg.realmUrl.resolve(method.loginUrl) + .replace(queryParameters: {'mobile_flow_otp': otp}); + check(testBinding.takeLaunchUrlCalls()) + .deepEquals([(url: expectedUrl, mode: UrlLaunchMode.inAppBrowserView)]); + + // TODO test _inProgress logic? + + final encoded = debugEncodeApiKey(eg.selfAccount.apiKey, otp); + final url = Uri(scheme: 'zulip', host: 'login', queryParameters: { + 'otp_encrypted_api_key': encoded, + 'email': eg.selfAccount.email, + 'user_id': eg.selfAccount.userId.toString(), + 'realm': eg.selfAccount.realmUrl.toString(), + }); + + final ByteData message = const JSONMethodCodec().encodeMethodCall( + MethodCall('pushRouteInformation', {'location': url.toString()})); + tester.binding.defaultBinaryMessenger.handlePlatformMessage( + 'flutter/navigation', message, null); + + await tester.idle(); + check(testBinding.takeCloseInAppWebViewCallCount()).equals(1); + + final account = testBinding.globalStore.accounts.single; + check(account).equals(eg.selfAccount.copyWith(id: account.id)); + check(pushedRoutes).single.isA() + ..accountId.equals(account.id) + ..page.isA(); + + debugNetworkImageHttpClientProvider = null; + }); + + // TODO failures, such as: invalid loginUrl; URL can't be launched; + // WebAuthPayload.realm doesn't match the realm the UI is about + }); }); } From 76ba652c016c80ddcfb035866ce220a8181f581f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 22 Mar 2024 16:35:36 -0700 Subject: [PATCH 14/14] login [nfc]: Express some unconditional padding more transparently The top of this area will never be in the top inset, since that's consumed by the app bar. So "minimum 8" really just means always 8. So, express that more directly and transparently. --- lib/widgets/login.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart index 6dd80e0959..ce70671d93 100644 --- a/lib/widgets/login.dart +++ b/lib/widgets/login.dart @@ -436,12 +436,12 @@ class _LoginPageState extends State { : null), body: SafeArea( minimum: const EdgeInsets.symmetric(horizontal: 8), - top: false, bottom: false, child: Center( child: SingleChildScrollView( + padding: const EdgeInsets.only(top: 8), child: SafeArea( - minimum: const EdgeInsets.symmetric(vertical: 8), + minimum: const EdgeInsets.only(bottom: 8), // TODO also detect vertical scroll gestures that start on the // left or the right of this box child: ConstrainedBox(