Skip to content

Commit d7f4780

Browse files
committed
feat(auth): migrate password hashing from bcrypt to Argon2id
Replace has_secure_password with a custom PasswordHasher service backed by Argon2id (m=64MiB, t=3, p=2), following the OWASP preferred profile. Lazy migration: existing bcrypt digests are verified as-is and silently re-hashed on the next successful login. No schema changes, no forced logouts, clean rollback. - Add gem argon2 ~> 2.3 - Add Authentication::PasswordHasher with bcrypt/argon2id detection, rescue BCrypt::Errors::InvalidHash and Argon2::Error, test-env fast params (m=16, t=1, p=1) - Add UpgradeablePassword concern used by User and Player - Remove has_secure_password from User and Player; replicate presence validation and virtual attr explicitly - Hash via before_validation callback, not in the setter - Add scripts/benchmark_argon2.rb for pre-deploy calibration
1 parent fffd37e commit d7f4780

7 files changed

Lines changed: 141 additions & 5 deletions

File tree

Gemfile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ gem 'redis', '~> 5.0'
3030
# Use Active Model has_secure_password [https://guides.rubyonrails.org/active_model_basics.html#securepassword]
3131
gem 'bcrypt', '~> 3.1.7'
3232

33+
# Argon2id password hashing (OWASP recommended, PHC winner 2015)
34+
gem 'argon2', '~> 2.3'
35+
3336
# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
3437
gem 'tzinfo-data', platforms: %i[mingw mswin x64_mingw jruby]
3538

Gemfile.lock

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ GEM
7979
annotate (3.2.0)
8080
activerecord (>= 3.2, < 8.0)
8181
rake (>= 10.4, < 14.0)
82+
argon2 (2.3.3)
83+
ffi (~> 1.15)
84+
ffi-compiler (~> 1.0)
8285
ast (2.4.3)
8386
aws-eventstream (1.4.0)
8487
aws-partitions (1.1229.0)
@@ -171,6 +174,10 @@ GEM
171174
net-http (~> 0.5)
172175
faraday-retry (2.3.2)
173176
faraday (~> 2.0)
177+
ffi (1.17.4-x86_64-linux-gnu)
178+
ffi-compiler (1.4.2)
179+
ffi (>= 1.15.5)
180+
rake
174181
fugit (1.12.1)
175182
et-orbi (~> 1.4)
176183
raabro (~> 1.4)
@@ -471,6 +478,7 @@ PLATFORMS
471478

472479
DEPENDENCIES
473480
annotate
481+
argon2 (~> 2.3)
474482
aws-sdk-s3 (~> 1.0)
475483
bcrypt (~> 3.1.7)
476484
blueprinter

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
```
4848
┌─────────────────────────────────────────────────────────────────────────────┐
4949
│ [■] JWT Authentication — Refresh tokens + token blacklisting │
50+
│ [■] Argon2id Password Hashing— OWASP preferred · lazy migration from bcrypt│
5051
│ [■] HashID URLs — Base62 encoding for obfuscated URLs │
5152
│ [■] Swagger Docs — 200+ endpoints documented interactively │
5253
│ [■] Riot Games API — Automatic match and player import │
@@ -182,7 +183,7 @@ open http://localhost:3333/api-docs
182183
║ Language ║ Ruby 3.4.8 ║
183184
║ Framework ║ Rails 7.2.3.1 (API-only mode) ║
184185
║ Database ║ PostgreSQL 14+ ║
185-
║ Authentication ║ JWT (access + refresh tokens)
186+
║ Authentication ║ JWT (access + refresh tokens) + Argon2id hashing
186187
║ URL Obfuscation ║ HashID with Base62 encoding ║
187188
║ Background Jobs ║ Sidekiq ║
188189
║ Caching ║ Redis (port 6380) ║
@@ -1037,6 +1038,7 @@ All cached responses include `X-Cache-Hit: true/false` header.
10371038
[✓] Timing oracle: login/register user enumeration
10381039
[✓] Mass assignment: StrongParameters coverage
10391040
[✓] CI/CD: security gates on every push + weekly CodeQL
1041+
[✓] Password hashing: Argon2id (m=64MiB, t=3, p=2) — bcrypt lazy migration on login
10401042
```
10411043

10421044
### Security Status
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# frozen_string_literal: true
2+
3+
module UpgradeablePassword
4+
extend ActiveSupport::Concern
5+
6+
# Verifies plain_password against the stored digest and, if the digest still
7+
# uses bcrypt, transparently re-hashes with Argon2id on the same request.
8+
#
9+
# @param plain_password [String] the password to verify
10+
# @param digest_attr [Symbol] the attribute name holding the stored digest
11+
# @param digest_setter [Symbol] the column name to write the upgraded digest
12+
# @return [self, nil] returns self on success, nil on failure
13+
def authenticate_with_upgrade(plain_password, digest_attr:, digest_setter:)
14+
digest = send(digest_attr)
15+
return nil unless Authentication::PasswordHasher.verify(plain_password, digest)
16+
17+
if Authentication::PasswordHasher.needs_upgrade?(digest)
18+
new_digest = Authentication::PasswordHasher.hash(plain_password)
19+
# Two separate update_column calls instead of update_columns so that Rails
20+
# dirty tracking is cleared field-by-field — avoids unexpected behavior on
21+
# read-replica setups where a bulk UPDATE could interleave with pending reads.
22+
# update_column bypasses callbacks intentionally (no before_save/after_save
23+
# during a transparent hash upgrade).
24+
update_column(digest_setter, new_digest)
25+
update_column(:updated_at, Time.current)
26+
end
27+
28+
self
29+
end
30+
end

app/models/user.rb

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@
22

33
# Authenticated user within an organization, with role-based access and notification support.
44
class User < ApplicationRecord
5-
has_secure_password
6-
75
# Concerns
86
include Constants
7+
include UpgradeablePassword
98

109
# Associations
1110
belongs_to :organization
@@ -21,7 +20,24 @@ class User < ApplicationRecord
2120
has_many :password_reset_tokens, dependent: :destroy
2221
has_many :messages, dependent: :nullify
2322

23+
# Virtual password attribute — set when changing password, nil otherwise.
24+
# has_secure_password is not used; hashing is handled by Authentication::PasswordHasher.
25+
attr_reader :password
26+
27+
def password=(plain_password)
28+
@password = plain_password.blank? ? nil : plain_password
29+
end
30+
31+
def authenticate(plain_password)
32+
authenticate_with_upgrade(
33+
plain_password,
34+
digest_attr: :password_digest,
35+
digest_setter: :password_digest
36+
)
37+
end
38+
2439
# Validations
40+
validates :password_digest, presence: true
2541
validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP }
2642
validates :full_name, presence: true, length: { maximum: 255 }
2743
validates :role, presence: true, inclusion: { in: Constants::User::ROLES }
@@ -42,6 +58,7 @@ class User < ApplicationRecord
4258

4359
# Callbacks
4460
before_validation :downcase_email
61+
before_validation :hash_password, if: -> { password.present? }
4562
after_update :log_audit_trail, if: :saved_changes?
4663

4764
# Scopes
@@ -92,6 +109,10 @@ def downcase_email
92109
self.email = email.downcase.strip if email.present?
93110
end
94111

112+
def hash_password
113+
self.password_digest = Authentication::PasswordHasher.hash(password)
114+
end
115+
95116
def log_audit_trail
96117
AuditLog.create!(
97118
organization: organization,

app/modules/players/models/player.rb

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class Player < ApplicationRecord # rubocop:disable Metrics/ClassLength
3535
include OrganizationScoped
3636
include SoftDeletable
3737
include Searchable
38+
include UpgradeablePassword
3839

3940
# Associations
4041
belongs_to :organization, optional: true
@@ -46,8 +47,21 @@ class Player < ApplicationRecord # rubocop:disable Metrics/ClassLength
4647
has_many :vod_timestamps, foreign_key: 'target_player_id', dependent: :nullify
4748
has_many :password_reset_tokens, dependent: :destroy
4849

49-
# Password authentication for individual player access
50-
has_secure_password :player_password, validations: false
50+
# Virtual attribute for the player password — has_secure_password is not used;
51+
# hashing is handled by Authentication::PasswordHasher.
52+
attr_reader :player_password
53+
54+
def player_password=(plain_password)
55+
@player_password = plain_password.blank? ? nil : plain_password
56+
end
57+
58+
def authenticate_player_password(plain_password)
59+
authenticate_with_upgrade(
60+
plain_password,
61+
digest_attr: :player_password_digest,
62+
digest_setter: :player_password_digest
63+
)
64+
end
5165

5266
# Validations
5367
validates :source_app, inclusion: { in: Constants::SOURCE_APPS }
@@ -68,6 +82,7 @@ class Player < ApplicationRecord # rubocop:disable Metrics/ClassLength
6882
validates :player_password, length: { minimum: 8 }, if: -> { player_password.present? }
6983

7084
# Callbacks
85+
before_validation :hash_player_password, if: -> { player_password.present? }
7186
before_save :normalize_summoner_name
7287
after_update_commit :enqueue_audit_log, if: :saved_changes?
7388
after_commit :clear_organization_cache, on: %i[create destroy]
@@ -222,6 +237,10 @@ def normalize_summoner_name
222237
self.summoner_name = summoner_name.strip if summoner_name.present?
223238
end
224239

240+
def hash_player_password
241+
self.player_password_digest = Authentication::PasswordHasher.hash(player_password)
242+
end
243+
225244
def enqueue_audit_log
226245
AuditLogJob.perform_later(
227246
organization_id: organization_id,
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# frozen_string_literal: true
2+
3+
module Authentication
4+
# Handles password hashing and verification with support for lazy migration
5+
# from bcrypt to Argon2id. The hash format is self-describing, so no extra
6+
# column or flag is needed to detect which algorithm was used.
7+
class PasswordHasher
8+
# Ultra-fast params in test to avoid adding 150-250ms per RSpec example
9+
# that touches authentication. Production values follow OWASP preferred profile.
10+
ARGON2_PARAMS = if Rails.env.test?
11+
{ m_cost: 16, t_cost: 1, p_cost: 1 }.freeze
12+
else
13+
{ m_cost: 65_536, t_cost: 3, p_cost: 2 }.freeze
14+
end
15+
16+
# Covers $2a$ (standard), $2b$ (canonical), $2x$/$2y$ (legacy JRuby/PHP variants)
17+
BCRYPT_PREFIX = /\A\$2[abxy]\$/
18+
19+
def self.hash(plain_password)
20+
Argon2::Password.create(plain_password, **ARGON2_PARAMS)
21+
end
22+
23+
def self.verify(plain_password, digest)
24+
return false if plain_password.blank? || digest.blank?
25+
26+
bcrypt?(digest) ? verify_bcrypt(plain_password, digest) : verify_argon2(plain_password, digest)
27+
end
28+
29+
def self.needs_upgrade?(digest)
30+
bcrypt?(digest)
31+
end
32+
33+
def self.bcrypt?(digest)
34+
digest.to_s.match?(BCRYPT_PREFIX)
35+
end
36+
37+
def self.verify_bcrypt(plain_password, digest)
38+
result = BCrypt::Password.new(digest) == plain_password
39+
Rails.logger.info('[PasswordHasher] bcrypt digest detected — upgrade queued') if result
40+
result
41+
rescue BCrypt::Errors::InvalidHash
42+
false
43+
end
44+
private_class_method :verify_bcrypt
45+
46+
def self.verify_argon2(plain_password, digest)
47+
Argon2::Password.verify_password(plain_password, digest)
48+
rescue Argon2::Error
49+
false
50+
end
51+
private_class_method :verify_argon2
52+
end
53+
end

0 commit comments

Comments
 (0)