Skip to content

Commit

Permalink
oauth2: parse RFC 6749 error response
Browse files Browse the repository at this point in the history 
Parse error response described in https://datatracker.ietf.org/doc/html/rfc6749#section-5.2

Handle unorthodox servers responding 200 in error case.

Implements API changes in accepted proposal golang/go#58125

Fixes #441
Fixes #274
Updates #173

Change-Id: If9399c3f952ac0501edbeefeb3a71ed057ca8d37
GitHub-Last-Rev: 0030e27
GitHub-Pull-Request: #610
Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/451076
Run-TryBot: Matt Hickford <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Matt Hickford <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Cody Oss <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
@hickford @neild
hickford authored and neild committed Apr 11, 2023
1 parent 3607514 commit cfe200d
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 14 deletions.
60 changes: 50 additions & 10 deletions internal/token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,18 @@ type Token struct {
}

// tokenJSON is the struct representing the HTTP response from OAuth2
// providers returning a token in JSON form.
// providers returning a token or error in JSON form.
// https://datatracker.ietf.org/doc/html/rfc6749#section-5.1
type tokenJSON struct {
AccessToken string `json:"access_token"`
TokenType string `json:"token_type"`
RefreshToken string `json:"refresh_token"`
ExpiresIn expirationTime `json:"expires_in"` // at least PayPal returns string, while most return number
// error fields
// https://datatracker.ietf.org/doc/html/rfc6749#section-5.2
ErrorCode string `json:"error"`
ErrorDescription string `json:"error_description"`
ErrorURI string `json:"error_uri"`
}

func (e *tokenJSON) expiry() (t time.Time) {
Expand Down Expand Up @@ -236,21 +242,29 @@ func doTokenRoundTrip(ctx context.Context, req *http.Request) (*Token, error) {
if err != nil {
return nil, fmt.Errorf("oauth2: cannot fetch token: %v", err)
}
if code := r.StatusCode; code < 200 || code > 299 {
return nil, &RetrieveError{
Response: r,
Body: body,
}

failureStatus := r.StatusCode < 200 || r.StatusCode > 299
retrieveError := &RetrieveError{
Response: r,
Body: body,
// attempt to populate error detail below
}

var token *Token
content, _, _ := mime.ParseMediaType(r.Header.Get("Content-Type"))
switch content {
case "application/x-www-form-urlencoded", "text/plain":
// some endpoints return a query string
vals, err := url.ParseQuery(string(body))
if err != nil {
return nil, err
if failureStatus {
return nil, retrieveError
}
return nil, fmt.Errorf("oauth2: cannot parse response: %v", err)
}
retrieveError.ErrorCode = vals.Get("error")
retrieveError.ErrorDescription = vals.Get("error_description")
retrieveError.ErrorURI = vals.Get("error_uri")
token = &Token{
AccessToken: vals.Get("access_token"),
TokenType: vals.Get("token_type"),
Expand All @@ -265,8 +279,14 @@ func doTokenRoundTrip(ctx context.Context, req *http.Request) (*Token, error) {
default:
var tj tokenJSON
if err = json.Unmarshal(body, &tj); err != nil {
return nil, err
if failureStatus {
return nil, retrieveError
}
return nil, fmt.Errorf("oauth2: cannot parse json: %v", err)
}
retrieveError.ErrorCode = tj.ErrorCode
retrieveError.ErrorDescription = tj.ErrorDescription
retrieveError.ErrorURI = tj.ErrorURI
token = &Token{
AccessToken: tj.AccessToken,
TokenType: tj.TokenType,
Expand All @@ -276,17 +296,37 @@ func doTokenRoundTrip(ctx context.Context, req *http.Request) (*Token, error) {
}
json.Unmarshal(body, &token.Raw) // no error checks for optional fields
}
// according to spec, servers should respond status 400 in error case
// https://www.rfc-editor.org/rfc/rfc6749#section-5.2
// but some unorthodox servers respond 200 in error case
if failureStatus || retrieveError.ErrorCode != "" {
return nil, retrieveError
}
if token.AccessToken == "" {
return nil, errors.New("oauth2: server response missing access_token")
}
return token, nil
}

// mirrors oauth2.RetrieveError
type RetrieveError struct {
Response *http.Response
Body []byte
Response *http.Response
Body []byte
ErrorCode string
ErrorDescription string
ErrorURI string
}

func (r *RetrieveError) Error() string {
if r.ErrorCode != "" {
s := fmt.Sprintf("oauth2: %q", r.ErrorCode)
if r.ErrorDescription != "" {
s += fmt.Sprintf(" %q", r.ErrorDescription)
}
if r.ErrorURI != "" {
s += fmt.Sprintf(" %q", r.ErrorURI)
}
return s
}
return fmt.Sprintf("oauth2: cannot fetch token: %v\nResponse: %s", r.Response.Status, r.Body)
}
39 changes: 36 additions & 3 deletions oauth2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ func TestTokenRetrieveError(t *testing.T) {
t.Errorf("Unexpected token refresh request URL, %v is found.", r.URL)
}
w.Header().Set("Content-type", "application/json")
// "The authorization server responds with an HTTP 400 (Bad Request)" https://www.rfc-editor.org/rfc/rfc6749#section-5.2
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte(`{"error": "invalid_grant"}`))
}))
Expand All @@ -493,15 +494,47 @@ func TestTokenRetrieveError(t *testing.T) {
if err == nil {
t.Fatalf("got no error, expected one")
}
_, ok := err.(*RetrieveError)
re, ok := err.(*RetrieveError)
if !ok {
t.Fatalf("got %T error, expected *RetrieveError; error was: %v", err, err)
}
// Test error string for backwards compatibility
expected := fmt.Sprintf("oauth2: cannot fetch token: %v\nResponse: %s", "400 Bad Request", `{"error": "invalid_grant"}`)
expected := `oauth2: "invalid_grant"`
if errStr := err.Error(); errStr != expected {
t.Fatalf("got %#v, expected %#v", errStr, expected)
}
expected = "invalid_grant"
if re.ErrorCode != expected {
t.Fatalf("got %#v, expected %#v", re.ErrorCode, expected)
}
}

// TestTokenRetrieveError200 tests handling of unorthodox server that returns 200 in error case
func TestTokenRetrieveError200(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.String() != "/token" {
t.Errorf("Unexpected token refresh request URL, %v is found.", r.URL)
}
w.Header().Set("Content-type", "application/json")
w.Write([]byte(`{"error": "invalid_grant"}`))
}))
defer ts.Close()
conf := newConf(ts.URL)
_, err := conf.Exchange(context.Background(), "exchange-code")
if err == nil {
t.Fatalf("got no error, expected one")
}
re, ok := err.(*RetrieveError)
if !ok {
t.Fatalf("got %T error, expected *RetrieveError; error was: %v", err, err)
}
expected := `oauth2: "invalid_grant"`
if errStr := err.Error(); errStr != expected {
t.Fatalf("got %#v, expected %#v", errStr, expected)
}
expected = "invalid_grant"
if re.ErrorCode != expected {
t.Fatalf("got %#v, expected %#v", re.ErrorCode, expected)
}
}

func TestRefreshToken_RefreshTokenReplacement(t *testing.T) {
Expand Down
19 changes: 18 additions & 1 deletion token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,31 @@ func retrieveToken(ctx context.Context, c *Config, v url.Values) (*Token, error)
}

// RetrieveError is the error returned when the token endpoint returns a
// non-2XX HTTP status code.
// non-2XX HTTP status code or populates RFC 6749's 'error' parameter.
// https://datatracker.ietf.org/doc/html/rfc6749#section-5.2
type RetrieveError struct {
Response *http.Response
// Body is the body that was consumed by reading Response.Body.
// It may be truncated.
Body []byte
// ErrorCode is RFC 6749's 'error' parameter.
ErrorCode string
// ErrorDescription is RFC 6749's 'error_description' parameter.
ErrorDescription string
// ErrorURI is RFC 6749's 'error_uri' parameter.
ErrorURI string
}

func (r *RetrieveError) Error() string {
if r.ErrorCode != "" {
s := fmt.Sprintf("oauth2: %q", r.ErrorCode)
if r.ErrorDescription != "" {
s += fmt.Sprintf(" %q", r.ErrorDescription)
}
if r.ErrorURI != "" {
s += fmt.Sprintf(" %q", r.ErrorURI)
}
return s
}
return fmt.Sprintf("oauth2: cannot fetch token: %v\nResponse: %s", r.Response.Status, r.Body)
}

0 comments on commit cfe200d

Please sign in to comment.