didemkkaslan
commented
1 week ago Added another screenshot, this time there is smth like username I'm not sure maybe its helpful
Open didemkkaslan opened 1 week ago
didemkkaslan
commented
1 week ago Added another screenshot, this time there is smth like username I'm not sure maybe its helpful
cwomack
commented
1 week ago Hello, @didemkkaslan 👋. Can you help me understand how you're looking to use that OptionsType import from the cookies-next library? It looks like you're setting secure: true within your cookieOptions, but in the screenshots the cookies don't look to be marked with the secure ✅ in the dev tools column.
Is this intentional? Just want to make sure we aren't missing any intended functionality here.
didemkkaslan
commented
1 week ago 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
cwomack
commented
6 days ago @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.
didemkkaslan
commented
3 days ago 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. And we also have our main app at app.spiky.ai
Is below implementation the way to go?
const isMSTeams = process.env.NEXT_PUBLIC_ENV === 'msteams';
cognitoUserPoolsTokenProvider.setKeyValueStorage(
new CookieStorage({
...(isMSTeams && {
domain: 'tab.app.spiky.ai',
}),
secure: true,
path: '/',
sameSite: isMSTeams ? 'none' : 'lax',
expires: 365,
}),
);
}I'm a bit confused shouldn't I be doing any customizations?
didemkkaslan
commented
1 day ago I've changed the cookie implementation to this:
const isMSTeams = process.env.NEXT_PUBLIC_ENV === 'msteams';
cognitoUserPoolsTokenProvider.setKeyValueStorage(
new CookieStorage({
...(isMSTeams && {
domain: 'tab.app.spiky.ai',
}),
secure: true,
path: '/',
sameSite: isMSTeams ? 'none' : 'lax',
expires: 365,
}),
);}
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
import { AuthSession, fetchAuthSession } from 'aws-amplify/auth';
/**
* This function creates a singleton that fetches the current session. This is necessary
* because the Amplify fetchAuthSession method does not de-duplicate credential requests to
* the Cognito server. That means that duplicate requests will be made to the server if
* multiple components call fetchAuthSession concurrently before the internal credential
* cache has been populated. This singleton ensures that only one request is made at a time.
*
* A feature request has been made to the Amplify team to add de-duplication
* to the fetchAuthSession method. If that feature is added, this layer ofz
* abstraction can be removed.
*
* @see https://github.com/aws-amplify/amplify-js/issues/13499
*/
function createDeduplicatedFetchAuthSessionSingleton() {
let pendingRequest: Promise<AuthSession> | null = null;
console.log('pendingRequest', pendingRequest);
return () => {
if (!pendingRequest) {
pendingRequest = new Promise((resolve, reject) => {
(async () => {
try {
const response = await fetchAuthSession();
resolve(response);
} catch (error) {
reject(error);
} finally {
pendingRequest = null;
}
})();
});
}
return pendingRequest;
};
}export const deduplicatedFetchAuthSession = createDeduplicatedFetchAuthSessionSingleton();
cwomack
commented
1 day ago @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 sameSite as a best practice for security. Was there a need for changing this in your cookieOptions? Ideally, you'd use our generic adapter when implementing your own cookie management solution.
didemkkaslan
commented
1 day ago Hello, no there isn't any logic it was just this code:
const isMSTeams = process.env.NEXT_PUBLIC_ENV === 'msteams';
cognitoUserPoolsTokenProvider.setKeyValueStorage(
new CookieStorage({
...(isMSTeams && {
domain: 'tab.app.spiky.ai',
}),
secure: true,
path: '/',
sameSite: isMSTeams ? 'none' : 'lax',
expires: 365,
}),
);So I'm not even touching how cookies are being added, deleted just some cookie options.
I also call fetchAuthSession from server inside getServerSideProps to access some user data. Can this be the reason? Like I saw couple of comments about server sets cookies too. When I check the duplicate cookie signInDetails then I see the same user actually: didem@spiky.ai
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:
cognitoUserPoolsTokenProvider.setKeyValueStorage(new CookieStorage());
Couldn't reproduce the token multiplication yet
cwomack
commented
11 hours ago @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.
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
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