diff options
author | Jonathan Huot <JonathanHuot@users.noreply.github.com> | 2018-05-25 16:09:44 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-05-25 16:09:44 +0200 |
commit | dd120a5c4ccdcc5e3de85746266990192202203d (patch) | |
tree | df62b1c48ced1a91facc8cb10167cec720679659 | |
parent | 8130cdc06fbf17d8cf2805a454eea3019e45c593 (diff) | |
parent | 6da09f284593546daac545d625f68014d7464c39 (diff) | |
download | oauthlib-dd120a5c4ccdcc5e3de85746266990192202203d.tar.gz |
Merge branch 'master' into master
-rw-r--r-- | .coveragerc | 20 | ||||
-rw-r--r-- | .travis.yml | 2 | ||||
-rw-r--r-- | AUTHORS | 1 | ||||
-rw-r--r-- | docs/oauth1/security.rst | 12 | ||||
-rw-r--r-- | examples/skeleton_oauth2_web_application_server.py | 2 | ||||
-rw-r--r-- | oauthlib/common.py | 11 | ||||
-rw-r--r-- | oauthlib/oauth1/rfc5849/parameters.py | 2 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/clients/base.py | 23 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/clients/mobile_application.py | 2 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/clients/service_application.py | 4 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/clients/web_application.py | 2 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/grant_types/authorization_code.py | 3 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/request_validator.py | 2 | ||||
-rw-r--r-- | oauthlib/signals.py | 2 | ||||
-rw-r--r-- | tests/oauth2/rfc6749/clients/test_base.py | 72 | ||||
-rw-r--r-- | tests/oauth2/rfc6749/clients/test_mobile_application.py | 12 | ||||
-rw-r--r-- | tests/oauth2/rfc6749/clients/test_service_application.py | 70 | ||||
-rw-r--r-- | tests/oauth2/rfc6749/clients/test_web_application.py | 19 |
18 files changed, 233 insertions, 28 deletions
diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000..70666c7 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,20 @@ +[run] +branch = 1 +cover_pylib = 0 +include=*oauthlib/* +omit = oauthlib.tests.* + +[report] +omit = + */python?.?/* + */site-packages/* + */pypy/* +exclude_lines = + pragma: no cover + def __repr__ + if __debug__: + raise AssertionError + raise NotImplementedError + if 0: + if __name__ == .__main__.: + noqa diff --git a/.travis.yml b/.travis.yml index 0a7d8ad..dd72d5c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,6 +34,7 @@ deploy: on: tags: true all_branches: true + condition: $TOXENV = py36 repo: oauthlib/oauthlib - provider: releases api_key: @@ -42,4 +43,5 @@ deploy: on: tags: true all_branches: true + condition: $TOXENV = py36 repo: oauthlib/oauthlib @@ -28,3 +28,4 @@ Joel Stevenson Brendan McCollam Jonathan Huot Pieter Ennes +Olaf Conradi diff --git a/docs/oauth1/security.rst b/docs/oauth1/security.rst index a1432a9..df1e2a0 100644 --- a/docs/oauth1/security.rst +++ b/docs/oauth1/security.rst @@ -16,11 +16,13 @@ A few important facts regarding OAuth security * **Tokens must be random**, OAuthLib provides a method for generating secure tokens and it's packed into ``oauthlib.common.generate_token``, - use it. If you decide to roll your own, use ``random.SystemRandom`` - which is based on ``os.urandom`` rather than the default ``random`` - based on the effecient but not truly random Mersenne Twister. - Predictable tokens allow attackers to bypass virtually all defences - OAuth provides. + use it. If you decide to roll your own, use ``secrets.SystemRandom`` + for Python 3.6 and later. The ``secrets`` module is designed for + generating cryptographically strong random numbers. For earlier versions + of Python, use ``random.SystemRandom`` which is based on ``os.urandom`` + rather than the default ``random`` based on the effecient but not truly + random Mersenne Twister. Predictable tokens allow attackers to bypass + virtually all defences OAuth provides. * **Timing attacks are real** and more than possible if you host your application inside a shared datacenter. Ensure all ``validate_`` methods diff --git a/examples/skeleton_oauth2_web_application_server.py b/examples/skeleton_oauth2_web_application_server.py index 8bfd936..e53232f 100644 --- a/examples/skeleton_oauth2_web_application_server.py +++ b/examples/skeleton_oauth2_web_application_server.py @@ -67,7 +67,7 @@ class SkeletonValidator(RequestValidator): # state and user to request.scopes and request.user. pass - def confirm_redirect_uri(self, client_id, code, redirect_uri, client, *args, **kwargs): + def confirm_redirect_uri(self, client_id, code, redirect_uri, client, request, *args, **kwargs): # You did save the redirect uri with the authorization code right? pass diff --git a/oauthlib/common.py b/oauthlib/common.py index afcc09c..f25656f 100644 --- a/oauthlib/common.py +++ b/oauthlib/common.py @@ -11,12 +11,17 @@ from __future__ import absolute_import, unicode_literals import collections import datetime import logging -import random import re import sys import time try: + from secrets import randbits + from secrets import SystemRandom +except ImportError: + from random import getrandbits as randbits + from random import SystemRandom +try: from urllib import quote as _quote from urllib import unquote as _unquote from urllib import urlencode as _urlencode @@ -202,7 +207,7 @@ def generate_nonce(): .. _`section 3.2.1`: https://tools.ietf.org/html/draft-ietf-oauth-v2-http-mac-01#section-3.2.1 .. _`section 3.3`: https://tools.ietf.org/html/rfc5849#section-3.3 """ - return unicode_type(unicode_type(random.getrandbits(64)) + generate_timestamp()) + return unicode_type(unicode_type(randbits(64)) + generate_timestamp()) def generate_timestamp(): @@ -225,7 +230,7 @@ def generate_token(length=30, chars=UNICODE_ASCII_CHARACTER_SET): and entropy when generating the random characters is important. Which is why SystemRandom is used instead of the default random.choice method. """ - rand = random.SystemRandom() + rand = SystemRandom() return ''.join(rand.choice(chars) for x in range(length)) diff --git a/oauthlib/oauth1/rfc5849/parameters.py b/oauthlib/oauth1/rfc5849/parameters.py index 2f068a7..db4400e 100644 --- a/oauthlib/oauth1/rfc5849/parameters.py +++ b/oauthlib/oauth1/rfc5849/parameters.py @@ -15,7 +15,7 @@ from . import utils try: from urlparse import urlparse, urlunparse -except ImportError: +except ImportError: # noqa from urllib.parse import urlparse, urlunparse diff --git a/oauthlib/oauth2/rfc6749/clients/base.py b/oauthlib/oauth2/rfc6749/clients/base.py index a07a5c9..07ef894 100644 --- a/oauthlib/oauth2/rfc6749/clients/base.py +++ b/oauthlib/oauth2/rfc6749/clients/base.py @@ -9,6 +9,7 @@ for consuming OAuth 2.0 RFC6749. from __future__ import absolute_import, unicode_literals import time +import warnings from oauthlib.common import generate_token from oauthlib.oauth2.rfc6749 import tokens @@ -111,8 +112,10 @@ class Client(object): self.state_generator = state_generator self.state = state self.redirect_url = redirect_url + self.code = None + self.expires_in = None self._expires_at = None - self._populate_attributes(self.token) + self.populate_token_attributes(self.token) @property def token_types(self): @@ -406,7 +409,7 @@ class Client(object): .. _`Section 7.1`: https://tools.ietf.org/html/rfc6749#section-7.1 """ self.token = parse_token_response(body, scope=scope) - self._populate_attributes(self.token) + self.populate_token_attributes(self.token) return self.token def prepare_refresh_body(self, body='', refresh_token=None, scope=None, **kwargs): @@ -460,7 +463,18 @@ class Client(object): return uri, headers, body def _populate_attributes(self, response): - """Add commonly used values such as access_token to self.""" + warnings.warn("Please switch to the public method " + "populate_token_attributes.", DeprecationWarning) + return self.populate_token_attributes(response) + + def populate_code_attributes(self, response): + """Add attributes from an auth code response to self.""" + + if 'code' in response: + self.code = response.get('code') + + def populate_token_attributes(self, response): + """Add attributes from a token exchange response to self.""" if 'access_token' in response: self.access_token = response.get('access_token') @@ -478,9 +492,6 @@ class Client(object): if 'expires_at' in response: self._expires_at = int(response.get('expires_at')) - if 'code' in response: - self.code = response.get('code') - if 'mac_key' in response: self.mac_key = response.get('mac_key') diff --git a/oauthlib/oauth2/rfc6749/clients/mobile_application.py b/oauthlib/oauth2/rfc6749/clients/mobile_application.py index 311aacf..aa20daa 100644 --- a/oauthlib/oauth2/rfc6749/clients/mobile_application.py +++ b/oauthlib/oauth2/rfc6749/clients/mobile_application.py @@ -168,5 +168,5 @@ class MobileApplicationClient(Client): .. _`Section 3.3`: https://tools.ietf.org/html/rfc6749#section-3.3 """ self.token = parse_implicit_response(uri, state=state, scope=scope) - self._populate_attributes(self.token) + self.populate_token_attributes(self.token) return self.token diff --git a/oauthlib/oauth2/rfc6749/clients/service_application.py b/oauthlib/oauth2/rfc6749/clients/service_application.py index 84ea0e9..7f336bb 100644 --- a/oauthlib/oauth2/rfc6749/clients/service_application.py +++ b/oauthlib/oauth2/rfc6749/clients/service_application.py @@ -146,8 +146,8 @@ class ServiceApplicationClient(Client): ' token requests.') claim = { 'iss': issuer or self.issuer, - 'aud': audience or self.issuer, - 'sub': subject or self.issuer, + 'aud': audience or self.audience, + 'sub': subject or self.subject, 'exp': int(expires_at or time.time() + 3600), 'iat': int(issued_at or time.time()), } diff --git a/oauthlib/oauth2/rfc6749/clients/web_application.py b/oauthlib/oauth2/rfc6749/clients/web_application.py index 14b5265..c14a5f8 100644 --- a/oauthlib/oauth2/rfc6749/clients/web_application.py +++ b/oauthlib/oauth2/rfc6749/clients/web_application.py @@ -172,5 +172,5 @@ class WebApplicationClient(Client): oauthlib.oauth2.rfc6749.errors.MismatchingStateError """ response = parse_authorization_code_response(uri, state=state) - self._populate_attributes(response) + self.populate_code_attributes(response) return response diff --git a/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py b/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py index 7bea650..0660263 100644 --- a/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py +++ b/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py @@ -421,7 +421,8 @@ class AuthorizationCodeGrant(GrantTypeBase): # authorization request as described in Section 4.1.1, and their # values MUST be identical. if not self.request_validator.confirm_redirect_uri(request.client_id, request.code, - request.redirect_uri, request.client): + request.redirect_uri, request.client, + request): log.debug('Redirect_uri (%r) invalid for client %r (%r).', request.redirect_uri, request.client_id, request.client) raise errors.MismatchingRedirectURIError(request=request) diff --git a/oauthlib/oauth2/rfc6749/request_validator.py b/oauthlib/oauth2/rfc6749/request_validator.py index 182642e..c0b69a1 100644 --- a/oauthlib/oauth2/rfc6749/request_validator.py +++ b/oauthlib/oauth2/rfc6749/request_validator.py @@ -82,7 +82,7 @@ class RequestValidator(object): """ raise NotImplementedError('Subclasses must implement this method.') - def confirm_redirect_uri(self, client_id, code, redirect_uri, client, + def confirm_redirect_uri(self, client_id, code, redirect_uri, client, request, *args, **kwargs): """Ensure that the authorization process represented by this authorization code began with this 'redirect_uri'. diff --git a/oauthlib/signals.py b/oauthlib/signals.py index 2f86650..22d47a4 100644 --- a/oauthlib/signals.py +++ b/oauthlib/signals.py @@ -8,7 +8,7 @@ signals_available = False try: from blinker import Namespace signals_available = True -except ImportError: +except ImportError: # noqa class Namespace(object): def signal(self, name, doc=None): return _FakeSignal(name, doc) diff --git a/tests/oauth2/rfc6749/clients/test_base.py b/tests/oauth2/rfc6749/clients/test_base.py index c788bc1..d48a944 100644 --- a/tests/oauth2/rfc6749/clients/test_base.py +++ b/tests/oauth2/rfc6749/clients/test_base.py @@ -4,7 +4,7 @@ from __future__ import absolute_import, unicode_literals import datetime from oauthlib import common -from oauthlib.oauth2 import Client, InsecureTransportError +from oauthlib.oauth2 import Client, InsecureTransportError, TokenExpiredError from oauthlib.oauth2.rfc6749 import utils from oauthlib.oauth2.rfc6749.clients import AUTH_HEADER, BODY, URI_QUERY @@ -51,10 +51,26 @@ class ClientTest(TestCase): self.assertFormBodyEqual(body, self.body) self.assertEqual(headers, self.bearer_header) + # Non-HTTPS + insecure_uri = 'http://example.com/path?query=world' + client = Client(self.client_id, access_token=self.access_token, token_type="Bearer") + self.assertRaises(InsecureTransportError, client.add_token, insecure_uri, + body=self.body, + headers=self.headers) + # Missing access token client = Client(self.client_id) self.assertRaises(ValueError, client.add_token, self.uri) + # Expired token + expired = 523549800 + expired_token = { + 'expires_at': expired, + } + client = Client(self.client_id, token=expired_token, access_token=self.access_token, token_type="Bearer") + self.assertRaises(TokenExpiredError, client.add_token, self.uri, + body=self.body, headers=self.headers) + # The default token placement, bearer in auth header client = Client(self.client_id, access_token=self.access_token) uri, headers, body = client.add_token(self.uri, body=self.body, @@ -150,8 +166,26 @@ class ClientTest(TestCase): self.assertEqual(uri, self.uri) self.assertEqual(body, self.body) self.assertEqual(headers, self.mac_00_header) + # Non-HTTPS + insecure_uri = 'http://example.com/path?query=world' + self.assertRaises(InsecureTransportError, client.add_token, insecure_uri, + body=self.body, + headers=self.headers, + issue_time=datetime.datetime.now()) + # Expired Token + expired = 523549800 + expired_token = { + 'expires_at': expired, + } + client = Client(self.client_id, token=expired_token, token_type="MAC", + access_token=self.access_token, mac_key=self.mac_key, + mac_algorithm="hmac-sha-1") + self.assertRaises(TokenExpiredError, client.add_token, self.uri, + body=self.body, + headers=self.headers, + issue_time=datetime.datetime.now()) - # Add the Authorization header (draft 00) + # Add the Authorization header (draft 01) client = Client(self.client_id, token_type="MAC", access_token=self.access_token, mac_key=self.mac_key, mac_algorithm="hmac-sha-1") @@ -160,7 +194,24 @@ class ClientTest(TestCase): self.assertEqual(uri, self.uri) self.assertEqual(body, self.body) self.assertEqual(headers, self.mac_01_header) - + # Non-HTTPS + insecure_uri = 'http://example.com/path?query=world' + self.assertRaises(InsecureTransportError, client.add_token, insecure_uri, + body=self.body, + headers=self.headers, + draft=1) + # Expired Token + expired = 523549800 + expired_token = { + 'expires_at': expired, + } + client = Client(self.client_id, token=expired_token, token_type="MAC", + access_token=self.access_token, mac_key=self.mac_key, + mac_algorithm="hmac-sha-1") + self.assertRaises(TokenExpiredError, client.add_token, self.uri, + body=self.body, + headers=self.headers, + draft=1) def test_revocation_request(self): client = Client(self.client_id) @@ -208,6 +259,21 @@ class ClientTest(TestCase): # NotImplementedError self.assertRaises(NotImplementedError, client.prepare_authorization_request, auth_url) + def test_prepare_token_request(self): + redirect_url = 'https://example.com/callback/' + scopes = 'read' + token_url = 'https://example.com/token/' + state = 'fake_state' + + client = Client(self.client_id, scope=scopes, state=state) + + # Non-HTTPS + self.assertRaises(InsecureTransportError, + client.prepare_token_request, 'http://example.com/token/') + + # NotImplementedError + self.assertRaises(NotImplementedError, client.prepare_token_request, token_url) + def test_prepare_refresh_token_request(self): client = Client(self.client_id) diff --git a/tests/oauth2/rfc6749/clients/test_mobile_application.py b/tests/oauth2/rfc6749/clients/test_mobile_application.py index 309220b..51e4dab 100644 --- a/tests/oauth2/rfc6749/clients/test_mobile_application.py +++ b/tests/oauth2/rfc6749/clients/test_mobile_application.py @@ -69,6 +69,18 @@ class MobileApplicationClientTest(TestCase): uri = client.prepare_request_uri(self.uri, **self.kwargs) self.assertURLEqual(uri, self.uri_kwargs) + def test_populate_attributes(self): + + client = MobileApplicationClient(self.client_id) + + response_uri = (self.response_uri + "&code=EVIL-CODE") + + client.parse_request_uri_response(response_uri, scope=self.scope) + + # We must not accidentally pick up any further security + # credentials at this point. + self.assertIsNone(client.code) + def test_parse_token_response(self): client = MobileApplicationClient(self.client_id) diff --git a/tests/oauth2/rfc6749/clients/test_service_application.py b/tests/oauth2/rfc6749/clients/test_service_application.py index 2dc633a..dc337cf 100644 --- a/tests/oauth2/rfc6749/clients/test_service_application.py +++ b/tests/oauth2/rfc6749/clients/test_service_application.py @@ -89,8 +89,8 @@ mfvGGg3xNjTMO7IdrwIDAQAB audience=self.audience, body=self.body) r = Request('https://a.b', body=body) - self.assertEqual(r.isnot, 'empty') - self.assertEqual(r.grant_type, ServiceApplicationClient.grant_type) + self.assertEqual(r.isnot, 'empty') + self.assertEqual(r.grant_type, ServiceApplicationClient.grant_type) claim = jwt.decode(r.assertion, self.public_key, audience=self.audience, algorithms=['RS256']) @@ -98,6 +98,72 @@ mfvGGg3xNjTMO7IdrwIDAQAB # audience verification is handled during decode now self.assertEqual(claim['sub'], self.subject) self.assertEqual(claim['iat'], int(t.return_value)) + self.assertNotIn('nbf', claim) + self.assertNotIn('jti', claim) + + # Missing issuer parameter + self.assertRaises(ValueError, client.prepare_request_body, + issuer=None, subject=self.subject, audience=self.audience, body=self.body) + + # Missing subject parameter + self.assertRaises(ValueError, client.prepare_request_body, + issuer=self.issuer, subject=None, audience=self.audience, body=self.body) + + # Missing audience parameter + self.assertRaises(ValueError, client.prepare_request_body, + issuer=self.issuer, subject=self.subject, audience=None, body=self.body) + + # Optional kwargs + not_before = time() - 3600 + jwt_id = '8zd15df4s35f43sd' + body = client.prepare_request_body(issuer=self.issuer, + subject=self.subject, + audience=self.audience, + body=self.body, + not_before=not_before, + jwt_id=jwt_id) + + r = Request('https://a.b', body=body) + self.assertEqual(r.isnot, 'empty') + self.assertEqual(r.grant_type, ServiceApplicationClient.grant_type) + + claim = jwt.decode(r.assertion, self.public_key, audience=self.audience, algorithms=['RS256']) + + self.assertEqual(claim['iss'], self.issuer) + # audience verification is handled during decode now + self.assertEqual(claim['sub'], self.subject) + self.assertEqual(claim['iat'], int(t.return_value)) + self.assertEqual(claim['nbf'], not_before) + self.assertEqual(claim['jti'], jwt_id) + + @patch('time.time') + def test_request_body_no_initial_private_key(self, t): + t.return_value = time() + self.token['expires_at'] = self.token['expires_in'] + t.return_value + + client = ServiceApplicationClient( + self.client_id, private_key=None) + + # Basic with private key provided + body = client.prepare_request_body(issuer=self.issuer, + subject=self.subject, + audience=self.audience, + body=self.body, + private_key=self.private_key) + r = Request('https://a.b', body=body) + self.assertEqual(r.isnot, 'empty') + self.assertEqual(r.grant_type, ServiceApplicationClient.grant_type) + + claim = jwt.decode(r.assertion, self.public_key, audience=self.audience, algorithms=['RS256']) + + self.assertEqual(claim['iss'], self.issuer) + # audience verification is handled during decode now + self.assertEqual(claim['sub'], self.subject) + self.assertEqual(claim['iat'], int(t.return_value)) + + # No private key provided + self.assertRaises(ValueError, client.prepare_request_body, + issuer=self.issuer, subject=self.subject, audience=self.audience, body=self.body) @patch('time.time') def test_parse_token_response(self, t): diff --git a/tests/oauth2/rfc6749/clients/test_web_application.py b/tests/oauth2/rfc6749/clients/test_web_application.py index 0a80c9a..4ecc3b3 100644 --- a/tests/oauth2/rfc6749/clients/test_web_application.py +++ b/tests/oauth2/rfc6749/clients/test_web_application.py @@ -117,6 +117,25 @@ class WebApplicationClientTest(TestCase): self.response_uri, state="invalid") + def test_populate_attributes(self): + + client = WebApplicationClient(self.client_id) + + response_uri = (self.response_uri + + "&access_token=EVIL-TOKEN" + "&refresh_token=EVIL-TOKEN" + "&mac_key=EVIL-KEY") + + client.parse_request_uri_response(response_uri, self.state) + + self.assertEqual(client.code, self.code) + + # We must not accidentally pick up any further security + # credentials at this point. + self.assertIsNone(client.access_token) + self.assertIsNone(client.refresh_token) + self.assertIsNone(client.mac_key) + def test_parse_token_response(self): client = WebApplicationClient(self.client_id) |