Merge pull request #125 from cedarcode/sr--backfill-webauthn-id

Fix ephemeral `webauthn_id` for existing users

Author: santiagorodriguez96

lib/devise/models/webauthn_credential_authenticatable.rb

@@ -12,7 +12,7 @@ module WebauthnCredentialAuthenticatable
 
         validates :webauthn_id, uniqueness: true, allow_blank: true
 
-        after_initialize do
+        before_validation do
           self.webauthn_id ||= WebAuthn.generate_user_id
         end
       end

lib/generators/devise/webauthn/webauthn_id/templates/add_webauthn_id.rb.erb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+class AddWebauthnIdTo<%= user_table_name.camelize %> < ActiveRecord::Migration[<%= Rails.version.to_f %>]
+  def up
+    add_column :<%= user_table_name %>, :webauthn_id, :string
+    add_index :<%= user_table_name %>, :webauthn_id, unique: true
+
+    # WARNING: The code below backfills webauthn_id for all existing records
+    # one row at a time. For larger tables, consider removing it and running
+    # the backfill separately (e.g., in a background job or maintenance task).
+    #
+    # Worth noting: PostgreSQL and MySQL support single-query backfills:
+    #
+    #   PostgreSQL:
+    #     UPDATE <%= user_table_name %> SET webauthn_id = encode(gen_random_bytes(64), 'base64') WHERE webauthn_id IS NULL
+    #
+    #   MySQL:
+    #     UPDATE <%= user_table_name %> SET webauthn_id = TO_BASE64(RANDOM_BYTES(64)) WHERE webauthn_id IS NULL
+    #
+    execute("SELECT id FROM <%= user_table_name %> WHERE webauthn_id IS NULL").each do |row|
+      webauthn_id = WebAuthn.generate_user_id
+      execute(ActiveRecord::Base.sanitize_sql_array(
+        ["UPDATE <%= user_table_name %> SET webauthn_id = ? WHERE id = ?", webauthn_id, row["id"]]
+      ))
+    end
+    # Note: if your application creates records using methods that skip
+    # callbacks (e.g., insert_all), consider adding a database default
+    # to ensure webauthn_id is always set. For example, in PostgreSQL:
+    #
+    #   change_column_default :<%= user_table_name %>, :webauthn_id, from: nil, to: -> { "encode(gen_random_bytes(64), 'base64')" }
+    #
+    # For the same reason, you may want to add a NOT NULL constraint in a
+    # separate migration after the backfill is complete:
+    #
+    #   change_column_null :<%= user_table_name %>, :webauthn_id, false
+  end
+
+  def down
+    remove_column :<%= user_table_name %>, :webauthn_id
+  end
+end

lib/generators/devise/webauthn/webauthn_id/webauthn_id_generator.rb

@@ -6,17 +6,22 @@
 module Devise
   module Webauthn
     class WebauthnIdGenerator < Rails::Generators::Base
+      include Rails::Generators::Migration
+
       hide!
       namespace "devise:webauthn:webauthn_id"
 
+      source_root File.expand_path("templates", __dir__)
+
       desc "Add webauthn_id field to User model"
       class_option :resource_name, type: :string, default: "user", desc: "The resource name for Devise (default: user)"
 
+      def self.next_migration_number(dirname)
+        ActiveRecord::Generators::Base.next_migration_number(dirname)
+      end
+
       def generate_migration
-        invoke "active_record:migration", [
-          "add_webauthn_id_to_#{user_table_name}",
-          "webauthn_id:string:uniq"
-        ]
+        migration_template "add_webauthn_id.rb.erb", "db/migrate/add_webauthn_id_to_#{user_table_name}.rb"
       end
 
       def show_instructions

spec/devise/models/passkey_authenticatable_spec.rb

@@ -2,16 +2,30 @@
 
 RSpec.describe Devise::Models::PasskeyAuthenticatable, type: :model do
   describe "webauthn_id initialization" do
-    it "generates a webauthn_id on initialize" do
-      user = Account.new(email: "user@example.com", password: "password", password_confirmation: "password")
+    it "generates a webauthn_id on create" do
+      user = Account.create!(email: "user@example.com", password: "password", password_confirmation: "password")
       expect(user.webauthn_id).to be_present
     end
 
-    it "keeps existing webauthn_id" do
-      user = Account.new(email: "user@example.com", password: "password", password_confirmation: "password",
-                         webauthn_id: "custom")
+    it "does not generate a webauthn_id on initialize" do
+      user = Account.new(email: "user@example.com", password: "password", password_confirmation: "password")
+      expect(user.webauthn_id).to be_nil
+    end
+
+    it "keeps webauthn_id if created with one" do
+      user = Account.create!(email: "user@example.com", password: "password", password_confirmation: "password",
+                             webauthn_id: "custom")
       expect(user.webauthn_id).to eq("custom")
     end
+
+    it "generates a webauthn_id on update if missing" do
+      user = Account.create!(email: "user@example.com", password: "password", password_confirmation: "password")
+      user.update_column(:webauthn_id, nil) # rubocop:disable Rails/SkipsModelValidations
+      user.reload
+
+      user.update!(email: "updated@example.com")
+      expect(user.webauthn_id).to be_present
+    end
   end
 
   describe "associations" do

spec/devise/models/webauthn_two_factor_authenticatable_spec.rb

@@ -2,16 +2,30 @@
 
 RSpec.describe Devise::Models::WebauthnTwoFactorAuthenticatable, type: :model do
   describe "webauthn_id initialization" do
-    it "generates a webauthn_id on initialize" do
-      user = Account.new(email: "user@example.com", password: "password", password_confirmation: "password")
+    it "generates a webauthn_id on create" do
+      user = Account.create!(email: "user@example.com", password: "password", password_confirmation: "password")
       expect(user.webauthn_id).to be_present
     end
 
-    it "keeps existing webauthn_id" do
-      user = Account.new(email: "user@example.com", password: "password", password_confirmation: "password",
-                         webauthn_id: "custom")
+    it "does not generate a webauthn_id on initialize" do
+      user = Account.new(email: "user@example.com", password: "password", password_confirmation: "password")
+      expect(user.webauthn_id).to be_nil
+    end
+
+    it "keeps webauthn_id if created with one" do
+      user = Account.create!(email: "user@example.com", password: "password", password_confirmation: "password",
+                             webauthn_id: "custom")
       expect(user.webauthn_id).to eq("custom")
     end
+
+    it "generates a webauthn_id on update if missing" do
+      user = Account.create!(email: "user@example.com", password: "password", password_confirmation: "password")
+      user.update_column(:webauthn_id, nil) # rubocop:disable Rails/SkipsModelValidations
+      user.reload
+
+      user.update!(email: "updated@example.com")
+      expect(user.webauthn_id).to be_present
+    end
   end
 
   describe "associations" do

spec/generators/devise/webauthn/webauthn_id_generator_spec.rb

@@ -9,25 +9,35 @@
 
   before do
     prepare_destination
-    allow(generator_instance).to receive(:invoke)
     invoke generator_instance
   end
 
   context "when using default resource name" do
     let(:generator_instance) { generator }
 
-    it "invokes the active_record:migration generator with correct arguments" do
-      expect(generator).to have_received(:invoke).with("active_record:migration",
-                                                       ["add_webauthn_id_to_users", "webauthn_id:string:uniq"])
+    it "creates a migration that adds webauthn_id to users" do
+      assert_migration "db/migrate/add_webauthn_id_to_users.rb" do |migration|
+        assert_match(/add_column :users, :webauthn_id, :string/, migration)
+        assert_match(/add_index :users, :webauthn_id, unique: true/, migration)
+      end
+    end
+
+    it "creates a migration that backfills existing records" do
+      assert_migration "db/migrate/add_webauthn_id_to_users.rb" do |migration|
+        assert_match(/SELECT id FROM users WHERE webauthn_id IS NULL/, migration)
+        assert_match(/WebAuthn\.generate_user_id/, migration)
+      end
     end
   end
 
   context "when using a custom resource name" do
     let(:generator_instance) { generator([destination_root], ["--resource_name=admin"]) }
 
-    it "invokes the active_record:migration generator with correct arguments" do
-      expect(generator).to have_received(:invoke).with("active_record:migration",
-                                                       ["add_webauthn_id_to_admins", "webauthn_id:string:uniq"])
+    it "creates a migration that adds webauthn_id to admins" do
+      assert_migration "db/migrate/add_webauthn_id_to_admins.rb" do |migration|
+        assert_match(/add_column :admins, :webauthn_id, :string/, migration)
+        assert_match(/add_index :admins, :webauthn_id, unique: true/, migration)
+      end
     end
   end
 end