From c0ceba41fbdfbd07c869ad104e87cb464a28f9c8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 27 Aug 2026 23:12:24 -0700 Subject: [PATCH 1/4] Expire OAuth access tokens and rotate refresh tokens OAuth-issued access tokens now expire an hour after issuance and carry a refresh token, generated at creation. Personal access tokens are untouched: no expiry, no refresh token. The token endpoint gains the refresh_token grant with OAuth 2.1 rotation semantics: each refresh rotates both the access and refresh token on the grant record, so the previous refresh token dies with the rotation and the Connected Apps list still shows one grant per client. Refresh requests must present the client_id the grant was issued to. Bearer authentication ignores expired tokens, and revocation accepts either token, killing the whole grant. Discovery and DCR responses advertise the refresh_token grant. --- app/controllers/oauth/clients_controller.rb | 2 +- app/controllers/oauth/metadata_controller.rb | 2 +- .../oauth/revocations_controller.rb | 4 +- app/controllers/oauth/tokens_controller.rb | 69 +++++++--- app/models/identity.rb | 2 +- app/models/identity/access_token.rb | 25 ++++ .../20260827100000_add_oauth_token_expiry.rb | 10 ++ db/schema.rb | 3 + db/schema_sqlite.rb | 3 + test/integration/oauth_flow_test.rb | 126 ++++++++++++++++++ test/models/identity/access_token_test.rb | 55 ++++++++ 11 files changed, 281 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20260827100000_add_oauth_token_expiry.rb diff --git a/app/controllers/oauth/clients_controller.rb b/app/controllers/oauth/clients_controller.rb index a2f07fee2e..9e5c772f40 100644 --- a/app/controllers/oauth/clients_controller.rb +++ b/app/controllers/oauth/clients_controller.rb @@ -74,7 +74,7 @@ def dynamic_client_registration_response(client) client_name: client.name, redirect_uris: client.redirect_uris, token_endpoint_auth_method: "none", - grant_types: %w[ authorization_code ], + grant_types: %w[ authorization_code refresh_token ], response_types: %w[ code ], scope: client.scopes.join(" ") } diff --git a/app/controllers/oauth/metadata_controller.rb b/app/controllers/oauth/metadata_controller.rb index 9bc84ea15e..4f9032bc88 100644 --- a/app/controllers/oauth/metadata_controller.rb +++ b/app/controllers/oauth/metadata_controller.rb @@ -9,7 +9,7 @@ def show registration_endpoint: oauth_clients_url, revocation_endpoint: oauth_revocation_url, response_types_supported: %w[ code ], - grant_types_supported: %w[ authorization_code ], + grant_types_supported: %w[ authorization_code refresh_token ], token_endpoint_auth_methods_supported: %w[ none ], code_challenge_methods_supported: %w[ S256 ], scopes_supported: %w[ read write ] diff --git a/app/controllers/oauth/revocations_controller.rb b/app/controllers/oauth/revocations_controller.rb index 5e49c5d1e9..124559a660 100644 --- a/app/controllers/oauth/revocations_controller.rb +++ b/app/controllers/oauth/revocations_controller.rb @@ -12,6 +12,8 @@ def create private def set_access_token - @access_token = Identity::AccessToken.find_by(token: params.require(:token)) + token = params.require(:token) + @access_token = Identity::AccessToken.find_by(token: token) || + Identity::AccessToken.find_by(refresh_token: token) end end diff --git a/app/controllers/oauth/tokens_controller.rb b/app/controllers/oauth/tokens_controller.rb index b5eba3e188..b3b3d7c97d 100644 --- a/app/controllers/oauth/tokens_controller.rb +++ b/app/controllers/oauth/tokens_controller.rb @@ -5,28 +5,42 @@ class Oauth::TokensController < Oauth::BaseController rate_limit to: 20, within: 1.minute, only: :create, with: :oauth_rate_limit_exceeded before_action :validate_grant_type - before_action :set_auth_code - before_action :set_client - before_action :validate_pkce - before_action :validate_redirect_uri - before_action :set_identity + + with_options if: :authorization_code_grant? do + before_action :set_auth_code + before_action :set_client + before_action :validate_pkce + before_action :validate_redirect_uri + before_action :set_identity + end + + with_options unless: :authorization_code_grant? do + before_action :set_refreshable_access_token + before_action :validate_refresh_client + end def create - granted = @auth_code.scope.to_s.split - permission = granted.include?("write") ? "write" : "read" - access_token = @identity.access_tokens.create! oauth_client: @client, permission: permission - - render json: { - access_token: access_token.token, - token_type: "Bearer", - scope: granted.join(" ") - } + if authorization_code_grant? + granted = @auth_code.scope.to_s.split + permission = granted.include?("write") ? "write" : "read" + access_token = @identity.access_tokens.create! oauth_client: @client, permission: permission + + render json: token_response(access_token, scope: granted.join(" ")) + else + @access_token.refresh! + + render json: token_response(@access_token) + end end private + def authorization_code_grant? + params[:grant_type] == "authorization_code" + end + def validate_grant_type - unless params[:grant_type] == "authorization_code" - oauth_error "unsupported_grant_type", "Only authorization_code grant is supported" + unless params[:grant_type].in?(%w[ authorization_code refresh_token ]) + oauth_error "unsupported_grant_type", "Only authorization_code and refresh_token grants are supported" end end @@ -59,4 +73,27 @@ def set_identity oauth_error "invalid_grant", "Identity not found" end end + + def set_refreshable_access_token + unless params[:refresh_token].present? && + @access_token = Identity::AccessToken.oauth.find_by(refresh_token: params[:refresh_token]) + oauth_error "invalid_grant", "Invalid refresh token" + end + end + + def validate_refresh_client + unless @access_token.oauth_client.client_id == params[:client_id] + oauth_error "invalid_grant", "Refresh token was not issued to this client" + end + end + + def token_response(access_token, scope: nil) + { + access_token: access_token.token, + token_type: "Bearer", + expires_in: access_token.expires_in, + refresh_token: access_token.refresh_token, + scope: scope + }.compact + end end diff --git a/app/models/identity.rb b/app/models/identity.rb index 059e8ac9c0..aa28518338 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -17,7 +17,7 @@ class Identity < ApplicationRecord normalizes :email_address, with: ->(value) { value.strip.downcase.presence } def self.find_by_permissable_access_token(token, method:) - if (access_token = AccessToken.find_by(token: token)) && access_token.allows?(method) + if (access_token = AccessToken.active.find_by(token: token)) && access_token.allows?(method) access_token.identity end end diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index 6cd1fc3b24..c723d00d2a 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -1,14 +1,39 @@ class Identity::AccessToken < ApplicationRecord + EXPIRES_IN = 1.hour + belongs_to :identity belongs_to :oauth_client, class_name: "Oauth::Client", optional: true scope :personal, -> { where oauth_client_id: nil } scope :oauth, -> { where.not oauth_client_id: nil } + scope :active, -> { where(expires_at: nil).or(where(expires_at: Time.current..)) } has_secure_token enum :permission, %w[ read write ].index_by(&:itself), default: :read + before_create :set_expiry_and_refresh_token, if: :oauth_client_id? + def allows?(method) method.in?(%w[ GET HEAD ]) || write? end + + def expired? + expires_at? && expires_at.past? + end + + def expires_in + (expires_at - Time.current).to_i if expires_at? + end + + def refresh! + update! token: self.class.generate_unique_secure_token, + refresh_token: self.class.generate_unique_secure_token, + expires_at: EXPIRES_IN.from_now + end + + private + def set_expiry_and_refresh_token + self.expires_at ||= EXPIRES_IN.from_now + self.refresh_token ||= self.class.generate_unique_secure_token + end end diff --git a/db/migrate/20260827100000_add_oauth_token_expiry.rb b/db/migrate/20260827100000_add_oauth_token_expiry.rb new file mode 100644 index 0000000000..bfb8a2fda2 --- /dev/null +++ b/db/migrate/20260827100000_add_oauth_token_expiry.rb @@ -0,0 +1,10 @@ +class AddOauthTokenExpiry < ActiveRecord::Migration[8.2] + def change + change_table :identity_access_tokens, bulk: true do |t| + t.string :refresh_token + t.datetime :expires_at + + t.index :refresh_token, unique: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 21e893e25d..5719fca6d4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -354,13 +354,16 @@ create_table "identity_access_tokens", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false t.text "description" + t.datetime "expires_at" t.uuid "identity_id", null: false t.uuid "oauth_client_id" t.string "permission" + t.string "refresh_token" t.string "token" t.datetime "updated_at", null: false t.index ["identity_id"], name: "index_access_token_on_identity_id" t.index ["oauth_client_id"], name: "index_identity_access_tokens_on_oauth_client_id" + t.index ["refresh_token"], name: "index_identity_access_tokens_on_refresh_token", unique: true end create_table "identity_transfers", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| diff --git a/db/schema_sqlite.rb b/db/schema_sqlite.rb index d6947a20de..661a6e9132 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -354,13 +354,16 @@ create_table "identity_access_tokens", id: :uuid, force: :cascade do |t| t.datetime "created_at", null: false t.text "description", limit: 65535 + t.datetime "expires_at" t.uuid "identity_id", null: false t.uuid "oauth_client_id" t.string "permission", limit: 255 + t.string "refresh_token", limit: 255 t.string "token", limit: 255 t.datetime "updated_at", null: false t.index ["identity_id"], name: "index_access_token_on_identity_id" t.index ["oauth_client_id"], name: "index_identity_access_tokens_on_oauth_client_id" + t.index ["refresh_token"], name: "index_identity_access_tokens_on_refresh_token", unique: true end create_table "identity_transfers", id: :uuid, force: :cascade do |t| diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb index 2e5f91baf9..b7dc6dcd72 100644 --- a/test/integration/oauth_flow_test.rb +++ b/test/integration/oauth_flow_test.rb @@ -157,6 +157,8 @@ class OauthFlowTest < ActionDispatch::IntegrationTest body = response.parsed_body assert_not_nil body["access_token"] + assert_not_nil body["refresh_token"] + assert_operator body["expires_in"], :>, 0 assert_equal "Bearer", body["token_type"] assert_equal "read", body["scope"] @@ -297,6 +299,118 @@ class OauthFlowTest < ActionDispatch::IntegrationTest end + # Refresh Grant + + test "refresh grant rotates access and refresh tokens" do + client = oauth_clients(:mcp_client) + token = identities(:david).access_tokens.create!(oauth_client: client) + old_access_token, old_refresh_token = token.token, token.refresh_token + + assert_no_difference "Identity::AccessToken.count" do + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: old_refresh_token, + client_id: client.client_id + }, as: :json + end + end + + assert_response :success + body = response.parsed_body + + assert_not_nil body["access_token"] + assert_not_nil body["refresh_token"] + assert_operator body["expires_in"], :>, 0 + assert_not_equal old_access_token, body["access_token"] + assert_not_equal old_refresh_token, body["refresh_token"] + end + + test "refresh grant works after the access token expires" do + client = oauth_clients(:mcp_client) + token = identities(:david).access_tokens.create!(oauth_client: client) + + travel Identity::AccessToken::EXPIRES_IN + 1.minute do + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: token.refresh_token, + client_id: client.client_id + }, as: :json + end + + assert_response :success + assert_not Identity::AccessToken.find_by(token: response.parsed_body["access_token"]).expired? + end + end + + test "refresh grant invalidates the previous refresh token" do + client = oauth_clients(:mcp_client) + token = identities(:david).access_tokens.create!(oauth_client: client) + old_refresh_token = token.refresh_token + + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: old_refresh_token, + client_id: client.client_id + }, as: :json + end + assert_response :success + + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: old_refresh_token, + client_id: client.client_id + }, as: :json + end + + assert_response :bad_request + assert_equal "invalid_grant", response.parsed_body["error"] + end + + test "refresh grant rejects a client mismatch" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) + + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: token.refresh_token, + client_id: oauth_clients(:trusted_client).client_id + }, as: :json + end + + assert_response :bad_request + assert_equal "invalid_grant", response.parsed_body["error"] + end + + test "refresh grant rejects unknown refresh token" do + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: "nonexistent", + client_id: oauth_clients(:mcp_client).client_id + }, as: :json + end + + assert_response :bad_request + assert_equal "invalid_grant", response.parsed_body["error"] + end + + test "refresh grant rejects blank refresh token" do + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + client_id: oauth_clients(:mcp_client).client_id + }, as: :json + end + + assert_response :bad_request + assert_equal "invalid_grant", response.parsed_body["error"] + end + + # Token Revocation (RFC 7009) test "revocation deletes access token" do @@ -311,6 +425,18 @@ class OauthFlowTest < ActionDispatch::IntegrationTest assert_response :success end + test "revocation by refresh token revokes the grant" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) + + assert_difference "Identity::AccessToken.count", -1 do + untenanted do + post oauth_revocation_path, params: { token: token.refresh_token }, as: :json + end + end + + assert_response :success + end + test "revocation returns 200 for nonexistent token" do untenanted do post oauth_revocation_path, params: { token: "nonexistent_token" }, as: :json diff --git a/test/models/identity/access_token_test.rb b/test/models/identity/access_token_test.rb index 7324d97ab1..15047d6d68 100644 --- a/test/models/identity/access_token_test.rb +++ b/test/models/identity/access_token_test.rb @@ -1,4 +1,59 @@ require "test_helper" class Identity::AccessTokenTest < ActiveSupport::TestCase + test "oauth tokens expire and get refresh tokens on create" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) + + assert_not_nil token.refresh_token + assert_in_delta Identity::AccessToken::EXPIRES_IN.from_now, token.expires_at, 5.seconds + end + + test "personal tokens don't expire and get no refresh token" do + token = identities(:david).access_tokens.create!(description: "Personal") + + assert_nil token.refresh_token + assert_nil token.expires_at + assert_not token.expired? + end + + test "expired?" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) + + assert_not token.expired? + travel Identity::AccessToken::EXPIRES_IN + 1.second do + assert token.expired? + end + end + + test "refresh! rotates both tokens and extends expiry" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) + old_token, old_refresh_token = token.token, token.refresh_token + + travel Identity::AccessToken::EXPIRES_IN + 1.minute do + token.refresh! + + assert_not_equal old_token, token.token + assert_not_equal old_refresh_token, token.refresh_token + assert_not token.expired? + end + end + + test "active scope excludes expired tokens" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) + + assert_includes Identity::AccessToken.active, token + travel Identity::AccessToken::EXPIRES_IN + 1.second do + assert_not_includes Identity::AccessToken.active, token + assert_includes Identity::AccessToken.active, identity_access_tokens(:davids_api_token) + end + end + + test "find_by_permissable_access_token rejects expired tokens" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client), permission: :write) + + assert_equal identities(:david), Identity.find_by_permissable_access_token(token.token, method: "GET") + travel Identity::AccessToken::EXPIRES_IN + 1.second do + assert_nil Identity.find_by_permissable_access_token(token.token, method: "GET") + end + end end From edcd070fa86ce0e5068a60f9e478c30362272163 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 27 Aug 2026 23:23:11 -0700 Subject: [PATCH 2/4] Rotate refresh tokens atomically and drop the bang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refresh-token consumption raced: two concurrent refreshes could both load the grant and both answer 200, one with already-dead credentials. Rotation now updates guarded on the presented refresh token, so the loser matches no row and gets invalid_grant. The rename to refresh follows STYLE.md's bang rule — no non-bang counterpart, no bang. Also assert grant_types in the discovery and DCR responses so the advertised refresh grant can't silently regress. --- app/controllers/oauth/tokens_controller.rb | 8 +++++--- app/models/identity/access_token.rb | 13 ++++++++++--- test/integration/oauth_flow_test.rb | 3 +++ test/models/identity/access_token_test.rb | 13 +++++++++++-- 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/app/controllers/oauth/tokens_controller.rb b/app/controllers/oauth/tokens_controller.rb index b3b3d7c97d..b42818971a 100644 --- a/app/controllers/oauth/tokens_controller.rb +++ b/app/controllers/oauth/tokens_controller.rb @@ -27,9 +27,11 @@ def create render json: token_response(access_token, scope: granted.join(" ")) else - @access_token.refresh! - - render json: token_response(@access_token) + if @access_token.refresh + render json: token_response(@access_token) + else + oauth_error "invalid_grant", "Invalid refresh token" + end end end diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index c723d00d2a..0efe308c60 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -25,10 +25,17 @@ def expires_in (expires_at - Time.current).to_i if expires_at? end - def refresh! - update! token: self.class.generate_unique_secure_token, + # Rotates atomically on the presented refresh token, so a concurrent + # rotation wins the row and the loser comes up empty-handed. + def refresh + rotated = { token: self.class.generate_unique_secure_token, refresh_token: self.class.generate_unique_secure_token, - expires_at: EXPIRES_IN.from_now + expires_at: EXPIRES_IN.from_now, updated_at: Time.current } + + if self.class.where(id: id, refresh_token: refresh_token).update_all(rotated) == 1 + assign_attributes rotated + true + end end private diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb index b7dc6dcd72..08f507d97b 100644 --- a/test/integration/oauth_flow_test.rb +++ b/test/integration/oauth_flow_test.rb @@ -468,6 +468,8 @@ class OauthFlowTest < ActionDispatch::IntegrationTest assert_match %r{/oauth/clients$}, body["registration_endpoint"] assert_includes body["response_types_supported"], "code" assert_includes body["code_challenge_methods_supported"], "S256" + assert_includes body["grant_types_supported"], "authorization_code" + assert_includes body["grant_types_supported"], "refresh_token" end test "protected resource metadata includes authorization server" do @@ -501,6 +503,7 @@ class OauthFlowTest < ActionDispatch::IntegrationTest assert_not_nil body["client_id"] assert_equal "Test MCP Client", body["client_name"] assert_equal [ "http://127.0.0.1:8888/callback" ], body["redirect_uris"] + assert_equal %w[ authorization_code refresh_token ], body["grant_types"] end test "DCR creates client with https redirect" do diff --git a/test/models/identity/access_token_test.rb b/test/models/identity/access_token_test.rb index 15047d6d68..6497eb75f9 100644 --- a/test/models/identity/access_token_test.rb +++ b/test/models/identity/access_token_test.rb @@ -25,12 +25,12 @@ class Identity::AccessTokenTest < ActiveSupport::TestCase end end - test "refresh! rotates both tokens and extends expiry" do + test "refresh rotates both tokens and extends expiry" do token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) old_token, old_refresh_token = token.token, token.refresh_token travel Identity::AccessToken::EXPIRES_IN + 1.minute do - token.refresh! + assert token.refresh assert_not_equal old_token, token.token assert_not_equal old_refresh_token, token.refresh_token @@ -38,6 +38,15 @@ class Identity::AccessTokenTest < ActiveSupport::TestCase end end + test "refresh fails when the presented refresh token was already rotated" do + token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) + stale = Identity::AccessToken.find(token.id) + + assert token.refresh + assert_not stale.refresh + assert_equal token.reload.token, token.token + end + test "active scope excludes expired tokens" do token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) From 0cd462c5361d981e5943e0ca0c57c4d5941c4b1b Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 31 Aug 2026 12:53:45 -0700 Subject: [PATCH 3/4] Validate and echo scope on refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A refresh request that carried a scope was silently ignored: the response omitted the scope field while the rotated token kept the grant's original permission, so a client narrowing a write token to read believed it held a read-only token but received a write-capable one, and a broadening request was accepted rather than rejected. Validate the requested scope against the grant (RFC 6749 §6): reject anything beyond it with invalid_scope, narrow the rotated token to a requested subset, and always echo the effective scope. --- app/controllers/oauth/tokens_controller.rb | 27 ++++++++++- app/models/identity/access_token.rb | 4 +- test/integration/oauth_flow_test.rb | 53 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/app/controllers/oauth/tokens_controller.rb b/app/controllers/oauth/tokens_controller.rb index b42818971a..ac06dd7428 100644 --- a/app/controllers/oauth/tokens_controller.rb +++ b/app/controllers/oauth/tokens_controller.rb @@ -17,6 +17,7 @@ class Oauth::TokensController < Oauth::BaseController with_options unless: :authorization_code_grant? do before_action :set_refreshable_access_token before_action :validate_refresh_client + before_action :set_refresh_scope end def create @@ -27,8 +28,8 @@ def create render json: token_response(access_token, scope: granted.join(" ")) else - if @access_token.refresh - render json: token_response(@access_token) + if @access_token.refresh(permission: @refresh_permission) + render json: token_response(@access_token, scope: scope_for(@access_token.permission)) else oauth_error "invalid_grant", "Invalid refresh token" end @@ -89,6 +90,28 @@ def validate_refresh_client end end + # A refresh request may narrow scope but never widen it (RFC 6749 §6). An + # omitted scope keeps the original grant; a requested subset narrows the + # rotated token; anything beyond the grant is invalid_scope. + def set_refresh_scope + granted = granted_scopes(@access_token.permission) + requested = params[:scope].present? ? params[:scope].split : granted + + if requested.present? && requested.all? { |scope| granted.include?(scope) } + @refresh_permission = requested.include?("write") ? "write" : "read" + else + oauth_error "invalid_scope", "Requested scope exceeds the original grant" + end + end + + def granted_scopes(permission) + permission == "write" ? %w[ read write ] : %w[ read ] + end + + def scope_for(permission) + granted_scopes(permission).join(" ") + end + def token_response(access_token, scope: nil) { access_token: access_token.token, diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index 0efe308c60..5ad7ebaf94 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -27,10 +27,10 @@ def expires_in # Rotates atomically on the presented refresh token, so a concurrent # rotation wins the row and the loser comes up empty-handed. - def refresh + def refresh(permission: self.permission) rotated = { token: self.class.generate_unique_secure_token, refresh_token: self.class.generate_unique_secure_token, - expires_at: EXPIRES_IN.from_now, updated_at: Time.current } + expires_at: EXPIRES_IN.from_now, permission: permission, updated_at: Time.current } if self.class.where(id: id, refresh_token: refresh_token).update_all(rotated) == 1 assign_attributes rotated diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb index 08f507d97b..6c7fc4fc32 100644 --- a/test/integration/oauth_flow_test.rb +++ b/test/integration/oauth_flow_test.rb @@ -326,6 +326,59 @@ class OauthFlowTest < ActionDispatch::IntegrationTest assert_not_equal old_refresh_token, body["refresh_token"] end + test "refresh grant echoes the granted scope" do + client = oauth_clients(:mcp_client) + token = identities(:david).access_tokens.create!(oauth_client: client, permission: :write) + + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: token.refresh_token, + client_id: client.client_id + }, as: :json + end + + assert_response :success + assert_equal "read write", response.parsed_body["scope"] + end + + test "refresh grant narrows the token to a requested subset scope" do + client = oauth_clients(:mcp_client) + token = identities(:david).access_tokens.create!(oauth_client: client, permission: :write) + + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: token.refresh_token, + client_id: client.client_id, + scope: "read" + }, as: :json + end + + assert_response :success + assert_equal "read", response.parsed_body["scope"] + assert_equal "read", Identity::AccessToken.find_by(token: response.parsed_body["access_token"]).permission + end + + test "refresh grant rejects a scope broader than the original grant" do + client = oauth_clients(:mcp_client) + token = identities(:david).access_tokens.create!(oauth_client: client, permission: :read) + old_refresh_token = token.refresh_token + + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: old_refresh_token, + client_id: client.client_id, + scope: "write" + }, as: :json + end + + assert_response :bad_request + assert_equal "invalid_scope", response.parsed_body["error"] + assert_equal old_refresh_token, token.reload.refresh_token + end + test "refresh grant works after the access token expires" do client = oauth_clients(:mcp_client) token = identities(:david).access_tokens.create!(oauth_client: client) From 8f5ef6958aa02bb0949457baa2eb2a19b23d08ea Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 10 Sep 2026 11:30:17 -0700 Subject: [PATCH 4/4] Reject a blank scope on refresh instead of restoring the full grant --- app/controllers/oauth/tokens_controller.rb | 7 +++++-- test/integration/oauth_flow_test.rb | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/app/controllers/oauth/tokens_controller.rb b/app/controllers/oauth/tokens_controller.rb index ac06dd7428..1778dfb0d4 100644 --- a/app/controllers/oauth/tokens_controller.rb +++ b/app/controllers/oauth/tokens_controller.rb @@ -92,10 +92,13 @@ def validate_refresh_client # A refresh request may narrow scope but never widen it (RFC 6749 §6). An # omitted scope keeps the original grant; a requested subset narrows the - # rotated token; anything beyond the grant is invalid_scope. + # rotated token; anything beyond the grant is invalid_scope. Only an absent + # parameter means "keep the grant" — a blank one names no scope, and the + # empty scope list below rejects it, since no scope-token is a malformed + # scope (RFC 6749 §3.3), not a request for the full grant. def set_refresh_scope granted = granted_scopes(@access_token.permission) - requested = params[:scope].present? ? params[:scope].split : granted + requested = params[:scope].nil? ? granted : params[:scope].to_s.split if requested.present? && requested.all? { |scope| granted.include?(scope) } @refresh_permission = requested.include?("write") ? "write" : "read" diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb index 6c7fc4fc32..b82e0778ab 100644 --- a/test/integration/oauth_flow_test.rb +++ b/test/integration/oauth_flow_test.rb @@ -379,6 +379,25 @@ class OauthFlowTest < ActionDispatch::IntegrationTest assert_equal old_refresh_token, token.reload.refresh_token end + test "refresh grant rejects a blank scope rather than restoring the full grant" do + client = oauth_clients(:mcp_client) + token = identities(:david).access_tokens.create!(oauth_client: client, permission: :write) + old_refresh_token = token.refresh_token + + untenanted do + post oauth_token_path, params: { + grant_type: "refresh_token", + refresh_token: old_refresh_token, + client_id: client.client_id, + scope: " " + }, as: :json + end + + assert_response :bad_request + assert_equal "invalid_scope", response.parsed_body["error"] + assert_equal old_refresh_token, token.reload.refresh_token + end + test "refresh grant works after the access token expires" do client = oauth_clients(:mcp_client) token = identities(:david).access_tokens.create!(oauth_client: client)