From 9c10c7dfbe941601fe12b3ebe98b5b15acd05348 Mon Sep 17 00:00:00 2001 From: Ali Hamdi Ali Fadel Date: Sun, 12 Jul 2026 17:21:51 +0300 Subject: [PATCH] Accept resource-less OAuth refreshes --- .../oauth/resource_indicator_enforcement.rb | 34 ++++++++++++++++--- app/controllers/oauth/tokens_controller.rb | 13 ++++--- .../oauth_resource_indicator_test.rb | 33 +++++++++++++++++- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/app/controllers/concerns/oauth/resource_indicator_enforcement.rb b/app/controllers/concerns/oauth/resource_indicator_enforcement.rb index cdc05c2..e35561e 100644 --- a/app/controllers/concerns/oauth/resource_indicator_enforcement.rb +++ b/app/controllers/concerns/oauth/resource_indicator_enforcement.rb @@ -1,10 +1,12 @@ # Hand-rolled RFC 8707 (Resource Indicators) enforcement -- Doorkeeper has no # native support. The MCP spec requires audience-bound tokens, and this # authorization server exists solely for the MCP endpoint, so both OAuth -# endpoints demand EXACTLY ONE `resource` parameter naming it: +# endpoints demand EXACTLY ONE `resource` parameter naming it, except that the +# token endpoint may allow refresh grants to omit it and inherit the resource +# already bound to the refresh token: # -# - Missing, repeated, or array-style `resource` values are rejected with the -# RFC 8707 `invalid_target` error (each controller renders its own shape). +# - Missing values are rejected unless a controller explicitly allows them; +# repeated and array-style values always fail with RFC 8707 `invalid_target`. # - The comparison follows RFC 3986 semantics: scheme and host are # case-insensitive, the path is byte-exact. `HTTPS://HOST/mcp` passes, # `/MCP` fails. @@ -16,11 +18,19 @@ module ResourceIndicatorEnforcement private def enforce_resource_indicator supplied = raw_resource_values + return if supplied.empty? && allow_omitted_resource_indicator? return if supplied.length == 1 && canonical_resource?(supplied.first) reject_invalid_target end + # Authorization requests and authorization-code exchanges require an + # explicit resource. The token controller overrides this narrowly for + # compatible refresh-token requests. + def allow_omitted_resource_indicator? + false + end + # Rails params collapse repeated keys (`resource=a&resource=b` becomes # just "b"), which would let a doubled parameter slip through looking # valid. Rack::Utils.parse_query keeps repeats as arrays, so parse the @@ -28,8 +38,22 @@ def enforce_resource_indicator # arrive under the raw key "resource[]" and therefore count as zero # `resource` values, rejecting that shape too. def raw_resource_values - [ request.query_string, url_encoded_body ].flat_map do |raw| - Array(Rack::Utils.parse_query(raw.to_s)["resource"]) + raw_parameter_values("resource") + end + + def raw_resource_parameter_omitted? + raw_parameter_sets.none? do |parameters| + parameters.keys.any? { |key| key == "resource" || key.start_with?("resource[") } + end + end + + def raw_parameter_values(name) + raw_parameter_sets.flat_map { |parameters| Array(parameters[name]) } + end + + def raw_parameter_sets + @raw_parameter_sets ||= [ request.query_string, url_encoded_body ].map do |raw| + Rack::Utils.parse_query(raw.to_s) end end diff --git a/app/controllers/oauth/tokens_controller.rb b/app/controllers/oauth/tokens_controller.rb index d2272bf..b63a89f 100644 --- a/app/controllers/oauth/tokens_controller.rb +++ b/app/controllers/oauth/tokens_controller.rb @@ -1,14 +1,19 @@ # Doorkeeper's token endpoint plus mandatory RFC 8707 resource validation for -# both grant types it serves (authorization_code and refresh_token). No -# canonicalization is needed here: the access token's `resource` is copied -# from the grant (or from the rotated-out token on refresh), which already -# stores the canonical value -- see Oauth::AuthorizationsController. +# both grant types it serves (authorization_code and refresh_token). Refresh +# grants may omit `resource` for clients whose OAuth library does not repeat +# the original resource during refresh; Doorkeeper copies the canonical value +# from the rotated-out token. Any explicitly supplied resource is still +# validated exactly -- see Oauth::AuthorizationsController. class Oauth::TokensController < Doorkeeper::TokensController include Oauth::ResourceIndicatorEnforcement before_action :enforce_resource_indicator, only: :create private + def allow_omitted_resource_indicator? + raw_resource_parameter_omitted? && raw_parameter_values("grant_type") == [ "refresh_token" ] + end + # Standard OAuth token-endpoint error: 400 JSON with the RFC 8707 # invalid_target error code. def reject_invalid_target diff --git a/test/integration/oauth_resource_indicator_test.rb b/test/integration/oauth_resource_indicator_test.rb index 8a207ee..33c1d05 100644 --- a/test/integration/oauth_resource_indicator_test.rb +++ b/test/integration/oauth_resource_indicator_test.rb @@ -151,10 +151,41 @@ class OauthResourceIndicatorTest < ActionDispatch::IntegrationTest assert response.parsed_body["result"].present? end - test "refresh without a resource parameter is rejected" do + test "refresh without a resource parameter inherits the original resource" do refresh_token = exchange_code_for_token["refresh_token"] post "/oauth/token", params: refresh_params(refresh_token).except(:resource) + assert_response :success + + refreshed = Doorkeeper::AccessToken.by_token(response.parsed_body["access_token"]) + assert_equal CANONICAL_RESOURCE, refreshed.resource + end + + test "refresh with a mismatched resource is rejected" do + refresh_token = exchange_code_for_token["refresh_token"] + + post "/oauth/token", params: refresh_params(refresh_token).merge(resource: "#{CANONICAL_RESOURCE}/other") + assert_response :bad_request + assert_equal "invalid_target", response.parsed_body["error"] + end + + test "refresh with a repeated resource is rejected" do + refresh_token = exchange_code_for_token["refresh_token"] + body = refresh_params(refresh_token).to_query + "&resource=#{CGI.escape(CANONICAL_RESOURCE)}" + + post "/oauth/token", params: body, + headers: { "Content-Type" => "application/x-www-form-urlencoded" } + assert_response :bad_request + assert_equal "invalid_target", response.parsed_body["error"] + end + + test "refresh with an array resource is rejected" do + refresh_token = exchange_code_for_token["refresh_token"] + body = refresh_params(refresh_token).except(:resource).to_query + + "&resource[]=#{CGI.escape(CANONICAL_RESOURCE)}" + + post "/oauth/token", params: body, + headers: { "Content-Type" => "application/x-www-form-urlencoded" } assert_response :bad_request assert_equal "invalid_target", response.parsed_body["error"] end