* added support for empty strings of client_secret

* added LegacyApplicationClient tests to ensure the grant supports a variety of allowed methods

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)
 ------------------
 

oauthlib/oauth2/rfc6749/clients/web_application.py

@@ -128,6 +128,14 @@ def prepare_request_body(self, code=None, redirect_uri=None, body='',
  >>> 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
  """

oauthlib/oauth2/rfc6749/parameters.py

@@ -124,6 +124,10 @@ def prepare_token_request(grant_type, body='', **kwargs):
  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)
 
 

tests/oauth2/rfc6749/clients/test_legacy_application.py

@@ -15,6 +15,7 @@
 class LegacyApplicationClientTest(TestCase):
 
  client_id = "someclientid"
+ client_secret = 'someclientsecret'
  scope = ["/profile"]
  kwargs = {
  "some": "providers",
@@ -88,3 +89,30 @@ def record_scope_change(sender, message, old, new):
  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, ))

tests/oauth2/rfc6749/clients/test_web_application.py

@@ -21,6 +21,7 @@
 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"
@@ -188,30 +189,39 @@ def test_prepare_request_body(self):
  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=someclientid')
+ 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=someclientid')
+ 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='someclientsecret')
- self.assertIn(r3, ('grant_type=authorization_code&client_secret=someclientsecret&client_id=someclientid',
- 'grant_type=authorization_code&client_id=someclientid&client_secret=someclientsecret',)
- )
- r3b = client.prepare_request_body(include_client_id=True, client_secret='someclientsecret')
- self.assertIn(r3b, ('grant_type=authorization_code&client_secret=someclientsecret&client_id=someclientid',
- 'grant_type=authorization_code&client_id=someclientid&client_secret=someclientsecret',)
- )
+ 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: