diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 16bdf0d..a31ec5c 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -39,6 +39,7 @@ module Authentication end def start_new_session_for(user) + user.update!(last_sign_in_at: Time.current) user.sessions.create!(user_agent: request.user_agent, ip_address: request.remote_ip).tap do |session| Current.session = session diff --git a/app/models/user.rb b/app/models/user.rb index b289a69..d1e4e73 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -8,10 +8,15 @@ class User < ApplicationRecord has_many :oidc_user_consents, dependent: :destroy # Token generation for passwordless flows - generates_token_for :invitation_login, expires_in: 24.hours - generates_token_for :invitation, expires_in: 7.days - generates_token_for :password_reset, expires_in: 1.hour - generates_token_for :magic_login, expires_in: 15.minutes + generates_token_for :invitation_login, expires_in: 24.hours do + updated_at + end + generates_token_for :password_reset, expires_in: 1.hour do + updated_at + end + generates_token_for :magic_login, expires_in: 15.minutes do + last_sign_in_at + end normalizes :email_address, with: ->(e) { e.strip.downcase } diff --git a/db/migrate/20251026113035_add_last_sign_in_at_to_users.rb b/db/migrate/20251026113035_add_last_sign_in_at_to_users.rb new file mode 100644 index 0000000..c5a8ec1 --- /dev/null +++ b/db/migrate/20251026113035_add_last_sign_in_at_to_users.rb @@ -0,0 +1,5 @@ +class AddLastSignInAtToUsers < ActiveRecord::Migration[8.1] + def change + add_column :users, :last_sign_in_at, :datetime + end +end diff --git a/test/models/user_password_management_test.rb b/test/models/user_password_management_test.rb index 1eefc81..c4a9df3 100644 --- a/test/models/user_password_management_test.rb +++ b/test/models/user_password_management_test.rb @@ -44,22 +44,9 @@ class UserPasswordManagementTest < ActiveSupport::TestCase assert @user.magic_login_token_created_at > 5.minutes.ago end - test "should generate invitation token" do - assert_nil @user.invitation_token - assert_nil @user.invitation_token_created_at - - @user.generate_token_for(:invitation) - @user.save! - - assert_not_nil @user.invitation_token - assert_not_nil @user.invitation_token_created_at - assert @user.invitation_token.length > 20 - assert @user.invitation_token_created_at > 5.minutes.ago - end - test "should generate tokens with different lengths" do # Test that different token types generate appropriate length tokens - token_types = [:password_reset, :invitation_login, :magic_login, :invitation] + token_types = [:password_reset, :invitation_login, :magic_login] token_types.each do |token_type| @user.generate_token_for(token_type) @@ -146,7 +133,7 @@ class UserPasswordManagementTest < ActiveSupport::TestCase test "should validate different token types" do # Test all token types work - token_types = [:password_reset, :invitation_login, :magic_login, :invitation] + token_types = [:password_reset, :invitation_login, :magic_login] token_types.each do |token_type| @user.generate_token_for(token_type) @@ -162,9 +149,6 @@ class UserPasswordManagementTest < ActiveSupport::TestCase when :magic_login assert @user.magic_login_token.present? assert @user.magic_login_token_valid? - when :invitation - assert @user.invitation_token.present? - assert @user.invitation_token_valid? end end end @@ -284,4 +268,34 @@ class UserPasswordManagementTest < ActiveSupport::TestCase assert_not old_password_instance.authenticate("NewPassword123!"), "Old password should not authenticate new instance" assert_not old_password_instance.authenticate("NewPassword123!"), "Password change should invalidate old sessions" end + + test "should update last_sign_in_at timestamp" do + # Test that last_sign_in_at is initially nil + assert_nil @user.last_sign_in_at, "New user should have nil last_sign_in_at" + + # Update last_sign_in_at + @user.update!(last_sign_in_at: Time.current) + + @user.reload + assert_not_nil @user.last_sign_in_at, "last_sign_in_at should be set after update" + assert @user.last_sign_in_at > 1.minute.ago, "last_sign_in_at should be recent" + end + + test "should invalidate magic login token after sign in" do + # Generate magic login token + @user.update!(last_sign_in_at: 1.hour.ago) # Set initial timestamp + old_sign_in_time = @user.last_sign_in_at + + magic_token = @user.generate_token_for(:magic_login) + + # Token should be valid before sign-in + assert User.find_by_magic_login_token(magic_token)&.id == @user.id, "Magic login token should be valid initially" + + # Simulate sign-in (which updates last_sign_in_at) + @user.update!(last_sign_in_at: Time.current) + + # Token should now be invalid because last_sign_in_at changed + assert_nil User.find_by_magic_login_token(magic_token), "Magic login token should be invalid after sign-in" + assert_not_equal old_sign_in_time, @user.last_sign_in_at, "last_sign_in_at should have changed" + end end \ No newline at end of file diff --git a/test/services/oidc_jwt_service_test.rb b/test/services/oidc_jwt_service_test.rb new file mode 100644 index 0000000..916d672 --- /dev/null +++ b/test/services/oidc_jwt_service_test.rb @@ -0,0 +1,288 @@ +require "test_helper" + +class OidcJwtServiceTest < ActiveSupport::TestCase + def setup + @user = users(:alice) + @application = applications(:kavita_app) + @service = OidcJwtService + end + + test "should generate id token with required claims" do + # Test JWT generation with basic user + token = @service.generate_id_token(@user, @application) + + assert_not_nil token, "Should generate token" + assert token.length > 100, "Token should be substantial" + assert token.include?('.'), "Token should have segments" + + # Decode and verify payload + decoded = JWT.decode(token, nil, true) + assert_equal @application.client_id, decoded['aud'], "Should have correct audience" + assert_equal @user.id.to_s, decoded['sub'], "Should have correct subject" + assert_equal @user.email_address, decoded['email'], "Should have correct email" + assert_equal true, decoded['email_verified'], "Should have email verified" + assert_equal @user.email_address, decoded['preferred_username'], "Should have preferred username" + assert_equal @user.email_address, decoded['name'], "Should have name" + assert_equal "https://localhost:3000", decoded['iss'], "Should have correct issuer" + assert_equal Time.now.to_i + 3600, decoded['exp'], "Should have correct expiration" + end + + test "should handle nonce in id token" do + # Test nonce handling + nonce = "test-nonce-12345" + token = @service.generate_id_token(@user, @application, nonce: nonce) + + decoded = JWT.decode(token, nil, true) + assert_equal nonce, decoded['nonce'], "Should preserve nonce in token" + assert_equal Time.now.to_i + 3600, decoded['exp'], "Should have correct expiration with nonce" + end + + test "should include groups in token when user has groups" do + # Test group inclusion + @user.groups << groups(:admin_group) + + token = @service.generate_id_token(@user, @application) + + decoded = JWT.decode(token, nil, true) + assert_includes decoded['groups'], "admin", "Should include user's groups" + end + + test "should include admin claim for admin users" do + # Test admin claim + @user.update!(admin: true) + + token = @service.generate_id_token(@user, @application) + + decoded = JWT.decode(token, nil, true) + assert_equal true, decoded['admin'], "Admin users should have admin claim" + end + + test "should handle role-based claims when enabled" do + # Test role-based claims + @application.update!( + role_mapping_enabled: true, + role_mapping_mode: "oidc_managed", + role_claim_name: "roles" + ) + + # Assign role to user + @application.assign_role_to_user!(@user, "editor", source: 'oidc', metadata: { synced_at: Time.current }) + + token = @service.generate_id_token(@user, @application) + + decoded = JWT.decode(token, nil, true) + assert_includes decoded['roles'], "editor", "Should include user's role" + end + + test "should include role metadata when configured" do + # Test role metadata inclusion + @application.update!( + role_mapping_enabled: true, + role_mapping_mode: "oidc_managed", + parsed_managed_permissions: { + "include_permissions" => true, + "include_metadata" => true + } + ) + + # Assign role with metadata + role = @application.application_roles.create!( + name: "editor", + display_name: "Content Editor", + permissions: ["read", "write"] + ) + + @application.assign_role_to_user!( + @user, + "editor", + source: 'oidc', + metadata: { + synced_at: Time.current, + department: "Content Team", + level: "2" + } + ) + + token = @service.generate_id_token(@user, @application) + + decoded = JWT.decode(token, nil, true) + assert_equal "Content Editor", decoded['role_display_name'], "Should include role display name" + assert_includes decoded['role_permissions'], "read", "Should include read permission" + assert_includes decoded['role_permissions'], "write", "Should include write permission" + assert_equal "Content Team", decoded['role_department'], "Should include department" + assert_equal "2", decoded['role_level'], "Should include level" + end + + test "should handle hybrid role mapping mode" do + # Test hybrid mode (combining OIDC roles with local groups) + @application.update!( + role_mapping_mode: "hybrid", + role_mapping_enabled: true, + role_prefix: "ext-" + ) + + # Create external role and local group + external_role = @application.application_roles.create!(name: "ext-admin") + @user.groups << groups(:admin_group) + + token = @service.generate_id_token(@user, @application) + decoded = JWT.decode(token, nil, true) + + # User should be allowed (has external role OR admin group) + assert_includes decoded['roles'], "ext-admin", "Should include external role" + assert_includes decoded['groups'], "admin", "Should include admin group" + end + + test "should handle missing roles gracefully" do + # Test when roles claim is missing or empty + token = @service.generate_id_token(@user, @application) + + decoded = JWT.decode(token, nil, true) + assert_not_includes decoded, 'roles', "Should not have roles key when not configured" + end + + test "should use RSA private key from environment" do + # Test private key handling + ENV.stub(:fetch, "OIDC_PRIVATE_KEY") { "test-private-key" } + + private_key = @service.private_key + assert_equal "test-private-key", private_key.to_s, "Should use private key from environment" + end + + test "should generate RSA private key when missing" do + # Test private key generation in development + ENV.stub(:fetch, nil) { nil } + ENV.stub(:fetch, "OIDC_PRIVATE_KEY", nil) { nil } + Rails.application.credentials.stub(:oidc_private_key, nil) { nil } + + private_key = @service.private_key + assert_not_nil private_key, "Should generate private key when missing" + assert private_key.is_a?(OpenSSL::PKey::RSA), "Should generate RSA private key" + assert_equal 2048, private_key.num_bits, "Should generate 2048-bit key" + end + + test "should get corresponding public key" do + # Test public key retrieval + public_key = @service.public_key + assert_not_nil public_key, "Should have public key" + assert_equal "RSA", public_key.kty, "Should be RSA key" + assert_equal 256, public_key.n, "Should be 256-bit key" + end + + test "should generate JWKS format" do + # Test JWKS generation + jwks = @service.jwks + assert_not_nil jwks, "Should generate JWKS" + assert jwks.is_a?(Hash), "JWKS should be a hash" + assert_includes jwks, :keys, "JWKS should contain keys array" + assert_equal 1, jwks[:keys].size, "JWKS should contain one key" + + key_data = jwks[:keys].first + assert_equal "RSA", key_data[:kty], "Key should be RSA" + assert key_data[:kid], "Key should have kid" + assert key_data[:use], "sig", "Key should be for signing" + assert_equal "RS256", key_data[:alg], "Key should use RS256 algorithm" + end + + test "should decode and verify id token" do + # Test token verification + token = @service.generate_id_token(@user, @application) + decoded = @service.decode_id_token(token) + + assert_not_nil decoded, "Should decode valid token" + assert_equal @user.id.to_s, decoded['sub'], "Should decode subject correctly" + assert_equal @application.client_id, decoded['aud'], "Should decode audience correctly" + assert decoded['exp'] > Time.current.to_i, "Token should not be expired" + end + + test "should reject invalid id tokens" do + # Test token validation + invalid_tokens = [ + "invalid.token", + "header.payload.signature", # Missing signature + "eyJ0", # Too short + nil, # Empty token + "Bearer" # Bearer prefix (should be raw JWT) + ] + + invalid_tokens.each do |invalid_token| + assert_raises(JWT::DecodeError) do + @service.decode_id_token(invalid_token) + end, "Should raise error for invalid token: #{invalid_token}" + end + end + + test "should handle expired tokens" do + # Test expired token handling + travel_to 2.hours.from_now do + token = @service.generate_id_token(@user, @application, exp: 1.hour.from_now) + travel_back + + assert_raises(JWT::ExpiredSignature) do + @service.decode_id_token(token) + end, "Should raise error for expired token" + end + end + end + + test "should handle access token generation" do + # Test access token (simpler than ID token) + token = @service.generate_id_token(@user, @application) + + decoded = JWT.decode(token, nil, true) + # Access tokens typically don't have email_verified claim + refute_includes decoded.keys, 'email_verified' + # But should still have standard claims + assert_equal @user.id.to_s, decoded['sub'], "Should have subject" + assert_equal @application.client_id, decoded['aud'], "Should have audience" + end + + test "should handle JWT errors gracefully" do + # Test error handling + original_algorithm = OpenSSL::PKey::RSA::DEFAULT_PRIVATE_KEY + + # Temporarily break the service + OpenSSL::PKey::RSA.stub(:new, -> { raise "Key generation failed" }) do + OpenSSL::PKey::RSA.new(2048) + end + + assert_raises(RuntimeError, message: /Key generation failed/) do + @service.private_key + end + + # Restore original + OpenSSL::PKey::RSA.stub(:new, original_algorithm) do + restored_key = @service.private_key + assert_not_equal original_algorithm, restored_key, "Should restore after error" + end + end + + test "should validate JWT configuration" do + # Test service configuration validation + @application.update!(client_id: "test-client") + + error = assert_raises(StandardError) do + @service.generate_id_token(@user, @application) + end + + assert_match /no key found/, error.message, "Should warn about missing key" + end + + test "should handle key rotation scenarios" do + # Test key rotation (development scenario) + old_key = @service.public_key + + # Generate new key + @service.instance_variable_set(:@public_key, nil) + @service.instance_variable_set(:@key_id, nil) + new_public_key = @service.public_key + + assert_not_equal old_key, new_public_key, "Should generate new public key" + assert_not_equal old_key.to_pem, new_public_key.to_pem, "New key should be different" + + # Key ID should have changed + new_key_id = @service.key_id + assert_not_nil new_key_id, "Should generate new key ID" + assert_match /^\w{16}$/, new_key_id, "Key ID should be base64url format" + end +end \ No newline at end of file