[Bug]: using github oauth with `step oauth --console-flow=device` does not work

[spacedub]

Steps to Reproduce

1. create an OAuth app off github to be able to test - be sure to allow device flow - and save the client id
2. step oauth --client-id XXXXX --console-flow device --token-endpoint https://github.com/login/oauth/access_token --scope "user" --device-authorization-endpoint https://github.com/login/device/code (<- replace XXX with client id from above)

Your Environment

• OS - macOS Montery 12.5.1
• step CLI Version - master, 746b47642de2fc4317e34d3e987267d4b2430ebe

Expected Behavior

Should work

Actual Behavior

This will first fail at the first request to Github, as step tries to parse the response as JSON:

failure decoding device authz response to JWON: invalid character 'd' looking for beginning of value

Unfortunately Github response by default is of content-type application/x-www-form-urlencoded - unless the client does send an Accept: application/json

https://github.com/smallstep/cli/blob/master/command/oauth/cmd.go#L853

I did workaround this for myself by having a custom http.RoundTripper that adds the Accept: application/json header if req.Host is github.com.

Once this one is fixed, step will properly read the response and starts polling the token endpoint, but will exit abnormally fast - at the first polling attempt, after 5 seconds (which is Github interval response), instead of properly waiting for the user to finish the browser flow.

The reason seems to be: https://github.com/smallstep/cli/blob/746b47642de2fc4317e34d3e987267d4b2430ebe/command/oauth/cmd.go#L909

Typically, while the authorization is pending, Github will return (200):

{"error":"authorization_pending","error_description":"The authorization request is still pending.","error_uri":"https://docs.github.com/developers/apps/authorizing-oauth-apps#error-codes-for-the-device-flow"}

which will marshal into token properly without "error", and return - though there is no actual AccessToken at that point of course.

... I believe this code should only return if tok.AccessToken != "" (or tok.Err == "").

(https://datatracker.ietf.org/doc/html/rfc6749#section-5.1)

Either way, after workaround-ing these two issues, I was able to complete the flow without errors.

Additional Context

Happy to try and send a PR to fix these, but would like an opinion first on the right solution for this (eg: if adding a custom roundtripper to the default http client is the right idea, where should that be best set?).

Contributing

No response

[spacedub]

... then it doesn't look like Github support OIDC :/... so...

[maraino]

@spacedub so ... is it still a bug?

[spacedub]

@maraino not sure... I guess it boils down to: is step oauth meant to be used solely with OIDC services? If "yes", then it doesn't make sense to try an make it work against GitHub as long as they do not offer OIDC.

[maraino]

We mainly use it for OIDC, but it should also work for plain OAuth 2.0, so I guess if it doesn't work with GitHub, it might be a bug. Here are their docs.

[spacedub]

Ok. So in a shell, to fix this as far as I can tell:

1. step oauth has to send Accept: application/json when interacting with Github
2. the return condition in that code need to verify that tok.Err is "" - right now it does not look like it is doing the right thing (and that part is also likely impacting other services as well): https://github.com/smallstep/cli/blob/746b47642de2fc4317e34d3e987267d4b2430ebe/command/oauth/cmd.go#L909

[spacedub]

diff --git a/command/oauth/cmd.go b/command/oauth/cmd.go
index 0ba6599..48db086 100644
--- a/command/oauth/cmd.go
+++ b/command/oauth/cmd.go
@@ -307,9 +307,24 @@ OpenID standard defines the following values, but your provider may support some
                Action: oauthCmd,
        }

+       http.DefaultClient.Transport = &transport{
+               RoundTripper: http.DefaultTransport,
+       }
+
        command.Register(cmd)
 }

+type transport struct {
+       http.RoundTripper
+}
+
+func (adt *transport) RoundTrip(req *http.Request) (*http.Response, error) {
+       if req.Host == "github.com" {
+               req.Header.Add("Accept", "application/json")
+       }
+       return adt.RoundTripper.RoundTrip(req)
+}
+
 type consoleFlow int

 const (
@@ -905,8 +920,10 @@ func (o *oauth) DoDeviceAuthorization() (*token, error) {
                                continue
                        } else if err != nil {
                                return nil, err
+                       } else if tok.Err == "" {
+                               // dmp: https://datatracker.ietf.org/doc/html/rfc6749#section-5.1
+                               return tok, nil
                        }
-                       return tok, nil
                case <-t.C:
                        return nil, errors.New("device authorization grant expired")
                }

This is my current quick and dirty ^

[maraino]

We should test that header on other IdPs and see if we can just send it always, I think it should work.