Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Add unauthenticated API integration test #1703

Merged
merged 6 commits into from
Sep 3, 2024

Conversation

sichanyoo
Copy link
Contributor

Issue #

Description of changes

  • Adds an integration test for an API call that doesn't use authentication

New/existing dependencies impact assessment, if applicable

Conventional Commits

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

_ = try await cognitoIdentityClient.getId(
input: GetIdInput(accountId: accountID, identityPoolId: identityPoolID)
)
// API call completed successfully
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we check that the deserialized response contains expected results?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, though reaching line 49 would mean the operation succeeded without throwing error, it wouldn't hurt to check it. Adopting suggestion.

@jbelkins jbelkins self-requested a review September 3, 2024 19:29
Copy link
Contributor

@jbelkins jbelkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of this is to prove that unauthenticated requests can be performed successfully. But the machine performing integration tests will have a set of AWS credentials configured for use in all the other tests.

How do we show that this request was generated unauthenticated, and isn't just succeeding because there happen to be credentials on the machine?

@sichanyoo
Copy link
Contributor Author

sichanyoo commented Sep 3, 2024

@jbelkins

We know for sure that the operation used in this new integration test doesn't use authentication and is sending an unauthenticated request because the generated auth scheme resolver for CognitoIdentity's getId operation returns only a single auth scheme smithy.api#noAuth. (see here).

This means when the auth scheme gets resolved during runtime, it resolves as a "no-auth" auth scheme in AuthSchemeMiddleware here, which then subsequently results in no signing occurring during SignerMiddleware here.


Edit:

Added sanity-check for checking that request is indeed unauthenticated using an interceptor, after offline discussion.

@sichanyoo sichanyoo merged commit 9369785 into main Sep 3, 2024
29 checks passed
@jbelkins
Copy link
Contributor

jbelkins commented Sep 3, 2024

A check was added to the test to ensure that the HTTP Authentication: header is not set when making an unauthenticated request. In response, I have approved.

@sichanyoo sichanyoo deleted the chore/unauthenticated-api-integ-test branch September 3, 2024 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants