client_id_secret should be client_secret

[michi-zuri]

auth/src/OAuth/AuthorizationCode.elm

Line 422 in 2215675

, Builder.string "client_id_secret" (credentials.secret |> Maybe.withDefault "")

When using lamdera/auth I had to rename client_id_secret to client_secret for the authorization request to orcid to be passed. Also in https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1 client_secret seems to be the correct name.

This line was added to the OAuth package by lamdera because for the implicit client-side request no client_secret is ever transmitted.

[supermario]

NTS: current call chain is

Flow.updateFromFrontend:Auth.Common.AuthCallbackReceived
Flow.backendUpdate:Auth.Common.AuthCallbackReceived_
Auth.Common.ProtocolOAuth -> Auth.Protocol.OAuth.onAuthCallbackReceived
Auth.Protocol.OAuth.validateCallbackToken
OAuth.makeTokenRequest
OAuth.makeTokenRequestWith

[michi-zuri]

Can only tell that without the change my credentials were rejected (by sandbox.orcid.org) and deleting the 3 characters solved this, so the code seemed to be used. It seems like the secret is also provided in the header, so other auth providers may have ignored the wrong parameter?

[michi-zuri]

What does NTS stand for? Any of these?

NTS — Need to see: Reporter must provide logs, repro steps, or files.
NTS — Not reproducible: Issue couldn't be reproduced with given steps.
NTS — Need to sync: Request to coordinate with reporter/dev for more info.
NTS — Not sure: Triage note meaning uncertainty; follow-up required.
NTS — New test scenario: Indicates adding a test case to cover the behavior.

[supermario]

@michi-zuri oh sorry! Note to self 😅

Just took a cursory look and wanted to write down a reminder of what I'd found in terms of how/where this code ended up in the auth chain.

Your theory may well be right especially if its noted in the spec, though a little surprising. If you have the time/inclination to test with a more mainstream auth provider i.e. Google or Github that would be cool.

[michi-zuri]

Related reading:
truqu/elm-oauth2#23 (comment)

It seems Orcid doesn't follow the spec, by not supporting the client_id and secret in the Header, just like Strava. Google, Github and Auth0 probably do follow the spec more closely, even though I didn't see the Basic Header mentioned Google OIDC docs.