Storing Authentication Tokens clears the entire storage state and then repopulates causing extra storage events.

[jon-armen]

Before opening, please confirm:

• [X] I have searched for duplicate or closed issues and discussions.
• [X] I have read the guide for submitting bug reports.
• [X] I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.

JavaScript Framework

Angular

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

None

Environment information

``` # Put output below this line ```

Describe the bug

Our application uses localStorage (the default) as the storage mechanism for Cognito Authentication Tokens. Using the shared storage mechanism, against all open browser tabs, we want to know when the authenticated user is changed (or signed out). We currently listen to the window storage events for local storage, which works great when users sign out. However, the implementation for assigning items to storage first clears the entire storage, and then populates storage. This causes an undesirable effect of appearing that the item is cleared from storage, and then inserted. I would consider this behavior a detriment to any storage back end implementation as every token refresh will have two operations - 1. delete existing item; 2. set new item value (if any). This would be better served instead by a single change to each storage key to either 1. delete existing item (if no new value); OR 2. set new item value.

Expected behavior

When setting tokens, the number of accesses against each storage key should be reduced. We should not clear an item just to set it later in the code flow.

Reproduction steps

1. Setup a listener to local storage to log storage events
2. Observe when token refreshes occur for tokens, first a clear is received, and then a new value.

Code Snippet

// Put your code below this line.

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

This is a proposed fix, which I will also submit a PR on.

...  TokenStore.ts
    async storeTokens(tokens: CognitoAuthTokens): Promise<void> {
        assert(tokens !== undefined, TokenProviderErrorCode.InvalidAuthTokens);

        const lastAuthUser = tokens.username;
        if (lastAuthUser) {
            await this.getKeyValueStorage().setItem(
                this.getLastAuthUserKey(),
                lastAuthUser,
            );
        } else {
            await this.getKeyValueStorage().removeItem(this.getLastAuthUserKey());
        }

        const authKeys = await this.getAuthKeys();
        if (authKeys.accessToken) {
            await this.getKeyValueStorage().setItem(
                authKeys.accessToken,
                tokens.accessToken.toString(),
            );
        } else {
            await this.getKeyValueStorage().removeItem(authKeys.accessToken);
        }

        if (tokens.idToken) {
            await this.getKeyValueStorage().setItem(
                authKeys.idToken,
                tokens.idToken.toString(),
            );
        } else {
            await this.getKeyValueStorage().removeItem(authKeys.idToken);
        }

        if (tokens.refreshToken) {
            await this.getKeyValueStorage().setItem(
                authKeys.refreshToken,
                tokens.refreshToken,
            );
        } else {
            await this.getKeyValueStorage().removeItem(authKeys.refreshToken);
        }

        if (tokens.deviceMetadata) {
            if (tokens.deviceMetadata.deviceKey) {
                await this.getKeyValueStorage().setItem(
                    authKeys.deviceKey,
                    tokens.deviceMetadata.deviceKey,
                );
            }
            if (tokens.deviceMetadata.deviceGroupKey) {
                await this.getKeyValueStorage().setItem(
                    authKeys.deviceGroupKey,
                    tokens.deviceMetadata.deviceGroupKey,
                );
            }

            await this.getKeyValueStorage().setItem(
                authKeys.randomPasswordKey,
                tokens.deviceMetadata.randomPassword,
            );
        }
        if (tokens.signInDetails) {
            await this.getKeyValueStorage().setItem(
                authKeys.signInDetails,
                JSON.stringify(tokens.signInDetails),
            );
        } else {
            await this.getKeyValueStorage().removeItem(authKeys.signInDetails);
        }

        if (authKeys.clockDrift) {
            await this.getKeyValueStorage().setItem(
                authKeys.clockDrift,
                `${tokens.clockDrift}`,
            );
        } else {
            await this.getKeyValueStorage().removeItem(authKeys.clockDrift);
        }
    }
...

[cwomack]

Hello, @jon-armen and thanks for opening this issue. We'd recommend using the Hub utility for this use case based on what you've described in the issue. There's a Hub event emitted for the Auth channel specific to the tokenRefresh (see here). Can you see if this will fit your use case or provide some context/clarity as to why it won't?

And thank you for taking the time to submit a PR for this as well. We'll leave feedback/comments on it and follow up there.

[jon-armen]

Hello @cwomack.

In my use case, I'm not concerned about the tokenRefresh itself, I am needing to identify the signedIn and signedOut events, however from my initial observations, the events from the Hub utility are only fired on the active tab that actually performed the signIn or signOut operation. I am looking to achieve an effect where we can display a notification on all tabs when the authentication state is changed.

I am close to achieving this using the local storage approach (although not as ideal as hub might be if it worked cross tabs), however the storage states receive two events during a refresh, which causes the other tabs to receive a clear and then a set operation. Ideally looking to track when the last signed in user id is changed (either to a different user or to null when signed out)

[cwomack]

@jon-armen, appreciate the follow up and additional context. Updating this issue to be a feature request for consideration and we'll review this further internally. If there's any additional questions or feedback for either this issue or the PR that you've submitted, we'll follow up accordingly. Thank you!

[Manizuca]

Hi! We use amplify for authentication on our react-native app. I've got multiple reports of failures while refreshing the tokens with:

TokenRefreshException: Token refresh is not supported when authenticated with the 'implicit grant' (token) oauth flow. 
     Please change your oauth configuration to use 'code grant' flow.

I've wrapped the storeTokens functions to log if all calls have a refreshToken present, and all of them do have it. However, since i've got the info that at least some of the errors have happened just after re-starting the app, it looks possible to me that sometimes the app is closed at the same time that storeTokens is running, and since the first thing it does is clear everything, we could end up in a case where the process is killed just before it stores the new token (or maybe storeTokens throws an error mid-execution, idk).

Anyways, i've patched the library with the fix proposed by @jon-armen , and (for redundancy) also added a copy of the auth info using just the file-system. The refresh issue seems to be fixed (the last part is probably not needed, but I added redundancy just to be sure i wont have problems)

[cwomack]

@Manizuca, thanks for the follow up and tentative confirmation that @jon-armen's PR may resolve this! Quick question for you since you've tried it... when you say that you're restarting the app, do you mean that you're simply putting it in a background state or completely killing the app and all processes?

[Manizuca]

@cwomack the time it happened to me was after killing the app, but i cannot confirm it was the same for the other reports i got

[cwomack]

@Manizuca, appreciate the quick reply and confirmation. We'll keep this as a feature request then at this point, and will follow up when progress is made or if we have more questions.