Fixes for tests and AR Encryption

Gemfile

@@ -47,6 +47,7 @@ gem "tzinfo-data", platforms: %i[ windows jruby ]
 # Use the database-backed adapters for Rails.cache and Action Cable
 gem "solid_cache"
 gem "solid_cable"
+gem "solid_queue", "~> 1.2"
 
 # Reduces boot times through caching; required in config/boot.rb
 gem "bootsnap", require: false
@@ -87,3 +88,4 @@ group :test do
   gem "capybara"
   gem "selenium-webdriver"
 end
+

Gemfile.lock

@@ -121,7 +121,8 @@ GEM
     ed25519 (1.4.0)
     erb (6.0.0)
     erubi (1.13.1)
-    ffi (1.17.2)
+    et-orbi (1.4.0)
+      tzinfo
     ffi (1.17.2-aarch64-linux-gnu)
     ffi (1.17.2-aarch64-linux-musl)
     ffi (1.17.2-arm-linux-gnu)
@@ -129,6 +130,9 @@ GEM
     ffi (1.17.2-arm64-darwin)
     ffi (1.17.2-x86_64-linux-gnu)
     ffi (1.17.2-x86_64-linux-musl)
+    fugit (1.12.1)
+      et-orbi (~> 1.4)
+      raabro (~> 1.4)
     globalid (1.3.0)
       activesupport (>= 6.1)
     i18n (1.14.7)
@@ -185,7 +189,6 @@ GEM
     mini_magick (5.3.1)
       logger
     mini_mime (1.1.5)
-    mini_portile2 (2.8.9)
     minitest (5.26.2)
     msgpack (1.8.0)
     net-imap (0.5.12)
@@ -203,9 +206,6 @@ GEM
       net-protocol
     net-ssh (7.3.0)
     nio4r (2.7.5)
-    nokogiri (1.18.10)
-      mini_portile2 (~> 2.8.2)
-      racc (~> 1.4)
     nokogiri (1.18.10-aarch64-linux-gnu)
       racc (~> 1.4)
     nokogiri (1.18.10-aarch64-linux-musl)
@@ -242,6 +242,7 @@ GEM
     public_suffix (7.0.0)
     puma (7.1.0)
       nio4r (~> 2.0)
+    raabro (1.4.0)
     racc (1.8.1)
     rack (3.2.4)
     rack-session (2.1.1)
@@ -353,8 +354,13 @@ GEM
       activejob (>= 7.2)
       activerecord (>= 7.2)
       railties (>= 7.2)
-    sqlite3 (2.8.1)
-      mini_portile2 (~> 2.8.0)
+    solid_queue (1.2.4)
+      activejob (>= 7.1)
+      activerecord (>= 7.1)
+      concurrent-ruby (>= 1.3.1)
+      fugit (~> 1.11)
+      railties (>= 7.1)
+      thor (>= 1.3.1)
     sqlite3 (2.8.1-aarch64-linux-gnu)
     sqlite3 (2.8.1-aarch64-linux-musl)
     sqlite3 (2.8.1-arm-linux-gnu)
@@ -460,6 +466,7 @@ DEPENDENCIES
   sentry-ruby (~> 6.2)
   solid_cable
   solid_cache
+  solid_queue (~> 1.2)
   sqlite3 (>= 2.1)
   stimulus-rails
   tailwindcss-rails

config/initializers/active_record_encryption.rb

@@ -0,0 +1,28 @@
+# ActiveRecord Encryption Configuration
+# Encryption keys derived from SECRET_KEY_BASE (no separate key storage needed)
+# Used for encrypting sensitive columns (currently: TOTP secrets)
+#
+# Optional: Override with env vars (for key rotation or explicit key management):
+#   - ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY
+#   - ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY
+#   - ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT
+
+# Use env vars if set, otherwise derive from SECRET_KEY_BASE (deterministic)
+primary_key = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY') do
+  Rails.application.key_generator.generate_key('active_record_encryption_primary', 32)
+end
+deterministic_key = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY') do
+  Rails.application.key_generator.generate_key('active_record_encryption_deterministic', 32)
+end
+key_derivation_salt = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT') do
+  Rails.application.key_generator.generate_key('active_record_encryption_salt', 32)
+end
+
+# Configure Rails 7.1+ ActiveRecord encryption
+Rails.application.config.active_record.encryption.primary_key = primary_key
+Rails.application.config.active_record.encryption.deterministic_key = deterministic_key
+Rails.application.config.active_record.encryption.key_derivation_salt = key_derivation_salt
+
+# Allow unencrypted data for existing records (new/updated records will be encrypted)
+# Set to false after all existing encrypted columns have been migrated
+Rails.application.config.active_record.encryption.support_unencrypted_data = true

config/initializers/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Clinch
-  VERSION = "0.8.0"
+  VERSION = "0.8.1"
 end

db/migrate/20251230073656_add_pkce_options_to_applications.rb

@@ -0,0 +1,14 @@
+class AddPkceOptionsToApplications < ActiveRecord::Migration[8.1]
+  def change
+    # Add require_pkce column for confidential clients
+    # Default true for new apps (secure by default), existing apps will be false
+    add_column :applications, :require_pkce, :boolean, default: true, null: false
+
+    # Set existing applications to not require PKCE (backwards compatibility)
+    reversible do |dir|
+      dir.up do
+        execute "UPDATE applications SET require_pkce = false WHERE id > 0"
+      end
+    end
+  end
+end

db/migrate/20251231043838_rename_code_to_code_hmac_and_add_token_hmac.rb

@@ -0,0 +1,20 @@
+class RenameCodeToCodeHmacAndAddTokenHmac < ActiveRecord::Migration[8.1]
+  def change
+    # Authorization codes: rename code to code_hmac
+    rename_column :oidc_authorization_codes, :code, :code_hmac
+
+    # Access tokens: add token_hmac, remove old columns
+    add_column :oidc_access_tokens, :token_hmac, :string
+    add_index :oidc_access_tokens, :token_hmac, unique: true
+
+    remove_column :oidc_access_tokens, :token_prefix
+    remove_column :oidc_access_tokens, :token_digest
+
+    # Refresh tokens: add token_hmac, remove old columns
+    add_column :oidc_refresh_tokens, :token_hmac, :string
+    add_index :oidc_refresh_tokens, :token_hmac, unique: true
+
+    remove_column :oidc_refresh_tokens, :token_prefix
+    remove_column :oidc_refresh_tokens, :token_digest
+  end
+end

test/fixtures/oidc_access_tokens.yml

@@ -1,16 +1,27 @@
 # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
 
+<%
+  # Generate a random token and compute HMAC
+  def generate_token_hmac
+    token = SecureRandom.urlsafe_base64(48)
+    hmac_key = Rails.application.key_generator.generate_key('oidc_token_prefix', 32)
+    hmac = OpenSSL::HMAC.hexdigest('SHA256', hmac_key, token)
+    [token, hmac]
+  end
+
+  token_one, hmac_one = generate_token_hmac
+  token_two, hmac_two = generate_token_hmac
+%>
+
 one:
-  token_digest: <%= BCrypt::Password.create(SecureRandom.urlsafe_base64(48)) %>
-  token_prefix: <%= SecureRandom.urlsafe_base64(8)[0..7] %>
+  token_hmac: <%= hmac_one %>
   application: kavita_app
   user: alice
   scope: "openid profile email"
   expires_at: 2025-12-31 23:59:59
 
 two:
-  token_digest: <%= BCrypt::Password.create(SecureRandom.urlsafe_base64(48)) %>
-  token_prefix: <%= SecureRandom.urlsafe_base64(8)[0..7] %>
+  token_hmac: <%= hmac_two %>
   application: another_app
   user: bob
   scope: "openid profile email"

test/fixtures/oidc_authorization_codes.yml

@@ -1,7 +1,20 @@
 # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
 
+<%
+  # Generate a random code and compute HMAC
+  def generate_code_hmac
+    code = SecureRandom.urlsafe_base64(32)
+    hmac_key = Rails.application.key_generator.generate_key('oidc_token_prefix', 32)
+    hmac = OpenSSL::HMAC.hexdigest('SHA256', hmac_key, code)
+    [code, hmac]
+  end
+
+  code_one, hmac_one = generate_code_hmac
+  code_two, hmac_two = generate_code_hmac
+%>
+
 one:
-  code: <%= SecureRandom.urlsafe_base64(32) %>
+  code_hmac: <%= hmac_one %>
   application: kavita_app
   user: alice
   redirect_uri: "https://kavita.example.com/signin-oidc"
@@ -10,7 +23,7 @@ one:
   used: false
 
 two:
-  code: <%= SecureRandom.urlsafe_base64(32) %>
+  code_hmac: <%= hmac_two %>
   application: another_app
   user: bob
   redirect_uri: "https://app.example.com/auth/callback"

test/integration/webauthn_credential_enumeration_test.rb

@@ -0,0 +1,107 @@
+require "test_helper"
+
+class WebauthnCredentialEnumerationTest < ActionDispatch::IntegrationTest
+  # ====================
+  # CREDENTIAL ENUMERATION PREVENTION TESTS
+  # ====================
+
+  test "prevents credential enumeration via delete endpoint" do
+    user1 = User.create!(email_address: "user1@example.com", password: "password123")
+    user2 = User.create!(email_address: "user2@example.com", password: "password123")
+
+    # Create a credential for user1
+    credential1 = user1.webauthn_credentials.create!(
+      external_id: Base64.urlsafe_encode64("user1_credential"),
+      public_key: Base64.urlsafe_encode64("public_key_1"),
+      sign_count: 0,
+      nickname: "User1 Key",
+      authenticator_type: "platform"
+    )
+
+    # Create a credential for user2
+    credential2 = user2.webauthn_credentials.create!(
+      external_id: Base64.urlsafe_encode64("user2_credential"),
+      public_key: Base64.urlsafe_encode64("public_key_2"),
+      sign_count: 0,
+      nickname: "User2 Key",
+      authenticator_type: "platform"
+    )
+
+    # Sign in as user1
+    post signin_path, params: { email_address: "user1@example.com", password: "password123" }
+    assert_response :redirect
+    follow_redirect!
+
+    # Try to delete user2's credential while authenticated as user1
+    # This should return 404 (not 403) to prevent enumeration
+    delete webauthn_credential_path(credential2.id), as: :json
+
+    assert_response :not_found
+    assert_includes JSON.parse(@response.body)["error"], "not found"
+
+    # Verify both credentials still exist
+    assert_equal 1, user1.webauthn_credentials.count
+    assert_equal 1, user2.webauthn_credentials.count
+
+    # Verify trying to delete a non-existent credential also returns 404
+    # This confirms identical responses for enumeration prevention
+    delete webauthn_credential_path(99999), as: :json
+
+    assert_response :not_found
+    assert_includes JSON.parse(@response.body)["error"], "not found"
+
+    user1.destroy
+    user2.destroy
+  end
+
+  test "allows users to delete their own credentials" do
+    user = User.create!(email_address: "user@example.com", password: "password123")
+
+    credential = user.webauthn_credentials.create!(
+      external_id: Base64.urlsafe_encode64("user_credential"),
+      public_key: Base64.urlsafe_encode64("public_key"),
+      sign_count: 0,
+      nickname: "My Key",
+      authenticator_type: "platform"
+    )
+
+    # Sign in
+    post signin_path, params: { email_address: "user@example.com", password: "password123" }
+    assert_response :redirect
+    follow_redirect!
+
+    # Delete own credential - should succeed
+    assert_difference "user.webauthn_credentials.count", -1 do
+      delete webauthn_credential_path(credential.id), as: :json
+    end
+
+    assert_response :success
+    assert_includes JSON.parse(@response.body)["message"], "has been removed"
+
+    user.destroy
+  end
+
+  test "unauthenticated user cannot delete credentials" do
+    user = User.create!(email_address: "user@example.com", password: "password123")
+
+    credential = user.webauthn_credentials.create!(
+      external_id: Base64.urlsafe_encode64("user_credential"),
+      public_key: Base64.urlsafe_encode64("public_key"),
+      sign_count: 0,
+      nickname: "My Key",
+      authenticator_type: "platform"
+    )
+
+    # Try to delete without authentication
+    delete webauthn_credential_path(credential.id), as: :json
+
+    # Should get redirect to signin (require_authentication before_action runs first)
+    assert_response :redirect
+    assert_redirected_to signin_path
+
+    # Verify credential still exists
+    assert_equal 1, user.webauthn_credentials.count
+
+    user.destroy
+  end
+end

test/models/oidc_access_token_test.rb

@@ -163,7 +163,7 @@ class OidcAccessTokenTest < ActiveSupport::TestCase
       user: users(:alice)
     )
 
-    assert access_token.plaintext_token.length > auth_code.code.length,
+    assert access_token.plaintext_token.length > auth_code.plaintext_code.length,
            "Access tokens should be longer than authorization codes"
   end
 

test/models/oidc_authorization_code_test.rb

@@ -25,10 +25,10 @@ class OidcAuthorizationCodeTest < ActiveSupport::TestCase
       user: users(:alice),
       redirect_uri: "https://example.com/callback"
     )
-    assert_nil new_code.code
+    assert_nil new_code.code_hmac
     assert new_code.save
-    assert_not_nil new_code.code
-    assert_match /^[A-Za-z0-9_-]+$/, new_code.code
+    assert_not_nil new_code.code_hmac
+    assert_match /^[a-f0-9]{64}$/, new_code.code_hmac # SHA256 hex digest
   end
 
   test "should set expiry before validation on create" do
@@ -44,22 +44,22 @@ class OidcAuthorizationCodeTest < ActiveSupport::TestCase
     assert new_code.expires_at <= 11.minutes.from_now # Allow some variance
   end
 
-  test "should validate presence of code" do
-    @auth_code.code = nil
+  test "should validate presence of code_hmac" do
+    @auth_code.code_hmac = nil
     assert_not @auth_code.valid?
-    assert_includes @auth_code.errors[:code], "can't be blank"
+    assert_includes @auth_code.errors[:code_hmac], "can't be blank"
   end
 
-  test "should validate uniqueness of code" do
+  test "should validate uniqueness of code_hmac" do
     @auth_code.save! if @auth_code.changed?
     duplicate = OidcAuthorizationCode.new(
-      code: @auth_code.code,
+      code_hmac: @auth_code.code_hmac,
       application: applications(:another_app),
       user: users(:bob),
       redirect_uri: "https://example.com/callback"
     )
     assert_not duplicate.valid?
-    assert_includes duplicate.errors[:code], "has already been taken"
+    assert_includes duplicate.errors[:code_hmac], "has already been taken"
   end
 
   test "should validate presence of redirect_uri" do
@@ -178,16 +178,16 @@ class OidcAuthorizationCodeTest < ActiveSupport::TestCase
         user: users(:alice),
         redirect_uri: "https://example.com/callback"
       )
-      codes << code.code
+      codes << code.code_hmac
     end
 
     # All codes should be unique
     assert_equal codes.length, codes.uniq.length
 
-    # All codes should match the expected pattern
+    # All codes should be SHA256 hex digests
     codes.each do |code|
-      assert_match /^[A-Za-z0-9_-]+$/, code
-      assert_equal 43, code.length # Base64 padding removed
+      assert_match /^[a-f0-9]{64}$/, code
+      assert_equal 64, code.length # SHA256 hex digest
     end
   end
 end