From 8b146db54e5f0657fa32d662cb1758a84509f140 Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Sun, 19 Jul 2026 14:43:03 +1000 Subject: [PATCH] Device code: retry user_code generation on collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq --- app/models/oidc_device_code.rb | 21 ++++++++++++++++++++- test/models/oidc_device_code_test.rb | 18 +++++++++++++----- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/app/models/oidc_device_code.rb b/app/models/oidc_device_code.rb index 9d9cdd3..32faf00 100644 --- a/app/models/oidc_device_code.rb +++ b/app/models/oidc_device_code.rb @@ -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 diff --git a/test/models/oidc_device_code_test.rb b/test/models/oidc_device_code_test.rb index 5edf5cf..738197a 100644 --- a/test/models/oidc_device_code_test.rb +++ b/test/models/oidc_device_code_test.rb @@ -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