truqu / elm-oauth2

OAuth 2.0 client-side utils in Elm
MIT License
81 stars 29 forks source link

Refresh token request rejected by auth server with PKCE flow due to missing client_id in refresh request #33

Open j-krose opened 2 weeks ago

j-krose commented 2 weeks ago

Using PCKE OAuth flow (i.e. no client secret available) to create a client-only web page I ran into the following issue; rather than being able to do an expected:

    Refresh.makeTokenRequest
        (resultMapper >> toMsg)
        { credentials = Nothing
        , url = tokenUrl
        , scope = scopes
        , token = accessToken.refreshToken
        }
        |> Http.request

or

    Refresh.makeTokenRequest
        (resultMapper >> toMsg)
        { credentials = Just { clientId = clientId, secret = "" }
        , url = tokenUrl
        , scope = scopes
        , token = accessToken.refreshToken
        }
        |> Http.request

I had to use the more custom makeTokenRequestWith in order to form the proper request:

    Refresh.makeTokenRequestWith
        OAuth.RefreshToken
        Refresh.defaultAuthenticationSuccessDecoder
        (Dict.singleton "client_id" clientId)
        (resultMapper >> toMsg)
        { credentials = Nothing
        , url = tokenUrl
        , scope = scopes
        , token = accessToken.refreshToken
        }
        |> Http.request

It seems that the library does not include the client_id in the form body; when supplied in credentials it just uses it in the headers rather than the body:

makeTokenRequestWith : GrantType -> Json.Decoder success -> Dict String String -> (Result Http.Error success -> msg) -> Authentication -> RequestParts msg
makeTokenRequestWith grantType decoder extraFields toMsg { credentials, scope, token, url } =
    let
        body =
            [ Builder.string "grant_type" (grantTypeToString grantType)
            , Builder.string "refresh_token" (extractTokenString token)
            ]
                |> urlAddList "scope" scope
                |> urlAddExtraFields extraFields
                |> Builder.toQuery
                |> String.dropLeft 1

        headers =
            makeHeaders credentials
    in
    makeRequest decoder toMsg url headers body

It seems like this client_id in the refresh token request is NOT mentioned in RFC: https://datatracker.ietf.org/doc/html/rfc6749#section-6

So maybe it is just strange that spotify auth server is requesting it: https://developer.spotify.com/documentation/web-api/tutorials/refreshing-tokens

Screen Shot 2024-07-10 at 01 03 06

Small nit as well, this documentation was a bit confusing https://github.com/truqu/elm-oauth2/blob/ef6a7bf29b361a2564b99b0daa79eb3b7ed74f45/src/OAuth/Refresh.elm#L56 :

  - `token` (_REQUIRED_):
    Token endpoint of the resource provider

I think this is supposed to be something more like "the refresh token issued by the authorization provider"

j-krose commented 2 weeks ago

Side note, really cool and useful library! Well documented and works really nicely!

I am happy to implement this one myself with some guidance on whether this actually merits a change or not, but I was not sure because I do not know so much about OAuth.