diff --git a/Gemfile b/Gemfile index dbac1c0531fb399f1bb7ac9ffdd1f3d29c8c3495..1acf2cb92456429173d8405265bdba25e26e6dca 100644 --- a/Gemfile +++ b/Gemfile @@ -125,7 +125,7 @@ gem 'akismet', '~> 3.0', feature_category: :insider_threat gem 'invisible_captcha', '~> 2.3.0', feature_category: :insider_threat # Two-factor authentication -gem 'devise-two-factor', '~> 4.1.1', feature_category: :system_access +gem 'devise-two-factor', '~> 5.1.0', feature_category: :system_access gem 'rqrcode', '~> 2.2', feature_category: :system_access gem 'webauthn', '~> 3.0', feature_category: :system_access diff --git a/Gemfile.checksum b/Gemfile.checksum index 2ea5a4bfd2f168ab226d3fcc10c9e1f2bbe0a210..ace9648c070f7fc199e6b3196ac1ad8d2bdba2b7 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -121,7 +121,7 @@ {"name":"devfile","version":"0.5.0","platform":"x86_64-linux","checksum":"44022ab3e37d13dd24daec4aa7471236b67e96d9d385966e6e7928535a518e3b"}, {"name":"device_detector","version":"1.1.3","platform":"ruby","checksum":"c5fe3fe42cab2e8aa01f193b2074b8bb1510373ce47127206f28c7dea75a9c79"}, {"name":"devise","version":"4.9.4","platform":"ruby","checksum":"920042fe5e704c548aa4eb65ebdd65980b83ffae67feb32c697206bfd975a7f8"}, -{"name":"devise-two-factor","version":"4.1.1","platform":"ruby","checksum":"c95f5b07533e62217aaed3c386874d94e2d472fb5f2b6598afe8600fc17a8b95"}, +{"name":"devise-two-factor","version":"5.1.0","platform":"ruby","checksum":"eae7a78d562e7ff623932d6c0b7f1bdbd4809c63e916875da3db7abaadf41ae1"}, {"name":"diff-lcs","version":"1.5.0","platform":"ruby","checksum":"49b934001c8c6aedb37ba19daec5c634da27b318a7a3c654ae979d6ba1929b67"}, {"name":"diffy","version":"3.4.4","platform":"ruby","checksum":"79384ab5ca82d0e115b2771f0961e27c164c456074bd2ec46b637ebf7b6e47e3"}, {"name":"digest-crc","version":"0.6.5","platform":"ruby","checksum":"5ca456f3352dc5ff17eb95deb3dd5a79dc79f8bf751d8005abca5b7b9b252124"}, diff --git a/Gemfile.lock b/Gemfile.lock index d808c4d21015934364953dd5f6e1b1191f3b39a5..a1bde64cf378d6bae0b0d5114295aad03426e34a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -146,7 +146,7 @@ PATH specs: devise-pbkdf2-encryptable (0.0.0) devise (~> 4.0) - devise-two-factor (~> 4.1.1) + devise-two-factor (~> 5.1.0) PATH remote: vendor/gems/diff_match_patch @@ -530,9 +530,8 @@ GEM railties (>= 4.1.0) responders warden (~> 1.2.3) - devise-two-factor (4.1.1) + devise-two-factor (5.1.0) activesupport (~> 7.0) - attr_encrypted (>= 1.3, < 5, != 2) devise (~> 4.0) railties (~> 7.0) rotp (~> 6.0) @@ -2142,7 +2141,7 @@ DEPENDENCIES device_detector devise (~> 4.9.3) devise-pbkdf2-encryptable (~> 0.0.0)! - devise-two-factor (~> 4.1.1) + devise-two-factor (~> 5.1.0) diff_match_patch (~> 0.1.0)! diffy (~> 3.4) doorkeeper (~> 5.8, >= 5.8.1) diff --git a/Gemfile.next.checksum b/Gemfile.next.checksum index 2ea5a4bfd2f168ab226d3fcc10c9e1f2bbe0a210..ace9648c070f7fc199e6b3196ac1ad8d2bdba2b7 100644 --- a/Gemfile.next.checksum +++ b/Gemfile.next.checksum @@ -121,7 +121,7 @@ {"name":"devfile","version":"0.5.0","platform":"x86_64-linux","checksum":"44022ab3e37d13dd24daec4aa7471236b67e96d9d385966e6e7928535a518e3b"}, {"name":"device_detector","version":"1.1.3","platform":"ruby","checksum":"c5fe3fe42cab2e8aa01f193b2074b8bb1510373ce47127206f28c7dea75a9c79"}, {"name":"devise","version":"4.9.4","platform":"ruby","checksum":"920042fe5e704c548aa4eb65ebdd65980b83ffae67feb32c697206bfd975a7f8"}, -{"name":"devise-two-factor","version":"4.1.1","platform":"ruby","checksum":"c95f5b07533e62217aaed3c386874d94e2d472fb5f2b6598afe8600fc17a8b95"}, +{"name":"devise-two-factor","version":"5.1.0","platform":"ruby","checksum":"eae7a78d562e7ff623932d6c0b7f1bdbd4809c63e916875da3db7abaadf41ae1"}, {"name":"diff-lcs","version":"1.5.0","platform":"ruby","checksum":"49b934001c8c6aedb37ba19daec5c634da27b318a7a3c654ae979d6ba1929b67"}, {"name":"diffy","version":"3.4.4","platform":"ruby","checksum":"79384ab5ca82d0e115b2771f0961e27c164c456074bd2ec46b637ebf7b6e47e3"}, {"name":"digest-crc","version":"0.6.5","platform":"ruby","checksum":"5ca456f3352dc5ff17eb95deb3dd5a79dc79f8bf751d8005abca5b7b9b252124"}, diff --git a/Gemfile.next.lock b/Gemfile.next.lock index d808c4d21015934364953dd5f6e1b1191f3b39a5..a1bde64cf378d6bae0b0d5114295aad03426e34a 100644 --- a/Gemfile.next.lock +++ b/Gemfile.next.lock @@ -146,7 +146,7 @@ PATH specs: devise-pbkdf2-encryptable (0.0.0) devise (~> 4.0) - devise-two-factor (~> 4.1.1) + devise-two-factor (~> 5.1.0) PATH remote: vendor/gems/diff_match_patch @@ -530,9 +530,8 @@ GEM railties (>= 4.1.0) responders warden (~> 1.2.3) - devise-two-factor (4.1.1) + devise-two-factor (5.1.0) activesupport (~> 7.0) - attr_encrypted (>= 1.3, < 5, != 2) devise (~> 4.0) railties (~> 7.0) rotp (~> 6.0) @@ -2142,7 +2141,7 @@ DEPENDENCIES device_detector devise (~> 4.9.3) devise-pbkdf2-encryptable (~> 0.0.0)! - devise-two-factor (~> 4.1.1) + devise-two-factor (~> 5.1.0) diff_match_patch (~> 0.1.0)! diffy (~> 3.4) doorkeeper (~> 5.8, >= 5.8.1) diff --git a/app/models/user.rb b/app/models/user.rb index 4c3b412b32e58e92b93e87791e3d5eaee2bc1716..e413cd19d13f8a3e9d5fd187daf5b1ceaaafbe6d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1350,6 +1350,7 @@ def disable_two_factor_otp! otp_grace_period_started_at: nil, otp_secret_expires_at: nil ) + update_column(:otp_secret, nil) end def disable_second_factor_webauthn! @@ -3119,6 +3120,27 @@ def organization_membership_exists?(organization, scope = nil) query = query.try(scope) if scope query.exists? end + + ## + # Decrypt and return the `encrypted_otp_secret` attribute which was used in + # prior versions of devise-two-factor + # @return [String] The decrypted OTP secret + def legacy_otp_secret + return unless self[:encrypted_otp_secret] + return unless self.class.otp_secret_encryption_key + + hmac_iterations = 2000 # a default set by the Encryptor gem + key = self.class.otp_secret_encryption_key + salt = Base64.decode64(encrypted_otp_secret_salt) + iv = Base64.decode64(encrypted_otp_secret_iv) + cipher_text = Base64.decode64(encrypted_otp_secret) + cipher = OpenSSL::Cipher.new('aes-256-cbc') + + cipher.decrypt + cipher.key = OpenSSL::PKCS5.pbkdf2_hmac_sha1(key, salt, hmac_iterations, cipher.key_len) + cipher.iv = iv + cipher.update(cipher_text) + cipher.final + end end User.prepend_mod_with('User') diff --git a/config/initializers/active_record_encryption.rb b/config/initializers/active_record_encryption.rb index 09cffed5be009f3d81d956c6f8b7b4c155909d4f..64d57915532744dcc3a50a2f083b88e2fa8b670f 100644 --- a/config/initializers/active_record_encryption.rb +++ b/config/initializers/active_record_encryption.rb @@ -3,7 +3,7 @@ # Normally, this would automatically be setup by `ActiveRecord::Encryption` initializer, see # https://github.com/rails/rails/blob/v7.0.8.4/activerecord/lib/active_record/railtie.rb#L331-L335, # but since we're setting `Rails.application.credentials.active_record_encryption` manually in -# `config/initializers/01_secret_token.rb`, the `ActiveRecord::Encryption` initializer runs prior +# `config/initializers/2_secret_token.rb`, the `ActiveRecord::Encryption` initializer runs prior # to that. We don't want to mess up with the initializer chain, so we configure # `ActiveRecord::Encryption` here instead. ActiveRecord::Encryption.configure( diff --git a/db/migrate/20251121113103_add_otp_secret_to_user.rb b/db/migrate/20251121113103_add_otp_secret_to_user.rb new file mode 100644 index 0000000000000000000000000000000000000000..788b51f223d711524a479c78582e64895ee7d7f4 --- /dev/null +++ b/db/migrate/20251121113103_add_otp_secret_to_user.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddOtpSecretToUser < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.8' + + def up + with_lock_retries do + add_column :users, :otp_secret, :text, if_not_exists: true # rubocop:disable Migration/PreventAddingColumns -- this column is used in devise-two-factor v5 + end + + add_text_limit :users, :otp_secret, 255 + end + + def down + with_lock_retries do + remove_column :users, :otp_secret, if_exists: true + end + end +end diff --git a/db/schema_migrations/20251121113103 b/db/schema_migrations/20251121113103 new file mode 100644 index 0000000000000000000000000000000000000000..93a09bdaf7bb22c49e74d870679d82b081cf684c --- /dev/null +++ b/db/schema_migrations/20251121113103 @@ -0,0 +1 @@ +ebd4add884748e42ebbb80a563d35142084ee55519630160581f1f01002b3c12 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3ce3ab37724117cca3e096bdc9aa82311a507be5..2464b26b0641aefc4ffe20014051cd2d3b641d94 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -577,12 +577,14 @@ CREATE TABLE users ( color_mode_id smallint DEFAULT 1 NOT NULL, composite_identity_enforced boolean DEFAULT false NOT NULL, organization_id bigint NOT NULL, + otp_secret text, CONSTRAINT check_061f6f1c91 CHECK ((project_view IS NOT NULL)), CONSTRAINT check_0dd5948e38 CHECK ((user_type IS NOT NULL)), CONSTRAINT check_3a60c18afc CHECK ((hide_no_password IS NOT NULL)), CONSTRAINT check_693c6f3aab CHECK ((hide_no_ssh_key IS NOT NULL)), CONSTRAINT check_7bde697e8e CHECK ((char_length(static_object_token_encrypted) <= 255)), - CONSTRAINT check_c737c04b87 CHECK ((notified_of_own_activity IS NOT NULL)) + CONSTRAINT check_c737c04b87 CHECK ((notified_of_own_activity IS NOT NULL)), + CONSTRAINT check_d0b84b7b3a CHECK ((char_length(otp_secret) <= 255)) ); CREATE FUNCTION find_users_by_id(users_id bigint) RETURNS users diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 773cd61e46093216d391bfff60ce4e8be58e1cd5..7b9bcc4249d79938c0319fa138ef26853e196f37 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3311,6 +3311,7 @@ expect(user).to be_two_factor_enabled expect(user.encrypted_otp_secret).not_to be_nil + expect(user.otp_secret).not_to be_nil expect(user.otp_backup_codes).not_to be_nil expect(user.otp_grace_period_started_at).not_to be_nil expect(user.email_otp_required_after).to be_nil @@ -3321,6 +3322,7 @@ expect(user.encrypted_otp_secret).to be_nil expect(user.encrypted_otp_secret_iv).to be_nil expect(user.encrypted_otp_secret_salt).to be_nil + expect(user.read_attribute(:otp_secret)).to be_nil expect(user.otp_backup_codes).to be_nil expect(user.otp_grace_period_started_at).to be_nil expect(user.otp_secret_expires_at).to be_nil @@ -10212,4 +10214,39 @@ def owner_class_attribute_default; end it { is_expected.to be_allow_user_to_create_group_and_project } end end + + describe '#legacy_otp_secret' do + let_it_be(:user) { create(:user) } + + subject(:legacy_otp_secret) { user.send(:legacy_otp_secret) } + + context 'when encrypted_otp_secret is nil' do + it { is_expected.to be_nil } + end + + context 'when otp_secret_encryption_key is nil' do + before do + user.update!(encrypted_otp_secret: 'dummy') + allow(described_class).to receive(:otp_secret_encryption_key).and_return(nil) + end + + it { is_expected.to be_nil } + end + + context 'when user has two factor auth enabled' do + let_it_be(:user) { create(:user, :two_factor) } + let(:otp_secret) { 'my-otp-secret' } + + before do + encrypted_secret = user.attr_encrypted_encrypt(:otp_secret, otp_secret) + user.update!(encrypted_otp_secret: encrypted_secret) + end + + it { is_expected.to eq(otp_secret) } + + it 'resolves the legacy otp secret' do + expect(user.reload.otp_secret).to eq(otp_secret) + end + end + end end diff --git a/vendor/gems/devise-pbkdf2-encryptable/Gemfile.lock b/vendor/gems/devise-pbkdf2-encryptable/Gemfile.lock index e522a8ce5bc83bbbe775436bc095eea836c5753d..57c88bfaf9ca9e8a85b5b91aa5915d3d9c6cae5e 100644 --- a/vendor/gems/devise-pbkdf2-encryptable/Gemfile.lock +++ b/vendor/gems/devise-pbkdf2-encryptable/Gemfile.lock @@ -3,7 +3,7 @@ PATH specs: devise-pbkdf2-encryptable (0.0.0) devise (~> 4.0) - devise-two-factor (~> 4.1.1) + devise-two-factor (~> 5.1.0) GEM remote: https://rubygems.org/ @@ -35,8 +35,6 @@ GEM minitest (>= 5.1) mutex_m tzinfo (~> 2.0) - attr_encrypted (4.0.0) - encryptor (~> 3.0.0) base64 (0.1.1) bcrypt (3.1.19) bigdecimal (3.1.4) @@ -50,16 +48,14 @@ GEM railties (>= 4.1.0) responders warden (~> 1.2.3) - devise-two-factor (4.1.1) + devise-two-factor (5.1.0) activesupport (~> 7.0) - attr_encrypted (>= 1.3, < 5, != 2) devise (~> 4.0) railties (~> 7.0) rotp (~> 6.0) diff-lcs (1.5.0) drb (2.1.1) ruby2_keywords - encryptor (3.0.0) erubi (1.12.0) i18n (1.14.1) concurrent-ruby (~> 1.0) @@ -148,7 +144,6 @@ CHECKSUMS actionview (7.1.1) sha256=36c737f2ab6f1b90633964ee2928a93d7d2911e83a885a881e564da1860b8509 activemodel (7.1.1) sha256=1f2fd3c2136b111fe523cf80ee5c141990290e8aedacb866b6fd68c8dfd7a69a activesupport (7.1.1) sha256=8770bca4af1cbd6e9ffb944b41056321499ff82e8e7c2ed34e48eff4a5ee58a2 - attr_encrypted (4.0.0) sha256=ccbe4abd2b6973929f119af3b50a3fa10dba51bd69a7b4076a57a59d57359d0c base64 (0.1.1) sha256=0c75d351a429b5176a476cd8a3740cff3277d2bac26a50b5c7456c266e9acd33 bcrypt (3.1.19) sha256=e94fe5cac1c9da5ddced396fb565789f9cdcc477ecbfdf539e3a7acdaa669991 bigdecimal (3.1.4) sha256=de0c967bb24afe45e0e3d2d65e376614a430c3bc70563ac21cb3518f7409c61f @@ -158,10 +153,9 @@ CHECKSUMS crass (1.0.6) sha256=dc516022a56e7b3b156099abc81b6d2b08ea1ed12676ac7a5657617f012bd45d devise (4.9.3) sha256=480638d6c51b97f56da6e28d4f3e2a1b8e606681b316aa594b87c6ab94923488 devise-pbkdf2-encryptable (0.0.0) - devise-two-factor (4.1.1) sha256=c95f5b07533e62217aaed3c386874d94e2d472fb5f2b6598afe8600fc17a8b95 + devise-two-factor (5.1.0) sha256=eae7a78d562e7ff623932d6c0b7f1bdbd4809c63e916875da3db7abaadf41ae1 diff-lcs (1.5.0) sha256=49b934001c8c6aedb37ba19daec5c634da27b318a7a3c654ae979d6ba1929b67 drb (2.1.1) sha256=6b8f481d9a9a7528c41d4f66484a4a73d2204f095da8ab141b5ea0aa22162c41 - encryptor (3.0.0) sha256=abf23f94ab4d864b8cea85b43f3432044a60001982cda7c33c1cd90da8db1969 erubi (1.12.0) sha256=27bedb74dfb1e04ff60674975e182d8ca787f2224f2e8143268c7696f42e4723 i18n (1.14.1) sha256=9d03698903547c060928e70a9bc8b6b87fda674453cda918fc7ab80235ae4a61 io-console (0.6.0) sha256=ce9d9a3455be48c56cac65b8ef7ec59b07526343905f9fda138791005b563336 diff --git a/vendor/gems/devise-pbkdf2-encryptable/devise-pbkdf2-encryptable.gemspec b/vendor/gems/devise-pbkdf2-encryptable/devise-pbkdf2-encryptable.gemspec index cd2c62b457d065796ed79b8ea7bd8dbce84b3edd..7573356e7c79194dc603e5ce76c5f7cf664ba871 100644 --- a/vendor/gems/devise-pbkdf2-encryptable/devise-pbkdf2-encryptable.gemspec +++ b/vendor/gems/devise-pbkdf2-encryptable/devise-pbkdf2-encryptable.gemspec @@ -19,7 +19,7 @@ Gem::Specification.new do |spec| spec.version = '0.0.0' spec.add_runtime_dependency 'devise', '~> 4.0' - spec.add_runtime_dependency 'devise-two-factor', '~> 4.1.1' + spec.add_runtime_dependency 'devise-two-factor', '~> 5.1.0' spec.add_development_dependency 'activemodel', '~> 7.0', '< 8' spec.add_development_dependency 'rspec', '~> 3.10.0'