* addresing ticket #585

* `prepare_request_body` client_id is deprecated in favor of include_client_id
* a new unit test `test_prepare_request_body` is added to ensure conformity of several use cases
* the docstrings for the `body` param have been consolidated and standardized across multiple functions linked to `prepare_request_body` for clarity

7 files changed, 90 insertions, 16 deletions

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..ec59b31 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
@@ -124,8 +132,18 @@ class WebApplicationClient(Client):
         .. _`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..2b6e854 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,6 +120,7 @@ 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]))
 
diff --git a/tests/oauth2/rfc6749/clients/test_web_application.py b/tests/oauth2/rfc6749/clients/test_web_application.py
index 4ecc3b3..d87005a 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
 
@@ -38,7 +39,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 +178,49 @@ 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)
+        """
+        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')
+
+        r1b = client.prepare_request_body(include_client_id=True)
+        self.assertEqual(r1b, 'grant_type=authorization_code&client_id=someclientid')
+
+        # 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)
+        r3 = client.prepare_request_body(client_secret='secret')
+        self.assertEqual(r3, 'grant_type=authorization_code&client_secret=secret&client_id=someclientid')
+        r3b = client.prepare_request_body(include_client_id=True, client_secret='secret')
+        self.assertEqual(r3b, 'grant_type=authorization_code&client_secret=secret&client_id=someclientid')
+
+        # 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)
+            self.assertEqual(w[0].message.message, """`client_id` has been deprecated in favor of `include_client_id`, a boolean value which will include the already configured `self.client_id`.""")
+
+        # 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')
+        self.assertEqual(cm.exception.message,
+                         "`client_id` was supplied as an argument, but it does not match `self.client_id`")