userinfo: expose http response information to library callers

[yanniszark]

The current UserInfo() function makes an HTTP request and if something goes wrong, it returns a string error:

go-oidc/oidc.go

Lines 229 to 231 in 8d77155

	if resp.StatusCode != http.StatusOK {
	return nil, fmt.Errorf("%s: %s", resp.Status, body)
	}

I need to check the return code of the request to find out more about the reason it was denied (if the request was unauthorized, an internal error occurred at the server, etc.).
See also: https://www.rfc-editor.org/rfc/rfc6750.html#section-3.1

Would it be reasonable to define an error type for this kind of information?
The oauth2 package uses a RetrieveError for these cases:

// RetrieveError is the error returned when the token endpoint returns a
// non-2XX HTTP status code.
type RetrieveError struct {
	Response *http.Response
	// Body is the body that was consumed by reading Response.Body.
	// It may be truncated.
	Body []byte
}

cc @ericchiang

[ericchiang]

Is this to figure out when you need to rotate the token?

There's a lot of customization that request could use, and I don't know if we want to start going down the path of exposing knobs. Maybe make it easier to make the request yourself? e.g. we could export the user info URL to the Provider struct:

type Provider struct {
    UserInfoURL string
}

As well as have UserInfo implement UnmarshalJSON:

func (u *UserInfo) UnmarshalJSON(b []byte) error

Then you could customize your error handling:

req, err := http.NewRequest("GET", p.UserInfo, nil)
if err != nil {
	// ...
}
token.SetAuthHeader(req)

resp, err := myClient.Do(req)
if err != nil {
	// ...
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
	// your custom error handling
}
var u oidc.UserInfo
if err := json.NewDecoder(resp.Body).Decode(&u); err != nil {
	// ...
}

I'd like to avoid dealing with OAuth2 semantics as much as possible 😄 that's what golang.org/x/oauth2 is for.

[damz]

I just bumped into this. I wanted to use *Provider.UserInfo() to check a token provided externally. Unfortunately there is no way to distinguish between "any random server error" and "this token is invalid" in the current error scheme.

[yanniszark]

@ericchiang thanks for taking the time such a detailed reply.
My thought was that the library could specifically enrich request-related errors with extra information, which would enable library users to do fine-grained error checking.
A RequestError could apply wherever a request is involved (discovery, token exchange, userinfo).
e.g., if the server returns 503, we might want to detect that and either retry the request or also return 503 to the user agent
In any case, both solutions are fine with me, please confirm what you think the best way to move forward is.