Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

signInWithRedirect sometimes does not forward to the auth provider #13751

Open
3 tasks done
ronbhomri opened this issue Aug 25, 2024 · 14 comments
Open
3 tasks done

signInWithRedirect sometimes does not forward to the auth provider #13751

ronbhomri opened this issue Aug 25, 2024 · 14 comments
Assignees
Labels
Auth Related to Auth components/category pending-community-response Issue is pending a response from the author or community. question General question

Comments

@ronbhomri
Copy link

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

CDK

Environment information

# Put output below this line


Describe the bug

Sometimes, totally random, when a user tries to login via signInWithRedirect with a custom provider, it seems the function get called, but the user is not redirected to the provider and therefore no user is in the session. There is not a clear pattern here, but it is only happens to users we use adminGlobalSignOut in the cdk (in the backend) to log them out of our app and revoke all of the tokens. We do it for the users after an hour if the user is not active in the app.

Expected behavior

Always redirect the user to the auth provider or throw a relevant error.

Reproduction steps

I can't reproduce as it is random, but I think it has a link to the Admin global sign out, so I would start there.

Code Snippet

const [email, setEmail] = useState();
const [user, setUser] = useState();
const submit = () => {
  if (email) {
    setAmplify();
    signIn();
 }
};

const setAmplify = () => {
  Amplify.configure({
    Auth: {
      Cognito: {
        userPoolClientId: <userPoolWebClientId>,
        userPoolId: <userPoolId>,
        loginWith: {
          oauth: {
            domain: <userPoolDomain>,
            scopes: ['email', 'profile', 'openid', 'aws.cognito.signin.user.admin'],
            redirectSignIn: [`${window.location.origin}`],
            redirectSignOut: [`${window.location.origin}`],
            responseType: 'code',
          },
        },
      },
    },
  });
};

export const signIn = async (state?) => {
  const signInDetails: any = {
    customState: `${encodeURIComponent(state || window.location.pathname + window.location.search)}`,
  };
  if (<userPoolIdentityProviderName>) {
    signInDetails.provider = { custom: <userPoolIdentityProviderName> };
  }
  return Auth.signInWithRedirect(signInDetails).catch(e => console.error(`Login error: ${e}`));
};

const unsubscribe = Hub.listen("auth", ({ payload }) => {
      const errorText = JSON.stringify(payload?.data?.error?.message);
      switch (payload.event) {
        case "customOAuthState":
          setTimeout(() => navigate(decodeURIComponent(payload.data)));
          break;
        case "signInWithRedirect":
          getUser();
          break;
        case "signInWithRedirect_failure":
          logout();
          navigate("");
          console.error(`[authentication error]: LOGIN FAIL: ${errorText}`);
          document.location.reload();
          break;
        case "tokenRefresh_failure":
          logout();
          console.error(
            `[authentication error]: REFRESH TOKEN FAIL: ${errorText}`
          );
          document.location.reload();
          break;
        default:
          break;
      }
    });

const getUser() => {
  try {
    const tokenUser = getCurrentUser();
    setUser(tokenUser);
  } catch (error){
    log.error(`no user is found ${error}`);
  }
};

return user ?  (<Box> <div>welcome</div></Box>) :
(<Box>
         <TextInput
            type="email"
            label="Email address"
            value={email}
            onChange={setEmail}
          />
          <Button variant="contained" disabled={isPending} onClick={submit()}>
            Continue
          </Button>
</Box>) 

Log output

// Put your logs below this line


aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

@ronbhomri ronbhomri added the pending-triage Issue is pending triage label Aug 25, 2024
@cwomack cwomack self-assigned this Aug 26, 2024
@cwomack cwomack added the Auth Related to Auth components/category label Aug 26, 2024
@cwomack
Copy link
Member

cwomack commented Aug 26, 2024

Hello, @ronbhomri and thanks for opening this issue. It sounds like you are calling the adminGlobalSignOut() which will revoke the tokens on the server side with Cognito, but you aren't clearing our the tokens on the client side as well. Is this correct? If so, then it sounds like what you're experiencing is expected to have some issues due to signInWithRedirect() calling getCurrentUser() under the hood. This would fail due to getCurrentUser getting a valid user locally and not allow the signInWithRedirect() to proceed.

@cwomack cwomack added question General question pending-response and removed pending-triage Issue is pending triage labels Aug 26, 2024
@ronbhomri
Copy link
Author

ronbhomri commented Aug 26, 2024

@cwomack thank you for the quick answer!
Few questions:

  1. Since I use the LS as the token storage, I can say for sure that when this error appears, there are no tokens available in the LS (it was the first thing I checked). Is it also saved in another place?
  2. If this is the case, so why does it fail randomly and not always?
  3. What is the common approach in this case? Since the adminGlobalSignOut is done in the BE, the user can be logged out even if they are not currently using our app.
  4. I couldn't find any example in amplify docs for the current way to build the login flow with the signInWithRedirect function. Can you please supply an example?

@ronbhomri
Copy link
Author

ronbhomri commented Aug 27, 2024

I see inside signInWithRedirect() there is this function:

export async function assertUserNotAuthenticated() {
	let authUser: AWSAuthUser | undefined;
	try {
		authUser = await getCurrentUser();
	} catch (error) {}

	if (authUser && authUser.userId && authUser.username) {
		throw new AuthError({
			name: USER_ALREADY_AUTHENTICATED_EXCEPTION,
			message: 'There is already a signed in user.',
			recoverySuggestion: 'Call signOut before calling signIn again.',
		});
	}
}

so from what you saying, I expect to get this error in this line:
return Auth.signInWithRedirect(signInDetails).catch(e => console.error(Login error: ${e}));
and this is not case. Moreover, I would expect in the flow you described that the getUser() won't fail too, but in the flow I described, it is failing with noUserAuthenticated error

@israx
Copy link
Member

israx commented Aug 27, 2024

Hello @ronbhomri. The adminGlobalSignOut api will invalidate all active user sessions on the server, but it won't clear authentication tokens stored in the browser's storage mechanism. That means that users will still be authenticated until the refresh_token has expired, but any authenticated calls made to Cognito or any service that checks for token validity won't work.

The signInWithRedirect API uses the getCurrentUser API to verify the absence of an authenticated user. Since this API doesn't send a network request, the library cannot check the token's validity, hence the user unable to authenticate again due to the UserAlreadyAuthenticated error.

The reason it happens randomly is that the refres_token might be already expired when a call to signInWithRedirect is done.

To make sure your users can login again after adminGlobalSignOut, you can call the signOut API when the signInWithRedirect throws the UserAlreadyAuthenticated error.

@ronbhomri
Copy link
Author

@israx something is still not clear to me.
I don't see the UserAlreadyAuthenticated errors in this case at all.
I only see the error: UserUnAuthenticatedException from my 'getUser' function.
Another thing - I think it does not have a direct connection to the adminGlobalSignOut as it started to happened to users which this function is not available for them.

@ronbhomri
Copy link
Author

@israx if you can also clarify what is the common approach for using adminGlobalSignOut? Since the adminGlobalSignOut is done in the BE, the user can be logged out even if they are not currently using our app.

@israx
Copy link
Member

israx commented Aug 30, 2024

Hello @ronbhomri. Could you share some code snippets of the following,

  1. module where signInWithRedirect is called.
  2. module where signInWithRedirect redirects to after authentication.

To answer your other question, a common approach of AdminGlobalSignOut is when,

  • Account suspension: if you need to temporarily or permanently suspend a user's account, this API ensures they're logged out everywhere.
  • Device management: if an employee loses a device or leaves the company, you can use this to ensure they're logged out of all company services
  • Security breach: If you suspect a user's account has been compromised, you can use this API to immediately invalidate all their sessions across devices.
  • Forget Password: If a user forgot their password, you want to use a lambda trigger to call AdminGlobalSignOut to invalidate all other active sessions.

For other cases where global revocation is needed, you are better off using the signOut API on the client and passing the global flag. One example is when the user changes their password after being authenticated.

@ronbhomri
Copy link
Author

@israx I meant what is the common approach of dealing it on the FE side.
Im not sure what you mean by these 2 different flow - isn't it the same function? we use the hub.Listen to catch the authentication after it was successful.

@israx
Copy link
Member

israx commented Sep 4, 2024

Hello @ronbhomri.

A common approach to use signOut({global:true}) on the client is when a user has updated their password while being authenticated.

Regarding the other question, I'm trying to understanding the failure scenario with signInWithRedirect. The ticket describes that some users are not able to get redirected to the custom provider to finish authenticating. If that is the case, do you see any of the following scenarios below when calling signInWithRedirect,

  1. is it throwing an UserAlreadyAuthenticated error?,
  2. is it throwing any other errors?
  3. are you able to get any logs from signInWithRedirect_failure?
  4. does it redirect to the auth provider but is failing to create auth tokens when the user is redirected back to the app?

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Sep 4, 2024
@ronbhomri
Copy link
Author

@israx thank for replying.
After further investigation, it does not related to the global sign out as it happens to multiple users, even users who does not use this feature.

I will explain how the error usually looks like:

  1. The LS is clean
  2. There is no Cognito's cookie
  3. The user puts its email, and then the signInWithRedirect is called.
  4. Before calling to signInWithRedirect, we put in LS "passedLogin" just to know the user already redirect to signIn.
  5. The user is not redirect to the custom provider, and then gets UserUnAuthenticatedException errors from my getUser function.

Replaying to your questions:

  1. is it throwing an UserAlreadyAuthenticated error? No
  2. is it throwing any other errors? No
  3. are you able to get any logs from signInWithRedirect_failure? No
  4. does it redirect to the auth provider but is failing to create auth tokens when the user is redirected back to the app? No, its not even redirect to the auth provider.

I will say that it usually work for the second time.

@cwomack
Copy link
Member

cwomack commented Sep 5, 2024

@ronbhomri, can you clarify what the URL is that users are being sent to when attempting to call signInWithRedirect() when this happens (if any URL at all)? Curious to see if you're properly getting a redirect to the user pool domain.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Sep 5, 2024
@israx
Copy link
Member

israx commented Sep 6, 2024

Hello @ronbhomri. In addition to @cwomack comment, I think the issue lies in how your App updates state. You can use conditional rendering to update the disabled option of the button until email is defined. It is also recommended to call Amplify.configure when the App renders. Below I added some code snippets. Let me know what you think

Before

const submit = () => {
  if (email) {
    setAmplify();
    signIn();
 }
};

  <Button variant="contained" disabled={isPending} onClick={submit()}>
    Continue
  </Button>

After

useEffect(()=>{
  setAmplify();
},[])

const submit = () => {
    signIn();
};

 <Button variant="contained" disabled={isPending && !email} onClick={submit}>
    Continue
 </Button>

@cwomack cwomack added pending-community-response Issue is pending a response from the author or community. and removed pending-response labels Sep 10, 2024
@ronbhomri
Copy link
Author

ronbhomri commented Sep 26, 2024

Hi @cwomack @israx,
it seems like first problem was solved - we had an extra rendering due to a state we saved before calling to signInWithRedirect().

Now I have another question regarding hub.listen. In the Amplify docs there is an example to use it like this:

useEffect(() => {
    const unsubscribe = Hub.listen('auth', ({ payload }) => {
      const errorText = JSON.stringify((payload as any)?.data?.error?.message);
      const underlyingError = JSON.stringify((payload as any)?.data?.error?.underlyingError);
      switch (payload.event) {
        case 'customOAuthState':
          setTimeout(() => navigate(decodeURIComponent(payload.data)));
          break;
        case 'signInWithRedirect':
          getUser();
          break;
        case 'signInWithRedirect_failure':
          console.log(`${errorText} ${underlyingError}`);
          logout();
          break;
        case 'tokenRefresh_failure':
          console.log(`${errorText} ${underlyingError}`);
          logout();
          break;
        default:
          break;
      }
    });
    getUser();

    return unsubscribe;
  }, []);

const getUser() => {
  try {
    const tokenUser = getCurrentUser();
    setUser(tokenUser);
  } catch (error){
    log.error(`no user is found ${error}`);
    logout();
  }
};

I've notice we have race condition here on login, since it calls the last getUser() always after getting redirect back from the IDP, and sometimes it ends before the signInWithRedirect event received and before the token request is sent to Cognito.

  • We are using the same URL before the login and after the redirect from the IDP, so thats why the getUser() is also needed outside the hub.listen (in a case of a refresh).

Is using setTimeout before the last getUser might effect the signInWithRedirect() process?

@github-actions github-actions bot added pending-maintainer-response Issue is pending a response from the Amplify team. and removed pending-community-response Issue is pending a response from the author or community. labels Sep 26, 2024
@cwomack
Copy link
Member

cwomack commented Oct 1, 2024

@ronbhomri, it does seem like one valid solution at this point would be to wrap your getUser() API calls with a setTimeout function to avoid the race condition by delaying its execution.

I believe the original scope of this issue was resolved tied to the signInWithRedirect() problem, but let us know if there are more questions from here. Thanks!

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Oct 1, 2024
@cwomack cwomack added the pending-community-response Issue is pending a response from the author or community. label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auth Related to Auth components/category pending-community-response Issue is pending a response from the author or community. question General question
Projects
None yet
Development

No branches or pull requests

3 participants