this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

OAuth client: Surface callback error responses (#1155)

Addresses #1152

This can be tested by initiating a login with the demo client app, but
hitting "cancel" on the AS login page (or by refusing to grant the
requested scopes).

authored by

David Buchanan and committed by
GitHub
cbaa8d37 c5eaa30f

+56 -6
+36 -6
atproto/auth/oauth/oauth.go
··· 579 579 580 580 // High-level helper for completing auth flow: verifies callback query parameters against persisted auth request info, makes initial token request to the auth server, validates account identifier, and persists session data. 581 581 func (app *ClientApp) ProcessCallback(ctx context.Context, params url.Values) (*ClientSessionData, error) { 582 + // There are two callback response formats, for error and non-error conditions, each expecting different 583 + // parameters. 584 + // 585 + // Error responses expect: state, error (and optionally: error_description, error_uri) 586 + // Non-error responses expect: state, iss, code 582 587 583 588 state := params.Get("state") 584 - authserverURL := params.Get("iss") 585 - authCode := params.Get("code") 586 - if state == "" || authserverURL == "" || authCode == "" { 587 - return nil, fmt.Errorf("missing required query param") 589 + if state == "" { 590 + return nil, fmt.Errorf("missing state query param") 588 591 } 589 592 590 593 info, err := app.Store.GetAuthRequestInfo(ctx, state) 591 594 if err != nil { 592 595 return nil, fmt.Errorf("loading auth request info: %w", err) 593 596 } 597 + // This check should never fail, but it guards against a faulty ClientAuthStore implementation 598 + if info.State != state { 599 + return nil, fmt.Errorf("callback state doesn't match request info") 600 + } 594 601 595 - if info.State != state || info.AuthServerURL != authserverURL { 596 - return nil, fmt.Errorf("callback params don't match request info") 602 + // NOTE: A corresponding `state` is expected even under error conditions, 603 + // hence we check error *after* checking state. 604 + errorCode := params.Get("error") 605 + if errorCode != "" { 606 + var errorUri *syntax.URI 607 + parsedUri, err := syntax.ParseURI(params.Get("error_uri")) 608 + if err == nil { 609 + errorUri = &parsedUri 610 + } 611 + return nil, &AuthRequestCallbackError{ 612 + ErrorCode: errorCode, 613 + ErrorDescription: params.Get("error_description"), 614 + ErrorURI: errorUri, 615 + } 616 + } 617 + 618 + // If we reached here, there was no `error` and we can process the rest of the parameters 619 + authserverURL := params.Get("iss") 620 + authCode := params.Get("code") 621 + if authserverURL == "" || authCode == "" { 622 + return nil, fmt.Errorf("missing required query param") 623 + } 624 + 625 + if info.AuthServerURL != authserverURL { 626 + return nil, fmt.Errorf("callback iss doesn't match request info") 597 627 } 598 628 599 629 tokenResp, err := app.SendInitialTokenRequest(ctx, authCode, *info)
+20
atproto/auth/oauth/types.go
··· 407 407 // Refresh token, for doing additional token requests to the auth server. 408 408 RefreshToken string `json:"refresh_token"` 409 409 } 410 + 411 + // Returned by [ClientApp.ProcessCallback] if the AS signals an error in the redirect URL parameters, per rfc6749 section 4.1.2.1 412 + // 413 + // NOTE: This is untrusted data and should not be e.g. rendered to HTML without appropriate escaping 414 + type AuthRequestCallbackError struct { 415 + ErrorCode string 416 + ErrorDescription string 417 + ErrorURI *syntax.URI 418 + } 419 + 420 + func (e *AuthRequestCallbackError) Error() string { 421 + res := "OAuth request callback error: " + e.ErrorCode 422 + if e.ErrorDescription != "" { 423 + res += ": " + e.ErrorDescription 424 + } 425 + if e.ErrorURI != nil { 426 + res += " (" + e.ErrorURI.String() + ")" 427 + } 428 + return res 429 + }