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

[v2] SSOTokenLoader Expiration Check #9356

Open
wants to merge 18 commits into
base: v2
Choose a base branch
from

Conversation

aemous
Copy link
Contributor

@aemous aemous commented Mar 11, 2025

Description of changes:

  • Updated SSOTokenLoader to check if the cached legacy token is expired according to the local clock. If expired, it will raise an UnauthorizedSSOTokenError instead of sending an expired token to Identity Center's GetRoleCredentials API.
  • Added a unit test to verify when an expired token is cached, that GetRoleCredentials is not called and UnauthorizedSSOTokenError is raised.

Description of tests:

  • Ran all test suites (unit, functional, integration, etc.)
  • The following manual workflow was completed to test the code.
    1. Login to a user account using the legacy flow via aws sso login.
    2. Let the cached token expire.
    3. Ran a command using the SSO profile (e.g. aws s3 ls --profile SSOProfile --debug)
    4. Verified an UnauthorizedSSOTokenError was raised, and checked the stacktrace (pasted below) to verify this exception was raised without GetRoleCredentials being called.

Stacktrace from manual workflow on this branch

 File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 519, in _protected_refresh
    metadata = self._refresh_using()
               ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 660, in fetch_credentials
    return self._get_cached_credentials()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 670, in _get_cached_credentials
    response = self._get_credentials()
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 2056, in _get_credentials
    raise UnauthorizedSSOTokenError()

Stacktrace from manual workflow on v2

Traceback (most recent call last):
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 518, in _protected_refresh
    metadata = self._refresh_using()
               ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 659, in fetch_credentials
    return self._get_cached_credentials()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 669, in _get_cached_credentials
    response = self._get_credentials()
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 2055, in _get_credentials
    raise UnauthorizedSSOTokenError()
botocore.exceptions.UnauthorizedSSOTokenError: The SSO session associated with this profile has expired or is otherwise invalid. To refresh this SSO session run aws sso login with the corresponding profile.
2025-03-13 10:45:14,232 - MainThread - awscli.clidriver - DEBUG - Exception caught in main()
Traceback (most recent call last):
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/credentials.py", line 2053, in _get_credentials
    response = client.get_role_credentials(**kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/client.py", line 365, in _api_call
    return self._make_api_call(operation_name, kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/context.py", line 124, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aemous/GitHub/aws-cli2/awscli/botocore/client.py", line 752, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.errorfactory.UnauthorizedException: An error occurred (UnauthorizedException) when calling the GetRoleCredentials operation: Session token not found or invalid

Notice the above exception is raised due to a returned UnauthorizedException from GetRoleCredentials, while this is not the case in the previous stacktrace.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@aemous aemous marked this pull request as ready for review March 12, 2025 22:22
@aemous aemous requested review from ashovlin and a team March 12, 2025 22:23
Copy link
Member

@ashovlin ashovlin left a comment

Choose a reason for hiding this comment

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

  1. I think it's still worth a changelog entry explaining the change in behavior.
  2. The Windows test failures look valid.
  3. Can you add a test case asserting that the expected error was thrown and that no calls were made to GetRoleCredentials?

@aemous aemous requested a review from ashovlin March 20, 2025 15:28
expiration = dateutil.parser.parse(token_dict['expiresAt'])
remaining = total_seconds(expiration - self._time_fetcher())
if remaining <= 0:
raise UnauthorizedSSOTokenError()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reviewer:

On one hand, I think it's unfair to claim the token is "Unauthorized" without the server stating this, since the server is the source of truth on this.

On the other hand, if we use any other exception (e.g. a new ExpiredTokenException), then that would theoretically be a breaking change, since we're changing the exception in a code path that always (?) throws an exception.

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.

2 participants