Skip to content

Commit 1b6c866

Browse files
committed
fix(oauth): check for oauth error responses even when status code is 200
Certain providers (**cough** GitHub **cough**) return HTTP 200 responses with an error in the body for their OAuth token exchange and token refresh endpoints. Previously, we only checked for non-200 status codes, which could lead to us missing the errors, attempting to unmarshal the body as a token, failing to detect the error, and even saving the empty token. This commit attempts to unmarshal the body as an OAuth error response even when the status code is 200. If the body is not an OAuth error response, we fall back to the previous behavior.
1 parent 238a36d commit 1b6c866

File tree

2 files changed

+530
-2
lines changed

2 files changed

+530
-2
lines changed

client/transport/oauth.go

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,21 @@ func (h *OAuthHandler) refreshToken(ctx context.Context, refreshToken string) (*
246246
return nil, extractOAuthError(body, resp.StatusCode, "refresh token request failed")
247247
}
248248

249+
// Read the response body for parsing
250+
body, err := io.ReadAll(resp.Body)
251+
if err != nil {
252+
return nil, fmt.Errorf("failed to read token response body: %w", err)
253+
}
254+
255+
// GitHub returns HTTP 200 even for errors, with error details in the JSON body
256+
// Check if the response contains an error field before parsing as Token
257+
var oauthErr OAuthError
258+
if err := json.Unmarshal(body, &oauthErr); err == nil && oauthErr.ErrorCode != "" {
259+
return nil, extractOAuthError(body, resp.StatusCode, "refresh token request failed")
260+
}
261+
249262
var tokenResp Token
250-
if err := json.NewDecoder(resp.Body).Decode(&tokenResp); err != nil {
263+
if err := json.Unmarshal(body, &tokenResp); err != nil {
251264
return nil, fmt.Errorf("failed to decode token response: %w", err)
252265
}
253266

@@ -279,6 +292,17 @@ func (h *OAuthHandler) GetClientID() string {
279292
return h.config.ClientID
280293
}
281294

295+
// truncateToken returns first 10 chars of token for logging, or "(empty)" if empty
296+
func truncateToken(token string) string {
297+
if token == "" {
298+
return "(empty)"
299+
}
300+
if len(token) > 10 {
301+
return token[:10]
302+
}
303+
return token
304+
}
305+
282306
// extractOAuthError attempts to parse an OAuth error response from the response body
283307
func extractOAuthError(body []byte, statusCode int, context string) error {
284308
// Try to parse the error as an OAuth error response
@@ -679,8 +703,21 @@ func (h *OAuthHandler) ProcessAuthorizationResponse(ctx context.Context, code, s
679703
return extractOAuthError(body, resp.StatusCode, "token request failed")
680704
}
681705

706+
// Read the response body for parsing
707+
body, err := io.ReadAll(resp.Body)
708+
if err != nil {
709+
return fmt.Errorf("failed to read token response body: %w", err)
710+
}
711+
712+
// GitHub returns HTTP 200 even for errors, with error details in the JSON body
713+
// Check if the response contains an error field before parsing as Token
714+
var oauthErr OAuthError
715+
if err := json.Unmarshal(body, &oauthErr); err == nil && oauthErr.ErrorCode != "" {
716+
return extractOAuthError(body, resp.StatusCode, "token request failed")
717+
}
718+
682719
var tokenResp Token
683-
if err := json.NewDecoder(resp.Body).Decode(&tokenResp); err != nil {
720+
if err := json.Unmarshal(body, &tokenResp); err != nil {
684721
return fmt.Errorf("failed to decode token response: %w", err)
685722
}
686723

0 commit comments

Comments
 (0)