-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Sometimes Amplify tokens get multiplied #14007
Comments
Hello, @didemkkaslan 👋. Can you help me understand how you're looking to use that Is this intentional? Just want to make sure we aren't missing any intended functionality here. |
Sure, we also have msteams tab app on the domain tab.app.spiky.ai. Its an iframe on msteams app so we weren't able to login there with the default local storage method amplify uses. So this is actually why we implemented custom cookie storage. But we get the duplicate token error when NEXT_PUBLIC_ENV is not msteams. so cookie options are empty in this case. I'm not sure if I'm missing smth |
@didemkkaslan, I'm a little confused here. If you're using Next.JS, the default implementation and usage for storage should be cookieStorage. Is there a reason why you were trying to utilize localStorage (and then needed to implement a custom cookie storage solution)? If the custom cookie storage is needed, you'll need to make sure you're persisting the cookie only if it's not a duplicate. This PR is an example of how we had to implement this fix for our cookie storage that is used out of the box with Next.JS. |
Hello @cwomack sorry for the confusion, I'm not really sure if I need to implement custom cookie storage here. The issue is we have a microsoft teams tab app where tab.app.spiky.ai domain is used in an iframe and its kind of embedded. It looks at the tab.app.spiky.ai domain and I need the amplify tokens to be stored there to be able to authenticate. Is below implementation the way to go?
I'm a bit confused shouldn't I be doing any customizations? |
I've changed the cookie implementation to this:
} and the token duplication issue still happens. After many token refresh events deduplicatedFetchAuthSession isn't able to get the tokens and I believe after that tokens get multiplied
export const deduplicatedFetchAuthSession = |
@didemkkaslan, is there any logic to check when setting a cookie if they are the same name/user? And are you trying to set different usernames in the cookies or have multiple users during a session? I only ask these because it looks like there are difference usernames in one of the screenshots you provided that were still being stored. Also, we don't recommend changing the value for |
Hello, no there isn't any logic it was just this code:
So I'm not even touching how cookies are being added, deleted just some cookie options. I also call fetchAuthSession from server inside idtoken content also belongs to the same user. I'm not sure what that username is in the cookie name. Today I converted cookie storage implementation to this:
Couldn't reproduce the token multiplication yet |
@didemkkaslan, appreciate your trying the default cookie storage and great to hear that the token multiplication is no longer happening! After implementing it, is there any other issues or is the app working as expected now? Maybe we can follow up in a week or so to see if it occurs again. |
Thanks for your help. I'm only using Amplify's cookie storage—nothing else. The issue doesn't always reproduce, but it tends to happen when multiple tabs are left inactive for about a day, followed by an ERROR_CONNECTION_RESET message. This current implementation is causing the bug, but when I remove the environment configurations, the API calls become unauthorized. Btw problem occurs both with deduplicatedFetchAuthSession and the default fetchAuthSession
We also call fetchAuthSession in getServerSideProps
|
Thanks for providing the sample code I couldn't however isolate the issue by just looking at the code... A few more questions for you to help us understand the situation better:
Quick suggestions:
|
Also you mentioned about We found an issue for versions < 6.6.7 that the library may wrongly clear tokens on |
I've also upgraded long time ago to these versions: "aws-amplify": "^6.8.0", When accessing the route that invokes getServerSideProps() in the 3rd block of your code, does your app also calls the client-side fetchAuthSession at the same time?
Looking at the 3rd block of your code, it looks like you are using the client side get API, please use the server one export from aws-amplify/api/server.
|
I've also removed |
What's the purpose of using ENV vars to override fields (such as userPoolId etc.) of the configuration object imported from amplifyconfiguration.json on the client side?
|
If
With this question, I'm curious when you accessing this route where the issue has occurred, do you have any business logic that calls Amplify APIs on both client side and server side that may trigger token refresh. E.g. you have logic running on the client-side calling the client-side |
And on the server I also need to check user permission so I'm calling the serverside |
Hi @didemkkaslan regarding the my comment calling fetchAuthSession on both client side and sever side may not be impactful for your case, as the client side fetchAuthSession should be guaranteed to run after the fetchAuthSession called in Could you verify whether the issue happens after the suggested clean ups? In addition, curious why do you choose to use |
I've merged the suggested cleanups of using server-side get function and removing the extra line of Not reallly we are actually thinking about using accessToken instead of idToken these days. Do you recommend putting scopes in the idtoken or accesstoken? We have many scopes for user permission stuff and cookie sizes are really large |
In general, access token is always recommended to be used as authorization information instead of the ID token. With Amazon Congnito for example, you can revoke tokens, but the tokens can actually be revoked are the access token and refresh token. I believe you are able to customize access token today with adding custom scopes, please see this AWS Security Blog for more details (please review the "consideration and best practices" section carefully). |
Hi, unfortunately after the latest cleanups issue is still continuing. One of our users got 431 request headers too large error. Can this be related to token revocation mechanism? I believe backend team told us they weren't revoking tokens or is it completely unrelated |
If the
Has this user had the same issue before the cleanups? If cookies are previously duplicated - they stay in cookie store until they expire. As the cookie names are messed up, the library won't able to detect them and clear. At this point the end user may need to clear cookie store manually. I'm really curious what's happening to cause this. If you are still able to reproduce on your end after the cleanups, can you generate a HAR file from the network traffic around the time you reproduce it? I will do some digging. |
I've asked the team to generate a HAR file when it happens, I will also try to reproduce it myself |
Hi @didemkkaslan, thanks for providing the HAR file. I'll try to reproduce this as well. |
Before opening, please confirm:
JavaScript Framework
Next.js
Amplify APIs
Authentication
Amplify Version
v6
Amplify Categories
auth
Backend
Amplify CLI
Environment information
Describe the bug
Hello:), I can rarely reproduce this but here is a user reporting it:
I was at home fully. And I used the platform let’s say 45 mins ago or so. Then I clicked on “Report Link” in my email and that’s when I saw this browser output.
Email link is just a redirection to meeting detail page. Since token sizes are big we also get request header too large error when they got multiplied and page crash
We use a custom cookie storage where I tried to prevent token multiplication error:
Expected behavior
Tokens shouldn't get multiplied
Reproduction steps
Not really sure why and when this happens but it usually happens when a tab is left open 45mins or so
Code Snippet
// Put your code below this line.
Log output
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
The text was updated successfully, but these errors were encountered: