diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-03-31 00:09:06 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-03-31 00:09:06 +0000 |
commit | ae6b4f857f51765dac310e8075c2c3f88e51dcab (patch) | |
tree | 7e350d6d94d6b9cae89b3cf4c79e9a8b09880842 | |
parent | ae92150461ad4cffcf85a4dc6313bc403e596391 (diff) | |
download | gitlab-ce-ae6b4f857f51765dac310e8075c2c3f88e51dcab.tar.gz |
Add latest changes from gitlab-org/security/gitlab@14-9-stable-ee
-rw-r--r-- | app/models/user.rb | 17 | ||||
-rw-r--r-- | spec/factories/users.rb | 4 | ||||
-rw-r--r-- | spec/features/users/login_spec.rb | 21 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 30 | ||||
-rw-r--r-- | spec/support/helpers/login_helpers.rb | 5 |
5 files changed, 75 insertions, 2 deletions
diff --git a/app/models/user.rb b/app/models/user.rb index b3bdc2c1c42..bc02f0ba55e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -879,6 +879,23 @@ class User < ApplicationRecord reset_password_sent_at.present? && reset_password_sent_at >= 1.minute.ago end + # See https://gitlab.com/gitlab-org/security/gitlab/-/issues/638 + DISALLOWED_PASSWORDS = %w[123qweQWE!@#000000000].freeze + + # Overwrites valid_password? from Devise::Models::DatabaseAuthenticatable + # In constant-time, check both that the password isn't on a denylist AND + # that the password is the user's password + def valid_password?(password) + password_allowed = true + DISALLOWED_PASSWORDS.each do |disallowed_password| + password_allowed = false if Devise.secure_compare(password, disallowed_password) + end + + original_result = super + + password_allowed && original_result + end + def remember_me! super if ::Gitlab::Database.read_write? end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 88ebe7ca606..70b0af8a36c 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -27,6 +27,10 @@ FactoryBot.define do after(:build) { |user, _| user.block! } end + trait :disallowed_password do + password { User::DISALLOWED_PASSWORDS.first } + end + trait :blocked_pending_approval do after(:build) { |user, _| user.block_pending_approval! } end diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 13d7078322e..8610cae58a4 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -150,6 +150,27 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do end end + describe 'with a disallowed password' do + let(:user) { create(:user, :disallowed_password) } + + before do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + end + + it 'disallows login' do + gitlab_sign_in(user, password: user.password) + + expect(page).to have_content('Invalid login or password.') + end + + it 'does not update Devise trackable attributes' do + expect { gitlab_sign_in(user, password: user.password) } + .not_to change { User.ghost.reload.sign_in_count } + end + end + describe 'with the ghost user' do it 'disallows login' do expect(authentication_metrics) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d4e82b5798f..6ee38048025 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5713,6 +5713,36 @@ RSpec.describe User do end end + describe '#valid_password?' do + subject { user.valid_password?(password) } + + context 'user with password not in disallowed list' do + let(:user) { create(:user) } + let(:password) { user.password } + + it { is_expected.to be_truthy } + + context 'using a wrong password' do + let(:password) { 'WRONG PASSWORD' } + + it { is_expected.to be_falsey } + end + end + + context 'user with disallowed password' do + let(:user) { create(:user, :disallowed_password) } + let(:password) { user.password } + + it { is_expected.to be_falsey } + + context 'using a wrong password' do + let(:password) { 'WRONG PASSWORD' } + + it { is_expected.to be_falsey } + end + end + end + describe '#password_expired?' do let(:user) { build(:user, password_expires_at: password_expires_at) } diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index 386988f6d52..7666d71f13c 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -91,11 +91,12 @@ module LoginHelpers # user - User instance to login with # remember - Whether or not to check "Remember me" (default: false) # two_factor_auth - If two-factor authentication is enabled (default: false) - def gitlab_sign_in_with(user, remember: false, two_factor_auth: false) + # password - password to attempt to login with + def gitlab_sign_in_with(user, remember: false, two_factor_auth: false, password: nil) visit new_user_session_path fill_in "user_login", with: user.email - fill_in "user_password", with: "12345678" + fill_in "user_password", with: (password || "12345678") check 'user_remember_me' if remember click_button "Sign in" |