[REVIEWABLE] Sign in with Apple on iOS #4034

chrisbobbe · 2020-04-10T23:43:26Z

This is a rough, incomplete draft of this feature (focused on iOS) intended to check my understanding of what needs to be done, broadly. Even if I've got some things wrong, I think I've reached a point where it would be helpful to test with a server-side implementation, using a key that @gnprice has.

I have several questions, some larger, some smaller. How about I write a separate message for each of them, here, and if it's big enough for a lot of discussion, we can go to CZO?

Fixes: #3964

chrisbobbe · 2020-04-10T23:44:02Z

After we decided to use a custom button everywhere, and never to use the system-provided button (Apple's HIG document on these), I haven't yet done the work to comply with the official rules for custom buttons. Part of me wishes this were acceptable, since the code makes it so easy for it to be uniform with the other social auth buttons, which (IMHO) looks better than having it be different, anyway. For the real version, it looks like the biggest differences we'll have to make are:

to use the SVG Apple provides, instead of the "apple-logo" SVG icon from Ionicons.
to match the height of the logo file to the height of the button
to not use our Zulip green color on the logo, or any custom color
Fix the English text to "Sign in with Apple" (though we could also update the others to match this.)
Translate the button's text. (I haven't found guidelines for this; the system-provided button would have handled this by rendering it in the device-level language.)

It looks so nice with the others, now, though!

chrisbobbe · 2020-04-10T23:44:21Z

You can pass a "state" to the authorization request to Apple, which gets "[...] returned to you unmodified in the corresponding credential after a successful authentication." (Apple doc; state is a property on an ASAuthorizationOpenIDRequest, which ASAuthorizationAppleIDRequest
extends, and here's the line where Expo uses an ASAuthorizationAppleIDRequest, also, Expo doc). It's optional. Expo says it's useful to help prevent replay attacks. Would it be appropriate to use an OTP from our existing OTP generator, perhaps? 🤷‍♂️

chrisbobbe · 2020-04-10T23:44:35Z

There's also a nonce property we can apparently use, which Expo exposes, and I'm having trouble distinguishing it from what state does. In the first of two attempts, Expo is, surprisingly, more helpful than the Apple doc about this property; Expo links to the OpenID Connect specification. There may be a standard, correct answer for what to do with this, but that spec says it's optional, so I'm appealing to others' judgment on this.

chrisbobbe · 2020-04-10T23:44:58Z

On purpose, in this PR, I haven't made any code changes to the web flow logic, with a glass-half-full mindset (and some thinking through) that it will Just Work, since that's what the existing web auth flow is designed for. The web flow will always be used on Android, and always on iOS versions before 13. For iOS >= 13, it will be used for non-Kandra realms.

chrisbobbe · 2020-04-10T23:45:08Z

As noted in a code comment, I think we'll need to carefully handle errors from the fetch of ${this.props.realm}/complete/apple/?mobile_flow_otp=${nativeAppleOTP}. I have it in my notes from a conversation with @gnprice that, when using the native flow, it would be nice to open Zulip's "create account" flow in the browser if it turns out that the user hasn't created a Zulip account yet. The mobile app doesn't have to be in charge of this for any of the now-existing auth flows (I think maybe the purely web-auth ones handle it this way, entirely in the browser?).

chrisbobbe · 2020-04-10T23:45:17Z

We can use fetch, right, or does this limitation apply?

chrisbobbe · 2020-04-10T23:45:33Z

I'm looking at this doc for this question. For the body of the POST request to /complete/apple/ (which we make after successful Apple auth), I'm trying to exactly replicate the shape of the request that Apple will send there, in the web flow. In the web flow, we get to tell Apple what we'd like their request to look like, with a response_mode flag, which can be query, fragment, or form_post. I'm having trouble finding where we do this in zulip/zulip#14235, but I expect it will be form_post, because the docs say, "If you requested any scopes, the value must be form_post", and I think we'll probably at least want the email scope, maybe full name too. Then,

For the form_post value, an HTTP POST request containing the results of the authorization is sent to the redirectURI. The HTTP body contains the result parameters with application/x-www-form-urlencoded content type.

Thus, the body I've chosen to use. Do I seem to be on the right track here?

chrisbobbe · 2020-04-10T23:45:43Z

I haven't thought much about the "Prevent Duplicate Accounts" strategy outlined at this Apple doc. Omitting that strategy doesn't seem to block a basic implementation, but I'll investigate further, if you like.

chrisbobbe · 2020-04-10T23:45:53Z

Apple has documentation (at "Manage the User Session") on, I think, a way to kick someone out of a Zulip account that they logged into with Apple, upon discovering that their account is no longer authenticated at the device level. It doesn't sound like we do something similar with Google on Android, but some users might feel more comfortable if this were done?

chrisbobbe · 2020-04-11T00:04:32Z

Just rebased and fixed some merge conflicts.

chdinesh1089 · 2020-04-11T18:20:40Z

I'm having trouble finding where we do this in zulip/zulip#14235, but I expect it will be form_post, because the docs say, "If you requested any scopes, the value must be form_post", and I think we'll probably at least want the email scope, maybe full name too.

You're right about it. That is taken care by Python-social-auth here https://github.com/python-social-auth/social-core/blob/master/social_core/backends/apple.py#L55
We send the scope as email name. We only get name the first time user makes a request, so Only on the initial try to register we get name but we get email every time the user tries to logs in.

I haven't thought much about the "Prevent Duplicate Accounts" strategy outlined at this Apple doc. Omitting that strategy doesn't seem to block a basic implementation,

I think a user can only make two accounts, one with original email through normal email registration and another with apple auth with a private email. And we get the same private email everytime as apple returns user data based on initial request as said in Retrieve the User’s Information from Apple ID Servers

The server returns the credentials and user information based on the initial request.

Looking at "Prevent Duplicate Accounts" in that doc, If my understanding is right, we send a request to the apple keychain(an apple user uses this to save their login credentials of a website/app in this afaik) credential sharing which helps us detect existing account only if it's credentials with url are saved in keychain. But I'm not really sure about that. I didn't really understand how to implement ASAuthorizationPasswordProvider from their docs.

chrisbobbe · 2020-04-14T01:25:24Z

Thanks, @chdinesh1089! It's helpful for me see that code in python-social-auth. 🙂

I'd better go help make dinner, but I just pushed some changes to add the custom button. Initially, I had thought it would be best to use the custom button for everything, since we had to make one anyway. But actually, I think we can be more targeted than that. In particular, @gnprice, what do you think about

on Android, using the existing ZulipButton that's already consistent with the other social auth buttons, since Apple's Human Interface Guidelines (which guide what the button should look like) don't apply on Android
on iOS, using the system-provided button when possible (i.e., iOS >=13), and the custom button otherwise? The custom button is kind of annoying because (at least currently) I don't know how to make the logo, which is a PNG, work well with any of RN's Touchable components that have feedback, since they all use opacity, and none seem to apply the opacity properly to the PNG

chrisbobbe · 2020-04-14T01:30:25Z

Here's the official button:

And the custom button:

I guess one thing that's nice about the custom button is that we're allowed to increase the size of the Apple logo to match the other logos/icons (doc):

Also, the left-aligned logos are available in small, medium, and large sizes, so you can match logo sizes in all the sign-up buttons you display.

chrisbobbe · 2020-04-14T17:22:05Z

src/start/IosCompliantAppleAuthButton/index.js

+}
+
+export default connect(state => ({
+  theme: getSettings(state).theme,


Oh, one question I had here — I know how to add to the React context for the theme colors, and consume them from there, but I wasn't sure if that's what we'd want given that this is definitely a single-use thing; the property names would have to be something like signInWithAppleButtonFill and signInWithAppleButtonText and we'd have to mark them with comments as not to be used anywhere else, which kind of goes against the purpose of React context.

gnprice

Thanks @chrisbobbe ! Some comments below on the auth flow in the native case (though I still don't yet fully understand how it's meant to work.)

gnprice · 2020-04-21T00:48:22Z

src/start/AuthScreen.js

+    const nativeAppleOTP = await webAuth.generateOtp();
+    const credential = await AppleAuthentication.signInAsync({
+      state: nativeAppleOTP,
+    });
+    if (credential.state !== nativeAppleOTP) {


I think the implementation of generateOtp is exactly what we want here, but the name doesn't match -- this isn't a one-time pad, but rather just a random token. Could just rename it.

Do you mean rename webAuth.generateOtp? It's currently got a useful, OTP-specific comment on it:

// Generate a one time pad (OTP) which the server XORs the API key with // in its response to protect against credentials intercept

I wonder about extracting the body into a separate function, maybe generateRandomToken:

if (Platform.OS === 'android') { return new Promise((resolve, reject) => { NativeModules.RNSecureRandom.randomBase64(32, (err, result) => { if (err) { reject(err); } resolve(base64ToHex(result)); }); }); } else { const rand = await NativeModules.UtilManager.randomBase64(32); return base64ToHex(rand); }

gnprice · 2020-04-21T01:00:37Z

src/start/AuthScreen.js

+    const { url: callbackUrl } = await fetch(
+      `${this.props.realm}/complete/apple/?mobile_flow_otp=${nativeAppleOTP}`,
+      {
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/x-www-form-urlencoded',
+        },
+        body: encodeParamsForUrl(credential),
+      },
+    );
+


I don't think this will work as is. I'm not sure we want to be using the /complete/apple/ route at all.

Here's what Apple will cause to be sent to that route (under "Handle the Response"):
https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_js/incorporating_sign_in_with_apple_into_other_platforms

Here's the data we have at this point, in the AppleAuthenticationCredential (from Expo, and corresponding closely to this from Apple):
https://docs.expo.io/versions/latest/sdk/apple-authentication/#appleauthenticationappleauthenticationcredential

Hmm, looking closer, I guess they actually align quite well in what information is present. They just aren't quite the same shape -- so that line body: encodeParamsForUrl(credential) will need to be expanded to do a bit more work 😉

Then, one thing that simplifies things (and may also call for adding a separate route) is: we don't have a need for the redirect to zulip://, or the OTP mechanism which exists to encrypt the API key through that channel. That redirect is needed for getting from a browser back to the app's code. If we're making our own fetch here, we can just get the data from that directly.

Alternatively, for the new-account case maybe we do want to open a browser. If we do open a browser, then we need the zulip:// redirect and the mobile_flow_otp mechanism. For simplicity we might, at least at first, just always open a browser, regardless of whether the user might already have an account. In that case we'll want to call something like webAuth.openBrowser instead of fetch.

For simplicity we might, at least at first, just always open a browser, regardless of whether the user might already have an account. In that case we'll want to call something like webAuth.openBrowser instead of fetch.

Mmm, this does make sense as a place to start; I was a bit worried about how to handle basically any case other than a completely successful login, with the browserless fetch way. All those contingencies (no Zulip account, etc.) could be handled in the browser. We would indeed need the zulip:// redirect, but I'm not sure I've understood whether you've decided to add the separate route and use it here.

Using the browser for everyone is not ideal for the long-term, though, right; users with an existing account would want to log in seamlessly, without the browser. I'd basically want to open an issue for this immediately upon merging this simpler approach; does that sound right? (And I hope opening the browser for people with existing accounts won't be enough to trigger an App Store review rejection.) One way this more complicated, but smoother, process could look is

We use fetch on either a) a route that never gives the zulip:// redirect, since, as you've said, we don't need it (in this case, adding a route is necessary), or b) a route that we can pass a flag to, saying we don't want the redirect (here, it would be possible, but maybe awkward, to use an altered form of the existing route).

Then, the result would either contain an API key, and we'd be ready to go, or (e.g., if there's no Zulip account) a signal that we need to try again, this time with webAuth.openBrowser and expecting the zulip:// redirect.

Using the browser for everyone is not ideal for the long-term, though, right; users with an existing account would want to log in seamlessly, without the browser. I'd basically want to open an issue for this immediately upon merging this simpler approach; does that sound right?

Yeah, that sounds right.

(And I hope opening the browser for people with existing accounts won't be enough to trigger an App Store review rejection.)

Yeah, me too. Good thing we're not right up against the deadline.

One way this more complicated, but smoother, process could look is [...]

This sounds like a good plan. The mobile_flow_otp option already serves as a flag that controls whether it does the zulip:// redirect, vs. redirecting to the webapp (in addition to providing a one-time pad to use in the zulip:// redirect.) So I think it'd probably make sense to go for (b) and just add another flag with a meaning like "just return the API key in a JSON response". But @mateuszmandera may have more to say about what'd be a convenient arrangement on the server side.

Hmm, so the plan would be to do the browser flow only for now?

For the native approach - I think /complete/apple/ is still a good endpoint to use for this as on the server side we can override the class method handling the endpoint to be able to deal with the "native flow" request as well. The upside of that being that then the rest of the pipeline for processing "social authentication" can run as normal (avoiding code duplication). And then we'd have two cases for the response you'll get from the server:

Complete success - you get the zulip:// redirect (yes, it's kind of redundant to do the OTP thing, but on the other hand - code reuse is nice)

Some kind of failure - you get a https redirect. If you get this, you could open the redirect in the browser and that will handle the error. "Bad" errors will get the same handling as in browser flow (show an error message if we're currently doing that in the browser flow). And the "user doesn't have an account" will take the user through the registration flow in the browser and upon success redirect back to the app with the newly created account through the zulip:// redirect.

How does the above sound? @gnprice @chrisbobbe

Hmm, so the plan would be to do the browser flow only for now?

Yes but there's a couple of things that can mean, so to be explicit:

For users on Android and on iOS <=12, we'll do the whole auth flow in a browser, same way we do Google and GitHub and SAML and the rest.

For users on iOS >=13 where we can, we'll

use the system-provided flow to get a credential from the user -- so the user gets some system UI that says like "do you want to sign into this app with your identity, what name do you want, do you want to use your real email address or an opaque alias", and when they say yes we get one of these AppleAuthenticationCredential objects;

then we'll turn that into a URL like https://example.zulipchat.com/complete/apple?id_token=…&mobile_flow_otp=…&… and open a browser with that URL, and pick up the browser flow from there. In particular if the user already has an account and the server accepts the login, it'll redirect the browser to a zulip://… URL with the encrypted API key.

Some kind of failure - you get a https redirect. If you get this, you could open the redirect in the browser and that will handle the error. "Bad" errors will get the same handling as in browser flow (show an error message if we're currently doing that in the browser flow).

Hmm. A couple of questions/thoughts I'd have are:

Will the URL in the redirect be enough to just pass to a fresh browser and it'll pick up the flow smoothly from there? No cookies or other state needed?

That seems like it wouldn't cleanly support the app trying to understand anything about the failure and give a structured error of its own -- it'd just always have to go to a browser. That might be OK, but it puts a ceiling on how polished the experience can be with this approach.

In any case I think we can discuss more and experiment when we get there. I would really like to get to a point where we have one flow that works solidly; among other things I think that will help us a lot in concretely understanding how all the steps of this are supposed to work. So I will be very happy for us to just get the flow with /complete/apple/ in a browser working.

chrisbobbe · 2020-05-22T01:58:55Z

OK, just pushed my latest revision, @gnprice. Still a draft, and Flow has some issues with

9812a3c [draft] Use "Sign in with Apple".

But just in case you wanted to check in on the code.

chrisbobbe · 2020-05-22T23:35:20Z

Fixed the Flow issues (and filed expo/expo#8446, hopefully that will clarify some things for us)!

chrisbobbe · 2020-06-24T05:36:24Z

I just pushed with some rebase conflicts resolved, and removed the edits of lots of translation files, as that'll be handled in Transifex as long as messages_en.json is correct (as we discussed on the phone).

gnprice · 2020-06-26T03:52:29Z

Tested this just now with Mateusz's test server, as discussed in chat here. And with one fix, it works!

diff --git src/start/AuthScreen.js src/start/AuthScreen.js
index a246baed9..af8714691 100644
--- src/start/AuthScreen.js
+++ src/start/AuthScreen.js
@@ -245,6 +245,7 @@ class AuthScreen extends PureComponent<Props> {
     const state = await webAuth.generateRandomToken();
     const credential: AppleAuthenticationCredential = await AppleAuthentication.signInAsync({
       state,
+      requestedScopes: [AppleAuthentication.AppleAuthenticationScope.FULL_NAME, AppleAuthentication.AppleAuthenticationScope.EMAIL],
     });

gnprice · 2020-06-26T04:04:14Z

Specifically with that change it worked on a simulated iPhone 11 running iOS 13.1.

It also worked, even without that change, on iOS 12 -- on a simulated iPhone 7 running iOS 12.2. As expected, on that OS version the "Sign in with Apple" button went straight to a browser view; I went through the auth flow on appleid.apple.com, got redirected back to the Zulip server, and then the app was successfully logged in.

chrisbobbe · 2020-06-30T01:31:59Z

OK, just pushed my latest changes, including the new way of establishing that a server is Kandra-hosted, as discussed around here.

gnprice

Thanks for the revisions! Comments below.

gnprice · 2020-06-30T01:15:16Z

src/config.js

@@ -39,6 +40,7 @@ const config: Config = {
    'update_message_flags',
    'user_status',
  ],
+  kandraHostedDomains: ['zulip.com', 'zulipchat.com', 'chat.zulip.org'],


Thinking about the name for this variable: I'd like it to be more generically about the relationship of this (build of the) app to some domains, rather than Kandra by name. In principle if someone else makes and distributes their own build with their own App ID, they'd want to edit this to refer to domains they control, even though these would still be the ones hosted by Kandra.

(Even if nobody ends up ever doing that, I think it keeps the logic around this variable clearer to think about if it's named so it works in that general case.)

Perhaps appAssociatedDomains? appOwnDomains?

Sure, makes a lot of sense!

src/start/AuthScreen.js

docs/howto/ios-tips.md

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.

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`.

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/.

chrisbobbe · 2020-06-30T19:08:58Z

@gnprice, thanks for the review! I've just pushed my latest changes, for another one.

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.

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

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<void>` doesn't seem to be well-considered.

…ional 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]: expo/expo#8446 (comment) [2]: expo/expo#8446 (comment)

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.

…tion. In an upcoming commit, we'll need to generate a random token (the same implementation is fine), but without the OTP semantics.

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.

chrisbobbe · 2020-06-30T19:29:21Z

Sorry, just pushed again—was getting those errors in Jest again; see #4034 (comment).

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.

…ons.

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.

gnprice · 2020-06-30T22:34:46Z

And merged -- thanks! Added a few small changes.

chrisbobbe · 2020-07-01T19:28:06Z

Great, thanks, and thanks for those changes! I just took a look and they definitely make sense.

chrisbobbe requested a review from gnprice April 10, 2020 23:43

chrisbobbe force-pushed the pr-ios-apple-auth branch from 08dbc37 to 90fe3c8 Compare April 11, 2020 00:03

chrisbobbe marked this pull request as draft April 13, 2020 16:36

chrisbobbe force-pushed the pr-ios-apple-auth branch from 90fe3c8 to 7fe0171 Compare April 14, 2020 01:10

chrisbobbe commented Apr 14, 2020

View reviewed changes

gnprice reviewed Apr 21, 2020

View reviewed changes

chrisbobbe mentioned this pull request May 1, 2020

Trailing slash on realm URL can happen, and breaks /events #3567

Closed

chrisbobbe force-pushed the pr-ios-apple-auth branch from 7fe0171 to ddd4898 Compare May 22, 2020 01:50

chrisbobbe force-pushed the pr-ios-apple-auth branch from ddd4898 to 4e0602c Compare May 22, 2020 18:58

chrisbobbe marked this pull request as ready for review June 22, 2020 23:15

chrisbobbe changed the title ~~[WIP] Sign in with Apple on iOS~~ [REVIEWABLE] Sign in with Apple on iOS Jun 22, 2020

chrisbobbe force-pushed the pr-ios-apple-auth branch from 4e0602c to 34784db Compare June 24, 2020 05:35

chrisbobbe force-pushed the pr-ios-apple-auth branch 2 times, most recently from deb1cdb to 3af2d03 Compare June 30, 2020 01:27

gnprice reviewed Jun 30, 2020

View reviewed changes

chrisbobbe added 3 commits June 30, 2020 11:11

ZulipButton types: Let text be LocalizableText, not just string.

6907fe7

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.

chrisbobbe force-pushed the pr-ios-apple-auth branch from 3af2d03 to a2c17f1 Compare June 30, 2020 19:07

chrisbobbe and others added 7 commits June 30, 2020 12:23

apple-auth libdef: Allow onPress to return Promise<void>

1c638f4

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<void>` doesn't seem to be well-considered.

webAuth [nfc]: Extract implementation into generateRandomToken func…

6a9db46

…tion. In an upcoming commit, we'll need to generate a random token (the same implementation is fine), but without the OTP semantics.

chrisbobbe force-pushed the pr-ios-apple-auth branch from a2c17f1 to 99d4391 Compare June 30, 2020 19:28

Chris Bobbe and others added 5 commits June 30, 2020 15:02

auth [nfc]: Expand comment on Apple-auth host check.

c7b37e2

ios docs: Add "Sign in with Apple" and NSAppTransportSecurity secti…

c19cbe2

…ons.

auth [nfc]: Simplify logic a bit with early return.

9c13381

gnprice force-pushed the pr-ios-apple-auth branch from 99d4391 to ba6b6b3 Compare June 30, 2020 22:34

gnprice merged commit ba6b6b3 into zulip:master Jun 30, 2020

chrisbobbe mentioned this pull request Jul 1, 2020

Support "Sign In with Apple" [Android] #3965

Closed

gnprice mentioned this pull request Jul 6, 2020

realm screen: Give early error when server URL is an email address. #4181

Merged

chrisbobbe deleted the pr-ios-apple-auth branch November 6, 2020 03:19

[REVIEWABLE] Sign in with Apple on iOS #4034

[REVIEWABLE] Sign in with Apple on iOS #4034

Conversation

chrisbobbe commented Apr 10, 2020 • edited by gnprice Loading

chrisbobbe commented Apr 10, 2020 • edited Loading

chrisbobbe commented Apr 10, 2020 • edited Loading

chrisbobbe commented Apr 10, 2020

chrisbobbe commented Apr 10, 2020

chrisbobbe commented Apr 10, 2020 • edited Loading

chrisbobbe commented Apr 10, 2020

chrisbobbe commented Apr 10, 2020

chrisbobbe commented Apr 10, 2020 • edited Loading

chrisbobbe commented Apr 10, 2020

chrisbobbe commented Apr 11, 2020

chdinesh1089 commented Apr 11, 2020

chrisbobbe commented Apr 14, 2020

chrisbobbe commented Apr 14, 2020

Choose a reason for hiding this comment

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe Apr 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe commented May 22, 2020

chrisbobbe commented May 22, 2020 • edited Loading

chrisbobbe commented Jun 24, 2020 • edited Loading

gnprice commented Jun 26, 2020

gnprice commented Jun 26, 2020

chrisbobbe commented Jun 30, 2020

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe commented Jun 30, 2020

chrisbobbe commented Jun 30, 2020

gnprice commented Jun 30, 2020

chrisbobbe commented Jul 1, 2020

chrisbobbe commented Apr 10, 2020 •

edited by gnprice

Loading

chrisbobbe commented Apr 10, 2020 •

edited

Loading

chrisbobbe commented Apr 10, 2020 •

edited

Loading

chrisbobbe commented Apr 10, 2020 •

edited

Loading

chrisbobbe commented Apr 10, 2020 •

edited

Loading

chrisbobbe Apr 23, 2020 •

edited

Loading

chrisbobbe commented May 22, 2020 •

edited

Loading

chrisbobbe commented Jun 24, 2020 •

edited

Loading