-
Notifications
You must be signed in to change notification settings - Fork 133
fix(client/oauth): preserve token endpoint error diagnostics #549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,7 +14,16 @@ module OAuth | |||||||||
| # `Provider`; this class consumes a Provider plus signal data extracted from | ||||||||||
| # the failing response (resource_metadata URL, scope challenge). | ||||||||||
| class Flow | ||||||||||
| class AuthorizationError < StandardError; end | ||||||||||
| class AuthorizationError < StandardError | ||||||||||
| attr_reader :http_status, :error, :error_description | ||||||||||
|
|
||||||||||
| def initialize(message = nil, http_status: nil, error: nil, error_description: nil) | ||||||||||
| super(message) | ||||||||||
| @http_status = http_status | ||||||||||
| @error = error | ||||||||||
| @error_description = error_description | ||||||||||
| end | ||||||||||
| end | ||||||||||
|
|
||||||||||
| # Raised specifically when the token endpoint rejects a grant with | ||||||||||
| # `error: "invalid_grant"` (RFC 6749 §5.2). Callers use this to | ||||||||||
|
|
@@ -1062,11 +1071,7 @@ def post_to_token_endpoint(as_metadata:, client_info:, form:) | |||||||||
| end | ||||||||||
|
|
||||||||||
| if response.status < 200 || response.status >= 300 | ||||||||||
| if token_endpoint_error_code(response) == "invalid_grant" | ||||||||||
| raise InvalidGrantError, "Token endpoint rejected the grant: invalid_grant." | ||||||||||
| end | ||||||||||
|
|
||||||||||
| raise AuthorizationError, "Token endpoint returned status #{response.status}." | ||||||||||
| raise token_endpoint_error(response) | ||||||||||
| end | ||||||||||
|
|
||||||||||
| parsed = begin | ||||||||||
|
|
@@ -1087,17 +1092,35 @@ def post_to_token_endpoint(as_metadata:, client_info:, form:) | |||||||||
| parsed | ||||||||||
| end | ||||||||||
|
|
||||||||||
| # Extracts the `error` code from an RFC 6749 §5.2 error response body | ||||||||||
| # when one is parseable. Returns nil on any parse failure or when | ||||||||||
| # the body is not JSON. | ||||||||||
| def token_endpoint_error_code(response) | ||||||||||
| body = response_body_string(response).to_s | ||||||||||
| return if body.empty? | ||||||||||
| # Surface only RFC 6749 §5.2 diagnostic fields, never the raw response, | ||||||||||
| # which may contain tokens or other credentials. Classify the original | ||||||||||
| # code so sanitization cannot turn malformed input into invalid_grant. | ||||||||||
| def token_endpoint_error(response) | ||||||||||
| parsed = begin | ||||||||||
| JSON.parse(response_body_string(response)) | ||||||||||
| rescue JSON::ParserError | ||||||||||
| nil | ||||||||||
| end | ||||||||||
| parsed = {} unless parsed.is_a?(Hash) | ||||||||||
|
|
||||||||||
| error_class = parsed["error"] == "invalid_grant" ? InvalidGrantError : AuthorizationError | ||||||||||
| error = token_endpoint_diagnostic(parsed["error"], limit: 128) | ||||||||||
| description = token_endpoint_diagnostic(parsed["error_description"], limit: 512) | ||||||||||
|
Comment on lines
+1107
to
+1108
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These limits define behavior the docs state; please name them next to the class's other constants so the code and the docs point at one place.
Suggested change
|
||||||||||
| message = "Token endpoint returned status #{response.status}." | ||||||||||
| message += " #{[error, description].compact.join(": ")}" if error || description | ||||||||||
|
|
||||||||||
| error_class.new(message, http_status: response.status, error: error, error_description: description) | ||||||||||
| end | ||||||||||
|
|
||||||||||
| def token_endpoint_diagnostic(value, limit:) | ||||||||||
| return unless value.is_a?(String) | ||||||||||
|
|
||||||||||
| parsed = JSON.parse(body) | ||||||||||
| parsed["error"] if parsed.is_a?(Hash) | ||||||||||
| rescue JSON::ParserError | ||||||||||
| nil | ||||||||||
| # RFC 6749 permits printable ASCII except double quotes and backslashes. | ||||||||||
| # Replace other characters to keep provider text on one log line. | ||||||||||
| value = value.gsub(/[^\x20-\x21\x23-\x5B\x5D-\x7E]/, " ").strip | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
value = JSON.parse("{\"error\":\"invalid_grant\",\"error_description\":\"bad \xFF byte\"}".b)["error_description"]
value.gsub(/[^\x20-\x21\x23-\x5B\x5D-\x7E]/, " ") # => ArgumentError: invalid byte sequence in UTF-8Scrub first, as the server-side
Suggested change
Also wrap the diagnostic extraction in |
||||||||||
| return if value.empty? | ||||||||||
|
|
||||||||||
| value.length > limit ? "#{value[0, limit - 3]}..." : value | ||||||||||
| end | ||||||||||
|
|
||||||||||
| # Per RFC 6749 Section 2.3.1, the `client_id` and `client_secret` MUST be | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.