Amplify-js: handleCodeFlow is executed twice

[ltankey]

Describe the bug
It seems handleCodeFlow is executed twice: one succeeds because it can access the PKCE code, and the second one fails as it does not have a code_verifier attribute. The second one kicks off a cascade of errors stemming from either invalid_request or invalid_grant. Sometimes this leads to authentication failure, even though the response from the hosted UI was successful.

Authentication fails sporadically even though my hosted UI flow was successful, and even though my code _could_ be redeemed successfully.

To Reproduce
Running on:
"aws-amplify": "^1.1.29",
"aws-amplify-react-native": "^2.1.9",

Steps to reproduce the behavior:

1. use Expo 33 (ejected) with RN 59.8
2. use hosted UI w/Cognito as the provider
3. configure oauth with the expo urlOpener
4. register for hub events in a login component
5. sign in and observe two "Calling token endpoint" debug messages
6. one will have a code_verifier attribute and one won't
7. sporadic authentication failure (keep retrying and it'll happen eventually)

The subsequent request that fails will bounce between invalid_grant and invalid_request. It alternates, and I can't seem to spot a pattern that explains why auth fails when it does vs. when it passes.

Expected behavior
I would expect only a single login event, and the handleCodeFlow code to only run once so that there's no conflict between what can consume the PKCE value.

Screenshots
If applicable, add screenshots to help explain your problem.

Smartphone (please complete the following information):

• Device: iPhone XR (simulator)
• OS: iOS 12

Additional context
Note: this implements a change to _handleCodeFlow to address the bug in #3247 -- the _only_ change that's made is to replace the body: body part of the fetch. The issue is as described in this comment, too.

Seems like it may be related to #3183

Sample code
Include additional sample code or a sample repository to help us reproduce the issue. (Be sure to remove any sensitive data)

[iamdavidmartin]

Looks like this issue you created could have led to this being fixed!

[ltankey]

@iamdavidmartin is it still happening for you? I’ve implemented a monkey patch to throw an exception for the second event dispatched so it doesn’t sporadically interfere with auth.

[MatthieuLab]

@ltankey hi Luke, could you provide the code of your monkey patch please ?

[ltankey]

In order to fix the issues raised in the related tickets (about missing URLSeachParams due to the upgrade to Expo 33/RN 59), I had this file anyway to change the body attribute of the POST to the token endpoint.

Because it seems like there's a duplicate event emitter registration triggered by a callback from the token endpoint, handleCodeFlow appears to be executed twice. The second time fails because the first consumes the PKCE code. I simply throw an exception early in the hopes to avoid a race with the first call. It seems if I don't do this, authentication can frequently fail.

This is a modified version of OAuth:

/* eslint-disable func-names */
/* eslint-disable consistent-return */
/* eslint-disable camelcase */
/* eslint-disable no-underscore-dangle */

import { parse } from 'url';
import {
  ConsoleLogger as Logger,
  Hub,
} from '@aws-amplify/core';
import OAuth from '@aws-amplify/auth/lib/OAuth/OAuth';
import { isCognitoHostedOpts } from '@aws-amplify/auth/lib/types/Auth';
import * as oAuthStorage from '@aws-amplify/auth/lib/OAuth/oauthStorage.native';

const AMPLIFY_SYMBOL = (typeof Symbol !== 'undefined' && typeof Symbol.for === 'function')
  ? Symbol.for('amplify_default')
  : '@@amplify_default';

const dispatchAuthEvent = (event, data, message) => {
  Hub.dispatch('auth', { event, data, message }, 'Auth', AMPLIFY_SYMBOL);
};

const logger = new Logger('OAuth');

console.log('[CUSTOM handleCodeFlow] Patching OAuth.prototype._handleCodeFlow');

OAuth.prototype._handleCodeFlow = async function (currentUrl) {
  /* Convert URL into an object with parameters as keys
    { redirect_uri: 'http://localhost:3000/', response_type: 'code', ...} */
  const { code } = (parse(currentUrl).query || '')
    .split('&')
    .map(pairings => pairings.split('='))
    .reduce((accum, [k, v]) => ({ ...accum, [k]: v }), { code: undefined });

  if (!code) { return; }

  const oAuthTokenEndpoint = `https://${this._config.domain}/oauth2/token`;

  dispatchAuthEvent(
    'codeFlow',
    {},
    `Retrieving tokens from ${oAuthTokenEndpoint}`,
  );

  const client_id = isCognitoHostedOpts(this._config)
    ? this._cognitoClientId
    : this._config.clientID;

  const redirect_uri = isCognitoHostedOpts(this._config)
    ? this._config.redirectSignIn
    : this._config.redirectUri;

  const code_verifier = oAuthStorage.getPKCE();

  // throw early if this is the second event emitted
  if (!code_verifier) {
    console.log('[CUSTOM handleCodeFlow] code_verifier is null');
    throw Error("You don't have a code verifier");
  }

  const oAuthTokenBody = {
    grant_type: 'authorization_code',
    code,
    client_id,
    redirect_uri,
    ...(code_verifier ? { code_verifier } : {}),
  };

  logger.debug(`Calling token endpoint: ${oAuthTokenEndpoint} with`, oAuthTokenBody);
  console.log(`[CUSTOM handleCodeFlow] Calling token endpoint: ${oAuthTokenEndpoint} with`, oAuthTokenBody);

  const body = Object.entries(oAuthTokenBody)
    .map(([k, v]) => `${encodeURIComponent(k)}=${encodeURIComponent(v)}`)
    .join('&');

  const {
    access_token, refresh_token, id_token, error,
  } = await (await fetch(oAuthTokenEndpoint, {
    method: 'POST',
    headers: {
      'Content-Type': 'application/x-www-form-urlencoded',
    },
    body,
  })).json();

  if (error) {
    console.log('[CUSTOM handleCodeFlow] error', error);
    throw new Error(error);
  }

  return {
    accessToken: access_token,
    refreshToken: refresh_token,
    idToken: id_token,
  };
};

[iamdavidmartin]

@ltankey I'm also on expo 33/RN 59. I agree with your diagnosis - the codeflow was executing twice. Hub.listen now gets the configured auth event and only gets it once. I don't think codeflow is happening twice anymore and I'm not getting invalid grant.

It seems to be working. I'm not confident enough to say it's solved. My testing hasn't been thorough enough, but something has changed on AWS's end.

Btw, if I remove the workaround that deletes global.URLSearchParams, I'm back to getting invalid_request.

[MatthieuLab]

@ltankey tanks a lot for your share

[schellack]

I am experiencing this issue as well. I use a Hub listener and see the codeFlow event more than once, which results in a subsequent invalid_request error. This occurs on both iOS and Android devices.

My environment:

• expo: 33.0.7 (not detached, only occurs in standalone build; strangely does not occur in Expo app)
• react-native 0.59.8
• @aws-amplify/auth: 1.2.25
• @aws-amplify/core: 1.0.28
• aws-amplify-react-native: 2.1.13
• expo-web-browser: 5.0.3

[ltankey]

We traced this down to the event listener for the link back into the app (for the redirect_uri).

I would have expected this to be affecting way more people, but it's possible people have been slow to upgrade to RN59/Expo 33.

Does the amplify team have an official solution?

[schellack]

I too had already patched @aws-amplify/auth/oauth due to the URLSeachParams issue (which yes is still a problem). 😞

Patching @aws-amplify/auth/oauth for this issue, as posted above, did help. The check if (!code_verifier) { works because the first time amplify get the code verifier (the call to oAuthStorage.getPKCE()) also removes the PKCE key from session storage.

Ultimately this is just a workaround, though, as we the OAuth code shouldn't be having to handle multiple code flow responses, right?

[ltankey]

Ultimately this is just a workaround, though, as we the OAuth code shouldn't be having to handle multiple code flow responses, right?

Definitely just a workaround. It's just to avoid a race between two events wanting to complete an auth flow. Without it, it seems auth is prone to failing (even though authentication may have been successful).

What I don't know is whether the flaw is with Linking with RN 59 or some fault of amplify's. Would be nice for them to provide a better (more official) solution

[ltankey]

I too had already patched @aws-amplify/auth/oauth due to the URLSeachParams issue (which yes is still a problem). 😞

I 100% this is something the amplify team needs to address btw, as if RN moving forward is seriously going to have a half-baked implementation of URLSearchParams, they can't rely on it as a platform indicator.

[alexofob]

Hi there,
I am using amplify on the web with react and I am experiencing this issue. So this issue is not only with react native. The amplify team needs to fix this.

[rkgrep]

@alexofob
I had the same issue with event triggered twice on web, because I had .configure executed twice in the code - so it assigned urlListener event twice.
In my case it was Amplify.configure({ Auth: ... }) and Auth.configure({ ... }) in another part.