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

fetchAuthSession triggering logout when offline #6057

Open
3 tasks done
dehli opened this issue Nov 11, 2024 · 18 comments
Open
3 tasks done

fetchAuthSession triggering logout when offline #6057

dehli opened this issue Nov 11, 2024 · 18 comments
Labels
Auth bug Something isn't working

Comments

@dehli
Copy link
Contributor

dehli commented Nov 11, 2024

Before opening, please confirm:

Fix can be found here: #6061

JavaScript Framework

React

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

CDK

Environment information

# Put output below this line


Describe the bug

If you call Auth.fetchAuthSession({ forceRefresh: true }) while offline, amplify will trigger the logout flow.

Expected behavior

I'd expect an error, but I wouldn't expect to be logged out.

Reproduction steps

  1. Install latest version of AWS amplify with auth.
  2. Sign in.
  3. Go offline.
  4. Call Auth.fetchAuthSession({ forceRefresh: true }) (I did this by adding the function to the window).

Code Snippet

// Put your code below this line.
window.TRIGGER_ERROR = () => Auth.fetchAuthSession({ forceRefresh: true });

Log output

// Put your logs below this line
{"channel":"auth","payload":{"event":"tokenRefresh_failure","data":{"error":{"name":"NetworkError","underlyingError":{}}}},"source":"Auth","patternInfo":[]}

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

Related to aws-amplify/amplify-js#13596 & aws-amplify/amplify-js#13993

The below chunk of code is NOT responsible for clearing the tokens. I confirmed that the signout workflow is triggered further downstream.

https://github.com/aws-amplify/amplify-js/blob/0f5091780046b9556b98300c29fb970a0358bd70/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts#L173-L176

If I return null (and prevent calling the subsequent Hub.dispatch) I am not logged out. Not sure what implications that will have so I didn't submit a PR.

        if (err.name !== AmplifyErrorCode.NetworkError) {
            // TODO(v6): Check errors on client
            this.clearTokens();
        } else {
            return null; // <-- Adding this "fixes" the issue
        }
        Hub.dispatch('auth', {
            event: 'tokenRefresh_failure',
            data: { error: err },
        }, 'Auth', AMPLIFY_SYMBOL);
@github-actions github-actions bot added pending-triage Issue is pending triage pending-maintainer-response Issue is pending response from an Amplify UI maintainer labels Nov 11, 2024
@dehli dehli closed this as completed Nov 11, 2024
@github-actions github-actions bot removed pending-triage Issue is pending triage pending-maintainer-response Issue is pending response from an Amplify UI maintainer labels Nov 11, 2024
@dehli dehli reopened this Nov 11, 2024
@didemkkaslan
Copy link

Having the same issue, I see tokenRefresh_failure event being fired and tokens are removed from cookie storage

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@cwomack cwomack self-assigned this Nov 11, 2024
@cwomack cwomack added the Auth label Nov 11, 2024
@ashika112
Copy link
Member

@dehli Thanks for taking a look. I can confirm i see the same behavior as well. I will look closer into this today and report back.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112
Copy link
Member

@dehli @didemkkaslan Can you confirm which version are you both on? There was a retry fix just recently, while i see the issue on 6.6.4 (I used this version since that was what @didemkkaslan issue mentioned and on another issue as well) when i try the most recent version 6.8.0 i dont see the logout behavior anymore. Can one of you confirm ?

@dehli
Copy link
Contributor Author

dehli commented Nov 11, 2024

Hey @ashika112, thanks for looking into it. I'm on 6.8.0 currently and seeing this behavior. I just removed my node_modules and reinstalled them to confirm that I was on that version. I was still able to trigger logout by calling fetchAuthSession({ forceRefresh: true }); while offline.

Here's my stacktrace:

image

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112
Copy link
Member

@dehli so this what i see, i do get network error but after that when network comes back on i am able to get credentials and token for the customer and not logged out.
Screenshot 2024-11-11 at 11 19 16 AM

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112
Copy link
Member

@dehli do you have any custom storage implementation? Would it be possible to put out a minimal code reproducing and share me the repo?

@dehli
Copy link
Contributor Author

dehli commented Nov 11, 2024

No custom storage. I'll work on putting together a minimal reproducing example though 👍

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112
Copy link
Member

@dehli , I was also testing in next js since some other issues was there. I will also try in react now at my end (which i notice is what you use at your end).

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112 ashika112 added the question General question label Nov 11, 2024
@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112 ashika112 added bug Something isn't working and removed question General question labels Nov 11, 2024
@ashika112
Copy link
Member

@dehli Im able to reproduce this consistently in React and hence i have marked this as bug. We will keep this ticket posted for updates.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@dehli
Copy link
Contributor Author

dehli commented Nov 11, 2024

Awesome! Thanks so much 🎉

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@didemkkaslan
Copy link

didemkkaslan commented Nov 11, 2024

I've tested both versions

With aws-amplify: "6.6.4"
When I go offline and triggered the method @dehli provides window.TRIGGER_ERROR = () => Auth.fetchAuthSession({ forceRefresh: true });

Got the below error and tokens are deleted from my custom cookie storage
Screenshot 2024-11-11 at 23 03 18
Screenshot 2024-11-11 at 23 03 39

With aws-amplify: "6.8.0"

Tokens are not deleted from my custom cookie storage, I can still access session idtoken data

Screenshot 2024-11-11 at 23 09 08 Screenshot 2024-11-11 at 23 09 34

My custom cookie storage implementation:

const cookieOptions: OptionsType =
  process.env.NEXT_PUBLIC_ENV === 'msteams'
    ? {
        domain: 'tab.app.spiky.ai' as string,
        sameSite: 'none' as 'lax' | 'strict' | 'none',
        secure: true,
      }
    : {};

const keyValueStorage = createKeyValueStorageFromCookieStorageAdapter({
  get(name) {
    const value = getCookie(name, cookieOptions);
    return { name, value };
  },
  getAll() {
    const cookies = getCookies(cookieOptions);
    return Object.keys(cookies).map((name) => ({ name, value: cookies[name] }));
  },
  set(name, value) {
    const cookieAlreadyExists = getCookie(name, cookieOptions);

    if (cookieAlreadyExists) {
      console.log(
        'cookieAlreadyExists so Im deleting it and setting new one',
        cookieAlreadyExists,
      );
      deleteCookie(name, cookieOptions);
    }
    setCookie(name, value, cookieOptions);
  },
  delete(name) {
    deleteCookie(name, cookieOptions);
  },
});

I have one question tho: I shouldn't signout the user when a tokenrefresh_failure event fires right?

@ashika112
Copy link
Member

@didemkkaslan Ideally you want log customer out when there is error except for Network error.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112
Copy link
Member

ashika112 commented Nov 11, 2024

@dehli Do you use Authenticator by any chance? taking another close look. I see this behavior only when using Authenticator component not when using the Auth APIs directly? If you do not use Authenticator, then I am going to need the reproducible code sample.

Sorry for the back and forth. As i investigate i see the behavior is scoped only to the component.

@dehli
Copy link
Contributor Author

dehli commented Nov 11, 2024

@ashika112 Yep, I am using the Authenticator component (although I am referencing Auth.fetchAuthSession directly). No need to apologize, happy to help how I can!

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112
Copy link
Member

@dehli I think this is scoped to Authenticator. Is it possible for you to verify the behavior without Authenticator? On my side, im going to reach out to amplify-ui repo.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@dehli
Copy link
Contributor Author

dehli commented Nov 11, 2024

It will take a bit to pull out the Authenticator with the way our project is setup. I did look through the amplify-js library for a bit this morning a bit and couldn't find anything that would cause the signout behavior so it being in a separate repo makes a lot of sense.

From my investigation this morning, it seemed to be related to the tokenRefresh_failure event that is sent over Hub. When searching over there for it, you can see a handful of references so I think it's safe to close this issue. https://github.com/search?q=repo%3Aaws-amplify%2Famplify-ui+tokenRefresh_failure&type=code

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112
Copy link
Member

@dehli I am working internally with the team right now, if this needs to be in their repo, i will re-route this ticket to them so they have context.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@dehli
Copy link
Contributor Author

dehli commented Nov 11, 2024

Sounds good! This seems to be what's causing the behavior:

case 'tokenRefresh_failure': {
if (event === 'signedOut' && isFunction(onSignOut)) {
onSignOut();
}
send('SIGN_OUT');

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Nov 11, 2024
@ashika112 ashika112 transferred this issue from aws-amplify/amplify-js Nov 11, 2024
@github-actions github-actions bot added the pending-triage Issue is pending triage label Nov 11, 2024
dehli added a commit to dehli/amplify-ui that referenced this issue Nov 12, 2024
@cwomack cwomack removed pending-triage Issue is pending triage pending-maintainer-response Issue is pending response from an Amplify UI maintainer labels Nov 13, 2024
@cwomack cwomack removed their assignment Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auth bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants