amplify-js icon indicating copy to clipboard operation
amplify-js copied to clipboard

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

Open asp3 opened this issue 1 year ago • 5 comments
trafficstars

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

asp3 avatar May 01 '24 19:05 asp3

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?

chrisbonifacio avatar May 03 '24 15:05 chrisbonifacio

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 avatar May 03 '24 18:05 asp3

@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 avatar May 03 '24 18:05 asp3

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

asp3 avatar May 07 '24 13:05 asp3

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?

chrisbonifacio avatar May 07 '24 18:05 chrisbonifacio

Hi 👋 Closing this as we have not heard back from you. If you are still experiencing this issue and in need of assistance, please feel free to comment and provide us with any information previously requested by our team members so we can re-open this issue and be better able to assist you.

Thank you!

chrisbonifacio avatar May 23 '24 13:05 chrisbonifacio