Skip to content

Commit c3f93fe

Browse files
committed
test(auth): add argon2id migration specs and fix Player password validation
- Add spec/services/authentication/password_hasher_spec.rb (16 examples) covering hash, verify (argon2id, bcrypt legacy, blank inputs), needs_upgrade? and bcrypt? - Add spec/models/concerns/upgradeable_password_spec.rb (10 examples) covering the bcrypt→argon2id lazy upgrade path for User and Player, including that wrong passwords do not trigger digest update - Fix Player#player_password validation: add format check (uppercase + lowercase + digit) to match User#password strength requirements - Remove password_confirmation from :user factory — attribute no longer exists after has_secure_password was removed - Set DatabaseCleaner.allow_remote_database_url = true: the existing guard at rails_helper.rb:57 already blocks supabase/prod URLs; this allows Docker-network hostnames in local test runs
1 parent 092a118 commit c3f93fe

5 files changed

Lines changed: 180 additions & 3 deletions

File tree

app/modules/players/models/player.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,13 @@ def authenticate_player_password(plain_password)
7979
validates :flex_queue_tier, inclusion: { in: Constants::Player::QUEUE_TIERS }, allow_blank: true
8080
validates :flex_queue_rank, inclusion: { in: Constants::Player::QUEUE_RANKS }, allow_blank: true
8181
validates :player_email, uniqueness: true, allow_blank: true, format: { with: URI::MailTo::EMAIL_REGEXP, allow_blank: true }
82-
validates :player_password, length: { minimum: 8 }, if: -> { player_password.present? }
82+
validates :player_password,
83+
length: { minimum: 8, message: 'must be at least 8 characters' },
84+
format: {
85+
with: /\A(?=.*[a-z])(?=.*[A-Z])(?=.*\d).*\z/,
86+
message: 'must contain at least one uppercase letter, one lowercase letter, and one number'
87+
},
88+
if: -> { player_password.present? }
8389

8490
# Callbacks
8591
before_validation :hash_player_password, if: -> { player_password.present? }

spec/factories/users.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
association :organization
66
email { Faker::Internet.email }
77
password { 'Test123!@#' }
8-
password_confirmation { 'Test123!@#' }
98
full_name { Faker::Name.name }
109
role { 'analyst' }
1110

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# frozen_string_literal: true
2+
3+
require 'rails_helper'
4+
5+
RSpec.describe UpgradeablePassword, type: :model do
6+
let(:password) { 'Test123!@#' }
7+
8+
describe 'User#authenticate (argon2id path)' do
9+
let(:user) { create(:user, password: password) }
10+
11+
it 'returns the user for the correct password' do
12+
expect(user.authenticate(password)).to eq(user)
13+
end
14+
15+
it 'returns nil for a wrong password' do
16+
expect(user.authenticate('WrongPass1')).to be_nil
17+
end
18+
19+
it 'does not modify the digest when already argon2id' do
20+
original = user.password_digest
21+
user.authenticate(password)
22+
expect(user.reload.password_digest).to eq(original)
23+
end
24+
end
25+
26+
describe 'User#authenticate — bcrypt to argon2id lazy migration' do
27+
let(:user) { create(:user, password: password) }
28+
let(:bcrypt_digest) { BCrypt::Password.create(password, cost: BCrypt::Engine::MIN_COST).to_s }
29+
30+
before { user.update_column(:password_digest, bcrypt_digest) }
31+
32+
it 'authenticates successfully against a bcrypt digest' do
33+
expect(user.authenticate(password)).to eq(user)
34+
end
35+
36+
it 'upgrades the digest to argon2id transparently after successful login' do
37+
user.authenticate(password)
38+
expect(user.reload.password_digest).to start_with('$argon2id$')
39+
end
40+
41+
it 'updates updated_at alongside the digest upgrade' do
42+
before = user.updated_at
43+
user.authenticate(password)
44+
expect(user.reload.updated_at).to be >= before
45+
end
46+
47+
it 'does not upgrade the digest when the password is wrong' do
48+
user.authenticate('WrongPass1')
49+
expect(user.reload.password_digest).to eq(bcrypt_digest)
50+
end
51+
end
52+
53+
describe 'Player#authenticate_player_password — bcrypt to argon2id lazy migration' do
54+
let(:player) do
55+
create(:player).tap do |p|
56+
argon2_digest = Authentication::PasswordHasher.hash(password)
57+
p.update_column(:player_password_digest, argon2_digest)
58+
end
59+
end
60+
let(:bcrypt_digest) { BCrypt::Password.create(password, cost: BCrypt::Engine::MIN_COST).to_s }
61+
62+
before { player.update_column(:player_password_digest, bcrypt_digest) }
63+
64+
it 'authenticates successfully against a bcrypt digest' do
65+
expect(player.authenticate_player_password(password)).to eq(player)
66+
end
67+
68+
it 'upgrades the digest to argon2id transparently after successful login' do
69+
player.authenticate_player_password(password)
70+
expect(player.reload.player_password_digest).to start_with('$argon2id$')
71+
end
72+
73+
it 'does not upgrade the digest when the password is wrong' do
74+
player.authenticate_player_password('WrongPass1')
75+
expect(player.reload.player_password_digest).to eq(bcrypt_digest)
76+
end
77+
end
78+
end

spec/rails_helper.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@
5858
abort('CRITICAL: Cannot run tests against production database! Use a local test database.')
5959
end
6060

61-
DatabaseCleaner.allow_remote_database_url = false
61+
# Custom guard above already aborts on 'supabase'/'prod' URLs, so we can
62+
# allow Docker-network hostnames (e.g. docker-postgres-1) here safely.
63+
DatabaseCleaner.allow_remote_database_url = true
6264

6365
config.before(:suite) do
6466
DatabaseCleaner.clean_with(:truncation)
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
# frozen_string_literal: true
2+
3+
require 'rails_helper'
4+
5+
RSpec.describe Authentication::PasswordHasher do
6+
let(:password) { 'Test123!@#' }
7+
8+
describe '.hash' do
9+
it 'returns a string with argon2id prefix' do
10+
expect(described_class.hash(password)).to start_with('$argon2id$')
11+
end
12+
13+
it 'produces a different digest on each call due to random salt' do
14+
expect(described_class.hash(password)).not_to eq(described_class.hash(password))
15+
end
16+
end
17+
18+
describe '.verify' do
19+
context 'with an argon2id digest' do
20+
let(:digest) { described_class.hash(password) }
21+
22+
it 'returns true for the correct password' do
23+
expect(described_class.verify(password, digest)).to be true
24+
end
25+
26+
it 'returns false for a wrong password' do
27+
expect(described_class.verify('WrongPass1', digest)).to be false
28+
end
29+
end
30+
31+
context 'with a bcrypt digest (legacy retrocompatibility)' do
32+
let(:digest) { BCrypt::Password.create(password, cost: BCrypt::Engine::MIN_COST) }
33+
34+
it 'returns true for the correct password' do
35+
expect(described_class.verify(password, digest)).to be true
36+
end
37+
38+
it 'returns false for a wrong password' do
39+
expect(described_class.verify('WrongPass1', digest)).to be false
40+
end
41+
42+
it 'returns false for a malformed bcrypt string' do
43+
expect(described_class.verify(password, '$2a$not_a_valid_hash')).to be false
44+
end
45+
end
46+
47+
context 'with blank inputs' do
48+
let(:digest) { described_class.hash(password) }
49+
50+
it 'returns false when password is blank' do
51+
expect(described_class.verify('', digest)).to be false
52+
end
53+
54+
it 'returns false when digest is blank' do
55+
expect(described_class.verify(password, '')).to be false
56+
end
57+
58+
it 'returns false when both are blank' do
59+
expect(described_class.verify('', '')).to be false
60+
end
61+
end
62+
end
63+
64+
describe '.needs_upgrade?' do
65+
it 'returns true for a bcrypt digest' do
66+
digest = BCrypt::Password.create(password, cost: BCrypt::Engine::MIN_COST)
67+
expect(described_class.needs_upgrade?(digest)).to be true
68+
end
69+
70+
it 'returns false for an argon2id digest' do
71+
expect(described_class.needs_upgrade?(described_class.hash(password))).to be false
72+
end
73+
end
74+
75+
describe '.bcrypt?' do
76+
it 'returns true for $2a$ prefix (standard bcrypt)' do
77+
expect(described_class.bcrypt?('$2a$12$somehashvalue')).to be true
78+
end
79+
80+
it 'returns true for $2b$ prefix (canonical bcrypt)' do
81+
expect(described_class.bcrypt?('$2b$12$somehashvalue')).to be true
82+
end
83+
84+
it 'returns false for an argon2id digest' do
85+
expect(described_class.bcrypt?('$argon2id$v=19$m=65536,t=3,p=2$salt$hash')).to be false
86+
end
87+
88+
it 'returns false for a blank string' do
89+
expect(described_class.bcrypt?('')).to be false
90+
end
91+
end
92+
end

0 commit comments

Comments
 (0)