diff options
| -rw-r--r-- | CHANGELOG.rst | 10 | ||||
| -rw-r--r-- | oauthlib/oauth2/rfc6749/clients/backend_application.py | 2 | ||||
| -rw-r--r-- | oauthlib/oauth2/rfc6749/clients/base.py | 6 | ||||
| -rw-r--r-- | oauthlib/oauth2/rfc6749/clients/legacy_application.py | 2 | ||||
| -rw-r--r-- | oauthlib/oauth2/rfc6749/clients/service_application.py | 13 | ||||
| -rw-r--r-- | oauthlib/oauth2/rfc6749/clients/web_application.py | 38 | ||||
| -rw-r--r-- | oauthlib/oauth2/rfc6749/parameters.py | 10 | ||||
| -rw-r--r-- | tests/oauth2/rfc6749/clients/test_legacy_application.py | 32 | ||||
| -rw-r--r-- | tests/oauth2/rfc6749/clients/test_web_application.py | 64 |
9 files changed, 158 insertions, 19 deletions
diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a8e1941..fd53769 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,16 @@ Changelog ========= +Unreleased +------------------ + +* OAuth2's `prepare_token_request` supports sending an empty string for `client_id` (#585) +* OAuth2's `WebApplicationClient.prepare_request_body` was refactored to better + support sending or omitting the `client_id` via a new `include_client_id` kwarg. + By default this is included. The method will also emit a DeprecationWarning if + a `client_id` parameter is submitted; the already configured `self.client_id` + is the preferred option. (#585) + 2.1.0 (2018-05-21) ------------------ diff --git a/oauthlib/oauth2/rfc6749/clients/backend_application.py b/oauthlib/oauth2/rfc6749/clients/backend_application.py index cbad8b7..99dbfc5 100644 --- a/oauthlib/oauth2/rfc6749/clients/backend_application.py +++ b/oauthlib/oauth2/rfc6749/clients/backend_application.py @@ -37,6 +37,8 @@ class BackendApplicationClient(Client): following parameters using the "application/x-www-form-urlencoded" format per `Appendix B`_ in the HTTP request entity-body: + :param body: Existing request body (URL encoded string) to embed parameters + into. This may contain extra paramters. Default ''. :param scope: The scope of the access request as described by `Section 3.3`_. :param kwargs: Extra credentials to include in the token request. diff --git a/oauthlib/oauth2/rfc6749/clients/base.py b/oauthlib/oauth2/rfc6749/clients/base.py index 406832d..d8ded50 100644 --- a/oauthlib/oauth2/rfc6749/clients/base.py +++ b/oauthlib/oauth2/rfc6749/clients/base.py @@ -254,7 +254,8 @@ class Client(object): :param redirect_url: The redirect_url supplied with the authorization request (if there was one). - :param body: Request body (URL encoded string). + :param body: Existing request body (URL encoded string) to embed parameters + into. This may contain extra paramters. Default ''. :param kwargs: Additional parameters to included in the request. @@ -286,7 +287,8 @@ class Client(object): :param refresh_token: Refresh token string. - :param body: Request body (URL encoded string). + :param body: Existing request body (URL encoded string) to embed parameters + into. This may contain extra paramters. Default ''. :param scope: List of scopes to request. Must be equal to or a subset of the scopes granted when obtaining the refresh diff --git a/oauthlib/oauth2/rfc6749/clients/legacy_application.py b/oauthlib/oauth2/rfc6749/clients/legacy_application.py index b16fc9f..8f03695 100644 --- a/oauthlib/oauth2/rfc6749/clients/legacy_application.py +++ b/oauthlib/oauth2/rfc6749/clients/legacy_application.py @@ -47,6 +47,8 @@ class LegacyApplicationClient(Client): :param username: The resource owner username. :param password: The resource owner password. + :param body: Existing request body (URL encoded string) to embed parameters + into. This may contain extra paramters. Default ''. :param scope: The scope of the access request as described by `Section 3.3`_. :param kwargs: Extra credentials to include in the token request. diff --git a/oauthlib/oauth2/rfc6749/clients/service_application.py b/oauthlib/oauth2/rfc6749/clients/service_application.py index 3045676..6bb784e 100644 --- a/oauthlib/oauth2/rfc6749/clients/service_application.py +++ b/oauthlib/oauth2/rfc6749/clients/service_application.py @@ -97,18 +97,19 @@ class ServiceApplicationClient(Client): :param issued_at: A unix timestamp of when the JWT was created. Defaults to now, i.e. ``time.time()``. + :param extra_claims: A dict of additional claims to include in the JWT. + + :param body: Existing request body (URL encoded string) to embed parameters + into. This may contain extra paramters. Default ''. + + :param scope: The scope of the access request. + :param not_before: A unix timestamp after which the JWT may be used. Not included unless provided. :param jwt_id: A unique JWT token identifier. Not included unless provided. - :param extra_claims: A dict of additional claims to include in the JWT. - - :param scope: The scope of the access request. - - :param body: Request body (string) with extra parameters. - :param kwargs: Extra credentials to include in the token request. The "scope" parameter may be used, as defined in the Assertion diff --git a/oauthlib/oauth2/rfc6749/clients/web_application.py b/oauthlib/oauth2/rfc6749/clients/web_application.py index c14a5f8..b4b109e 100644 --- a/oauthlib/oauth2/rfc6749/clients/web_application.py +++ b/oauthlib/oauth2/rfc6749/clients/web_application.py @@ -8,6 +8,8 @@ for consuming and providing OAuth 2.0 RFC6749. """ from __future__ import absolute_import, unicode_literals +import warnings + from ..parameters import (parse_authorization_code_response, parse_token_response, prepare_grant_uri, prepare_token_request) @@ -85,17 +87,14 @@ class WebApplicationClient(Client): return prepare_grant_uri(uri, self.client_id, 'code', redirect_uri=redirect_uri, scope=scope, state=state, **kwargs) - def prepare_request_body(self, client_id=None, code=None, body='', - redirect_uri=None, **kwargs): + def prepare_request_body(self, code=None, redirect_uri=None, body='', + include_client_id=True, **kwargs): """Prepare the access token request body. The client makes a request to the token endpoint by adding the following parameters using the "application/x-www-form-urlencoded" format in the HTTP request entity-body: - :param client_id: REQUIRED, if the client is not authenticating with the - authorization server as described in `Section 3.2.1`_. - :param code: REQUIRED. The authorization code received from the authorization server. @@ -103,6 +102,15 @@ class WebApplicationClient(Client): authorization request as described in `Section 4.1.1`_, and their values MUST be identical. + :param body: Existing request body (URL encoded string) to embed parameters + into. This may contain extra paramters. Default ''. + + :param include_client_id: `True` (default) to send the `client_id` in the + body of the upstream request. This is required + if the client is not authenticating with the + authorization server as described in `Section 3.2.1`_. + :type include_client_id: Boolean + :param kwargs: Extra parameters to include in the token request. In addition OAuthLib will add the ``grant_type`` parameter set to @@ -120,12 +128,30 @@ class WebApplicationClient(Client): >>> client.prepare_request_body(code='sh35ksdf09sf', foo='bar') 'grant_type=authorization_code&code=sh35ksdf09sf&foo=bar' + `Section 3.2.1` also states: + In the "authorization_code" "grant_type" request to the token + endpoint, an unauthenticated client MUST send its "client_id" to + prevent itself from inadvertently accepting a code intended for a + client with a different "client_id". This protects the client from + substitution of the authentication code. (It provides no additional + security for the protected resource.) + .. _`Section 4.1.1`: https://tools.ietf.org/html/rfc6749#section-4.1.1 .. _`Section 3.2.1`: https://tools.ietf.org/html/rfc6749#section-3.2.1 """ code = code or self.code + if 'client_id' in kwargs: + warnings.warn("`client_id` has been deprecated in favor of " + "`include_client_id`, a boolean value which will " + "include the already configured `self.client_id`.", + DeprecationWarning) + if kwargs['client_id'] != self.client_id: + raise ValueError("`client_id` was supplied as an argument, but " + "it does not match `self.client_id`") + if include_client_id: + kwargs['client_id'] = self.client_id return prepare_token_request('authorization_code', code=code, body=body, - client_id=client_id, redirect_uri=redirect_uri, **kwargs) + redirect_uri=redirect_uri, **kwargs) def parse_request_uri_response(self, uri, state=None): """Parse the URI query for code and state. diff --git a/oauthlib/oauth2/rfc6749/parameters.py b/oauthlib/oauth2/rfc6749/parameters.py index 74a15f9..0a36e53 100644 --- a/oauthlib/oauth2/rfc6749/parameters.py +++ b/oauthlib/oauth2/rfc6749/parameters.py @@ -96,7 +96,8 @@ def prepare_token_request(grant_type, body='', **kwargs): :param grant_type: To indicate grant type being used, i.e. "password", "authorization_code" or "client_credentials". - :param body: Existing request body to embed parameters in. + :param body: Existing request body (URL encoded string) to embed parameters + into. This may contain extra paramters. Default ''. :param code: If using authorization code grant, pass the previously obtained authorization code as the ``code`` argument. :param redirect_uri: If the "redirect_uri" parameter was included in the @@ -119,9 +120,14 @@ def prepare_token_request(grant_type, body='', **kwargs): kwargs['scope'] = list_to_scope(kwargs['scope']) for k in kwargs: + # this handles: `code`, `redirect_uri`, or undocumented params if kwargs[k]: params.append((unicode_type(k), kwargs[k])) + if ('client_secret' in kwargs) and ('client_secret' not in params): + if kwargs['client_secret'] == '': + params.append((unicode_type('client_secret'), kwargs['client_secret'])) + return add_params_to_qs(body, params) @@ -144,7 +150,7 @@ def prepare_token_revocation_request(url, token, token_type_hint="access_token", token types. An authorization server MAY ignore this parameter, particularly if it is able to detect the token type automatically. - + This specification defines two values for `token_type_hint`: * access_token: An access token as defined in [RFC6749], diff --git a/tests/oauth2/rfc6749/clients/test_legacy_application.py b/tests/oauth2/rfc6749/clients/test_legacy_application.py index 3f97c02..4e518e8 100644 --- a/tests/oauth2/rfc6749/clients/test_legacy_application.py +++ b/tests/oauth2/rfc6749/clients/test_legacy_application.py @@ -15,14 +15,15 @@ from ....unittest import TestCase class LegacyApplicationClientTest(TestCase): client_id = "someclientid" + client_secret = 'someclientsecret' scope = ["/profile"] kwargs = { "some": "providers", "require": "extra arguments" } - username = "foo" - password = "bar" + username = "user_username" + password = "user_password" body = "not=empty" body_up = "not=empty&grant_type=password&username=%s&password=%s" % (username, password) @@ -88,3 +89,30 @@ class LegacyApplicationClientTest(TestCase): finally: signals.scope_changed.disconnect(record_scope_change) del os.environ['OAUTHLIB_RELAX_TOKEN_SCOPE'] + + def test_prepare_request_body(self): + """ + see issue #585 + https://github.com/oauthlib/oauthlib/issues/585 + """ + client = LegacyApplicationClient(self.client_id) + + # scenario 1, default behavior to not include `client_id` + r1 = client.prepare_request_body(username=self.username, password=self.password) + self.assertEqual(r1, 'grant_type=password&username=user_username&password=user_password') + + # scenario 2, include `client_id` in the body + r2 = client.prepare_request_body(username=self.username, password=self.password, client_id=self.client_id) + self.assertEqual(r2, 'grant_type=password&username=user_username&password=user_password&client_id=%s' % self.client_id) + + # scenario 3, include `client_id` + `client_secret` in the body + r3 = client.prepare_request_body(username=self.username, password=self.password, client_id=self.client_id, client_secret=self.client_secret) + self.assertEqual(r3, 'grant_type=password&username=user_username&password=user_password&client_id=%s&client_secret=%s' % (self.client_id, self.client_secret)) + + # scenario 4, `client_secret` is an empty string + r4 = client.prepare_request_body(username=self.username, password=self.password, client_id=self.client_id, client_secret='') + self.assertEqual(r4, 'grant_type=password&username=user_username&password=user_password&client_id=%s&client_secret=%s' % (self.client_id, '')) + + # scenario 4b`,` client_secret is `None` + r4b = client.prepare_request_body(username=self.username, password=self.password, client_id=self.client_id, client_secret=None) + self.assertEqual(r4b, 'grant_type=password&username=user_username&password=user_password&client_id=%s' % (self.client_id, )) diff --git a/tests/oauth2/rfc6749/clients/test_web_application.py b/tests/oauth2/rfc6749/clients/test_web_application.py index 4ecc3b3..fb800f7 100644 --- a/tests/oauth2/rfc6749/clients/test_web_application.py +++ b/tests/oauth2/rfc6749/clients/test_web_application.py @@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals import datetime import os +import warnings from mock import patch @@ -20,6 +21,7 @@ from ....unittest import TestCase class WebApplicationClientTest(TestCase): client_id = "someclientid" + client_secret = 'someclientsecret' uri = "https://example.com/path?query=world" uri_id = uri + "&response_type=code&client_id=" + client_id uri_redirect = uri_id + "&redirect_uri=http%3A%2F%2Fmy.page.com%2Fcallback" @@ -38,7 +40,7 @@ class WebApplicationClientTest(TestCase): code = "zzzzaaaa" body = "not=empty" - body_code = "not=empty&grant_type=authorization_code&code=%s" % code + body_code = "not=empty&grant_type=authorization_code&code=%s&client_id=%s" % (code, client_id) body_redirect = body_code + "&redirect_uri=http%3A%2F%2Fmy.page.com%2Fcallback" body_kwargs = body_code + "&some=providers&require=extra+arguments" @@ -177,3 +179,63 @@ class WebApplicationClientTest(TestCase): # verify default header and body only self.assertEqual(header, {'Content-Type': 'application/x-www-form-urlencoded'}) self.assertEqual(body, '') + + def test_prepare_request_body(self): + """ + see issue #585 + https://github.com/oauthlib/oauthlib/issues/585 + + `prepare_request_body` should support the following scenarios: + 1. Include client_id alone in the body (default) + 2. Include client_id and client_secret in auth and not include them in the body (RFC preferred solution) + 3. Include client_id and client_secret in the body (RFC alternative solution) + 4. Include client_id in the body and an empty string for client_secret. + """ + client = WebApplicationClient(self.client_id) + + # scenario 1, default behavior to include `client_id` + r1 = client.prepare_request_body() + self.assertEqual(r1, 'grant_type=authorization_code&client_id=%s' % self.client_id) + + r1b = client.prepare_request_body(include_client_id=True) + self.assertEqual(r1b, 'grant_type=authorization_code&client_id=%s' % self.client_id) + + # scenario 2, do not include `client_id` in the body, so it can be sent in auth. + r2 = client.prepare_request_body(include_client_id=False) + self.assertEqual(r2, 'grant_type=authorization_code') + + # scenario 3, Include client_id and client_secret in the body (RFC alternative solution) + # the order of kwargs being appended is not guaranteed. for brevity, check the 2 permutations instead of sorting + r3 = client.prepare_request_body(client_secret=self.client_secret) + self.assertIn(r3, ('grant_type=authorization_code&client_secret=%s&client_id=%s' % (self.client_secret, self.client_id, ), + 'grant_type=authorization_code&client_id=%s&client_secret=%s' % (self.client_id, self.client_secret, ), + )) + r3b = client.prepare_request_body(include_client_id=True, client_secret=self.client_secret) + self.assertIn(r3b, ('grant_type=authorization_code&client_secret=%s&client_id=%s' % (self.client_secret, self.client_id, ), + 'grant_type=authorization_code&client_id=%s&client_secret=%s' % (self.client_id, self.client_secret, ), + )) + + # scenario 4, `client_secret` is an empty string + r4 = client.prepare_request_body(include_client_id=True, client_secret='') + self.assertEqual(r4, 'grant_type=authorization_code&client_id=%s&client_secret=' % self.client_id) + + # scenario 4b, `client_secret` is `None` + r4b = client.prepare_request_body(include_client_id=True, client_secret=None) + self.assertEqual(r4b, 'grant_type=authorization_code&client_id=%s' % self.client_id) + + # scenario Warnings + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") # catch all + + # warning1 - raise a DeprecationWarning if a `client_id` is submitted + rWarnings1 = client.prepare_request_body(client_id=self.client_id) + self.assertEqual(len(w), 1) + self.assertIsInstance(w[0].message, DeprecationWarning) + + # testing the exact warning message in Python2&Python3 is a pain + + # scenario Exceptions + # exception1 - raise a ValueError if the a different `client_id` is submitted + with self.assertRaises(ValueError) as cm: + client.prepare_request_body(client_id='different_client_id') + # testing the exact exception message in Python2&Python3 is a pain |