Skip to content

Commit 814bf18

Browse files
chrisbobbegnprice
authored andcommitted
AuthScreen [nfc]: Make realm-url expressions less of a mouthful
And put them in a consistent format, `serverSettings.realm_url`, that'll be easy to handle when we eventually convert this component to a function component. Some of these intermediate declarations are placed such that there's now a yield between when we read the current realm-URL prop and when we use it: - In `beginWebAuth`, there's an `await webAuth.generateOtp()` - In `handleNativeAppleAuth`, there are these: await webAuth.generateRandomToken() await AppleAuthentication.signInAsync(…) await webAuth.generateOtp() But that's fine and NFC, because the `serverSettings` route param doesn't change and isn't supposed to change. Since that constraint wasn't yet explicit, add a comment in the route-params type. (The UX/security reason given in the comment has always applied, but the constraint conveniently helps us verify this commit as NFC.)
1 parent e83938b commit 814bf18

File tree

1 file changed

+24
-19
lines changed

1 file changed

+24
-19
lines changed

src/start/AuthScreen.js

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,17 @@ export const activeAuthentications = (
173173
type OuterProps = $ReadOnly<{|
174174
// These should be passed from React Navigation
175175
navigation: AppNavigationProp<'auth'>,
176-
route: RouteProp<'auth', {| serverSettings: ServerSettings |}>,
176+
route: RouteProp<
177+
'auth',
178+
{|
179+
// Keep constant through the life of an 'auth' route: don't
180+
// `navigation.navigate` or `navigation.setParams` or do anything else
181+
// that can change this. We use it to identify the server to the user,
182+
// and also to identify which server to send auth credentials to. So
183+
// we mustn't let it jump out from under the user.
184+
serverSettings: ServerSettings,
185+
|},
186+
>,
177187
|}>;
178188

179189
type SelectorProps = $ReadOnly<{||}>;
@@ -230,27 +240,26 @@ class AuthScreenInner extends PureComponent<Props> {
230240
* `external_authentication_method` object from `/server_settings`.
231241
*/
232242
beginWebAuth = async (url: string) => {
243+
const { serverSettings } = this.props.route.params;
233244
otp = await webAuth.generateOtp();
234-
webAuth.openBrowser(
235-
new URL(url, this.props.route.params.serverSettings.realm_uri).toString(),
236-
otp,
237-
);
245+
webAuth.openBrowser(new URL(url, serverSettings.realm_uri).toString(), otp);
238246
};
239247

240248
endWebAuth = (event: LinkingEvent) => {
241249
webAuth.closeBrowser();
242250

243251
const { dispatch } = this.props;
244-
const realm = this.props.route.params.serverSettings.realm_uri;
245-
const auth = webAuth.authFromCallbackUrl(event.url, otp, realm);
252+
const { serverSettings } = this.props.route.params;
253+
const auth = webAuth.authFromCallbackUrl(event.url, otp, serverSettings.realm_uri);
246254
if (auth) {
247255
dispatch(loginSuccess(auth.realm, auth.email, auth.apiKey));
248256
}
249257
};
250258

251259
handleDevAuth = () => {
260+
const { serverSettings } = this.props.route.params;
252261
this.props.navigation.push('dev-auth', {
253-
realm: this.props.route.params.serverSettings.realm_uri,
262+
realm: serverSettings.realm_uri,
254263
});
255264
};
256265

@@ -264,6 +273,8 @@ class AuthScreenInner extends PureComponent<Props> {
264273
};
265274

266275
handleNativeAppleAuth = async () => {
276+
const { serverSettings } = this.props.route.params;
277+
267278
const state = await webAuth.generateRandomToken();
268279
const credential: AppleAuthenticationCredential = await AppleAuthentication.signInAsync({
269280
state,
@@ -284,12 +295,7 @@ class AuthScreenInner extends PureComponent<Props> {
284295
id_token: credential.identityToken,
285296
});
286297

287-
openLinkEmbedded(
288-
new URL(
289-
`/complete/apple/?${params}`,
290-
this.props.route.params.serverSettings.realm_uri,
291-
).toString(),
292-
);
298+
openLinkEmbedded(new URL(`/complete/apple/?${params}`, serverSettings.realm_uri).toString());
293299

294300
// Currently, the rest is handled with the `zulip://` redirect,
295301
// same as in the web flow.
@@ -300,6 +306,8 @@ class AuthScreenInner extends PureComponent<Props> {
300306
};
301307

302308
canUseNativeAppleFlow = async () => {
309+
const { serverSettings } = this.props.route.params;
310+
303311
if (!(Platform.OS === 'ios' && (await AppleAuthentication.isAvailableAsync()))) {
304312
return false;
305313
}
@@ -311,7 +319,7 @@ class AuthScreenInner extends PureComponent<Props> {
311319
//
312320
// (For other realms, we'll simply fall back to the web flow, which
313321
// handles things appropriately without relying on that assumption.)
314-
return isAppOwnDomain(this.props.route.params.serverSettings.realm_uri);
322+
return isAppOwnDomain(serverSettings.realm_uri);
315323
};
316324

317325
handleAuth = async (method: AuthenticationMethodDetails) => {
@@ -336,10 +344,7 @@ class AuthScreenInner extends PureComponent<Props> {
336344
<Centerer>
337345
<RealmInfo
338346
name={serverSettings.realm_name}
339-
iconUrl={new URL(
340-
serverSettings.realm_icon,
341-
this.props.route.params.serverSettings.realm_uri,
342-
).toString()}
347+
iconUrl={new URL(serverSettings.realm_icon, serverSettings.realm_uri).toString()}
343348
/>
344349
{activeAuthentications(
345350
serverSettings.authentication_methods,

0 commit comments

Comments
 (0)