diff --git a/Gemfile b/Gemfile index 9a14236..e9b8058 100644 --- a/Gemfile +++ b/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 + diff --git a/Gemfile.lock b/Gemfile.lock index cab9d18..f11c70b 100644 --- a/Gemfile.lock +++ b/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 diff --git a/config/initializers/active_record_encryption.rb b/config/initializers/active_record_encryption.rb new file mode 100644 index 0000000..e325fb3 --- /dev/null +++ b/config/initializers/active_record_encryption.rb @@ -0,0 +1,22 @@ +# 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 +Rails.application.config.after_initialize do + # Use env vars if set, otherwise derive from SECRET_KEY_BASE (deterministic) + primary_key = ENV['ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY'] || Rails.application.key_generator.generate_key('active_record_encryption_primary', 32) + deterministic_key = ENV['ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY'] || Rails.application.key_generator.generate_key('active_record_encryption_deterministic', 32) + key_derivation_salt = ENV['ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT'] || Rails.application.key_generator.generate_key('active_record_encryption_salt', 32) + + # 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 + + # Ensure encryption is enabled + Rails.application.config.active_record.encryption.support_unencrypted_data = false +end diff --git a/db/migrate/20251230073656_add_pkce_options_to_applications.rb b/db/migrate/20251230073656_add_pkce_options_to_applications.rb new file mode 100644 index 0000000..b95a86e --- /dev/null +++ b/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 diff --git a/db/migrate/20251231043838_rename_code_to_code_hmac_and_add_token_hmac.rb b/db/migrate/20251231043838_rename_code_to_code_hmac_and_add_token_hmac.rb new file mode 100644 index 0000000..54bb5fa --- /dev/null +++ b/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 diff --git a/test/fixtures/oidc_access_tokens.yml b/test/fixtures/oidc_access_tokens.yml index 276b932..fe73d12 100644 --- a/test/fixtures/oidc_access_tokens.yml +++ b/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" diff --git a/test/fixtures/oidc_authorization_codes.yml b/test/fixtures/oidc_authorization_codes.yml index 3163646..072e37e 100644 --- a/test/fixtures/oidc_authorization_codes.yml +++ b/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" diff --git a/test/integration/webauthn_credential_enumeration_test.rb b/test/integration/webauthn_credential_enumeration_test.rb new file mode 100644 index 0000000..43d3547 --- /dev/null +++ b/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