Commit e9da17c4 authored by Bogdan Denkovych's avatar Bogdan Denkovych

Add `User#can_log_in_with_non_expired_password?` method

In https://gitlab.com/gitlab-org/security/gitlab/-/commit/e521e9736b3d2d444d239636f5af780d4df98bcc#a08075e47107d59496393338d2b8a4aef0014955_388_387
and https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/2042/diffs#note_755152255
we added `can?(user, :log_in) && !user.password_expired_if_applicable?`
condition to some part of application to prevent users to authenticate if they
have expired passwords. To prevent code duplication and make the code more
readable this commit suggests extracting this condition into
`User#can_log_in_with_non_expired_password?` method.
parent d3bece3a
...@@ -20,7 +20,7 @@ module SessionlessAuthentication ...@@ -20,7 +20,7 @@ module SessionlessAuthentication
end end
def sessionless_sign_in(user) def sessionless_sign_in(user)
if can?(user, :log_in) && !user.password_expired_if_applicable? if user.can_log_in_with_non_expired_password?
# Notice we are passing store false, so the user is not # Notice we are passing store false, so the user is not
# actually stored in the session and a token is needed # actually stored in the session and a token is needed
# for every request. If you want the token to work as a # for every request. If you want the token to work as a
......
...@@ -1901,6 +1901,10 @@ class User < ApplicationRecord ...@@ -1901,6 +1901,10 @@ class User < ApplicationRecord
true true
end end
def can_log_in_with_non_expired_password?
can?(:log_in) && !password_expired_if_applicable?
end
def can_be_deactivated? def can_be_deactivated?
active? && no_recent_activity? && !internal? active? && no_recent_activity? && !internal?
end end
......
...@@ -84,7 +84,7 @@ module Gitlab ...@@ -84,7 +84,7 @@ module Gitlab
Gitlab::Auth::UniqueIpsLimiter.limit_user! do Gitlab::Auth::UniqueIpsLimiter.limit_user! do
user = User.by_login(login) user = User.by_login(login)
break if user && !can_user_login_with_non_expired_password?(user) break if user && !user.can_log_in_with_non_expired_password?
authenticators = [] authenticators = []
...@@ -187,7 +187,7 @@ module Gitlab ...@@ -187,7 +187,7 @@ module Gitlab
if valid_oauth_token?(token) if valid_oauth_token?(token)
user = User.id_in(token.resource_owner_id).first user = User.id_in(token.resource_owner_id).first
return unless user && can_user_login_with_non_expired_password?(user) return unless user && user.can_log_in_with_non_expired_password?
Gitlab::Auth::Result.new(user, nil, :oauth, abilities_for_scopes(token.scopes)) Gitlab::Auth::Result.new(user, nil, :oauth, abilities_for_scopes(token.scopes))
end end
...@@ -210,7 +210,7 @@ module Gitlab ...@@ -210,7 +210,7 @@ module Gitlab
return unless token_bot_in_project?(token.user, project) || token_bot_in_group?(token.user, project) return unless token_bot_in_project?(token.user, project) || token_bot_in_group?(token.user, project)
end end
if can_user_login_with_non_expired_password?(token.user) || token.user.project_bot? if token.user.can_log_in_with_non_expired_password? || token.user.project_bot?
Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes)) Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes))
end end
end end
...@@ -309,7 +309,7 @@ module Gitlab ...@@ -309,7 +309,7 @@ module Gitlab
return unless build.project.builds_enabled? return unless build.project.builds_enabled?
if build.user if build.user
return unless can_user_login_with_non_expired_password?(build.user) || (build.user.project_bot? && build.project.bots&.include?(build.user)) return unless build.user.can_log_in_with_non_expired_password? || (build.user.project_bot? && build.project.bots&.include?(build.user))
# If user is assigned to build, use restricted credentials of user # If user is assigned to build, use restricted credentials of user
Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities) Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities)
...@@ -406,10 +406,6 @@ module Gitlab ...@@ -406,10 +406,6 @@ module Gitlab
user.increment_failed_attempts! user.increment_failed_attempts!
end end
def can_user_login_with_non_expired_password?(user)
user.can?(:log_in) && !user.password_expired_if_applicable?
end
end end
end end
end end
...@@ -5784,6 +5784,48 @@ RSpec.describe User do ...@@ -5784,6 +5784,48 @@ RSpec.describe User do
end end
end end
describe '#can_log_in_with_non_expired_password?' do
let(:user) { build(:user) }
subject { user.can_log_in_with_non_expired_password? }
context 'when user can log in' do
it 'returns true' do
is_expected.to be_truthy
end
context 'when user with expired password' do
before do
user.password_expires_at = 2.minutes.ago
end
it 'returns false' do
is_expected.to be_falsey
end
context 'when password expiration is not applicable' do
context 'when ldap user' do
let(:user) { build(:omniauth_user, provider: 'ldap') }
it 'returns true' do
is_expected.to be_truthy
end
end
end
end
end
context 'when user cannot log in' do
context 'when user is blocked' do
let(:user) { build(:user, :blocked) }
it 'returns false' do
is_expected.to be_falsey
end
end
end
end
describe '#read_only_attribute?' do describe '#read_only_attribute?' do
context 'when synced attributes metadata is present' do context 'when synced attributes metadata is present' do
it 'delegates to synced_attributes_metadata' do it 'delegates to synced_attributes_metadata' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment