From 3d1875fcdd519b043951b9dd37ef9b7c68a81e10 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 18 Mar 2024 17:34:51 -0700 Subject: [PATCH] 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 787a3ab1d3..9d812097f7 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 + }); }); }