aws-amplify / amplify-backend

Home to all tools related to Amplify's code-first DX (Gen 2) for building fullstack apps on AWS
Apache License 2.0
184 stars 62 forks source link

Use global lock for auth credential access in tests. #2038

Closed sobolk closed 1 month ago

sobolk commented 1 month ago

Problem

We have at least two tests that are using Amplify Auth to get Cognito credentials. There's a lock guarding access to Amplify Auth apis. However, these tests are using separate factory instances due to different configuration and effectively two separate locks. This is wrong, because Amplify Auth apis need global locking (they're static, i.e. attached to global session).

This sometimes results in errors like this:

https://github.com/aws-amplify/amplify-backend/actions/runs/10964134931/job/30447221560

test at file:/Users/runner/work/amplify-backend/amplify-backend/packages/integration-tests/lib/test-e2e/sandbox.test.js:41:26
✖ generates config after sandbox --once deployment (22038.52175ms)
  NotAuthorizedException: Invalid session for the user.
      at <anonymous> (/Users/runner/work/amplify-backend/amplify-backend/node_modules/@aws-amplify/auth/src/providers/cognito/utils/clients/CognitoIdentityProvider/index.ts:18:19)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at Module.confirmSignIn (/Users/runner/work/amplify-backend/amplify-backend/node_modules/@aws-amplify/auth/src/providers/cognito/apis/confirmSignIn.ts:53:138)
      at AmplifyAuthCredentialsFactory.getNewAuthenticatedUserCredentials (/Users/runner/work/amplify-backend/amplify-backend/packages/integration-tests/src/amplify_auth_credentials_factory.ts:88:7)
      at ConversationHandlerTestProject.assertPostDeployment (/Users/runner/work/amplify-backend/amplify-backend/packages/integration-tests/src/test-project-setup/conversation_handler_project.ts:165:7)
      at TestContext.<anonymous> (/Users/runner/work/amplify-backend/amplify-backend/packages/integration-tests/src/test-e2e/sandbox.test.ts:77:13)
      at async Test.run (node:internal/test_runner/test:632:9)
      at async Suite.processPendingSubtests (node:internal/test_runner/test:374:7) {
    underlyingError: undefined,
    recoverySuggestion: undefined
  }

Changes

Make lock global to match Amplify Auth api scope.

Validation

PR checks with e2e.

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: cee93438cc80aba4d48ca1fb344173711e393e4e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR