oauthlib: Client web application does no longer send client_id

I found a regression in master when used with requests/requests-oauthlib since https://github.com/oauthlib/oauthlib/issues/495 has been merged. It’s related to authorization grant/web application only.

Basic usage of requests-oauthlib is :

sess = OAuth2Session(client_id)
token = sess.fetch_token(token_url, client_secret=client_secret, authorization_response=request.url)

However, since the changes, client_id of the session is ignored. I think https://github.com/oauthlib/oauthlib/pull/505 fixed an use-case but broke another one. We should find a win-win solution.

requests-oauthlib code call at https://github.com/requests/requests-oauthlib/blob/master/requests_oauthlib/oauth2_session.py#L196-L198 and oauthlib issue here https://github.com/oauthlib/oauthlib/blame/master/oauthlib/oauth2/rfc6749/clients/web_application.py#L128.

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 26 (25 by maintainers)

Commits related to this issue

* 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 ... — committed to jvanasco/oauthlib by jvanasco 6 years ago
Merge pull request #593 from jvanasco/fix-585_client_id PR for #585, `client_id` behavior with `prepare_request_body` — committed to oauthlib/oauthlib by JonathanHuot 6 years ago

Most upvoted comments

Wouldn’t see how you’d ever want to override client_id with a different value so would vote for raising an exception if they differ.

Should we, in addition, log a DeprecationWarning if client_id was provided at all as a kwarg?

skion on Sep 13, 2018

@JonathanHuot the existing implementation does not support sending an empty string for client_secret. It is removed in this logic https://github.com/oauthlib/oauthlib/blob/master/oauthlib/oauth2/rfc6749/parameters.py#L90-L125 – specifically line 122

Supporting it can be adding something like this right after that routine:

if ('client_secret' in kwargs) and ('client_secret' not in params):
    if kwargs['client_secret'] == '':
        params.append((unicode_type('client_secret'), kwargs['client_secret']))

That would send an empty string for client_secret when the secret is an empty string, but not send the client_secret if the value is None.

I think it is worth supporting this, because if the RFC supports either of the two variants… there are likely to be many broken implementations that only support one variant.

jvanasco on Dec 3, 2018

Some server reject the request (400) if the client_id is in the request body. I think the default should be what is recommended by the spec.

Diaoul on Sep 19, 2018

Thanks for running into this.

username and password must always be present in request’s body. They must be used only for password grant aka legacy. Those must not be used for other grants (implicit, code, client credentials).
HTTP Basic Auth is optional for client credentials (recommended for confidential clients i.e. with a client_secret). User credentials must never be in HTTP Basic Auth.
Yes

JonathanHuot on Sep 13, 2018