python-social-auth / social-core

Python Social Auth - Core
BSD 3-Clause "New" or "Revised" License
851 stars 546 forks source link

OAuth2 grant type `refresh_token` sends the access token when no refresh token is available #826

Open kysrpex opened 1 year ago

kysrpex commented 1 year ago

Expected behaviour

According to RFC6749 section 4.1.4, when requesting an authorization code via OAuth2 (grant_type=authorization_code) using a request like this,

POST /token HTTP/1.1
Host: server.example.com
Authorization: Basic czZCaGRSa3F0MzpnWDFmQmF0M2JW
Content-Type: application/x-www-form-urlencoded

grant_type=authorization_code&code=SplxlOBeZQQYbYS6WxSbIA
&redirect_uri=https%3A%2F%2Fclient%2Eexample%2Ecom%2Fcb

the response may contain a refresh_token.

HTTP/1.1 200 OK
Content-Type: application/json;charset=UTF-8
Cache-Control: no-store
Pragma: no-cache

{
    "access_token": "2YotnFZFEjr1zCsicMWpAA",
    "token_type": "example",
    "expires_in": 3600,
    "refresh_token": "tGzv3JOkF0XG5Qx2TlKWIA",
    "example_parameter": "example_value"
}

If the response contains no refresh_token and later refresh_token() is called, I would expect social-core to raise an exception (or at least not send a request to the authorization server).

Actual behaviour

social-core sends the access token as if it was a refresh token and the authorization server reports that the refresh token is invalid.

What are the steps to reproduce this issue?

Request an authorization code via OAuth2 to an authorization server that does not return a refresh_token (for example, LS Login). Then call refresh_token().

Any logs, error output, etc?

The authorization server returns a response with error code 401.

An error occurred when refreshing user token: 401 Client Error: 401 for url: https://login.elixir-czech.org/oidc/token

The error is arising in version 4.5.4.

Any other comments?

We have spotted this issue on UseGalaxy.eu, that makes use of LS Login in production. This problem arises about one hundred thousand times a day since September 1, 2023.

kysrpex commented 4 months ago

The source of the HTTP 401 error is a malformed request.

social-auth-core is sending this sort of request (a few headers are omitted)

POST /oidc/token HTTP/1.1
Host: login.elixir-czech.org
Accept: application/json
Content-Type: application/x-www-form-urlencoded
Content-Length: ***

{"grant_type": "refresh_token", "refresh_token": "******", "client_id": "******", "client_secret": "******"}

and getting this sort of response (again some headers are omitted).

HTTP/1.1 401 401
Content-Type: application/json

{"error":"invalid_client","error_description":"Bad client credentials"}

If the requests are modified so that they look like it is described in RFC6749

POST /oidc/token HTTP/1.1
Host: login.elixir-czech.org
Accept: application/json
Content-Type: application/x-www-form-urlencoded
Content-Length: ***

grant_type=refresh_token&refresh_token=******&client_id=******&client_secret=******

then everything works smoothly.

nijel commented 4 months ago

The request really looks wrong. Reading the code, it ends up calling requests.request(method="POST",..., data={"grant_type": "refresh_token", "refresh_token": "******", "client_id": "******", "client_secret": "******"}) and requests should properly encode the payload in that case. So instead of fixing this in the elixir backed (as you've attempted in #927), the reason it is sent as a string should be investigated.

kysrpex commented 4 months ago

@nijel Sorry, you are right. This was true on Sep 6 2023, but I checked now and the request is properly constructed. I paid attention only to the status code of the error I was seeing and did not realize that while the status code coincides, the error message is actually different. What I am observing now is a different problem.

kysrpex commented 1 month ago

@nijel We spotted the actual issue, I just edited the title and updated the initial message.

kysrpex commented 1 month ago

@bgruening FYI

nijel commented 1 month ago

This logic is there since ever (https://github.com/python-social-auth/social-core/commit/a7f8ff4471baee1d728fa168408b01060a5f2496), I'm not sure if removing fallback to access_token will not break something.