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

Do not sign requests with boto client #59

Merged

Conversation

balloob
Copy link
Contributor

@balloob balloob commented Oct 14, 2017

Not all requests to Cognito require requests to be signed in. Examples of these are register, authenticate, forgot password, confirm forgot password.

This PR will make sure that request signing is disabled when no auth is passed in. Without this change, botocore will blow up with a NoCredentialsError exception:

  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/warrant/__init__.py", line 289, in authenticate
    tokens = aws.authenticate_user()
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/warrant/aws_srp.py", line 187, in authenticate_user
    ClientId=self.client_id
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/client.py", line 251, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/client.py", line 526, in _make_api_call
    operation_model, request_dict)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/endpoint.py", line 141, in make_request
    return self._send_request(request_dict, operation_model)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/endpoint.py", line 166, in _send_request
    request = self.create_request(request_dict, operation_model)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/endpoint.py", line 150, in create_request
    operation_name=operation_model.name)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/hooks.py", line 227, in emit
    return self._emit(event_name, kwargs)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/hooks.py", line 210, in _emit
    response = handler(**kwargs)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/signers.py", line 90, in handler
    return self.sign(operation_name, request)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/signers.py", line 147, in sign
    auth.add_auth(request)
  File "/Users/paulus/dev/python/home-assistant/lib/python3.6/site-packages/botocore/auth.py", line 316, in add_auth
    raise NoCredentialsError
botocore.exceptions.NoCredentialsError: Unable to locate credentials

@bjinwright bjinwright changed the base branch from master to develop October 14, 2017 14:18
@balloob
Copy link
Contributor Author

balloob commented Nov 18, 2017

@bjinwright anything I can do to help this PR get merged?

@bjinwright
Copy link
Member

Can you add tests?

@bjinwright bjinwright changed the base branch from develop to feature/dont-sign-requests November 25, 2017 15:24
@bjinwright bjinwright merged commit c17715f into capless:feature/dont-sign-requests Nov 25, 2017
@bjinwright
Copy link
Member

I get two failing tests.

======================================================================
ERROR: test_admin_get_user (warrant.tests.tests.CognitoAuthTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/brian/workspace/warrant/warrant/tests/tests.py", line 119, in test_admin_get_user
    u = self.user.admin_get_user()
  File "/home/brian/workspace/warrant/warrant/__init__.py", line 492, in admin_get_user
    Username=self.username)
  File "/home/brian/.virtualenvs/warrant/local/lib/python2.7/site-packages/botocore/client.py", line 251, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/home/brian/.virtualenvs/warrant/local/lib/python2.7/site-packages/botocore/client.py", line 537, in _make_api_call
    raise ClientError(parsed_response, operation_name)
ClientError: An error occurred (MissingAuthenticationTokenException) when calling the AdminGetUser operation: Missing Authentication Token

and

======================================================================
ERROR: test_admin_authenticate (warrant.tests.tests.CognitoAuthTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/brian/workspace/warrant/warrant/tests/tests.py", line 167, in test_admin_authenticate
    self.user.admin_authenticate(self.password)
  File "/home/brian/workspace/warrant/warrant/__init__.py", line 370, in admin_authenticate
    AuthParameters=auth_params,
  File "/home/brian/.virtualenvs/warrant/local/lib/python2.7/site-packages/botocore/client.py", line 251, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/home/brian/.virtualenvs/warrant/local/lib/python2.7/site-packages/botocore/client.py", line 537, in _make_api_call
    raise ClientError(parsed_response, operation_name)
ClientError: An error occurred (MissingAuthenticationTokenException) when calling the AdminInitiateAuth operation: Missing Authentication Token

@bjinwright
Copy link
Member

Please create another PR with the tests fixed.

@balloob balloob deleted the unsigned-client branch November 25, 2017 19:02
@balloob balloob restored the unsigned-client branch November 29, 2017 06:16
@balloob
Copy link
Contributor Author

balloob commented Nov 29, 2017

I think that the way those 2 tests worked before is that they rely on the credentials in ~/.aws of the user running the tests because I did not change the if-statement that is responsible for adding the keys.

alastairmccormack pushed a commit to alastairmccormack/pycognito that referenced this pull request Feb 2, 2022
Bumps [pytest](https://github.com/pytest-dev/pytest) from 6.2.3 to 6.2.4.
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@6.2.3...6.2.4)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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