From c96be10c14e13dfeb4e05a034ecee7809048205f Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 00:27:26 -0700 Subject: [PATCH 1/6] Lapse OAuth grants left idle for 90 days Before this, a grant lived until it was revoked: a refresh token left on a laptop that was never opened again stayed good indefinitely. bc3 handles this through its refresh tokens. Each rotation mints a token that is good for refresh_token_ttl (90 days), so a grant lapses once it has gone 90 days without a refresh. This applies the same rule to Fizzy grants. - Each grant carries refresh_token_expires_at. It is set 90 days out when the grant is issued and again on every rotation. As in bc3, idle means no refresh: using the access token doesn't extend the grant, because that token expires an hour after the last refresh anyway. - Refreshing a lapsed grant answers invalid_grant. The client has to send the user through authorization again, and the consent screen always shows. - Lapsed grants drop out of Connected Apps, and a daily sweep deletes them. - The boundary matches bc3's Oauth::RefreshToken#expired? (expires_at < now). A refresh at exactly 90 days works. One second later, it doesn't. - Retired refresh tokens are kept for the same window, since a lapsed grant leaves no replay to catch. Existing OAuth grants are backfilled to lapse 90 days after their last rotation. --- .../my/connected_apps_controller.rb | 2 +- app/controllers/oauth/tokens_controller.rb | 7 ++ app/models/identity/access_token.rb | 19 ++++- app/models/oauth/retired_refresh_token.rb | 6 +- config/recurring.yml | 3 + ...en_expires_at_to_identity_access_tokens.rb | 17 +++++ db/schema.rb | 4 +- db/schema_sqlite.rb | 4 +- .../oauth_grant_idle_expiry_test.rb | 76 +++++++++++++++++++ 9 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20261006130000_add_refresh_token_expires_at_to_identity_access_tokens.rb create mode 100644 test/integration/oauth_grant_idle_expiry_test.rb diff --git a/app/controllers/my/connected_apps_controller.rb b/app/controllers/my/connected_apps_controller.rb index 94da5900e2..8517622edb 100644 --- a/app/controllers/my/connected_apps_controller.rb +++ b/app/controllers/my/connected_apps_controller.rb @@ -19,7 +19,7 @@ def destroy private def set_connected_apps - tokens = oauth_tokens.includes(:oauth_client).order(:created_at) + tokens = oauth_tokens.unlapsed.includes(:oauth_client).order(:created_at) @connected_apps = tokens.group_by(&:oauth_client).sort_by { |client, _| client.name.downcase } end diff --git a/app/controllers/oauth/tokens_controller.rb b/app/controllers/oauth/tokens_controller.rb index d7e7a42d56..64fce07424 100644 --- a/app/controllers/oauth/tokens_controller.rb +++ b/app/controllers/oauth/tokens_controller.rb @@ -24,6 +24,7 @@ class Oauth::TokensController < Oauth::BaseController before_action :set_refreshable_access_token, unless: :authorization_code_grant? before_action :validate_client_id + before_action :reject_lapsed_grant, unless: -> { authorization_code_grant? || @retired_refresh_token } with_options if: :authorization_code_grant? do before_action :validate_pkce @@ -151,6 +152,12 @@ def unretryable_replay? @retired_refresh_token && !@retired_refresh_token.retryable?(@access_token) end + def reject_lapsed_grant + if @access_token.lapsed? + oauth_error "invalid_grant", "Refresh token expired" + 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 diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index 715c107fe1..50edf18ffa 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -1,6 +1,11 @@ class Identity::AccessToken < ApplicationRecord EXPIRES_IN = 1.hour + # A grant whose refresh token goes unused this long lapses, and the client + # must ask the user again. Each refresh restarts the clock, as each rotation + # in bc3 mints a refresh token good for its 90-day refresh_token_ttl. + REFRESH_IDLE_LIMIT = 90.days + belongs_to :identity belongs_to :oauth_client, class_name: "Oauth::Client", optional: true has_many :retired_refresh_tokens, class_name: "Oauth::RetiredRefreshToken", dependent: :delete_all @@ -13,6 +18,8 @@ class Identity::AccessToken < ApplicationRecord 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..)) } + scope :lapsed, -> { oauth.where(refresh_token_expires_at: ...Time.current) } + scope :unlapsed, -> { where(refresh_token_expires_at: Time.current..) } has_secure_token enum :permission, %w[ read write ].index_by(&:itself), default: :read @@ -28,6 +35,10 @@ def find_honored(token) end end + def cleanup + lapsed.find_each(&:destroy) + end + def find_by_refresh_token(refresh_token) oauth.find_by(refresh_token: refresh_token) end @@ -62,6 +73,10 @@ def expires_in (expires_at - Time.current).to_i if expires_at? end + def lapsed? + refresh_token_expires_at? && refresh_token_expires_at.past? + end + # Rotates atomically on the presented refresh token, so a concurrent # rotation wins the row and the loser comes up empty-handed. The presented # token is retired, not forgotten, so presenting it again is recognized as @@ -69,7 +84,8 @@ def expires_in 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 } + expires_at: EXPIRES_IN.from_now, refresh_token_expires_at: REFRESH_IDLE_LIMIT.from_now, + permission: permission, updated_at: Time.current } transaction do if self.class.where(id: id, refresh_token: refresh_token).update_all(rotated) == 1 @@ -88,5 +104,6 @@ def lock_grant def set_expiry_and_refresh_token self.expires_at ||= EXPIRES_IN.from_now self.refresh_token ||= self.class.generate_unique_secure_token + self.refresh_token_expires_at ||= REFRESH_IDLE_LIMIT.from_now end end diff --git a/app/models/oauth/retired_refresh_token.rb b/app/models/oauth/retired_refresh_token.rb index d1b16b2643..76fde92525 100644 --- a/app/models/oauth/retired_refresh_token.rb +++ b/app/models/oauth/retired_refresh_token.rb @@ -8,9 +8,9 @@ class Oauth::RetiredRefreshToken < ApplicationRecord # bc3's refresh_replay_grace default (Oauth::RefreshToken::Rotation). GRACE = 60.seconds - # As long as a replay can still be recognized. It matches bc3's 90-day - # refresh_token_ttl, after which a rotated token there has expired too. - RETENTION = 90.days + # As long as a replay can still be recognized: past the idle limit, the + # grant a retired token came from has lapsed unless it rotated since. + RETENTION = Identity::AccessToken::REFRESH_IDLE_LIMIT belongs_to :access_token, class_name: "Identity::AccessToken" diff --git a/config/recurring.yml b/config/recurring.yml index 7097319553..66c8d1a1bf 100644 --- a/config/recurring.yml +++ b/config/recurring.yml @@ -30,6 +30,9 @@ production: &production cleanup_oauth_retired_refresh_tokens: command: "Oauth::RetiredRefreshToken.cleanup" schedule: every day at 04:22 + cleanup_lapsed_oauth_grants: + command: "Identity::AccessToken.cleanup" + schedule: every day at 04:32 cleanup_exports: command: "Export.cleanup" schedule: every hour at minute 20 diff --git a/db/migrate/20261006130000_add_refresh_token_expires_at_to_identity_access_tokens.rb b/db/migrate/20261006130000_add_refresh_token_expires_at_to_identity_access_tokens.rb new file mode 100644 index 0000000000..b379894bfe --- /dev/null +++ b/db/migrate/20261006130000_add_refresh_token_expires_at_to_identity_access_tokens.rb @@ -0,0 +1,17 @@ +class AddRefreshTokenExpiresAtToIdentityAccessTokens < ActiveRecord::Migration[8.2] + def up + add_column :identity_access_tokens, :refresh_token_expires_at, :datetime + add_index :identity_access_tokens, :refresh_token_expires_at + + # Grants issued before this have been idle since their last rotation. + grants = Class.new(ActiveRecord::Base) { self.table_name = "identity_access_tokens" } + grants.where.not(oauth_client_id: nil).find_each do |grant| + grant.update_columns refresh_token_expires_at: grant.updated_at + 90.days + end + end + + def down + remove_index :identity_access_tokens, :refresh_token_expires_at + remove_column :identity_access_tokens, :refresh_token_expires_at + end +end diff --git a/db/schema.rb b/db/schema.rb index a95b556b21..5e462097ac 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2026_10_06_120000) do +ActiveRecord::Schema[8.2].define(version: 2026_10_06_130000) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -360,12 +360,14 @@ t.uuid "oauth_client_id" t.string "permission" t.string "refresh_token" + t.datetime "refresh_token_expires_at" t.string "token" t.datetime "updated_at", null: false t.index ["authorization_code_jti"], name: "index_identity_access_tokens_on_authorization_code_jti", unique: true 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 + t.index ["refresh_token_expires_at"], name: "index_identity_access_tokens_on_refresh_token_expires_at" 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 4b67d952b6..7a72a72be8 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2026_10_06_120000) do +ActiveRecord::Schema[8.2].define(version: 2026_10_06_130000) do create_table "accesses", id: :uuid, force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -362,10 +362,12 @@ t.string "refresh_token", limit: 255 t.string "token", limit: 255 t.datetime "updated_at", null: false + t.datetime "refresh_token_expires_at" t.index ["authorization_code_jti"], name: "index_identity_access_tokens_on_authorization_code_jti", unique: true 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 + t.index ["refresh_token_expires_at"], name: "index_identity_access_tokens_on_refresh_token_expires_at" end create_table "identity_transfers", id: :uuid, force: :cascade do |t| diff --git a/test/integration/oauth_grant_idle_expiry_test.rb b/test/integration/oauth_grant_idle_expiry_test.rb new file mode 100644 index 0000000000..f43ab3ed49 --- /dev/null +++ b/test/integration/oauth_grant_idle_expiry_test.rb @@ -0,0 +1,76 @@ +require "test_helper" + +# A grant lapses once its refresh token goes unused for the idle window, as +# bc3's refresh tokens do: each refresh restarts the clock, and a lapsed grant +# takes a fresh authorization, with consent. +class OauthGrantIdleExpiryTest < ActionDispatch::IntegrationTest + setup do + @client = oauth_clients(:mcp_client) + freeze_time + @grant = identities(:david).access_tokens.create!(oauth_client: @client, permission: :read) + end + + test "a grant refreshes up to the end of the idle window" do + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + refresh @grant.refresh_token + + assert_response :success + end + + test "a grant idle past the window lapses" do + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second + refresh @grant.refresh_token + + assert_response :bad_request + assert_equal "invalid_grant", response.parsed_body["error"] + end + + test "each refresh restarts the idle clock" do + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT - 1.day + refresh @grant.refresh_token + assert_response :success + + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT - 1.day + refresh response.parsed_body["refresh_token"] + + assert_response :success + end + + test "a lapsed grant leaves Connected Apps" do + sign_in_as :david + + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second + get my_connected_apps_path + + assert_response :success + assert_no_match @client.name, response.body + end + + test "the sweep removes lapsed grants and keeps live ones" do + live = identities(:david).access_tokens.create!(oauth_client: @client, permission: :read) + personal = identities(:david).access_tokens.create!(permission: :read, description: "PAT") + + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT - 1.day + live.refresh + + later_by 1.day + 1.second + Identity::AccessToken.cleanup + + assert_not Identity::AccessToken.exists?(@grant.id) + assert Identity::AccessToken.exists?(live.id) + assert Identity::AccessToken.exists?(personal.id) + end + + private + # Elapsed time in the app's zone, as the model counts it: travel would add + # calendar days in the local zone, an hour off across a DST change. + def later_by(duration) + travel_to Time.current + duration + end + + def refresh(refresh_token) + untenanted do + post oauth_token_path, params: { grant_type: "refresh_token", refresh_token: refresh_token, client_id: @client.client_id } + end + end +end From b8df79a1d162f4c4d2dbd035471957719799fb5a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 00:40:59 -0700 Subject: [PATCH 2/6] Enforce the idle limit in the rotation itself, and lapse grants written without an expiry Rotation now matches only unlapsed grants. A refresh that passed the lapse check just before the deadline can no longer renew the grant after it, and the daily sweep can no longer delete a grant that was renewed between selecting it and destroying it. During a rolling deploy, old code can write grants without refresh_token_expires_at. Such a grant is now idle from its last write (updated_at), both at refresh and in the sweep. Connected Apps keeps listing it, so it can't become a grant that is live but hidden. --- app/models/identity/access_token.rb | 14 +++++--- .../oauth_grant_idle_expiry_test.rb | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index 50edf18ffa..06f95a0ef4 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -18,8 +18,10 @@ class Identity::AccessToken < ApplicationRecord 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..)) } - scope :lapsed, -> { oauth.where(refresh_token_expires_at: ...Time.current) } - scope :unlapsed, -> { where(refresh_token_expires_at: Time.current..) } + # A grant written without an expiry (by code from before idle expiry, while + # a deploy rolls) is idle since its last write. + scope :unlapsed, -> { where(refresh_token_expires_at: Time.current..).or(where(refresh_token_expires_at: nil, updated_at: REFRESH_IDLE_LIMIT.ago..)) } + scope :lapsed, -> { oauth.where(refresh_token_expires_at: ...Time.current).or(oauth.where(refresh_token_expires_at: nil, updated_at: ...REFRESH_IDLE_LIMIT.ago)) } has_secure_token enum :permission, %w[ read write ].index_by(&:itself), default: :read @@ -74,11 +76,13 @@ def expires_in end def lapsed? - refresh_token_expires_at? && refresh_token_expires_at.past? + oauth_client_id? && (refresh_token_expires_at || updated_at + REFRESH_IDLE_LIMIT).past? end # Rotates atomically on the presented refresh token, so a concurrent - # rotation wins the row and the loser comes up empty-handed. The presented + # rotation wins the row and the loser comes up empty-handed. A grant that + # lapses mid-request doesn't rotate either, so a sweep that found it lapsed + # never deletes a grant that was just renewed. The presented # token is retired, not forgotten, so presenting it again is recognized as # a retry or a replay (see Oauth::RetiredRefreshToken). def refresh(permission: self.permission) @@ -88,7 +92,7 @@ def refresh(permission: self.permission) permission: permission, updated_at: Time.current } transaction do - if self.class.where(id: id, refresh_token: refresh_token).update_all(rotated) == 1 + if self.class.unlapsed.where(id: id, refresh_token: refresh_token).update_all(rotated) == 1 retired_refresh_tokens.create! refresh_token: refresh_token, successor_refresh_token: rotated[:refresh_token] assign_attributes rotated true diff --git a/test/integration/oauth_grant_idle_expiry_test.rb b/test/integration/oauth_grant_idle_expiry_test.rb index f43ab3ed49..00ee3bfbb1 100644 --- a/test/integration/oauth_grant_idle_expiry_test.rb +++ b/test/integration/oauth_grant_idle_expiry_test.rb @@ -61,6 +61,38 @@ class OauthGrantIdleExpiryTest < ActionDispatch::IntegrationTest assert Identity::AccessToken.exists?(personal.id) end + test "rotation itself refuses a lapsed grant, so one racing the deadline can't renew it" do + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second + presented = @grant.refresh_token + + assert_nil @grant.refresh + assert_equal presented, @grant.reload.refresh_token + end + + test "a grant written without an expiry, as by old code mid-deploy, lapses from its last rotation" do + Identity::AccessToken.where(id: @grant.id).update_all(refresh_token_expires_at: nil) + + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second + refresh @grant.refresh_token + + assert_response :bad_request + assert_equal "invalid_grant", response.parsed_body["error"] + end + + test "a grant written without an expiry still refreshes inside the window, gains an expiry, and shows in Connected Apps" do + Identity::AccessToken.where(id: @grant.id).update_all(refresh_token_expires_at: nil) + sign_in_as :david + + get my_connected_apps_path + assert_match @client.name, response.body + + later_by 1.day + refresh @grant.refresh_token + + assert_response :success + assert_not_nil @grant.reload.read_attribute(:refresh_token_expires_at) + end + private # Elapsed time in the app's zone, as the model counts it: travel would add # calendar days in the local zone, an hour off across a DST change. From d0efc0ebf00f042644e5fe3b5b14e19bea676053 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 00:54:33 -0700 Subject: [PATCH 3/6] Measure idleness from the grant's last rotation, not a stored deadline The refresh_token_expires_at column existed only to record a deadline that updated_at already implies. Only issuance and rotation write a grant, and every rotation sets updated_at, including rotations by code that predates idle expiry. The column, on the other hand, could fall out of step during a rolling deploy. Old code could write a grant without it, or rotate a grant and leave a backfilled deadline in place, which would lapse the fresh token early. Removing the column removes the backfill, the NULL fallback and the stale-deadline case together. The profile page's "apps you've authorized" link now hides once every grant has lapsed, matching Connected Apps. --- app/models/identity/access_token.rb | 16 +++++------ app/views/users/show.html.erb | 2 +- ...en_expires_at_to_identity_access_tokens.rb | 17 ----------- db/schema.rb | 4 +-- db/schema_sqlite.rb | 4 +-- .../oauth_grant_idle_expiry_test.rb | 28 +++++++++---------- 6 files changed, 24 insertions(+), 47 deletions(-) delete mode 100644 db/migrate/20261006130000_add_refresh_token_expires_at_to_identity_access_tokens.rb diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index 06f95a0ef4..b9f55e91d7 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -4,6 +4,10 @@ class Identity::AccessToken < ApplicationRecord # A grant whose refresh token goes unused this long lapses, and the client # must ask the user again. Each refresh restarts the clock, as each rotation # in bc3 mints a refresh token good for its 90-day refresh_token_ttl. + # + # The clock is updated_at. Only issuance and rotation write a grant, and + # every rotation (this code's or any earlier version's) sets updated_at, so + # no stored deadline can fall out of step with the token it governs. REFRESH_IDLE_LIMIT = 90.days belongs_to :identity @@ -18,10 +22,8 @@ class Identity::AccessToken < ApplicationRecord 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..)) } - # A grant written without an expiry (by code from before idle expiry, while - # a deploy rolls) is idle since its last write. - scope :unlapsed, -> { where(refresh_token_expires_at: Time.current..).or(where(refresh_token_expires_at: nil, updated_at: REFRESH_IDLE_LIMIT.ago..)) } - scope :lapsed, -> { oauth.where(refresh_token_expires_at: ...Time.current).or(oauth.where(refresh_token_expires_at: nil, updated_at: ...REFRESH_IDLE_LIMIT.ago)) } + scope :unlapsed, -> { where(updated_at: REFRESH_IDLE_LIMIT.ago..) } + scope :lapsed, -> { oauth.where(updated_at: ...REFRESH_IDLE_LIMIT.ago) } has_secure_token enum :permission, %w[ read write ].index_by(&:itself), default: :read @@ -76,7 +78,7 @@ def expires_in end def lapsed? - oauth_client_id? && (refresh_token_expires_at || updated_at + REFRESH_IDLE_LIMIT).past? + oauth_client_id? && (updated_at + REFRESH_IDLE_LIMIT).past? end # Rotates atomically on the presented refresh token, so a concurrent @@ -88,8 +90,7 @@ def lapsed? 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, refresh_token_expires_at: REFRESH_IDLE_LIMIT.from_now, - permission: permission, updated_at: Time.current } + expires_at: EXPIRES_IN.from_now, permission: permission, updated_at: Time.current } transaction do if self.class.unlapsed.where(id: id, refresh_token: refresh_token).update_all(rotated) == 1 @@ -108,6 +109,5 @@ def lock_grant def set_expiry_and_refresh_token self.expires_at ||= EXPIRES_IN.from_now self.refresh_token ||= self.class.generate_unique_secure_token - self.refresh_token_expires_at ||= REFRESH_IDLE_LIMIT.from_now end end diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index a6e941d269..650bffde0a 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -59,7 +59,7 @@
<%= render "users/theme" %> <%= render "users/transfer", user: @user %> - <%= render "users/connected_apps" if Current.identity.access_tokens.oauth.exists? %> + <%= render "users/connected_apps" if Current.identity.access_tokens.oauth.unlapsed.exists? %> <%= render "users/access_tokens" %> <%= render "users/data_export" %>
diff --git a/db/migrate/20261006130000_add_refresh_token_expires_at_to_identity_access_tokens.rb b/db/migrate/20261006130000_add_refresh_token_expires_at_to_identity_access_tokens.rb deleted file mode 100644 index b379894bfe..0000000000 --- a/db/migrate/20261006130000_add_refresh_token_expires_at_to_identity_access_tokens.rb +++ /dev/null @@ -1,17 +0,0 @@ -class AddRefreshTokenExpiresAtToIdentityAccessTokens < ActiveRecord::Migration[8.2] - def up - add_column :identity_access_tokens, :refresh_token_expires_at, :datetime - add_index :identity_access_tokens, :refresh_token_expires_at - - # Grants issued before this have been idle since their last rotation. - grants = Class.new(ActiveRecord::Base) { self.table_name = "identity_access_tokens" } - grants.where.not(oauth_client_id: nil).find_each do |grant| - grant.update_columns refresh_token_expires_at: grant.updated_at + 90.days - end - end - - def down - remove_index :identity_access_tokens, :refresh_token_expires_at - remove_column :identity_access_tokens, :refresh_token_expires_at - end -end diff --git a/db/schema.rb b/db/schema.rb index 5e462097ac..a95b556b21 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2026_10_06_130000) do +ActiveRecord::Schema[8.2].define(version: 2026_10_06_120000) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -360,14 +360,12 @@ t.uuid "oauth_client_id" t.string "permission" t.string "refresh_token" - t.datetime "refresh_token_expires_at" t.string "token" t.datetime "updated_at", null: false t.index ["authorization_code_jti"], name: "index_identity_access_tokens_on_authorization_code_jti", unique: true 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 - t.index ["refresh_token_expires_at"], name: "index_identity_access_tokens_on_refresh_token_expires_at" 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 7a72a72be8..4b67d952b6 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2026_10_06_130000) do +ActiveRecord::Schema[8.2].define(version: 2026_10_06_120000) do create_table "accesses", id: :uuid, force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -362,12 +362,10 @@ t.string "refresh_token", limit: 255 t.string "token", limit: 255 t.datetime "updated_at", null: false - t.datetime "refresh_token_expires_at" t.index ["authorization_code_jti"], name: "index_identity_access_tokens_on_authorization_code_jti", unique: true 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 - t.index ["refresh_token_expires_at"], name: "index_identity_access_tokens_on_refresh_token_expires_at" end create_table "identity_transfers", id: :uuid, force: :cascade do |t| diff --git a/test/integration/oauth_grant_idle_expiry_test.rb b/test/integration/oauth_grant_idle_expiry_test.rb index 00ee3bfbb1..c7c9140353 100644 --- a/test/integration/oauth_grant_idle_expiry_test.rb +++ b/test/integration/oauth_grant_idle_expiry_test.rb @@ -69,28 +69,26 @@ class OauthGrantIdleExpiryTest < ActionDispatch::IntegrationTest assert_equal presented, @grant.reload.refresh_token end - test "a grant written without an expiry, as by old code mid-deploy, lapses from its last rotation" do - Identity::AccessToken.where(id: @grant.id).update_all(refresh_token_expires_at: nil) + test "the idle clock is the grant's last rotation, so a rotation by older code mid-deploy restarts it too" do + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT - 1.day + rotated_by_older_code = Identity::AccessToken.generate_unique_secure_token + Identity::AccessToken.where(id: @grant.id).update_all(refresh_token: rotated_by_older_code, updated_at: Time.current) - later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second - refresh @grant.refresh_token + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT - 1.day + refresh rotated_by_older_code - assert_response :bad_request - assert_equal "invalid_grant", response.parsed_body["error"] + assert_response :success end - test "a grant written without an expiry still refreshes inside the window, gains an expiry, and shows in Connected Apps" do - Identity::AccessToken.where(id: @grant.id).update_all(refresh_token_expires_at: nil) + test "a lapsed grant takes its entry point off the profile page too" do sign_in_as :david - get my_connected_apps_path - assert_match @client.name, response.body - - later_by 1.day - refresh @grant.refresh_token + get user_path(users(:david)) + assert_match my_connected_apps_path, response.body - assert_response :success - assert_not_nil @grant.reload.read_attribute(:refresh_token_expires_at) + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second + get user_path(users(:david)) + assert_no_match my_connected_apps_path, response.body end private From 50499f883be2ea974db411a40edc5d0091e66239 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 01:38:46 -0700 Subject: [PATCH 4/6] Recheck each lapsed grant under its lock before the sweep deletes it During a rolling deploy, code from before idle expiry can rotate a grant without checking for lapse. A grant the sweep selected could therefore be renewed before the sweep reached it. The sweep now locks each grant, rechecks it, and spares one that is no longer lapsed. --- app/models/identity/access_token.rb | 9 ++++++++- test/integration/oauth_grant_idle_expiry_test.rb | 11 +++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index b9f55e91d7..efdbd3e8b6 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -39,8 +39,15 @@ def find_honored(token) end end + # Each grant is rechecked under its lock, so one renewed after it was + # selected (by code from before idle expiry, mid-deploy, whose rotation + # doesn't check for lapse) is spared. def cleanup - lapsed.find_each(&:destroy) + lapsed.find_each do |grant| + grant.with_lock { grant.destroy if grant.lapsed? } + rescue ActiveRecord::RecordNotFound + # Already gone. + end end def find_by_refresh_token(refresh_token) diff --git a/test/integration/oauth_grant_idle_expiry_test.rb b/test/integration/oauth_grant_idle_expiry_test.rb index c7c9140353..9559199a9c 100644 --- a/test/integration/oauth_grant_idle_expiry_test.rb +++ b/test/integration/oauth_grant_idle_expiry_test.rb @@ -61,6 +61,17 @@ class OauthGrantIdleExpiryTest < ActionDispatch::IntegrationTest assert Identity::AccessToken.exists?(personal.id) end + test "the sweep spares a grant renewed after it was selected" do + selected = Identity::AccessToken.where(id: @grant.id) + Identity::AccessToken.stubs(:lapsed).returns(selected) + + later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second + selected.update_all(updated_at: Time.current) + Identity::AccessToken.cleanup + + assert Identity::AccessToken.exists?(@grant.id) + end + test "rotation itself refuses a lapsed grant, so one racing the deadline can't renew it" do later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second presented = @grant.refresh_token From d96cc01b40fe72da343cecbbfd47a0c41e78c250 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 03:40:10 -0700 Subject: [PATCH 5/6] Measure the idle limit in fixed seconds Requests run in the browser's time zone, where 90.days is calendar arithmetic. Across a daylight-saving change the token endpoint's cutoff moved by an hour, and it then disagreed with the UTC sweep. The limit is now 90 days of seconds everywhere. --- app/models/identity/access_token.rb | 6 +++++- test/models/identity/access_token_test.rb | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index efdbd3e8b6..dd7ac7b693 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -8,7 +8,11 @@ class Identity::AccessToken < ApplicationRecord # The clock is updated_at. Only issuance and rotation write a grant, and # every rotation (this code's or any earlier version's) sets updated_at, so # no stored deadline can fall out of step with the token it governs. - REFRESH_IDLE_LIMIT = 90.days + # + # Fixed seconds, not calendar days: requests run in the browser's time zone, + # where 90.days would shift by an hour across a daylight-saving change, and + # the UTC sweep would then disagree with the token endpoint. + REFRESH_IDLE_LIMIT = 90.days.in_seconds.seconds belongs_to :identity belongs_to :oauth_client, class_name: "Oauth::Client", optional: true diff --git a/test/models/identity/access_token_test.rb b/test/models/identity/access_token_test.rb index c8daf759c5..a0f1a83654 100644 --- a/test/models/identity/access_token_test.rb +++ b/test/models/identity/access_token_test.rb @@ -78,4 +78,18 @@ class Identity::AccessTokenTest < ActiveSupport::TestCase test "find_honored rejects unknown tokens" do assert_nil Identity::AccessToken.find_honored("nonsense") end + + test "the idle limit is a fixed span of seconds, whatever the zone's daylight saving" do + travel_to Time.utc(2026, 12, 1, 12) do + cutoff = Time.current - Identity::AccessToken::REFRESH_IDLE_LIMIT.to_i + grant = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client)) + grant.update_columns updated_at: cutoff - 30.minutes + + Time.use_zone("America/New_York") do + assert grant.lapsed? + assert_includes Identity::AccessToken.lapsed, grant + assert_not_includes Identity::AccessToken.unlapsed, grant + end + end + end end From 9bf097414cdb6fe87e5026778de922ac02783633 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 17 Oct 2026 01:10:59 -0700 Subject: [PATCH 6/6] Hand the sweep the grant as it was read, so the race pin needs the lock The stubbed selection was a lazy relation, read after the renewal, so the pin held without with_lock's reload. Now the sweep gets the pre-renewal record, and the test fails without the lock. --- test/integration/oauth_grant_idle_expiry_test.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/integration/oauth_grant_idle_expiry_test.rb b/test/integration/oauth_grant_idle_expiry_test.rb index 9559199a9c..3ab8e96de1 100644 --- a/test/integration/oauth_grant_idle_expiry_test.rb +++ b/test/integration/oauth_grant_idle_expiry_test.rb @@ -62,11 +62,13 @@ class OauthGrantIdleExpiryTest < ActionDispatch::IntegrationTest end test "the sweep spares a grant renewed after it was selected" do - selected = Identity::AccessToken.where(id: @grant.id) - Identity::AccessToken.stubs(:lapsed).returns(selected) - later_by Identity::AccessToken::REFRESH_IDLE_LIMIT + 1.second - selected.update_all(updated_at: Time.current) + # The sweep holds the grant as it was read when selected, before the renewal. + selection = stub + selection.stubs(:find_each).yields(Identity::AccessToken.find(@grant.id)) + Identity::AccessToken.stubs(:lapsed).returns(selection) + + Identity::AccessToken.where(id: @grant.id).update_all(updated_at: Time.current) Identity::AccessToken.cleanup assert Identity::AccessToken.exists?(@grant.id)