diff --git a/app/controllers/oauth/clients_controller.rb b/app/controllers/oauth/clients_controller.rb index a2f07fee2..9e5c772f4 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 9bc84ea15..4f9032bc8 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 5e49c5d1e..124559a66 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 b5eba3e18..1778dfb0d 100644 --- a/app/controllers/oauth/tokens_controller.rb +++ b/app/controllers/oauth/tokens_controller.rb @@ -5,28 +5,45 @@ 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 + before_action :set_refresh_scope + 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 + 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 + 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 +76,52 @@ 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 + + # 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. 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].nil? ? granted : params[:scope].to_s.split + + 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, + 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 059e8ac9c..aa2851833 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 6cd1fc3b2..5ad7ebaf9 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -1,14 +1,46 @@ 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 + + # Rotates atomically on the presented refresh token, so a concurrent + # rotation wins the row and the loser comes up empty-handed. + 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, permission: permission, 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 + 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 000000000..bfb8a2fda --- /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 21e893e25..5719fca6d 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 d6947a20d..661a6e913 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 2e5f91baf..b82e0778a 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,190 @@ 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 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 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) + + 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 +497,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 @@ -342,6 +540,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 @@ -375,6 +575,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 7324d97ab..6497eb75f 100644 --- a/test/models/identity/access_token_test.rb +++ b/test/models/identity/access_token_test.rb @@ -1,4 +1,68 @@ 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 + assert 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 "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)) + + 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