From 5d82ed0b6d6cff40005ff9a426670d647c5d8dc4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 10 Apr 2020 11:34:27 -0700 Subject: [PATCH 01/16] webAuth types: Annotate generateOtp return value. --- src/start/webAuth.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/start/webAuth.js b/src/start/webAuth.js index 71a1adb160f..5c07318e6e9 100644 --- a/src/start/webAuth.js +++ b/src/start/webAuth.js @@ -29,7 +29,7 @@ import { base64ToHex, hexToAscii, xorHexStrings } from '../utils/encoding'; // Generate a one time pad (OTP) which the server XORs the API key with // in its response to protect against credentials intercept -export const generateOtp = async () => { +export const generateOtp = async (): Promise => { if (Platform.OS === 'android') { return new Promise((resolve, reject) => { NativeModules.RNSecureRandom.randomBase64(32, (err, result) => { From 6907fe7a3efa48145c424a325263274df2e8d7c8 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 30 Jun 2020 11:05:25 -0700 Subject: [PATCH 02/16] ZulipButton types: Let `text` be LocalizableText, not just string. Now we can use `values` in the text we pass in. The only thing ZulipButton currently does with `text` is pass it to TranslatedText, which already accepts LocalizableText. --- src/common/ZulipButton.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/ZulipButton.js b/src/common/ZulipButton.js index f3bd729644d..45a76313c35 100644 --- a/src/common/ZulipButton.js +++ b/src/common/ZulipButton.js @@ -4,6 +4,7 @@ import { StyleSheet, Text, View, ActivityIndicator } from 'react-native'; import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; import TranslatedText from './TranslatedText'; +import type { LocalizableText } from '../types'; import type { SpecificIconType } from './Icons'; import { BRAND_COLOR } from '../styles'; import Touchable from './Touchable'; @@ -64,7 +65,7 @@ type Props = $ReadOnly<{| progress: boolean, disabled: boolean, Icon?: SpecificIconType, - text: string, + text: LocalizableText, secondary: boolean, onPress: () => void | Promise, |}>; From 8db5a4da4ccae370326c5de0c26ed12e96a0ccff Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 30 Jun 2020 11:08:17 -0700 Subject: [PATCH 03/16] AuthScreen: Translate "Log in with..." button for all auth methods. Before this, it wouldn't work when `auth.displayName` is something other than the values we know about in advance. So, make it a fixed string with a placeholder and pass the value of `auth.displayName` in through `values`. --- src/start/AuthScreen.js | 5 ++++- static/translations/messages_en.json | 6 +----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/start/AuthScreen.js b/src/start/AuthScreen.js index 99accf9d42f..7dffde00d60 100644 --- a/src/start/AuthScreen.js +++ b/src/start/AuthScreen.js @@ -256,7 +256,10 @@ class AuthScreen extends PureComponent { key={auth.name} style={styles.halfMarginTop} secondary - text={`Log in with ${auth.displayName}`} + text={{ + text: 'Log in with {method}', + values: { method: auth.displayName }, + }} Icon={auth.Icon} onPress={() => this.handleAuth(auth)} /> diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index caf775bb032..8eafe8278d5 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -34,11 +34,7 @@ "Password": "Password", "Why not start the conversation?": "Why not start the conversation?", "Chat": "Chat", - "Log in with Google": "Log in with Google", - "Log in with GitHub": "Log in with GitHub", - "Log in with GitLab": "Log in with GitLab", - "Log in with password": "Log in with password", - "Log in with dev account": "Log in with dev account", + "Log in with {method}": "Log in with {method}", "Cannot connect to server": "Cannot connect to server", "Wrong email or password. Try again.": "Wrong email or password. Try again.", "Wrong username or password. Try again.": "Wrong username or password. Try again.", From 98a626a609a984011cbde6dfb2bd18c47886269e Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 7 May 2020 14:01:32 -0700 Subject: [PATCH 04/16] button text: Change "Log in with" to "Sign in with" everywhere. Apple's Human Interface Guidelines doc on the "Sign in with Apple" button [1] (to be added in an upcoming commit) is quite clear that, for logging in, the text must say "Sign in with Apple", with that exact capitalization and wording. In order to keep the buttons consistent, change the others to match. [1]: https://developer.apple.com/design/human-interface-guidelines/sign-in-with-apple/overview/buttons/. --- docs/howto/dev-server.md | 2 +- src/start/AuthScreen.js | 2 +- static/translations/messages_en.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/howto/dev-server.md b/docs/howto/dev-server.md index 3eff7657138..a4c61490703 100644 --- a/docs/howto/dev-server.md +++ b/docs/howto/dev-server.md @@ -202,7 +202,7 @@ This will be `http://ADDRESS:9991`, where `ADDRESS` is the address you identified in step 2. (Be sure to type the `http://`.) This should get you the login screen! Unless you're working on the login -flow itself, tap "Log in with dev account"; then pick any user to log in as. +flow itself, tap "Sign in with dev account"; then pick any user to log in as. If you need to work more closely with authentication systems, or if you need to use the [Zulip REST API][rest-api], which requires an API key, this diff --git a/src/start/AuthScreen.js b/src/start/AuthScreen.js index 7dffde00d60..a6f7a51a256 100644 --- a/src/start/AuthScreen.js +++ b/src/start/AuthScreen.js @@ -257,7 +257,7 @@ class AuthScreen extends PureComponent { style={styles.halfMarginTop} secondary text={{ - text: 'Log in with {method}', + text: 'Sign in with {method}', values: { method: auth.displayName }, }} Icon={auth.Icon} diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index 8eafe8278d5..5bdfd7efd90 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -34,7 +34,7 @@ "Password": "Password", "Why not start the conversation?": "Why not start the conversation?", "Chat": "Chat", - "Log in with {method}": "Log in with {method}", + "Sign in with {method}": "Sign in with {method}", "Cannot connect to server": "Cannot connect to server", "Wrong email or password. Try again.": "Wrong email or password. Try again.", "Wrong username or password. Try again.": "Wrong username or password. Try again.", From 6e0d3bff49969e28d733f56b1e323b3b9f0d10e2 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 30 Jun 2020 12:23:16 -0700 Subject: [PATCH 05/16] jest: Add `@unimodules/` to `transformModulesWhitelist`. Empirically (when running Jest), there is some uncompiled JavaScript in this directory; we see errors with stack traces that suggest that when we use Expo modules, e.g., `expo-apple-authentication`. Mocking those Expo modules seems to be also necessary, sometimes, as we still get errors about things being undefined, after we've avoided the syntax errors. --- jest.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/jest.config.js b/jest.config.js index 22810f3ba57..55f3b8b07f2 100644 --- a/jest.config.js +++ b/jest.config.js @@ -10,6 +10,7 @@ const transformModulesWhitelist = [ '@expo/react-native-action-sheet', 'react-navigation', '@sentry/react-native', + '@unimodules/', '@zulip/', ]; From c7d982e17b8904f93134feeb76a8c64c9a068fed Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 3 Apr 2020 12:23:10 -0700 Subject: [PATCH 06/16] iOS deps: Add expo-apple-authentication at latest. Following their instructions for a "bare" app (as opposed to "managed" by Expo) [1], which include the addition of the "Sign in with Apple" entitlement, which we did through the Xcode UI as instructed. We don't have to worry about excluding this package from being linked on Android, it seems; its `unimodule.json` specifies `"platforms": ["ios"]`. Empirically (when running Jest), there seems to be uncompiled JavaScript in this package, so add an entry in `transformModulesWhitelist`. Also mock the module, as we then get errors about things being undefined. [1]: https://github.com/expo/expo/tree/1c5eb9818/packages/expo-apple-authentication --- docs/howto/libdefs.md | 40 +++ .../expo-apple-authentication_vx.x.x.js | 299 ++++++++++++++++++ ios/Podfile.lock | 7 + ios/ZulipMobile/ZulipMobile.entitlements | 4 + jest.config.js | 1 + jest/jestSetup.js | 7 + package.json | 1 + yarn.lock | 5 + 8 files changed, 364 insertions(+) create mode 100644 flow-typed/expo-apple-authentication_vx.x.x.js diff --git a/docs/howto/libdefs.md b/docs/howto/libdefs.md index 5faa90da019..871d0c8e924 100644 --- a/docs/howto/libdefs.md +++ b/docs/howto/libdefs.md @@ -152,3 +152,43 @@ Flow and FlowTyped about not being able to import third-party types into one's own libdefs that haven't been resolved. [9] [9]: https://github.com/zulip/zulip-mobile/issues/3458#issuecomment-639859987 + +## Expo packages (made available through Unimodules) + +We're starting to see a pattern developing with these, e.g.: + +- `expo-apple-authentication` +- `expo-screen-orientation` + +Namely: + +1. See what `node_modules/expo-name-of-package/build/index.d.ts` + depends on; it's probably at least `'./NameOfPackage'` and + `'./NameOfPackage.types'`. + + Assuming so, make a `declare module expo-name-of-package` block and + have it do what that `index.d.ts` does, maybe + + ```javascript + declare module 'expo-name-of-package' { + declare export * from 'expo-name-of-package/build/NameOfPackage' + declare export * from 'expo-name-of-package/build/NameOfPackage.types' + } + ``` + +2. Run `node_modules/expo-name-of-package/build/NameOfPackage.d.ts` + through Flowgen and paste the output into a + `declare module 'expo-name-of-package/build/NameOfPackage'` + block. +2. Run `node_modules/expo-name-of-package/build/PackageName.types'` + through Flowgen and paste the output into a + `declare module 'expo-screen-orientation/build/ScreenOrientation.types'` + block. +3. Make any necessary syntactic fixes based on error messages (in + particular, replacing `export` with `declare export` everywhere may + be necessary) or adjustments to imports. You may only import from + something that's been declared in that same file, with + `declare export` [1] [2]. + +[1]: https://github.com/flow-typed/flow-typed/blob/master/CONTRIBUTING.md#dont-import-types-from-other-libdefs +[2]: See discussion around https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/libdef.3A.20react-native-webview/near/896713. diff --git a/flow-typed/expo-apple-authentication_vx.x.x.js b/flow-typed/expo-apple-authentication_vx.x.x.js new file mode 100644 index 00000000000..e43f9ee088c --- /dev/null +++ b/flow-typed/expo-apple-authentication_vx.x.x.js @@ -0,0 +1,299 @@ +// Assembled with help from Flowgen v1.10.0. +// +// The modules 'expo-apple-authentication/build/AppleAuthentication', +// 'expo-apple-authentication/build/AppleAuthenticationButton', and +// 'expo-apple-authentication/build/AppleAuthentication.types' are the +// result of passing those files in node_modules through Flowgen and +// doing some minor syntactic tweaks. + +declare module 'expo-apple-authentication/build/AppleAuthentication' { + import type { + AppleAuthenticationSignInOptions, + AppleAuthenticationRefreshOptions, + AppleAuthenticationSignOutOptions, + AppleAuthenticationCredential, + AppleAuthenticationRevokeListener, + } from 'expo-apple-authentication/build/AppleAuthentication.types'; + import typeof { AppleAuthenticationCredentialState } from 'expo-apple-authentication/build/AppleAuthentication.types'; + + declare type Subscription = { + remove: () => void, + }; + + declare export function isAvailableAsync(): Promise; + declare export function signInAsync( + options?: AppleAuthenticationSignInOptions, + ): Promise; + declare export function refreshAsync( + options: AppleAuthenticationRefreshOptions, + ): Promise; + declare export function signOutAsync( + options: AppleAuthenticationSignOutOptions, + ): Promise; + declare export function getCredentialStateAsync( + user: string, + ): Promise; + declare export function addRevokeListener( + listener: AppleAuthenticationRevokeListener, + ): Subscription; +} + +declare module 'expo-apple-authentication/build/AppleAuthentication.types' { + declare export type AppleAuthenticationButtonProps = { + onPress: () => void, + buttonType: $Values, + buttonStyle: $Values, + cornerRadius?: number, + style?: mixed, + ... + }; + /** + * The options you can supply when making a call to `AppleAuthentication.signInAsync()`. None of + * these options are required. + * @see [Apple + * Documentation](https://developer.apple.com/documentation/authenticationservices/asauthorizationopenidrequest) + * for more details. + */ + declare export type AppleAuthenticationSignInOptions = { + /** + * The scope of personal information to which your app is requesting access. The user can choose + * to deny your app access to any scope at the time of logging in. + * @defaults `[]` (no scopes). + */ + requestedScopes?: $Values[], + + /** + * Data that's returned to you unmodified in the corresponding credential after a successful + * authentication. Used to verify that the response was from the request you made. Can be used to + * avoid replay attacks. + */ + state?: string, + + /** + * Data that is used to verify the uniqueness of a response and prevent replay attacks. + */ + nonce?: string, + ... + }; + /** + * The options you can supply when making a call to `AppleAuthentication.refreshAsync()`. You must + * include the ID string of the user whose credentials you'd like to refresh. + * @see [Apple + * Documentation](https://developer.apple.com/documentation/authenticationservices/asauthorizationopenidrequest) + * for more details. + */ + declare export type AppleAuthenticationRefreshOptions = { + user: string, + + /** + * The scope of personal information to which your app is requesting access. The user can choose + * to deny your app access to any scope at the time of refreshing. + * @defaults `[]` (no scopes). + */ + requestedScopes?: $Values[], + + /** + * Data that's returned to you unmodified in the corresponding credential after a successful + * authentication. Used to verify that the response was from the request you made. Can be used to + * avoid replay attacks. + */ + state?: string, + ... + }; + /** + * The options you can supply when making a call to `AppleAuthentication.signOutAsync()`. You must + * include the ID string of the user to sign out. + * @see [Apple + * Documentation](https://developer.apple.com/documentation/authenticationservices/asauthorizationopenidrequest) + * for more details. + */ + declare export type AppleAuthenticationSignOutOptions = { + user: string, + + /** + * Data that's returned to you unmodified in the corresponding credential after a successful + * authentication. Used to verify that the response was from the request you made. Can be used to + * avoid replay attacks. + */ + state?: string, + ... + }; + /** + * The user credentials returned from a successful call to `AppleAuthentication.signInAsync()`, + * `AppleAuthentication.refreshAsync()`, or `AppleAuthentication.signOutAsync()`. + * @see [Apple + * Documentation](https://developer.apple.com/documentation/authenticationservices/asauthorizationappleidcredential) + * for more details. + */ + declare export type AppleAuthenticationCredential = { + /** + * An identifier associated with the authenticated user. You can use this to check if the user is + * still authenticated later. This is stable and can be shared across apps released under the same + * development team. The same user will have a different identifier for apps released by other + * developers. + */ + user: string, + + /** + * An arbitrary string that your app provided as `state` in the request that generated the + * credential. Used to verify that the response was from the request you made. Can be used to + * avoid replay attacks. + */ + state: string | null, + + /** + * The user's name. May be `null` or contain `null` values if you didn't request the `FULL_NAME` + * scope, if the user denied access, or if this is not the first time the user has signed into + * your app. + */ + fullName: AppleAuthenticationFullName | null, + + /** + * The user's email address. Might not be present if you didn't request the `EMAIL` scope. May + * also be null if this is not the first time the user has signed into your app. If the user chose + * to withhold their email address, this field will instead contain an obscured email address with + * an Apple domain. + */ + email: string | null, + + /** + * A value that indicates whether the user appears to the system to be a real person. + */ + realUserStatus: $Values, + + /** + * A JSON Web Token (JWT) that securely communicates information about the user to your app. + */ + identityToken: string | null, + + /** + * A short-lived session token used by your app for proof of authorization when interacting with + * the app's server counterpart. Unlike `user`, this is ephemeral and will change each session. + */ + authorizationCode: string | null, + ... + }; + /** + * An object representing the tokenized portions of the user's full name. + */ + declare export type AppleAuthenticationFullName = { + namePrefix: string | null, + givenName: string | null, + middleName: string | null, + familyName: string | null, + nameSuffix: string | null, + nickname: string | null, + ... + }; + declare export type AppleAuthenticationRevokeListener = () => void; + /** + * Scopes you can request when calling `AppleAuthentication.signInAsync()` or + * `AppleAuthentication.refreshAsync()`. + * @note Note that it is possible that you will not be granted all of the scopes which you request. + * You will still need to handle null values for any fields you request. + * @see [Apple + * Documentation](https://developer.apple.com/documentation/authenticationservices/asauthorizationscope) + * for more details. + */ + + declare export var AppleAuthenticationScope: {| + +FULL_NAME: 0, // 0 + +EMAIL: 1, // 1 + |}; + + declare export var AppleAuthenticationOperation: {| + +IMPLICIT: 0, // 0 + +LOGIN: 1, // 1 + +REFRESH: 2, // 2 + +LOGOUT: 3, // 3 + |}; + + /** + * The state of the credential when checked with `AppleAuthentication.getCredentialStateAsync()`. + * @see [Apple + * Documentation](https://developer.apple.com/documentation/authenticationservices/asauthorizationappleidprovidercredentialstate) + * for more details. + */ + + declare export var AppleAuthenticationCredentialState: {| + +REVOKED: 0, // 0 + +AUTHORIZED: 1, // 1 + +NOT_FOUND: 2, // 2 + +TRANSFERRED: 3, // 3 + |}; + + /** + * A value that indicates whether the user appears to be a real person. You get this in the + * realUserStatus property of a `Credential` object. It can be used as one metric to help prevent + * fraud. + * @see [Apple + * Documentation](https://developer.apple.com/documentation/authenticationservices/asuserdetectionstatus) + * for more details. + */ + + declare export var AppleAuthenticationUserDetectionStatus: {| + +UNSUPPORTED: 0, // 0 + +UNKNOWN: 1, // 1 + +LIKELY_REAL: 2, // 2 + |}; + + /** + * Controls the predefined text shown on the authentication button. + */ + + declare export var AppleAuthenticationButtonType: {| + +SIGN_IN: 0, // 0 + +CONTINUE: 1, // 1 + |}; + + /** + * Controls the predefined style of the authenticating button. + */ + + declare export var AppleAuthenticationButtonStyle: {| + +WHITE: 0, // 0 + +WHITE_OUTLINE: 1, // 1 + +BLACK: 2, // 2 + |}; +} + +declare module 'expo-apple-authentication/build/AppleAuthenticationButton' { + import type { StatelessFunctionalComponent } from 'react'; + /* eslint-disable-next-line */ + import type { AppleAuthenticationButtonProps } from 'expo-apple-authentication/build/AppleAuthentication.types'; + + /** + * This component displays the proprietary "Sign In with Apple" / "Continue with Apple" button on + * your screen. The App Store Guidelines require you to use this component to start the sign in + * process instead of a custom button. You can customize the design of the button using the + * properties. You should start the sign in process when the `onPress` property is called. + * + * You should only attempt to render this if `AppleAuthentication.isAvailableAsync()` resolves to + * true. This component will render nothing if it is not available and you will get a warning if + * `__DEV__ === true`. + * + * The properties of this component extend from `View`; however, you should not attempt to set + * `backgroundColor` or `borderRadius` with the `style` property. This will not work and is against + * the App Store Guidelines. Instead, you should use the `buttonStyle` property to choose one of the + * predefined color styles and the `cornerRadius` property to change the border radius of the + * button. + * @see [Apple + * Documentation](https://developer.apple.com/documentation/authenticationservices/asauthorizationappleidbutton) + * for more details. + */ + declare type AppleAuthenticationButton = StatelessFunctionalComponent; + declare export default AppleAuthenticationButton; +} + +/* + * Flowtype definitions for AppleAuthenticationButton + * Generated by Flowgen from a Typescript Definition + * Flowgen v1.10.0 + */ +declare module 'expo-apple-authentication' { + declare export * from 'expo-apple-authentication/build/AppleAuthentication' + declare export * from 'expo-apple-authentication/build/AppleAuthentication.types' + declare export { + default as AppleAuthenticationButton, + } from 'expo-apple-authentication/build/AppleAuthenticationButton'; +} diff --git a/ios/Podfile.lock b/ios/Podfile.lock index a0f456b9368..46eb62229d5 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -1,6 +1,8 @@ PODS: - boost-for-react-native (1.63.0) - DoubleConversion (1.1.6) + - EXAppleAuthentication (2.1.1): + - UMCore - EXApplication (2.1.1): - UMCore - EXAppLoaderProvider (7.0.0) @@ -154,6 +156,7 @@ PODS: DEPENDENCIES: - DoubleConversion (from `../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec`) + - EXAppleAuthentication (from `../node_modules/expo-apple-authentication/ios`) - EXApplication (from `../node_modules/expo-application/ios`) - EXAppLoaderProvider (from `../node_modules/expo-app-loader-provider/ios`) - EXConstants (from `../node_modules/expo-constants/ios`) @@ -218,6 +221,9 @@ SPEC REPOS: EXTERNAL SOURCES: DoubleConversion: :podspec: "../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec" + EXAppleAuthentication: + :path: !ruby/object:Pathname + path: "../node_modules/expo-apple-authentication/ios" EXApplication: :path: !ruby/object:Pathname path: "../node_modules/expo-application/ios" @@ -348,6 +354,7 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: boost-for-react-native: 39c7adb57c4e60d6c5479dd8623128eb5b3f0f2c DoubleConversion: 5805e889d232975c086db112ece9ed034df7a0b2 + EXAppleAuthentication: 046c76335343eaa97f6ed8d35a9cf493a2c4d351 EXApplication: 7cf81de6fafccff42f5d1caa5c24a159db6b9437 EXAppLoaderProvider: 5d348813a9cf09b03bbe5b8b55437bc1bfbddbd1 EXConstants: 857aa7b1c84e2878f8402d712061860bca16a697 diff --git a/ios/ZulipMobile/ZulipMobile.entitlements b/ios/ZulipMobile/ZulipMobile.entitlements index 903def2af53..80b5221de76 100644 --- a/ios/ZulipMobile/ZulipMobile.entitlements +++ b/ios/ZulipMobile/ZulipMobile.entitlements @@ -4,5 +4,9 @@ aps-environment development + com.apple.developer.applesignin + + Default + diff --git a/jest.config.js b/jest.config.js index 55f3b8b07f2..7b16fdf9eb1 100644 --- a/jest.config.js +++ b/jest.config.js @@ -3,6 +3,7 @@ // // These will be used as regexp fragments. const transformModulesWhitelist = [ + 'expo-apple-authentication', 'react-native', // @rnc/async-storage itself is precompiled, but its mock-helper is not '@react-native-community/async-storage', diff --git a/jest/jestSetup.js b/jest/jestSetup.js index 86e85e62435..97f1a29db41 100644 --- a/jest/jestSetup.js +++ b/jest/jestSetup.js @@ -36,3 +36,10 @@ jest.mock('react-native-device-info', () => ({ getSystemName: jest.fn().mockReturnValue('ios'), getSystemVersion: jest.fn().mockReturnValue('13.3.1'), })); + +jest.mock('expo-apple-authentication', () => ({ + AppleAuthenticationButton: jest.fn(), + isAvailableAsync: jest.fn(), + signInAsync: jest.fn(), + // etc. (incomplete) +})); diff --git a/package.json b/package.json index 83721483d11..e8e852bbf9c 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "immutable": "^4.0.0-rc.12", "json-stringify-safe": "^5.0.1", "katex": "^0.11.1", + "expo-apple-authentication": "^2.1.1", "lodash.escape": "^4.0.1", "lodash.isequal": "^4.4.0", "lodash.isplainobject": "^4.0.6", diff --git a/yarn.lock b/yarn.lock index 2af07960f69..52f138bd875 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3709,6 +3709,11 @@ expo-app-loader-provider@~7.0.0: resolved "https://registry.yarnpkg.com/expo-app-loader-provider/-/expo-app-loader-provider-7.0.0.tgz#9bfff831a204d0a8896e0120bce2209c4304ef03" integrity sha512-C+5zpZN2T7PCj7weLs/ZgAC+y9dvu0VdTXD00Jf9Wo7Pxu/lsLh6ljg9JL91c+2tYDzMEODPNmT+JOUIxAr5zQ== +expo-apple-authentication@^2.1.1: + version "2.1.1" + resolved "https://registry.yarnpkg.com/expo-apple-authentication/-/expo-apple-authentication-2.1.1.tgz#ecba053fb6ae688f6a83553a192403e968eef931" + integrity sha512-E3k0Poo53N3pXimRVYpmxXhF+0PutQWYlQpfNFzRzZX+TjsHVehrTKmtd1KOimFpIs+u39MhsDQIIoMas12/dw== + expo-application@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/expo-application/-/expo-application-2.1.1.tgz#7cd61ccc53d7a0c85c8aac64b5ea59b20a1cb803" From 1c638f49b71df58c6a59a22b71b152617b58e9d3 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 Apr 2020 17:44:46 -0700 Subject: [PATCH 07/16] apple-auth libdef: Allow onPress to return Promise The example in the doc at https://docs.expo.io/versions/latest/sdk/apple-authentication/#usage has this return type for `onPress`, and we want it too. Omitting `Promise` doesn't seem to be well-considered. --- flow-typed/expo-apple-authentication_vx.x.x.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flow-typed/expo-apple-authentication_vx.x.x.js b/flow-typed/expo-apple-authentication_vx.x.x.js index e43f9ee088c..ef8d5c659bd 100644 --- a/flow-typed/expo-apple-authentication_vx.x.x.js +++ b/flow-typed/expo-apple-authentication_vx.x.x.js @@ -40,7 +40,7 @@ declare module 'expo-apple-authentication/build/AppleAuthentication' { declare module 'expo-apple-authentication/build/AppleAuthentication.types' { declare export type AppleAuthenticationButtonProps = { - onPress: () => void, + onPress: () => void | Promise, buttonType: $Values, buttonStyle: $Values, cornerRadius?: number, From 8d056ed6392c6fb3e6ba2d96f73f4024cd0aaa52 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 22 May 2020 11:48:58 -0700 Subject: [PATCH 08/16] expo-apple-auth libdef: Make identityToken, authorizationCode non-optional The Apple docs are frustratingly vague, but it doesn't seem to be the case that "no credential" and "yes credential, but no `identityToken`/`authorizationCode`" are separate auth failure modes that we have to distinguish. As I theorize in an Expo issue, these fields being marked optional does correspond to the Swift version of an Apple doc [1], where they're also marked optional. We still don't know exactly why that Apple doc has them marked optional, but corresponding values in a different protocol, the one you use on other platforms (i.e., not the native iOS flow), *are* marked optional for a particular reason [2]. In particular, they might be missing if you said you didn't want them. But...here, there doesn't seem to be a way to say you don't want them. So, they must be there. Also, in the JavaScript layer of `expo-apple-authentication`, an error is thrown if `identityToken` or `authorizationCode` is missing. (See `signInAsync` in `build/AppleAuthentication.js`.) [1]: https://github.com/expo/expo/issues/8446#issuecomment-634997122 [2]: https://github.com/expo/expo/issues/8446#issuecomment-635000576 --- flow-typed/expo-apple-authentication_vx.x.x.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flow-typed/expo-apple-authentication_vx.x.x.js b/flow-typed/expo-apple-authentication_vx.x.x.js index ef8d5c659bd..b72b1cc72ba 100644 --- a/flow-typed/expo-apple-authentication_vx.x.x.js +++ b/flow-typed/expo-apple-authentication_vx.x.x.js @@ -164,13 +164,13 @@ declare module 'expo-apple-authentication/build/AppleAuthentication.types' { /** * A JSON Web Token (JWT) that securely communicates information about the user to your app. */ - identityToken: string | null, + identityToken: string, /** * A short-lived session token used by your app for proof of authorization when interacting with * the app's server counterpart. Unlike `user`, this is ephemeral and will change each session. */ - authorizationCode: string | null, + authorizationCode: string, ... }; /** From cc8b569f8bfcbc7d5a91267e4fa581f5ea3fc43f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 Apr 2020 17:20:24 -0700 Subject: [PATCH 09/16] auth ui: Add iOS-compliant "Sign in with Apple" button component. In Apple's Human Interface Guidelines doc about the button to be used for "Sign in with Apple", they recommend using a standard component from the iOS SDK if possible, but that you can use a custom button, subject to design constraints, otherwise. That doc is https://developer.apple.com/design/human-interface-guidelines/sign-in-with-apple/overview/buttons/. So, make a component, only to be used on iOS, that will check if the standard button is available, and use it, if so. The fallback custom button will show on iOS <13. Since Apple doesn't review our Google Play Store submissions, we shouldn't even have to use the custom button on Android; we can just use the same ZulipButton as the other social auth buttons use. I think it looks better when they all look uniform like this. --- .../IosCompliantAppleAuthButton/Custom.js | 80 ++++++++++++++++++ .../IosCompliantAppleAuthButton/index.js | 75 ++++++++++++++++ static/img/apple-logo-black.png | Bin 0 -> 846 bytes static/img/apple-logo-white.png | Bin 0 -> 813 bytes 4 files changed, 155 insertions(+) create mode 100644 src/start/IosCompliantAppleAuthButton/Custom.js create mode 100644 src/start/IosCompliantAppleAuthButton/index.js create mode 100644 static/img/apple-logo-black.png create mode 100644 static/img/apple-logo-white.png diff --git a/src/start/IosCompliantAppleAuthButton/Custom.js b/src/start/IosCompliantAppleAuthButton/Custom.js new file mode 100644 index 00000000000..937ac48ae48 --- /dev/null +++ b/src/start/IosCompliantAppleAuthButton/Custom.js @@ -0,0 +1,80 @@ +/* @flow strict-local */ +import React, { PureComponent } from 'react'; +import { StyleSheet, Text, View, Image, TouchableWithoutFeedback } from 'react-native'; +import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; + +import type { ThemeName } from '../../reduxTypes'; +import TranslatedText from '../../common/TranslatedText'; +import appleLogoBlackImg from '../../../static/img/apple-logo-black.png'; +import appleLogoWhiteImg from '../../../static/img/apple-logo-white.png'; + +const styles = StyleSheet.create({ + buttonContent: { + flexDirection: 'row', + alignItems: 'center', + justifyContent: 'center', + height: 44, + }, + frame: { + height: 44, + justifyContent: 'center', + borderRadius: 22, + overflow: 'hidden', + }, + nightFrame: { + backgroundColor: 'black', + }, + dayFrame: { + backgroundColor: 'white', + borderWidth: 1.5, + borderColor: 'black', + }, + text: { + fontSize: 16, + }, + nightText: { + color: 'white', + }, + dayText: { + color: 'black', + }, +}); + +type Props = $ReadOnly<{| + style?: ViewStyleProp, + onPress: () => void | Promise, + theme: ThemeName, +|}>; + +/** + * The custom "Sign in with Apple" button that follows the rules. + * + * Do not reuse this component; it is only meant to be rendered by the + * IosCompliantAppleAuthButton, which controls whether the custom + * button should be used. + */ +export default class Custom extends PureComponent { + render() { + const { style, onPress, theme } = this.props; + const logoSource = theme === 'default' ? appleLogoBlackImg : appleLogoWhiteImg; + const frameStyle = [ + styles.frame, + theme === 'default' ? styles.dayFrame : styles.nightFrame, + style, + ]; + const textStyle = [styles.text, theme === 'default' ? styles.dayText : styles.nightText]; + + return ( + + + + + + + + + + + ); + } +} diff --git a/src/start/IosCompliantAppleAuthButton/index.js b/src/start/IosCompliantAppleAuthButton/index.js new file mode 100644 index 00000000000..653028184e3 --- /dev/null +++ b/src/start/IosCompliantAppleAuthButton/index.js @@ -0,0 +1,75 @@ +/* @flow strict-local */ +import React, { PureComponent } from 'react'; +import { View } from 'react-native'; +import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; +import * as AppleAuthentication from 'expo-apple-authentication'; +import { connect } from '../../react-redux'; + +import Custom from './Custom'; +import type { ThemeName } from '../../reduxTypes'; +import type { Dispatch } from '../../types'; +import { getSettings } from '../../selectors'; + +type SelectorProps = $ReadOnly<{| + theme: ThemeName, +|}>; + +type Props = $ReadOnly<{| + style?: ViewStyleProp, + onPress: () => void | Promise, + + dispatch: Dispatch, + ...SelectorProps, +|}>; + +type State = $ReadOnly<{| + isNativeButtonAvailable: boolean | void, +|}>; + +/** + * A "Sign in with Apple" button (iOS only) that follows the rules. + * + * These official guidelines from Apple are at + * https://developer.apple.com/design/human-interface-guidelines/sign-in-with-apple/overview/buttons/. + * + * Not to be used on Android. There, we also offer "Sign in with + * Apple", but without marking it with a different style from the + * other buttons. + */ +class IosCompliantAppleAuthButton extends PureComponent { + state = { + isNativeButtonAvailable: undefined, + }; + + async componentDidMount() { + this.setState({ isNativeButtonAvailable: await AppleAuthentication.isAvailableAsync() }); + } + + render() { + const { style, onPress, theme } = this.props; + const { isNativeButtonAvailable } = this.state; + if (isNativeButtonAvailable === undefined) { + return ; + } else if (isNativeButtonAvailable) { + return ( + + ); + } else { + return ; + } + } +} + +export default connect(state => ({ + theme: getSettings(state).theme, +}))(IosCompliantAppleAuthButton); diff --git a/static/img/apple-logo-black.png b/static/img/apple-logo-black.png new file mode 100644 index 0000000000000000000000000000000000000000..b099599b934c18d357b06060d9b249914fbb4a79 GIT binary patch literal 846 zcmeAS@N?(olHy`uVBq!ia0vp^>Oic+!3HFsr7Zjgq!^2X+?^QKos)S9===tRHP6h_19iA?Z zArYK!r+a%>If@+UU+)n0Md7kpkV1!Umhui^(IdTjF0rh-zgX(J&FWeYEEU+IQ7~O$ zSAYP^!BCA3{*r}17PPE*W?%eYrPB2M%$=;d?{fASKmR)0`1!n@uQ+5sxVQi7FY=6# zn)Bji#>~q`$0s-T_w%24{`o@Q_R2HUO6r$B%i1cnBS!DT&zcJvCexUTd?SSaot$|3 zX~)f+wBETdzE&B{^l7=UcK5h>c>*~*3)U5xi?(C9Bd@P z^Fbxsq<+gYt*KlRy>2`9>}d&F$`%;V~Ee~%mHi_AM zGiD11&j~BpTi;LQnEe)W({nhiD#RHYnY`3MhVSV|^K*d(>({SOsjiL|z4P_!SGNhJ zP9fTvTcbYw`Qzj7Ra0RjmzOMZC&T1XnU47a^Y$9g!oH^4H*W@(ELn2lWyzF~uqPT8 zv(IizIXg$!S)e#^lGg`-J=l5h7*X%*+3uG>Gq{&~F4z5DkYV-x%P z`{TnzkW?Ds6l~b#$y!Pj3C03!#yYA+RNCZD>TI{-3>fOJ8|H>cC zxSn93!6U)felfvkqIU6_1=n8-uD*JTx1yxP<=0=k;zqeU1!s)^o>2My`?q()@#Du; zJ;85z&q{SuP9!bD#FEj zPx#L}ge>W=%~1DgXcg2mk?xX#fNO00031000^Q000001E2u_0{{R30RRC20H6W@ z1ONa40RR91C!hlW1ONa40RR91EC2ui07>NYxc~qHmq|oHR9Fe^mc431F%-v-S_eN6 zaj+CyL~s*CP*5nvugSqdu((QJ!Ns@m30wphD+oI2pwLZRNBOmrf`rQ7Xt z`!d0i<=`Ipufupe{@Zl)Hxt}qu{b)=F@x*t>tiMd{Y-F^$>hM1x8eT&p4UaPXWFR4 z;ZV^Zf*fmjdwa_ii%>~+Ov=$IkQ^n=MD(T8>3|+#@pznT2&UP@VlhR^$lr7!cjoK-;dMj^cNsG0r2th5v$cIzP-Ku>Z6nK`Fy-!D0CC1ayuLj z$Y!%(G#a7VY{Jvi6K$4?Mx&8Z#@*c=kV__%%Vi+961khX7KUSvpumth2`h*p?5e_v rN~bEPDkrLdsGqv$ROLh!5cT^8nZgj)42w&|00000NkvXXu0mjfH&JHW literal 0 HcmV?d00001 From 6a9db46d107a3047ca934ee1a47ca67a0eea6a8c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 21 May 2020 11:37:53 -0700 Subject: [PATCH 10/16] webAuth [nfc]: Extract implementation into `generateRandomToken` function. In an upcoming commit, we'll need to generate a random token (the same implementation is fine), but without the OTP semantics. --- src/start/webAuth.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/start/webAuth.js b/src/start/webAuth.js index 5c07318e6e9..8e043a9f687 100644 --- a/src/start/webAuth.js +++ b/src/start/webAuth.js @@ -27,9 +27,7 @@ import { base64ToHex, hexToAscii, xorHexStrings } from '../utils/encoding'; https://chat.zulip.org/#narrow/stream/16-desktop/topic/desktop.20app.20OAuth/near/803919 */ -// Generate a one time pad (OTP) which the server XORs the API key with -// in its response to protect against credentials intercept -export const generateOtp = async (): Promise => { +export const generateRandomToken = async (): Promise => { if (Platform.OS === 'android') { return new Promise((resolve, reject) => { NativeModules.RNSecureRandom.randomBase64(32, (err, result) => { @@ -45,6 +43,10 @@ export const generateOtp = async (): Promise => { } }; +// Generate a one time pad (OTP) which the server XORs the API key with +// in its response to protect against credentials intercept +export const generateOtp = async (): Promise => generateRandomToken(); + export const openBrowser = (url: string, otp: string) => { openLink(`${url}?mobile_flow_otp=${otp}`); }; From f9005213943bc1c06af724326b773758a948810d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 29 Jun 2020 14:52:07 -0700 Subject: [PATCH 11/16] config: Add `appOwnDomains`. These will be useful, in an upcoming commit, to grant the privilege of the native "Sign in with Apple" flow (in which sensitive `id_token`s will be sent to the server) only to domains that have trust with the mobile app. To do that securely, the base URL we'll be sending requests to (basically what the user entered on the first screen) will be checked to see if its host matches one of these. --- src/config.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/config.js b/src/config.js index 1a893b21223..ccc0610c6e9 100644 --- a/src/config.js +++ b/src/config.js @@ -11,6 +11,7 @@ type Config = {| sentryKey: string | null, enableErrorConsoleLogging: boolean, serverDataOnStartup: string[], + appOwnDomains: string[], |}; const config: Config = { @@ -39,6 +40,7 @@ const config: Config = { 'update_message_flags', 'user_status', ], + appOwnDomains: ['zulip.com', 'zulipchat.com', 'chat.zulip.org'], }; export default config; From b72e9a33fb01c4cce13761d45b4936dfd9fdf52d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 10 Apr 2020 14:26:24 -0700 Subject: [PATCH 12/16] auth: Handle "Sign in with Apple". From our perspective, this works just like any other social auth in many cases. It's set up to conform to our protocol of giving a redirect to a URL with the `zulip://` scheme containing the API key. These cases (where the normal protocol is used) are - Android - iOS before version 13 - On servers not operated by the same people as the publishers of the mobile app (so for the official Zulip app, by Kandra Labs) [1] In the remaining cases (Kandra-hosted realms on iOS 13+), we'll do the native flow. This means we initiate the auth by using an iOS API that natively handles querying for, e.g., the user's fingerprint, face, or password, and gives us an `id_token`, which we send to the server. Currently, we do this by opening the browser and awaiting the same `zulip://` redirect, same as in the normal protocol. As a followup, we may want to tweak this so it's not necessary to ever open the browser in the native flow. We could just use `fetch` and expect the API key in the response. [1]: We don't want to send an `id_token` from the native flow to one of these realms; see discussion around https://chat.zulip.org/#narrow/stream/3-backend/topic/apple.20auth/near/918714. --- src/common/Icons.js | 1 + src/start/AuthScreen.js | 111 ++++++++++++++++++++++++++++++++++------ 2 files changed, 95 insertions(+), 17 deletions(-) diff --git a/src/common/Icons.js b/src/common/Icons.js index 5097e6ab34f..6b609595327 100644 --- a/src/common/Icons.js +++ b/src/common/Icons.js @@ -63,6 +63,7 @@ export const IconStream = makeIcon(Feather, 'hash'); export const IconPin = makeIcon(SimpleLineIcons, 'pin'); export const IconPrivate = makeIcon(Feather, 'lock'); export const IconPrivateChat = makeIcon(Feather, 'mail'); +export const IconApple = makeIcon(IoniconsIcon, 'logo-apple'); export const IconGoogle = makeIcon(IoniconsIcon, 'logo-google'); export const IconGitHub = makeIcon(Feather, 'github'); export const IconWindows = makeIcon(IoniconsIcon, 'logo-windows'); diff --git a/src/start/AuthScreen.js b/src/start/AuthScreen.js index a6f7a51a256..5d9b1484331 100644 --- a/src/start/AuthScreen.js +++ b/src/start/AuthScreen.js @@ -1,26 +1,38 @@ /* @flow strict-local */ import React, { PureComponent } from 'react'; -import { Linking } from 'react-native'; +import { Linking, Platform } from 'react-native'; import type { NavigationScreenProp } from 'react-navigation'; import { URL as WhatwgURL } from 'react-native-url-polyfill'; +import type { AppleAuthenticationCredential } from 'expo-apple-authentication'; +import * as AppleAuthentication from 'expo-apple-authentication'; +import config from '../config'; import type { AuthenticationMethods, Dispatch, ExternalAuthenticationMethod, ApiResponseServerSettings, } from '../types'; -import { IconPrivate, IconGoogle, IconGitHub, IconWindows, IconTerminal } from '../common/Icons'; +import { + IconApple, + IconPrivate, + IconGoogle, + IconGitHub, + IconWindows, + IconTerminal, +} from '../common/Icons'; import type { SpecificIconType } from '../common/Icons'; import { connect } from '../react-redux'; import styles from '../styles'; import { Centerer, Screen, ZulipButton } from '../common'; import { getCurrentRealm } from '../selectors'; import RealmInfo from './RealmInfo'; -import { getFullUrl } from '../utils/url'; +import { getFullUrl, encodeParamsForUrl } from '../utils/url'; import * as webAuth from './webAuth'; import { loginSuccess, navigateToDev, navigateToPassword } from '../actions'; +import IosCompliantAppleAuthButton from './IosCompliantAppleAuthButton'; +import openLink from '../utils/openLink'; /** * Describes a method for authenticating to the server. @@ -100,6 +112,7 @@ const externalMethodIcons = new Map([ ['google', IconGoogle], ['github', IconGitHub], ['azuread', IconWindows], + ['apple', IconApple], ]); /** Exported for tests only. */ @@ -227,12 +240,68 @@ class AuthScreen extends PureComponent { this.props.dispatch(navigateToPassword(serverSettings.require_email_format_usernames)); }; - handleAuth = (method: AuthenticationMethodDetails) => { + handleNativeAppleAuth = async () => { + const state = await webAuth.generateRandomToken(); + const credential: AppleAuthenticationCredential = await AppleAuthentication.signInAsync({ + state, + requestedScopes: [ + AppleAuthentication.AppleAuthenticationScope.FULL_NAME, + AppleAuthentication.AppleAuthenticationScope.EMAIL, + ], + }); + if (credential.state !== state) { + throw new Error('`state` mismatch'); + } + + otp = await webAuth.generateOtp(); + + const params = encodeParamsForUrl({ + mobile_flow_otp: otp, + native_flow: true, + id_token: credential.identityToken, + }); + + openLink(`${this.props.realm}/complete/apple/?${params}`); + + // Currently, the rest is handled with the `zulip://` redirect, + // same as in the web flow. + // + // TODO: Maybe have an endpoint we can just send a request to, + // with `fetch`, and get the API key right away, without ever + // having to open the browser. + }; + + canUseNativeAppleFlow = async () => { + if (Platform.OS === 'ios' && (await AppleAuthentication.isAvailableAsync())) { + let host: string | void; + try { + host = new WhatwgURL(this.props.realm).host; + } catch (e) { + // `this.props.realm` invalid. + // TODO: Check this much sooner. + } + + // Check that the realm we're actually sending requests to, + // which is basically the URL the user entered on the first + // screen, is trusted by the official mobile app. + const isTrusted = config.appOwnDomains.some( + domain => host !== undefined && (host === domain || host.endsWith(`.${domain}`)), + ); + return isTrusted; + } + + return false; + }; + + handleAuth = async (method: AuthenticationMethodDetails) => { const { action } = method; + if (action === 'dev') { this.handleDevAuth(); } else if (action === 'password') { this.handlePassword(); + } else if (method.name === 'apple' && (await this.canUseNativeAppleFlow())) { + this.handleNativeAppleAuth(); } else { this.beginWebAuth(action.url); } @@ -251,19 +320,27 @@ class AuthScreen extends PureComponent { {activeAuthentications( serverSettings.authentication_methods, serverSettings.external_authentication_methods, - ).map(auth => ( - this.handleAuth(auth)} - /> - ))} + ).map(auth => + auth.name === 'apple' && Platform.OS === 'ios' ? ( + this.handleAuth(auth)} + /> + ) : ( + this.handleAuth(auth)} + /> + ), + )} ); From c7b37e274a4c7365781367533382633bc3c1b641 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 30 Jun 2020 14:57:37 -0700 Subject: [PATCH 13/16] auth [nfc]: Expand comment on Apple-auth host check. --- src/start/AuthScreen.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/start/AuthScreen.js b/src/start/AuthScreen.js index 5d9b1484331..c96105fe92c 100644 --- a/src/start/AuthScreen.js +++ b/src/start/AuthScreen.js @@ -281,9 +281,13 @@ class AuthScreen extends PureComponent { // TODO: Check this much sooner. } - // Check that the realm we're actually sending requests to, - // which is basically the URL the user entered on the first - // screen, is trusted by the official mobile app. + // The native flow for Apple auth assumes that the app and the server + // are operated by the same organization, so that for a user to + // entrust private information to either one is the same as entrusting + // it to the other. Check that this realm is on such a server. + // + // (For other realms, we'll simply fall back to the web flow, which + // handles things appropriately without relying on that assumption.) const isTrusted = config.appOwnDomains.some( domain => host !== undefined && (host === domain || host.endsWith(`.${domain}`)), ); From c19cbe2f2c0bda9cbef89a9a4b3435c297f74b41 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 29 Jun 2020 14:47:09 -0700 Subject: [PATCH 14/16] ios docs: Add "Sign in with Apple" and `NSAppTransportSecurity` sections. --- docs/howto/ios-tips.md | 84 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/docs/howto/ios-tips.md b/docs/howto/ios-tips.md index d8685386e8f..b6089bc5a21 100644 --- a/docs/howto/ios-tips.md +++ b/docs/howto/ios-tips.md @@ -84,3 +84,87 @@ It seems like there's some caching strategy to avoid fetching `.podspec` files unnecessarily, potentially with network requests. (See [discussion](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M3548.20RN.20v0.2E60.2E0.20upgrade/near/896746).) + +## Sign in with Apple + +To set up your [development server](./dev-server.md) to use Apple +authentication ("Sign in with Apple"), you'll want to follow almost +[these +steps](https://zulip.readthedocs.io/en/latest/production/authentication-methods.html#sign-in-with-apple), +but with a few things to keep in mind: + +- If you don't have your own Apple Developer account (there's an + annual fee), please ask Greg to set up test credentials and send + them to you. + These will be associated with the Kandra team, so + [please](https://chat.zulip.org/#narrow/stream/3-backend/topic/apple.20auth/near/915391) + let him know when you're finished with the credentials so he can + revoke them. Please don't abuse them with deliberate spam, as + that goes on our reputation. +- Use the domain `zulipdev.com` where Apple asks for a domain; + [`localhost` won't + work](https://chat.zulip.org/#narrow/stream/3-backend/topic/Apple.20Auth/near/831533). + On the public Internet, `zulipdev.com` resolves to `127.0.0.1`. + - `127.0.0.1` (also what `localhost` points to) points to the + machine you're on. When you're on a physical device, that's the + device itself, not the device (your computer) that's running the + dev server. So you won't be able to connect using `zulipdev.com` + on a physical device. + - Empirically, there's no problem using the iOS simulator on the + computer running the dev server; it seems the iOS simulator shares + its network interface with the computer it's running on. To use + the native flow, you will be able to sign into the simulator at + the "device" level just as you would on a real device. + - Temporarily allow the app to access `http://zulipdev.com` as + described in the section on `NSAppTransportSecurity` exceptions, + below. + +To test the native flow, which uses an Apple ID you've authenticated +with in System Preferences, go to the ZulipMobile target in the +project and targets list, and, under General > Identity, set the +Bundle Identifier field to your development App ID (a.k.a. Bundle ID). +If you've already installed a build that used the canonical Bundle +Identifier, you'll see two app icons on your home screen. Be sure to +open the correct one; it might be easiest to delete them both and +reinstall to prevent any doubt. + +You should now be able to enter `http://zulipdev.com:9991` (not +`https://`), see the "Sign in with Apple" button, and use it +successfully. + +## Adding `http://` exceptions to `NSAppTransportSecurity` in `Info.plist` + +If you need to connect to `http://zulipdev.com` or another host with +the insecure `http://`, you'll need to tell the app to make an +exception under iOS's "App Transport Security", either to allow access +any host with `http://`, or just to specific domains. + +These exceptions should never be committed to master, as there aren't +any insecure domains we want to connect to in production. + +To add an exception for the `zulipdev.com` domain, add the following +in `ios/ZulipMobile/Info.plist`: + +```diff + NSAppTransportSecurity + + NSExceptionDomains + + localhost + + NSTemporaryExceptionAllowsInsecureHTTPLoads + + ++ zulipdev.com ++ ++ NSTemporaryExceptionAllowsInsecureHTTPLoads ++ ++ + + +``` + +See +[discussion](https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Apple.20ATS.20for.20debug/near/883318) +for more convenient solutions if we find we have to allow this +regularly. From 9c13381cc62e4470ec82df5f8d41f15f5cd2f6ca Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 30 Jun 2020 14:21:01 -0700 Subject: [PATCH 15/16] auth [nfc]: Simplify logic a bit with early return. --- src/start/AuthScreen.js | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/start/AuthScreen.js b/src/start/AuthScreen.js index c96105fe92c..b3a13b03ecc 100644 --- a/src/start/AuthScreen.js +++ b/src/start/AuthScreen.js @@ -272,29 +272,28 @@ class AuthScreen extends PureComponent { }; canUseNativeAppleFlow = async () => { - if (Platform.OS === 'ios' && (await AppleAuthentication.isAvailableAsync())) { - let host: string | void; - try { - host = new WhatwgURL(this.props.realm).host; - } catch (e) { - // `this.props.realm` invalid. - // TODO: Check this much sooner. - } + if (!(Platform.OS === 'ios' && (await AppleAuthentication.isAvailableAsync()))) { + return false; + } - // The native flow for Apple auth assumes that the app and the server - // are operated by the same organization, so that for a user to - // entrust private information to either one is the same as entrusting - // it to the other. Check that this realm is on such a server. - // - // (For other realms, we'll simply fall back to the web flow, which - // handles things appropriately without relying on that assumption.) - const isTrusted = config.appOwnDomains.some( - domain => host !== undefined && (host === domain || host.endsWith(`.${domain}`)), - ); - return isTrusted; + let host: string | void; + try { + host = new WhatwgURL(this.props.realm).host; + } catch (e) { + // `this.props.realm` invalid. + // TODO: Check this much sooner. } - return false; + // The native flow for Apple auth assumes that the app and the server + // are operated by the same organization, so that for a user to + // entrust private information to either one is the same as entrusting + // it to the other. Check that this realm is on such a server. + // + // (For other realms, we'll simply fall back to the web flow, which + // handles things appropriately without relying on that assumption.) + return config.appOwnDomains.some( + domain => host !== undefined && (host === domain || host.endsWith(`.${domain}`)), + ); }; handleAuth = async (method: AuthenticationMethodDetails) => { From ba6b6b319ed3ce8183188d093aba31520bf3c7d6 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 30 Jun 2020 14:34:47 -0700 Subject: [PATCH 16/16] url [nfc]: Factor out helper `tryParseUrl`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows callers like `canUseNativeAppleFlow` to use plain conditionals instead of try/catch, and use more `const` so that the type-checker is better able to see when a variable can no longer be a value like `undefined`. In this case, for example, if instead of this change a `return false` is simply added to the `catch` block, then Flow still believes the `host !== undefined` check below is necessary (and that without it the `host.endsWith(…)` call is ill-typed), because `host` is a mutable binding and Flow doesn't see how to prove that it doesn't get set back to `undefined`. With `const` everywhere, the logic to rule out such possibilities is more straightforward and the type-checker is able to see it. --- src/start/AuthScreen.js | 13 +++++-------- src/utils/url.js | 10 ++++++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/start/AuthScreen.js b/src/start/AuthScreen.js index b3a13b03ecc..e9c84477548 100644 --- a/src/start/AuthScreen.js +++ b/src/start/AuthScreen.js @@ -28,7 +28,7 @@ import styles from '../styles'; import { Centerer, Screen, ZulipButton } from '../common'; import { getCurrentRealm } from '../selectors'; import RealmInfo from './RealmInfo'; -import { getFullUrl, encodeParamsForUrl } from '../utils/url'; +import { getFullUrl, encodeParamsForUrl, tryParseUrl } from '../utils/url'; import * as webAuth from './webAuth'; import { loginSuccess, navigateToDev, navigateToPassword } from '../actions'; import IosCompliantAppleAuthButton from './IosCompliantAppleAuthButton'; @@ -276,12 +276,11 @@ class AuthScreen extends PureComponent { return false; } - let host: string | void; - try { - host = new WhatwgURL(this.props.realm).host; - } catch (e) { + const host = tryParseUrl(this.props.realm)?.host; + if (host === undefined) { // `this.props.realm` invalid. // TODO: Check this much sooner. + return false; } // The native flow for Apple auth assumes that the app and the server @@ -291,9 +290,7 @@ class AuthScreen extends PureComponent { // // (For other realms, we'll simply fall back to the web flow, which // handles things appropriately without relying on that assumption.) - return config.appOwnDomains.some( - domain => host !== undefined && (host === domain || host.endsWith(`.${domain}`)), - ); + return config.appOwnDomains.some(domain => host === domain || host.endsWith(`.${domain}`)); }; handleAuth = async (method: AuthenticationMethodDetails) => { diff --git a/src/utils/url.js b/src/utils/url.js index b5164c229d5..d33873a871c 100644 --- a/src/utils/url.js +++ b/src/utils/url.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import urlRegex from 'url-regex'; +import { URL as WhatwgURL } from 'react-native-url-polyfill'; import type { Auth } from '../types'; import { getAuthHeaders } from '../api/transport'; @@ -31,6 +32,15 @@ export const encodeParamsForUrl = (params: UrlParams): string => .map(([key, value]) => `${encodeURIComponent(key)}=${encodeURIComponent(value.toString())}`) .join('&'); +/** Just like `new WhatwgURL`, but on error return undefined instead of throwing. */ +export const tryParseUrl = (url: string, base?: string | WhatwgURL): WhatwgURL | void => { + try { + return new WhatwgURL(url, base); + } catch (e) { + return undefined; + } +}; + /** * Turn a relative or absolute URL into an absolute URL. *