fix(amplify-category-auth): clean up OIDC provider on auth stack teardown

[ahmedhamouda78]

Description of changes

The auth category's OpenId custom resource Lambda (openIdLambda.js) only handled Create/Update events. On stack deletion CloudFormation sends a Delete event, which fell through to a no-op — so the account-global IAM OIDC provider (accounts.google.com) was never removed. Orphaned providers accumulate in the account on every teardown of an auth resource configured with Google/OpenID federation.

This PR:

• Adds a Delete handler to openIdLambda.js. Because the provider is account-global and keyed by URL (the create path reuses it and appends client IDs), the handler removes only the client IDs this resource registered and deletes the provider only once no client IDs remain — preserving providers shared by other Amplify apps in the same account. All IAM calls tolerate an already-removed provider/client ID, so the handler is idempotent.
• Grants the OpenId Lambda role iam:RemoveClientIDFromOpenIDConnectProvider and iam:DeleteOpenIDConnectProvider (scoped to the accounts.google.com provider ARN) in auth-cognito-stack-builder.ts.
• Adds unit tests for create, sole-owner delete, shared-provider delete, and idempotent delete.

Issue #, if available

N/A

Description of how you validated changes

• New Jest unit test openIdLambda.test.js passes (6/6), driving the real handler through: create, update-appends, sole-owner delete, shared-provider delete, concurrent-deletion race, and idempotent delete.
• Confirmed the IAM policy change does not alter existing stack-builder snapshots.

Checklist

• PR description included
• Tests are changed or added
• New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies

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

[ahmedhamouda78]

Thanks for the thorough review @soberm — all six points addressed in b16780b:

• [major] Concurrent-deletion race: After removing our client IDs I now re-read the provider and delete it if the list is empty, so two stacks racing on a shared provider can't leave it orphaned with zero client IDs. The existing NoSuchEntityException catch handles the case where the other deleter wins the final delete. Added a deletes the provider when a concurrent stack empties it during our Delete test that simulates the interleaving.
• [minor] Fragile URL matching: findProviderArn now derives the host via new URL(url).host and matches the provider ARN suffix. One correction to the suggested snippet: real OIDC provider ARNs use :oidc-provider/<host> (colon), e.g. arn:aws:iam::<account>:oidc-provider/accounts.google.com, so I matched on :oidc-provider/${providerHost} rather than /oidc-provider/... — the latter never matches.
• [minor] ListOpenIDConnectProviders: It's granted in the adjacent statement (account-level, Resource: *) since it can't be scoped to a single provider ARN. Added a comment there noting it's required by findProviderArn.
• [minor] invoke() guard: Now throws handler never called response.send instead of returning undefined.
• [nit] URL shadowing: Renamed the test constant to GOOGLE_OIDC_URL.
• [nit] Update coverage: Added a test that creates, then updates with an additional client ID and asserts it's appended.

All six unit tests pass locally under CI-like conditions (runtime-only deps mocked virtually).

[sarayev]

Review — Approve with comments ✅

Solid fix for a real latent leak: the OpenId custom-resource Lambda never handled Delete, so the accounts.google.com OIDC provider (+ IAM resources) leaked on stack teardown. The Delete handler (removes only this stack's client IDs, deletes the provider only when empty), scoped IAM perms, idempotency (NoSuchEntityException tolerance), shared-provider safety, and the new URL(url).host refactor are all good. 6-scenario test file is a nice addition.

Should-fix (cheap, none break the happy path)

1. Inaccurate comment — auth-cognito-stack-builder.ts:~1073. Says ListOpenIDConnectProviders must use Resource '*', but the code scopes to arn:aws:iam::<acct>:oidc-provider/* (via selector: '*'). Misleading — reword to describe the wildcard provider ARN.
2. Asymmetric race — openIdLambda.js:~93 (sole-owner branch). Shared-provider branch re-reads before deleting; sole-owner deletes from the stale initial read. A concurrent Create could be destroyed. Mirror the re-read (Get → confirm empty → delete).
3. Missing url null-guard. clientIdList got a guard this PR; url didn't → new URL(undefined) throws TypeError. Add event.ResourceProperties.url ?? '' (or validate early).

Nits

4. PR body says 4 tests; file has 6 — update the count.
5. openIdLambda.test.js:5 mutates module-level mockState (guideline: reset a let in beforeEach).
6. Shared-provider test injects state directly; prefer a 2nd Create to set up shared state (also exercises add-client-ID path).
7. Pre-existing gap (not this PR): no snapshot coverage of the OpenId IAM policy.

Please run the full split e2e + all PR checks and get them green before merge.

[ahmedhamouda78]

Thanks @sarayev — addressed in d153b19:

Should-fix

1. Inaccurate IAM comment: Reworded — ListOpenIDConnectProviders is granted against the wildcard provider ARN (oidc-provider/*) in this account, not Resource '*'.
2. Asymmetric race (sole-owner branch): The sole-owner branch now re-reads immediately before deleting (Get → confirm nothing but our client IDs remain → delete). If a concurrent Create added client IDs (or re-created the provider) after our first read, we fall back to removing only our client IDs instead of deleting. Both branches are now symmetric.
3. url null-guard: findProviderArn returns early when url is falsy, so new URL(undefined) can no longer throw.

Nits
4. Updated the PR description (6 tests, not 4).
5. mockState is now reset via reassignment (let + beforeEach).
6. The shared-provider test now sets up shared state with a real second Create (which finds the existing provider and adds its client ID), exercising the add-client-ID path instead of injecting state.
7. Snapshot coverage of the OpenId IAM policy — agreed it's a pre-existing gap; leaving it out of this PR's scope.

All 6 unit tests pass, and prettier/lint is now clean (the earlier _lint failure was a prettier formatting miss on the test file, fixed here).

[sarayev]

Thanks for addressing the feedback — approving, this looks good to me.