Device code: retry user_code generation on collision
Regenerate the user_code when a freshly generated one collides with an existing
code (up to USER_CODE_MAX_ATTEMPTS), so a client never gets a uniqueness error
just because two codes happened to match; the DB unique index stays the final
guard against a concurrent-insert race.
Replace the model test that asserted Rails' uniqueness validator message
("has already been taken") — per CLAUDE.md we don't test framework behavior —
with one that exercises the app-specific property: generation retries past a
collision and yields a unique code.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
6c4be7199c
commit
8b146db54e
@@ -110,10 +110,29 @@ class OidcDeviceCode < ApplicationRecord
|
||||
self.device_code_hmac ||= self.class.compute_device_code_hmac(plaintext_device_code)
|
||||
end
|
||||
|
||||
# Number of fresh candidates to try before falling back to the DB unique index.
|
||||
USER_CODE_MAX_ATTEMPTS = 10
|
||||
|
||||
def generate_user_code
|
||||
return if user_code.present?
|
||||
|
||||
# Regenerate on the (astronomically rare) collision with an existing code so a
|
||||
# client never gets an error just because two codes happened to match. The DB
|
||||
# unique index remains the final guard against a concurrent-insert race.
|
||||
USER_CODE_MAX_ATTEMPTS.times do
|
||||
candidate = random_user_code
|
||||
unless self.class.exists?(user_code: candidate)
|
||||
self.user_code = candidate
|
||||
return
|
||||
end
|
||||
end
|
||||
self.user_code = random_user_code
|
||||
end
|
||||
|
||||
def random_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_GROUPS.times.map do
|
||||
USER_CODE_GROUP_SIZE.times.map { USER_CODE_ALPHABET.sample(random: SecureRandom) }.join
|
||||
end.join
|
||||
end
|
||||
|
||||
@@ -39,12 +39,20 @@ class OidcDeviceCodeTest < ActiveSupport::TestCase
|
||||
assert_nil OidcDeviceCode.find_by_user_code("nope")
|
||||
end
|
||||
|
||||
test "user_code is unique" do
|
||||
dc = OidcDeviceCode.create!(application: @application)
|
||||
dup = OidcDeviceCode.new(application: @application, user_code: dc.user_code)
|
||||
test "regenerates the user_code when generation collides with an existing code" do
|
||||
existing = OidcDeviceCode.create!(application: @application)
|
||||
taken = existing.user_code
|
||||
fresh = "ABCDEFGH" # in-alphabet, effectively guaranteed != the random `taken`
|
||||
|
||||
assert_not dup.valid?
|
||||
assert_includes dup.errors[:user_code], "has already been taken"
|
||||
# First candidate collides with the existing code, the second is unique — the
|
||||
# generator must retry rather than surface a uniqueness error.
|
||||
candidates = [taken, fresh].each
|
||||
dc = OidcDeviceCode.new(application: @application)
|
||||
dc.define_singleton_method(:random_user_code) { candidates.next }
|
||||
dc.save!
|
||||
|
||||
assert_equal fresh, dc.user_code
|
||||
assert_not_equal taken, dc.user_code
|
||||
end
|
||||
|
||||
test "starts pending and approve! attaches the user and auth context" do
|
||||
|
||||
Reference in New Issue
Block a user