fix: Add getId to enhanced authflow

[qhanam]

Cognito recently onboarded CloudWatch RUM to its enhanced authflow. However, the enhanced authflow implemented in the web client is currently broken because it uses the identity pool Id, instead of the identity Id, as the argument for getCredentialsForIdentity.

This change modifies the enhanced authflow to fetch the identity Id before calling getCredentialsForIdentity.

---

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

[williazz]

At first glance, the unit test when getCredentialsForIdentity called then credentials are returned is failing. I reran to approval workflow and it failed a second time. Will take a closer look

[ps863]

question: Should we update the documentation here (specifically for the Guest Role ARN), to reflect that Enhanced auth flow will be used if its omitted?

[ps863]

I think the title for this unit test and some others in this file still reference basic auth flow.

1. The names of the tests need to be updated. (Example)
2. We should remove guest role arn from the mocked web client config when using enhanced auth flow (Example)

[qhanam]

Good catch, however I don't think fixing these fall within the scope of this change.

[qhanam]

question: Should we update the documentation here (specifically for the Guest Role ARN), to reflect that Enhanced auth flow will be used if its omitted?

Yes, and probably update the examples to show the enhanced authflow instead of the basic authflow.

I think we should do make these doc changes in a separate PR, as the purpose of this change is to patch a bug.

[qhanam]

At first glance, the unit test when getCredentialsForIdentity called then credentials are returned is failing. I reran to approval workflow and it failed a second time. Will take a closer look

Oops, I didn't wait for the CI checks to finish 😬 -- fixed

[ps863]

Looks good to me.
We can do future PRs with:

• New docs and doc updates
• Update the test names

[williazz]

nit: is GetCredentialsForIdentity supposed to return secretKey instead of secretAccessKey?

https://docs.aws.amazon.com/cognitoidentity/latest/APIReference/API_GetCredentialsForIdentity.html

[qhanam]

The CognitoIdentityClient, which is implemented by the RUM web client (recall we don't use the AWS SDK here), returns secretAccessKey. This is by design, because it returns a Credential object, which has the property secretAccessKey.

One could make the argument that the CognitoIdentityClient should have behavior which is consistent with the actual Cognito client. However, I've altered this behavior for the sake of maintainability and reducing the size of the web client bundle.

[williazz]

Looks good. Double checked by reviewing the enhanced auth flow and looking at the documentation for GetCredentialsForIdentity.

Q: What was the impact of this bug? And how long has GetCredentialsForIdentity required identityId instead of identityPoolId?

[qhanam]

Q: What was the impact of this bug? And how long has GetCredentialsForIdentity required identityId instead of identityPoolId?

Cognito's enhanced authflow doesn't support RUM, so it has never been integration tested and no one is using it.

That does raise the question of "How do we integration test unauthenicated authorization?". I think the answer to this question is that we test during smoke tests. We should update the smoke tests accordingly before we "release" enhanced authflow by updating docs.