aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.36k stars 2.1k forks source link

Cognito Identity API Call made even when auth is explicitly passed in #13327

Open asp3 opened 2 weeks ago

asp3 commented 2 weeks ago

Before opening, please confirm:

JavaScript Framework

React Native, Next.js

Amplify APIs

Authentication, GraphQL API

Amplify Version

v6

Amplify Categories

auth, api

Backend

None

Environment information

``` System: OS: macOS 14.4.1 CPU: (12) arm64 Apple M2 Max Memory: 2.65 GB / 64.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 20.12.1 - ~/.nvm/versions/node/v20.12.1/bin/node Yarn: 1.22.5 - ~/.yarn/bin/yarn npm: 10.5.0 - ~/.nvm/versions/node/v20.12.1/bin/npm Watchman: 2024.01.22.00 - /usr/local/bin/watchman Browsers: Brave Browser: 114.1.52.130 Chrome: 124.0.6367.118 Safari: 17.4.1 npmPackages: @knowt/eslint-config: * => 0.0.0 dotenv-cli: latest => 7.4.1 husky: ^8.0.0 => 8.0.3 lint-staged: ^12.4.0 => 12.5.0 prettier: ^2.7.1 => 2.8.8 turbo: ^1.10.12 => 1.13.0 npmGlobalPackages: @aws-amplify/cli: 12.11.0 corepack: 0.25.2 npm: 10.5.0 ytdl: 1.4.1 ```

Describe the bug

After reading #13323, I noticed that we are seeing some related problems on our end as well.

image

We fetch our own idToken using fetchAuthSession (which i think completes without an internet connection).

export const amplifyAuthOverride = {
    API: {
        GraphQL: {
            headers: async () => {
                const idToken = (await fetchAuthSession()).tokens?.idToken?.toString();

                if (!idToken) return {};
                return {
                    Authorization: idToken,
                };
            },
        },
    },
};

....
// ConfigureAmplify.tsx
Amplify.configure(awsConfig, {
    ssr: true,
    ...amplifyAuthOverride,
});

export default function ConfigureAmplify() {
    return null;
}

However, the ensuing graphql calls seem to call the IDP api for the session values, even when an Auth header has been explicitly passed in

image

Expected behavior

I expect that if an auth is explicity provided, we do not need to make a call to cognito IDP to revalidate the token. Even if a network call is needed, the returned value of the identity call

AccessKeyId: "..."
Expiration: 1714595836 (in one hour)
SecretKey: "...."
SessionToken: "..."

should be reused instead of making another call to fetch it again.

Reproduction steps

  1. set up GraphQL API in a next js app as per the docs.
  2. Set up API override to use idToken given
  3. make an API call on client or server side, and inspect the network logs (you will see that for N GraphQL calls, even if the user is properly authenticated, N cognito-identity.us-east-1.amazonaws.com/ calls are made.

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

No response

chrisbonifacio commented 2 weeks ago

Hi @asp3, I was not able to reproduce this issue on the latest version of aws-amplify.

I tried with and without overriding the Authorization header with an id token

Both resulted in only two initial calls to Cognito, one for GetId and one for GetCredentialsForIdentity

image

Can you share what version of the aws-ampllify library you are using and any relevant code snippets to help with reproduction on our end?

Also, just a thought, but are you configuring the expiration for your tokens anywhere?

asp3 commented 2 weeks ago

Hi @chrisbonifacio, thanks for the quick response! So in that repro example, if you reload the page, are there no more cognito-identity calls on the second one since we pull it from the cookies? Or do those 2 calls happen each time regardless?

we are currently on amplify v6.0.21, as we had some issues when upgrading to the new version around our patch files around api-graphql.

Here's our patch, if it gives some insight, but i dont think it should have anything to do with the error.

The goals of this patch:

  1. for iam, we dont need to make a server call, we are sure the session will be defined at the time of the API call.
  2. Since we are always passing in our own Authorization, theres no need for the fetchAuthSession call (which on the server, makes an API call)
  3. if we don't pass an auth (in the future), and the user isnt logged in, fallback to an IAM call after userpool call fails.
diff --git a/node_modules/@aws-amplify/api-graphql/dist/cjs/internals/InternalGraphQLAPI.js b/node_modules/@aws-amplify/api-graphql/dist/cjs/internals/InternalGraphQLAPI.js
index c5d9b6a..f0c6ce7 100644
--- a/node_modules/@aws-amplify/api-graphql/dist/cjs/internals/InternalGraphQLAPI.js
+++ b/node_modules/@aws-amplify/api-graphql/dist/cjs/internals/InternalGraphQLAPI.js
@@ -51,16 +51,16 @@ class InternalGraphQLAPIClass {
                 };
                 break;
             case 'iam':
-                const session = await amplify.Auth.fetchAuthSession();
-                if (session.credentials === undefined) {
-                    throw new Error(types_1.GraphQLAuthError.NO_CREDENTIALS);
-                }
                 break;
             case 'oidc':
             case 'userPool':
                 try {
+                    if (additionalHeaders.Authorization) {
+                       break;
+                   }
+
                     let token;
-                    token = (await amplify.Auth.fetchAuthSession()).tokens?.accessToken.toString();
+                    token = (await amplify.Auth.fetchAuthSession()).tokens?.idToken.toString();
                     if (!token) {
                         throw new Error(types_1.GraphQLAuthError.NO_FEDERATED_JWT);
                     }
@@ -69,7 +69,7 @@ class InternalGraphQLAPIClass {
                     };
                 }
                 catch (e) {
-                    throw new Error(types_1.GraphQLAuthError.NO_CURRENT_USER);
+                    // pass, call with no authorization header
                 }
                 break;
             case 'lambda':

// duplicated for the esm module, and original ts file
...

Otherwise, all code is standard, except for the fact that we are passing in our own authorization headers, with the snippet I attached in the original issue:

export const amplifyAuthOverride = {
    API: {
        GraphQL: {
            headers: async () => {
                const idToken = (await fetchAuthSession()).tokens?.idToken?.toString();

                if (!idToken) return {};
                return {
                    Authorization: idToken,
                };
            },
        },
    },
};
asp3 commented 2 weeks ago

@chrisbonifacio upgraded to 6.2.0, and still having the same issue, so I don't think its a version issue! Please let me know if there's any other information I can provide.

Seems like there are 6 GetId calls, and 6 GetCredentialsForIdentity calls, followed by 6 graphql calls.

https://github.com/aws-amplify/amplify-js/assets/11220954/18eedb67-5090-41c9-9ac2-eba0efab1363

asp3 commented 1 week ago

Hi @chrisbonifacio, just wondering if there is an update here!

chrisbonifacio commented 1 week ago

Hi @asp3 , I noticed that you had ssr enabled and missed that on my initial reproduction. However, even enabling it I am unable to reproduce the issue. Would you be able to provide a minimal sample app that reproduces the issue for us to troubleshoot on our end?