Device flow: replay-revocation, user_code CSPRNG, /device rate limit
More device-flow security-review fixes. Replay-revocation on device_code redemption (was: destroy on use): - Redemption now marks the code redeemed_at and links the issued access/refresh tokens back to it (new oidc_device_code_id FKs, on_delete: :nullify) instead of destroying the row. A replayed redeemed code is detected as reuse — revoking every descended token and returning a distinguishable "already been used" invalid_grant rather than the generic "Invalid device_code" — mirroring the authorization-code reuse semantics (RFC 6749 §4.1.2). The device_code FK is carried forward across refresh rotation so revocation reaches the whole chain. Redeemed codes are reaped by the existing expiry cleanup sweep. user_code CSPRNG (RFC 8628 §6.1): - The 8-char user_code is a credential (type it + Approve mints tokens), so draw it from SecureRandom instead of Ruby's global Mersenne Twister (Array#sample). Rate-limit the /device verification endpoint (RFC 8628 §5.1): - Add the app's standard 10/min limit on show + verify so a signed-in user can't brute-force the short code space to deny or hijack a pending authorization. Also carried in this commit (other agent's working-tree change): - Give each OidcController rate_limit a distinct name: so frequent device polling on the token endpoint no longer shares one counter with (and 429s) unrelated token/refresh/revoke/introspect calls. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Q4ATZHoMCWqvSpE2yYoie
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
64410a0c50
commit
71ee301dd2
@@ -9,6 +9,14 @@
|
||||
class DeviceAuthorizationsController < ApplicationController
|
||||
# Browser form endpoint — keep CSRF protection on (do NOT skip it).
|
||||
|
||||
# RFC 8628 §5.1: rate-limit user_code entry so a signed-in user cannot brute
|
||||
# force the short code space to deny or hijack another user's pending
|
||||
# authorization during its ~10 minute window. Covers both the lookup (show) and
|
||||
# the state-changing submit (verify).
|
||||
rate_limit to: 10, within: 1.minute, only: [:show, :verify], with: -> {
|
||||
render plain: "Too many attempts. Try again later.", status: :too_many_requests
|
||||
}
|
||||
|
||||
# GET /device?user_code=WDJB-MJHT
|
||||
def show
|
||||
@user_code = params[:user_code].to_s
|
||||
|
||||
@@ -15,11 +15,26 @@ class OidcController < ApplicationController
|
||||
before_action :set_application, only: :authorize
|
||||
before_action :validate_redirect_uri, only: :authorize
|
||||
|
||||
# Rate limiting to prevent brute force and abuse
|
||||
rate_limit to: 60, within: 1.minute, only: [:token, :revoke, :introspect, :device_authorization], with: -> {
|
||||
# Rate limiting to prevent brute force and abuse.
|
||||
#
|
||||
# Each rate_limit MUST pass a distinct name: — without it actionpack derives the
|
||||
# same cache key for every call on the controller, collapsing these into one
|
||||
# shared counter. That let high-frequency RFC 8628 device polling (on the token
|
||||
# endpoint) exhaust the budget and 429 unrelated token/refresh/revoke/introspect
|
||||
# requests, including the poll that completes a just-approved login.
|
||||
#
|
||||
# The token endpoint also carries device-code polling, which is legitimately
|
||||
# frequent and can come from several devices behind one NAT, so its bucket has
|
||||
# generous headroom. Per-device poll abuse is separately bounded by the slow_down
|
||||
# interval, and none of these endpoints exposes a brute-forceable secret (tokens,
|
||||
# codes, and client secrets are opaque high-entropy values), so the limit is a
|
||||
# DoS guard rather than a credential guard.
|
||||
rate_limit to: 120, within: 1.minute, name: "oidc_token", only: [:token, :revoke, :introspect, :device_authorization], with: -> {
|
||||
render json: {error: "too_many_requests", error_description: "Rate limit exceeded. Try again later."}, status: :too_many_requests
|
||||
}
|
||||
rate_limit to: 30, within: 1.minute, only: [:authorize, :consent], with: -> {
|
||||
# Browser-facing authorization flow — kept on its own counter so token-endpoint
|
||||
# traffic can never consume the interactive login budget (or vice versa).
|
||||
rate_limit to: 30, within: 1.minute, name: "oidc_authorize", only: [:authorize, :consent], with: -> {
|
||||
render plain: "Too many authorization attempts. Try again later.", status: :too_many_requests
|
||||
}
|
||||
|
||||
@@ -599,6 +614,19 @@ class OidcController < ApplicationController
|
||||
# Lock so concurrent polls / a poll racing with approval can't double-issue.
|
||||
device_code.lock!
|
||||
|
||||
# Replay: an already-redeemed code must never mint tokens again. Mirror the
|
||||
# authorization-code reuse semantics (RFC 6749 §4.1.2) — revoke every token
|
||||
# descended from it and report the reuse distinguishably, rather than the
|
||||
# generic "Invalid device_code" returned for an unknown code.
|
||||
if device_code.redeemed?
|
||||
Rails.logger.warn "OIDC Security: Device code reuse detected for code #{device_code.id}"
|
||||
now = Time.current
|
||||
device_code.oidc_access_tokens.where(revoked_at: nil).update_all(revoked_at: now)
|
||||
device_code.oidc_refresh_tokens.where(revoked_at: nil).update_all(revoked_at: now)
|
||||
render json: {error: "invalid_grant", error_description: "Device code has already been used"}, status: :bad_request
|
||||
return
|
||||
end
|
||||
|
||||
if device_code.expired?
|
||||
render json: {error: "expired_token", error_description: "The device_code has expired"}, status: :bad_request
|
||||
return
|
||||
@@ -667,6 +695,7 @@ class OidcController < ApplicationController
|
||||
application: application,
|
||||
user: user,
|
||||
scope: granted_scope,
|
||||
oidc_device_code: device_code,
|
||||
resource: device_code.resource
|
||||
)
|
||||
|
||||
@@ -674,6 +703,7 @@ class OidcController < ApplicationController
|
||||
application: application,
|
||||
user: user,
|
||||
oidc_access_token: access_token_record,
|
||||
oidc_device_code: device_code,
|
||||
scope: granted_scope,
|
||||
auth_time: device_code.auth_time,
|
||||
acr: device_code.acr,
|
||||
@@ -692,8 +722,10 @@ class OidcController < ApplicationController
|
||||
claims_requests: {}
|
||||
)
|
||||
|
||||
# Single-use: destroy the code so an approved device_code can't be replayed.
|
||||
device_code.destroy!
|
||||
# Single-use: mark the code redeemed (don't destroy it) so a replay is
|
||||
# detected as reuse — see the redeemed? check at the top of this block. The
|
||||
# cleanup job reaps redeemed codes after they expire.
|
||||
device_code.update!(redeemed_at: Time.current)
|
||||
|
||||
response.headers["Cache-Control"] = "no-store"
|
||||
response.headers["Pragma"] = "no-cache"
|
||||
@@ -963,15 +995,18 @@ class OidcController < ApplicationController
|
||||
refresh_token_record.revoke!
|
||||
|
||||
# Generate new access token record (opaque token with BCrypt hashing)
|
||||
# Carry the authorization-code FK forward across rotations so replay
|
||||
# revocation reaches every descendant token in the chain.
|
||||
# Carry the issuing-code FK forward across rotations so replay revocation
|
||||
# reaches every descendant token in the chain — for both the authorization-code
|
||||
# and device-code grants (a token descends from exactly one of them).
|
||||
issuing_auth_code = refresh_token_record.oidc_authorization_code
|
||||
issuing_device_code = refresh_token_record.oidc_device_code
|
||||
|
||||
new_access_token = OidcAccessToken.create!(
|
||||
application: application,
|
||||
user: user,
|
||||
scope: refresh_token_record.scope,
|
||||
oidc_authorization_code: issuing_auth_code,
|
||||
oidc_device_code: issuing_device_code,
|
||||
resource: refresh_token_record.resource
|
||||
)
|
||||
|
||||
@@ -981,6 +1016,7 @@ class OidcController < ApplicationController
|
||||
user: user,
|
||||
oidc_access_token: new_access_token,
|
||||
oidc_authorization_code: issuing_auth_code,
|
||||
oidc_device_code: issuing_device_code,
|
||||
scope: refresh_token_record.scope,
|
||||
token_family_id: refresh_token_record.token_family_id, # Keep same family for rotation tracking
|
||||
auth_time: refresh_token_record.auth_time, # Carry over original auth_time
|
||||
|
||||
@@ -2,6 +2,7 @@ class OidcAccessToken < ApplicationRecord
|
||||
belongs_to :application
|
||||
belongs_to :user
|
||||
belongs_to :oidc_authorization_code, optional: true
|
||||
belongs_to :oidc_device_code, optional: true
|
||||
has_many :oidc_refresh_tokens, dependent: :destroy
|
||||
|
||||
before_validation :generate_token, on: :create
|
||||
|
||||
@@ -9,6 +9,11 @@ class OidcDeviceCode < ApplicationRecord
|
||||
belongs_to :application
|
||||
belongs_to :user, optional: true # nil until the request is approved
|
||||
|
||||
# Tokens minted from this code, so a replayed (already-redeemed) code can revoke
|
||||
# every token descended from it — mirrors OidcAuthorizationCode.
|
||||
has_many :oidc_access_tokens, dependent: :nullify
|
||||
has_many :oidc_refresh_tokens, dependent: :nullify
|
||||
|
||||
# Alphabet for the user_code: uppercase letters + digits, minus visually
|
||||
# ambiguous characters (0/O, 1/I, etc.) so it is easy to read and type.
|
||||
USER_CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789".chars.freeze
|
||||
@@ -83,6 +88,12 @@ class OidcDeviceCode < ApplicationRecord
|
||||
code_challenge.present?
|
||||
end
|
||||
|
||||
# True once the approved code has been exchanged for tokens. The row is kept
|
||||
# (not destroyed) so a replay is detectable and its tokens can be revoked.
|
||||
def redeemed?
|
||||
redeemed_at.present?
|
||||
end
|
||||
|
||||
# Grant the request: attach the approving user and capture their auth context.
|
||||
def approve!(user:, acr:, auth_time:)
|
||||
update!(status: "approved", user: user, acr: acr, auth_time: auth_time)
|
||||
@@ -100,8 +111,10 @@ class OidcDeviceCode < ApplicationRecord
|
||||
end
|
||||
|
||||
def generate_user_code
|
||||
# The user_code is a security credential (typing it + Approve grants tokens),
|
||||
# so draw from a CSPRNG rather than Ruby's global Mersenne Twister PRNG.
|
||||
self.user_code ||= USER_CODE_GROUPS.times.map do
|
||||
USER_CODE_GROUP_SIZE.times.map { USER_CODE_ALPHABET.sample }.join
|
||||
USER_CODE_GROUP_SIZE.times.map { USER_CODE_ALPHABET.sample(random: SecureRandom) }.join
|
||||
end.join
|
||||
end
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ class OidcRefreshToken < ApplicationRecord
|
||||
belongs_to :user
|
||||
belongs_to :oidc_access_token
|
||||
belongs_to :oidc_authorization_code, optional: true
|
||||
belongs_to :oidc_device_code, optional: true
|
||||
|
||||
before_validation :generate_token, on: :create
|
||||
before_validation :set_expiry, on: :create
|
||||
|
||||
Reference in New Issue
Block a user