Fix/add some tests. Configure email sending address

2025-10-26 20:13:39 +11:00
parent d98f777e7d
commit 227e29ce0a
7 changed files with 644 additions and 30 deletions
--- a/test/fixtures/application_groups.yml
+++ b/test/fixtures/application_groups.yml
@@ -1,9 +1,9 @@
 # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html

-one:
-  application: one
-  group: one
+kavita_admin_group:
+  application: kavita_app
+  group: admin_group

-two:
-  application: two
-  group: two
+kavita_editor_group:
+  application: kavita_app
+  group: editor_group
--- a/test/fixtures/oidc_user_consents.yml
+++ b/test/fixtures/oidc_user_consents.yml
@@ -1,13 +1,13 @@
 # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html

-one:
-  user: one
-  application: one
-  scopes_granted: MyText
+alice_consent:
+  user: alice
+  application: kavita_app
+  scopes_granted: openid profile email
  granted_at: 2025-10-24 16:57:39

-two:
-  user: two
-  application: two
-  scopes_granted: MyText
+bob_consent:
+  user: bob
+  application: another_app
+  scopes_granted: openid email groups
  granted_at: 2025-10-24 16:57:39
--- a/test/fixtures/user_groups.yml
+++ b/test/fixtures/user_groups.yml
@@ -1,9 +1,9 @@
 # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html

-one:
-  user: one
-  group: one
+alice_admin_group:
+  user: alice
+  group: admin_group

-two:
-  user: two
-  group: two
+bob_editor_group:
+  user: bob
+  group: editor_group
--- a/test/models/oidc_access_token_test.rb
+++ b/test/models/oidc_access_token_test.rb
@@ -1,7 +1,216 @@
 require "test_helper"

 class OidcAccessTokenTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  def setup
+    @access_token = oidc_access_tokens(:one)
+  end
+
+  test "should be valid with all required attributes" do
+    assert @access_token.valid?
+  end
+
+  test "should belong to an application" do
+    assert_respond_to @access_token, :application
+    assert_equal applications(:kavita_app), @access_token.application
+  end
+
+  test "should belong to a user" do
+    assert_respond_to @access_token, :user
+    assert_equal users(:alice), @access_token.user
+  end
+
+  test "should generate token before validation on create" do
+    new_token = OidcAccessToken.new(
+      application: applications(:kavita_app),
+      user: users(:alice)
+    )
+    assert_nil new_token.token
+    assert new_token.save
+    assert_not_nil new_token.token
+    assert_match /^[A-Za-z0-9_-]+$/, new_token.token
+  end
+
+  test "should set expiry before validation on create" do
+    new_token = OidcAccessToken.new(
+      application: applications(:kavita_app),
+      user: users(:alice)
+    )
+    assert_nil new_token.expires_at
+    assert new_token.save
+    assert_not_nil new_token.expires_at
+    assert new_token.expires_at > Time.current
+    assert new_token.expires_at <= 61.minutes.from_now # Allow some variance
+  end
+
+  test "should validate presence of token" do
+    @access_token.token = nil
+    assert_not @access_token.valid?
+    assert_includes @access_token.errors[:token], "can't be blank"
+  end
+
+  test "should validate uniqueness of token" do
+    @access_token.save! if @access_token.changed?
+    duplicate = OidcAccessToken.new(
+      token: @access_token.token,
+      application: applications(:another_app),
+      user: users(:bob)
+    )
+    assert_not duplicate.valid?
+    assert_includes duplicate.errors[:token], "has already been taken"
+  end
+
+  test "should identify expired tokens correctly" do
+    @access_token.expires_at = 5.minutes.ago
+    assert @access_token.expired?, "Should identify past expiry as expired"
+
+    @access_token.expires_at = 5.minutes.from_now
+    assert_not @access_token.expired?, "Should identify future expiry as not expired"
+
+    @access_token.expires_at = Time.current
+    assert @access_token.expired?, "Should identify current time as expired"
+  end
+
+  test "should identify active tokens correctly" do
+    # Non-expired token should be active
+    @access_token.expires_at = 5.minutes.from_now
+    assert @access_token.active?, "Future expiry should be active"
+
+    # Expired token should not be active
+    @access_token.expires_at = 5.minutes.ago
+    assert_not @access_token.active?, "Past expiry should not be active"
+
+    # Current time should be considered expired (not active)
+    @access_token.expires_at = Time.current
+    assert_not @access_token.active?, "Current time should not be active"
+  end
+
+  test "should revoke token correctly" do
+    @access_token.expires_at = 1.hour.from_now
+    original_expiry = @access_token.expires_at
+    assert @access_token.active?, "Token should be active before revocation"
+
+    @access_token.revoke!
+    @access_token.reload
+
+    assert @access_token.expired?, "Token should be expired after revocation"
+    assert @access_token.expires_at <= Time.current, "Expiry should be set to current time or earlier"
+    assert @access_token.expires_at < original_expiry, "Expiry should be changed from original"
+  end
+
+  test "valid scope should return only non-expired tokens" do
+    # Create tokens with different states
+    valid_token = OidcAccessToken.create!(
+      application: applications(:kavita_app),
+      user: users(:alice)
+    )
+
+    expired_token = OidcAccessToken.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      expires_at: 5.minutes.ago
+    )
+
+    valid_tokens = OidcAccessToken.valid
+    assert_includes valid_tokens, valid_token
+    assert_not_includes valid_tokens, expired_token
+  end
+
+  test "expired scope should return only expired tokens" do
+    # Create tokens with different expiry states
+    non_expired_token = OidcAccessToken.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      expires_at: 1.hour.from_now
+    )
+
+    expired_token = OidcAccessToken.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      expires_at: 5.minutes.ago
+    )
+
+    expired_tokens = OidcAccessToken.expired
+    assert_includes expired_tokens, expired_token
+    assert_not_includes expired_tokens, non_expired_token
+  end
+
+  test "should handle concurrent revocation safely" do
+    @access_token.expires_at = 1.hour.from_now
+    @access_token.save!
+
+    original_active = @access_token.active?
+    @access_token.revoke!
+
+    assert original_active, "Token should be active before revocation"
+    assert @access_token.expired?, "Token should be expired after revocation"
+  end
+
+  test "should generate secure random tokens" do
+    tokens = []
+    5.times do
+      token = OidcAccessToken.create!(
+        application: applications(:kavita_app),
+        user: users(:alice)
+      )
+      tokens << token.token
+    end
+
+    # All tokens should be unique
+    assert_equal tokens.length, tokens.uniq.length
+
+    # All tokens should match the expected pattern
+    tokens.each do |token|
+      assert_match /^[A-Za-z0-9_-]+$/, token
+      assert_equal 63, token.length # Base64 with padding removed (48 bytes = 64 chars, minus padding)
+    end
+  end
+
+  test "should have longer token than authorization codes" do
+    auth_code = OidcAuthorizationCode.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback"
+    )
+
+    access_token = OidcAccessToken.create!(
+      application: applications(:kavita_app),
+      user: users(:alice)
+    )
+
+    assert access_token.token.length > auth_code.code.length,
+           "Access tokens should be longer than authorization codes"
+  end
+
+  test "should have appropriate expiry times" do
+    auth_code = OidcAuthorizationCode.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback"
+    )
+
+    access_token = OidcAccessToken.create!(
+      application: applications(:kavita_app),
+      user: users(:alice)
+    )
+
+    # Authorization codes expire in 10 minutes, access tokens in 1 hour
+    assert access_token.expires_at > auth_code.expires_at,
+           "Access tokens should have longer expiry than authorization codes"
+  end
+
+  test "revoked tokens should not appear in valid scope" do
+    access_token = OidcAccessToken.create!(
+      application: applications(:kavita_app),
+      user: users(:alice)
+    )
+
+    # Token should be in valid scope initially
+    assert_includes OidcAccessToken.valid, access_token
+
+    # Revoke the token
+    access_token.revoke!
+
+    # Token should no longer be in valid scope
+    assert_not_includes OidcAccessToken.valid, access_token
+  end
 end
--- a/test/models/oidc_authorization_code_test.rb
+++ b/test/models/oidc_authorization_code_test.rb
@@ -1,7 +1,193 @@
 require "test_helper"

 class OidcAuthorizationCodeTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  def setup
+    @auth_code = oidc_authorization_codes(:one)
+  end
+
+  test "should be valid with all required attributes" do
+    assert @auth_code.valid?
+  end
+
+  test "should belong to an application" do
+    assert_respond_to @auth_code, :application
+    assert_equal applications(:kavita_app), @auth_code.application
+  end
+
+  test "should belong to a user" do
+    assert_respond_to @auth_code, :user
+    assert_equal users(:alice), @auth_code.user
+  end
+
+  test "should generate code before validation on create" do
+    new_code = OidcAuthorizationCode.new(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback"
+    )
+    assert_nil new_code.code
+    assert new_code.save
+    assert_not_nil new_code.code
+    assert_match /^[A-Za-z0-9_-]+$/, new_code.code
+  end
+
+  test "should set expiry before validation on create" do
+    new_code = OidcAuthorizationCode.new(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback"
+    )
+    assert_nil new_code.expires_at
+    assert new_code.save
+    assert_not_nil new_code.expires_at
+    assert new_code.expires_at > Time.current
+    assert new_code.expires_at <= 11.minutes.from_now # Allow some variance
+  end
+
+  test "should validate presence of code" do
+    @auth_code.code = nil
+    assert_not @auth_code.valid?
+    assert_includes @auth_code.errors[:code], "can't be blank"
+  end
+
+  test "should validate uniqueness of code" do
+    @auth_code.save! if @auth_code.changed?
+    duplicate = OidcAuthorizationCode.new(
+      code: @auth_code.code,
+      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"
+  end
+
+  test "should validate presence of redirect_uri" do
+    @auth_code.redirect_uri = nil
+    assert_not @auth_code.valid?
+    assert_includes @auth_code.errors[:redirect_uri], "can't be blank"
+  end
+
+  test "should identify expired codes correctly" do
+    @auth_code.expires_at = 5.minutes.ago
+    assert @auth_code.expired?, "Should identify past expiry as expired"
+
+    @auth_code.expires_at = 5.minutes.from_now
+    assert_not @auth_code.expired?, "Should identify future expiry as not expired"
+
+    @auth_code.expires_at = Time.current
+    assert @auth_code.expired?, "Should identify current time as expired"
+  end
+
+  test "should identify usable codes correctly" do
+    # Fresh, unused code should be usable
+    @auth_code.expires_at = 5.minutes.from_now
+    @auth_code.used = false
+    assert @auth_code.usable?, "Fresh unused code should be usable"
+
+    # Used code should not be usable
+    @auth_code.used = true
+    assert_not @auth_code.usable?, "Used code should not be usable"
+
+    # Expired code should not be usable
+    @auth_code.used = false
+    @auth_code.expires_at = 5.minutes.ago
+    assert_not @auth_code.usable?, "Expired code should not be usable"
+
+    # Used and expired code should not be usable
+    @auth_code.used = true
+    @auth_code.expires_at = 5.minutes.ago
+    assert_not @auth_code.usable?, "Used and expired code should not be usable"
+  end
+
+  test "should consume code correctly" do
+    @auth_code.used = false
+    assert_not @auth_code.used?, "Code should initially be unused"
+
+    @auth_code.consume!
+    @auth_code.reload
+    assert @auth_code.used?, "Code should be marked as used after consumption"
+  end
+
+  test "valid scope should return only unused and non-expired codes" do
+    # Create codes with different states
+    valid_code = OidcAuthorizationCode.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback"
+    )
+
+    used_code = OidcAuthorizationCode.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback",
+      used: true
+    )
+
+    expired_code = OidcAuthorizationCode.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback",
+      expires_at: 5.minutes.ago
+    )
+
+    valid_codes = OidcAuthorizationCode.valid
+    assert_includes valid_codes, valid_code
+    assert_not_includes valid_codes, used_code
+    assert_not_includes valid_codes, expired_code
+  end
+
+  test "expired scope should return only expired codes" do
+    # Create codes with different expiry states
+    non_expired_code = OidcAuthorizationCode.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback",
+      expires_at: 5.minutes.from_now
+    )
+
+    expired_code = OidcAuthorizationCode.create!(
+      application: applications(:kavita_app),
+      user: users(:alice),
+      redirect_uri: "https://example.com/callback",
+      expires_at: 5.minutes.ago
+    )
+
+    expired_codes = OidcAuthorizationCode.expired
+    assert_includes expired_codes, expired_code
+    assert_not_includes expired_codes, non_expired_code
+  end
+
+  test "should handle concurrent consumption safely" do
+    @auth_code.used = false
+    @auth_code.save!
+
+    # Simulate concurrent consumption
+    original_used = @auth_code.used?
+    @auth_code.consume!
+
+    assert_not original_used, "Code should be unused before consumption"
+    assert @auth_code.used?, "Code should be used after consumption"
+  end
+
+  test "should generate secure random codes" do
+    codes = []
+    5.times do
+      code = OidcAuthorizationCode.create!(
+        application: applications(:kavita_app),
+        user: users(:alice),
+        redirect_uri: "https://example.com/callback"
+      )
+      codes << code.code
+    end
+
+    # All codes should be unique
+    assert_equal codes.length, codes.uniq.length
+
+    # All codes should match the expected pattern
+    codes.each do |code|
+      assert_match /^[A-Za-z0-9_-]+$/, code
+      assert_equal 43, code.length # Base64 padding removed
+    end
+  end
 end
--- a/test/models/oidc_user_consent_test.rb
+++ b/test/models/oidc_user_consent_test.rb
@@ -1,7 +1,226 @@
 require "test_helper"

 class OidcUserConsentTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  def setup
+    @consent = oidc_user_consents(:alice_consent)
+  end
+
+  test "should be valid with all required attributes" do
+    assert @consent.valid?
+  end
+
+  test "should belong to a user" do
+    assert_respond_to @consent, :user
+    assert_equal users(:alice), @consent.user
+  end
+
+  test "should belong to an application" do
+    assert_respond_to @consent, :application
+    assert_equal applications(:kavita_app), @consent.application
+  end
+
+  test "should validate presence of user" do
+    @consent.user = nil
+    assert_not @consent.valid?
+    assert_includes @consent.errors[:user], "can't be blank"
+  end
+
+  test "should validate presence of application" do
+    @consent.application = nil
+    assert_not @consent.valid?
+    assert_includes @consent.errors[:application], "can't be blank"
+  end
+
+  test "should validate presence of scopes_granted" do
+    @consent.scopes_granted = nil
+    assert_not @consent.valid?
+    assert_includes @consent.errors[:scopes_granted], "can't be blank"
+  end
+
+  test "should validate presence of granted_at" do
+    @consent.granted_at = nil
+    assert_not @consent.valid?
+    assert_includes @consent.errors[:granted_at], "can't be blank"
+  end
+
+  test "should validate uniqueness of user_id scoped to application_id" do
+    # Should be able to create consent for different user with same app
+    new_consent = OidcUserConsent.new(
+      user: users(:bob),
+      application: @consent.application,
+      scopes_granted: "openid email"
+    )
+    assert new_consent.valid?
+
+    # Should NOT be able to create consent for same user with same app
+    duplicate_consent = OidcUserConsent.new(
+      user: @consent.user,
+      application: @consent.application,
+      scopes_granted: "openid profile"
+    )
+    assert_not duplicate_consent.valid?
+    assert_includes duplicate_consent.errors[:user_id], "has already been taken"
+  end
+
+  test "should set granted_at before validation on create" do
+    new_consent = OidcUserConsent.new(
+      user: users(:bob),
+      application: applications(:another_app),
+      scopes_granted: "openid email"
+    )
+    assert_nil new_consent.granted_at
+    assert new_consent.save
+    assert_not_nil new_consent.granted_at
+    assert new_consent.granted_at <= Time.current
+  end
+
+  test "scopes should parse space-separated scopes into array" do
+    @consent.scopes_granted = "openid profile email groups"
+    assert_equal ["openid", "profile", "email", "groups"], @consent.scopes
+
+    # Handle single scope
+    @consent.scopes_granted = "openid"
+    assert_equal ["openid"], @consent.scopes
+
+    # Handle empty string
+    @consent.scopes_granted = ""
+    assert_equal [], @consent.scopes
+
+    # Handle extra spaces
+    @consent.scopes_granted = "openid  profile   email"
+    assert_equal ["openid", "profile", "email"], @consent.scopes
+  end
+
+  test "scopes= should join array into space-separated string" do
+    @consent.scopes = ["openid", "profile", "email"]
+    assert_equal "openid profile email", @consent.scopes_granted
+
+    # Handle single item array
+    @consent.scopes = ["openid"]
+    assert_equal "openid", @consent.scopes_granted
+
+    # Handle empty array
+    @consent.scopes = []
+    assert_equal "", @consent.scopes_granted
+
+    # Handle duplicates
+    @consent.scopes = ["openid", "profile", "openid"]
+    assert_equal "openid profile", @consent.scopes_granted
+  end
+
+  test "should handle string input for scopes=" do
+    @consent.scopes = "openid profile"
+    assert_equal "openid profile", @consent.scopes_granted
+    assert_equal ["openid", "profile"], @consent.scopes
+  end
+
+  test "covers_scopes? should correctly identify scope coverage" do
+    @consent.scopes_granted = "openid profile email groups"
+
+    # Should cover when all requested scopes are granted
+    assert @consent.covers_scopes?(["openid"]), "Should cover single requested scope"
+    assert @consent.covers_scopes?(["openid", "profile"]), "Should cover multiple requested scopes"
+    assert @consent.covers_scopes?(["email", "groups"]), "Should cover different combination"
+    assert @consent.covers_scopes?(["openid", "profile", "email", "groups"]), "Should cover all granted scopes"
+
+    # Should not cover when requested includes non-granted scope
+    assert_not @consent.covers_scopes?(["admin"]), "Should not cover non-granted scope"
+    assert_not @consent.covers_scopes?(["openid", "admin"]), "Should not cover mixed granted/non-granted"
+    assert_not @consent.covers_scopes?(["admin", "write"]), "Should not cover all non-granted"
+
+    # Handle string input
+    assert @consent.covers_scopes?("openid"), "Should handle string input"
+    assert_not @consent.covers_scopes?("admin"), "Should handle string input for non-granted scope"
+
+    # Handle empty requested scopes
+    assert @consent.covers_scopes?([]), "Should cover empty array"
+    assert @consent.covers_scopes?(nil), "Should handle nil input"
+  end
+
+  test "covers_scopes? should handle edge cases" do
+    # Consent with no scopes
+    @consent.scopes_granted = ""
+    assert_not @consent.covers_scopes?(["openid"]), "Should not cover any scope when no scopes granted"
+    assert @consent.covers_scopes?([]), "Should cover empty request when no scopes granted"
+
+    # Consent with one scope
+    @consent.scopes_granted = "openid"
+    assert @consent.covers_scopes?(["openid"]), "Should cover matching single scope"
+    assert_not @consent.covers_scopes?(["profile"]), "Should not cover different single scope"
+  end
+
+  test "formatted_scopes should provide human-readable scope names" do
+    @consent.scopes_granted = "openid profile email groups"
+    expected = "Basic authentication, Profile information, Email address, Group membership"
+    assert_equal expected, @consent.formatted_scopes
+
+    # Test single scope
+    @consent.scopes_granted = "openid"
+    assert_equal "Basic authentication", @consent.formatted_scopes
+
+    # Test unknown scope
+    @consent.scopes_granted = "unknown_scope"
+    assert_equal "Unknown scope", @consent.formatted_scopes
+
+    # Test mixed known and unknown
+    @consent.scopes_granted = "openid custom_scope"
+    assert_equal "Basic authentication, Custom scope", @consent.formatted_scopes
+
+    # Test empty scopes
+    @consent.scopes_granted = ""
+    assert_equal "", @consent.formatted_scopes
+  end
+
+  test "should maintain consistency between scopes getter and setter" do
+    original_scopes = ["openid", "profile", "email"]
+    @consent.scopes = original_scopes
+    assert_equal original_scopes, @consent.scopes
+
+    # Modify scopes
+    new_scopes = ["openid", "groups"]
+    @consent.scopes = new_scopes
+    assert_equal new_scopes, @consent.scopes
+  end
+
+  test "should handle consent updates correctly" do
+    # Create initial consent
+    consent = OidcUserConsent.create!(
+      user: users(:bob),
+      application: applications(:another_app),
+      scopes_granted: "openid email"
+    )
+
+    # Update to include more scopes
+    consent.scopes = ["openid", "email", "profile"]
+    consent.save!
+
+    consent.reload
+    assert_equal ["openid", "email", "profile"], consent.scopes
+    assert_equal "openid email profile", consent.scopes_granted
+
+    # Should still cover original scopes
+    assert consent.covers_scopes?(["openid", "email"])
+    # Should cover new scopes
+    assert consent.covers_scopes?(["profile"])
+    # Should cover all scopes
+    assert consent.covers_scopes?(["openid", "email", "profile"])
+  end
+
+  test "should validate scope coverage logic with real OIDC scenarios" do
+    # Typical OIDC consent scenario
+    @consent.scopes_granted = "openid profile email"
+
+    # Application requests only openid (required for OIDC)
+    assert @consent.covers_scopes?(["openid"]), "Should cover required openid scope"
+
+    # Application requests standard scopes
+    assert @consent.covers_scopes?(["openid", "profile"]), "Should cover standard OIDC scopes"
+
+    # Application requests more than granted
+    assert_not @consent.covers_scopes?(["openid", "profile", "groups"]),
+                  "Should not cover scopes not granted"
+
+    # Application requests subset
+    assert @consent.covers_scopes?(["email"]), "Should cover subset of granted scopes"
+  end
 end