diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index b837afb..eacd3ca 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -611,9 +611,12 @@ class OidcController < ApplicationController if device_code.pending? # Enforce the polling interval; too-frequent polls get slow_down, and the - # client is expected to add 5s to its interval (RFC 8628 §3.5). + # client is expected to add 5s to its interval (RFC 8628 §3.5). The bump is + # capped at MAX_INTERVAL so a persistently fast poller can't grow it without + # bound and starve a legitimate client before the code expires. if device_code.last_polled_at && (Time.current - device_code.last_polled_at) < device_code.interval - device_code.update!(interval: device_code.interval + 5, last_polled_at: Time.current) + bumped_interval = [device_code.interval + OidcDeviceCode::INTERVAL_INCREMENT, OidcDeviceCode::MAX_INTERVAL].min + device_code.update!(interval: bumped_interval, last_polled_at: Time.current) render json: {error: "slow_down"}, status: :bad_request else device_code.update!(last_polled_at: Time.current) diff --git a/app/jobs/oidc_token_cleanup_job.rb b/app/jobs/oidc_token_cleanup_job.rb index 47c72ef..11d00fb 100644 --- a/app/jobs/oidc_token_cleanup_job.rb +++ b/app/jobs/oidc_token_cleanup_job.rb @@ -25,5 +25,15 @@ class OidcTokenCleanupJob < ApplicationJob old_auth_codes = OidcAuthorizationCode.where("created_at < ?", 7.days.ago) deleted_count = old_auth_codes.delete_all Rails.logger.info "OIDC Token Cleanup: Deleted #{deleted_count} old authorization codes" + + # Delete expired device codes (RFC 8628). They have a ~10 minute TTL and no + # audit value; a redeemed code is already destroyed at token issuance. Once + # expired a code can never be redeemed, so this single expiry sweep clears the + # expired, denied, and abandoned rows that would otherwise accumulate forever + # (anonymous callers can create them via /oauth/device_authorization). The + # short grace keeps this clear of any in-flight redemption near expiry. + expired_device_codes = OidcDeviceCode.where("expires_at < ?", 1.hour.ago) + deleted_count = expired_device_codes.delete_all + Rails.logger.info "OIDC Token Cleanup: Deleted #{deleted_count} expired device codes" end end diff --git a/app/models/oidc_device_code.rb b/app/models/oidc_device_code.rb index 1caa7ce..b1f63b4 100644 --- a/app/models/oidc_device_code.rb +++ b/app/models/oidc_device_code.rb @@ -17,6 +17,14 @@ class OidcDeviceCode < ApplicationRecord STATUSES = %w[pending approved denied].freeze + # Polling interval, in seconds. The token endpoint bumps the persisted interval + # by INTERVAL_INCREMENT on each too-fast poll (RFC 8628 §3.5 slow_down), but + # clamps it to MAX_INTERVAL so a client polling slightly fast — or an attacker + # spamming a known device_code — can't balloon it past the expiry window and + # starve a well-behaved client of its token. + INTERVAL_INCREMENT = 5 + MAX_INTERVAL = 30 + attr_accessor :plaintext_device_code before_validation :generate_device_code, on: :create diff --git a/test/controllers/oidc_device_flow_controller_test.rb b/test/controllers/oidc_device_flow_controller_test.rb index ad77be1..cef7090 100644 --- a/test/controllers/oidc_device_flow_controller_test.rb +++ b/test/controllers/oidc_device_flow_controller_test.rb @@ -119,6 +119,14 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest assert_equal "slow_down", JSON.parse(@response.body)["error"] end + test "slow_down interval is capped and does not grow without bound" do + dc = OidcDeviceCode.create!(application: @cli, scope: "openid") + # Hammer the code far more times than it would take to exceed the cap if the + # interval grew by 5 unbounded (20 * 5 = 100s >> MAX_INTERVAL). + 20.times { poll(dc) } + assert_operator dc.reload.interval, :<=, OidcDeviceCode::MAX_INTERVAL + end + test "token endpoint returns expired_token for an expired code" do dc = OidcDeviceCode.create!(application: @cli, scope: "openid", expires_at: 1.minute.ago) poll(dc) diff --git a/test/jobs/oidc_token_cleanup_job_test.rb b/test/jobs/oidc_token_cleanup_job_test.rb index ef25c36..9395b52 100644 --- a/test/jobs/oidc_token_cleanup_job_test.rb +++ b/test/jobs/oidc_token_cleanup_job_test.rb @@ -48,4 +48,35 @@ class OidcTokenCleanupJobTest < ActiveJob::TestCase user&.destroy application&.destroy end + + # Device codes (RFC 8628) are created by anonymous callers and would grow the + # table without bound; the cleanup job must purge expired ones (pending, denied, + # or abandoned) while leaving live codes alone. + test "deletes expired device codes and keeps live ones" do + application = Application.create!( + name: "Device Cleanup App", + slug: "device-cleanup-app", + app_type: "oidc", + is_public_client: true, + active: true + ) + + expired_pending = nil + expired_denied = nil + travel_to(2.hours.ago) do + expired_pending = OidcDeviceCode.create!(application: application, scope: "openid") + expired_denied = OidcDeviceCode.create!(application: application, scope: "openid") + expired_denied.deny! + end + live = OidcDeviceCode.create!(application: application, scope: "openid") + + OidcTokenCleanupJob.new.perform + + assert_not OidcDeviceCode.exists?(expired_pending.id), "expired pending code should be deleted" + assert_not OidcDeviceCode.exists?(expired_denied.id), "expired denied code should be deleted" + assert OidcDeviceCode.exists?(live.id), "live code should be kept" + ensure + OidcDeviceCode.where(application: application).delete_all if application + application&.destroy + end end