* added support for empty strings of `client_secret`

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

5 files changed, 70 insertions, 10 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/web_application.py b/oauthlib/oauth2/rfc6749/clients/web_application.py
index ec59b31..b4b109e 100644
--- a/oauthlib/oauth2/rfc6749/clients/web_application.py
+++ b/oauthlib/oauth2/rfc6749/clients/web_application.py
@@ -128,6 +128,14 @@ 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
         """
diff --git a/oauthlib/oauth2/rfc6749/parameters.py b/oauthlib/oauth2/rfc6749/parameters.py
index 1229f31..0a36e53 100644
--- a/oauthlib/oauth2/rfc6749/parameters.py
+++ b/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)
 
 
diff --git a/tests/oauth2/rfc6749/clients/test_legacy_application.py b/tests/oauth2/rfc6749/clients/test_legacy_application.py
index 1e11112..4e518e8 100644
--- a/tests/oauth2/rfc6749/clients/test_legacy_application.py
+++ b/tests/oauth2/rfc6749/clients/test_legacy_application.py
@@ -15,6 +15,7 @@ from ....unittest import TestCase
 class LegacyApplicationClientTest(TestCase):
 
     client_id = "someclientid"
+    client_secret = 'someclientsecret'
     scope = ["/profile"]
     kwargs = {
         "some": "providers",
@@ -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 9144659..fb800f7 100644
--- a/tests/oauth2/rfc6749/clients/test_web_application.py
+++ b/tests/oauth2/rfc6749/clients/test_web_application.py
@@ -21,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"
@@ -188,15 +189,16 @@ class WebApplicationClientTest(TestCase):
             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)
@@ -204,14 +206,22 @@ class WebApplicationClientTest(TestCase):
 
         # 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: