truqu / elm-oauth2

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

Strava asking for 'client_secret' field #23

Closed peterjamesward closed 3 years ago

peterjamesward commented 3 years ago

Hi. Getting to grips with this code, with the aim of connecting my Elm app to Strava (popular with runners and cyclists, you may know). I am using OAuth.AuthorizationCode and getting as far as requesting a token. I am supplying my client secret in getAccessToken

getAccessToken : Configuration -> Url -> OAuth.AuthorizationCode -> Cmd OAuthMsg
getAccessToken { clientId, tokenEndpoint } redirectUri code =
    Http.request <|
        OAuth.makeTokenRequest GotAccessToken
            { credentials =
                { clientId = clientId
                , secret = Just StravaClientSecret.clientSecret
                }
            , code = code
            , url = tokenEndpoint
            , redirectUri = redirectUri
            }

but Strava's response is

{
    "message": "Bad Request",
    "errors": [
        {
            "resource": "Application",
            "field": "client_secret",
            "code": "invalid"
        }
    ]
}

Looking at your code, it seems to append the secret to the id :. What am I missing? Could I just append another field to the url?

Pete Ward

peterjamesward commented 3 years ago

To be sure, the Strava Authentication guide gives this example.

curl -X POST https://www.strava.com/api/v3/oauth/token \
  -d client_id=ReplaceWithClientID \
  -d client_secret=ReplaceWithClientSecret \
  -d code=ReplaceWithCode \
  -d grant_type=authorization_code
peterjamesward commented 3 years ago

I added this line to makeTokenRequest and Strava is happy now.

    [ Builder.string "client_secret" secret ]

Perhaps this should be an option somehow?

KtorZ commented 3 years ago

Jeez... One more major provider which can't read the specs :sob: ...

From the OAuth 2.0 RFC - section 2.3.1:

The authorization server MUST support the HTTP Basic authentication scheme for authenticating clients that were issued a client password. For example [..]:

     Authorization: Basic czZCaGRSa3F0Mzo3RmpmcDBaQnIxS3REUmJuZlZkbUl3

Alternatively, the authorization server MAY support including the client credentials in the request-body using the following parameters:

   client_id
         REQUIRED.  The client identifier issued to the client during the registration process described by Section 2.2.

   client_secret
         REQUIRED.  The client secret.  The client MAY omit the parameter if the client secret is an empty string.

Including the client credentials in the request-body using the two parameters is NOT RECOMMENDED and SHOULD be limited to clients unable to directly utilize the HTTP Basic authentication scheme [...]

In this context the "authorization server" is the Strava API. This library sticks to the former approach and always provides the client secret as a HTTP Authentication header as it should since the server MUST support it. I'd suggest contacting Strava about this if you can because that's a fault on their side.

In the meantime, your work-around would do indeed.

peterjamesward commented 3 years ago

I came to the same conclusion, but without knowledge of the standards. I shall contact Strava. Who knows, they may fix it. Thank you for confirming that my change was valid. I have to say, it was very easy to find the right place - your code is better organised than mine!

KtorZ commented 3 years ago

Who knows, they may fix it

In their case it's easy to fix because it'll be an additive change. It's not like Facebook who has it completely broken from the start using non standard naming and deviating from the specs in many aspects. They can't change anything without inducing a breaking change in all their clients.

I have to say, it was very easy to find the right place - your code is better organised than mine!

Thank you! Keep in mind it's version 7.x.x :smile: ... that has been refined a few times already!