diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb
index 16d4a81ee9f4532149bfbca2a7ebf3605c366f9a..954d4ba9c98e3e387509289e53a301e4022fdf1c 100644
--- a/app/controllers/invites_controller.rb
+++ b/app/controllers/invites_controller.rb
@@ -63,7 +63,7 @@ def member?
def member
strong_memoize(:member) do
- @token = params[:id]
+ @token = params[:id].to_s
Member.find_by_invite_token(@token)
end
end
diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb
index a541e7e703fee5cb6d1e7eb18ad762b95c51c581..2d408a4635f852200d664d864dc5acf6867f22eb 100644
--- a/app/controllers/oauth/authorizations_controller.rb
+++ b/app/controllers/oauth/authorizations_controller.rb
@@ -82,7 +82,7 @@ def add_read_user_scope!
end
def doorkeeper_application
- strong_memoize(:doorkeeper_application) { ::Doorkeeper::OAuth::Client.find(params['client_id'])&.application }
+ strong_memoize(:doorkeeper_application) { ::Doorkeeper::OAuth::Client.find(params['client_id'].to_s)&.application }
end
def application_has_read_user_scope?
diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb
index 42c65a845c6387cba6762e305f43b2921cb53069..314d1e8db8bab2c486c3653cb53c76aeb54b6a87 100644
--- a/app/controllers/oauth/authorized_applications_controller.rb
+++ b/app/controllers/oauth/authorized_applications_controller.rb
@@ -14,9 +14,9 @@ def index
def destroy
if params[:token_id].present?
- current_resource_owner.oauth_authorized_tokens.find(params[:token_id]).revoke
+ current_resource_owner.oauth_authorized_tokens.find(params[:token_id].to_s).revoke
else
- Doorkeeper::Application.revoke_tokens_and_grants_for(params[:id], current_resource_owner)
+ Doorkeeper::Application.revoke_tokens_and_grants_for(params[:id].to_s, current_resource_owner)
end
redirect_to user_settings_applications_url,
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index c352ea456def17cf186a9b951a9644e0d78c4175..60316e7d76037505f677c78c7c7ec8bfad947165 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -42,11 +42,12 @@ def failure
update_login_counter_metric(failed_strategy.name, 'failed')
log_saml_response if params['SAMLResponse']
- if params[:username].present? && AuthHelper.form_based_provider?(failed_strategy.name)
- user = User.find_by_login(params[:username])
+ username = params[:username].to_s
+ if username.present? && AuthHelper.form_based_provider?(failed_strategy.name)
+ user = User.find_by_login(username)
user&.increment_failed_attempts!
- log_failed_login(params[:username], failed_strategy.name)
+ log_failed_login(username, failed_strategy.name)
end
super
diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index 48ce6479f07643a5a2484280903f6cad4b1e2f0a..d66d0c2b874aee7b252f77db59c76999ba37c459 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -55,8 +55,7 @@ def update
def log_audit_reset_failure(_user); end
def resource_from_email
- email = resource_params[:email]
- self.resource = resource_class.find_by_email(email)
+ self.resource = resource_class.find_by_email(resource_params[:email].to_s)
end
def check_password_authentication_available
diff --git a/app/services/auth/dependency_proxy_authentication_service.rb b/app/services/auth/dependency_proxy_authentication_service.rb
index 29f5a50d809ffb5f53a6c5a55dc0fb8f7714dd0d..84cbbc59cc2141d1d3ed010fb7240b80b43a8a40 100644
--- a/app/services/auth/dependency_proxy_authentication_service.rb
+++ b/app/services/auth/dependency_proxy_authentication_service.rb
@@ -54,7 +54,7 @@ def has_required_abilities?(authentication_abilities)
end
def group_access_token
- PersonalAccessTokensFinder.new(state: 'active').find_by_token(raw_token)
+ PersonalAccessTokensFinder.new(state: 'active').find_by_token(raw_token.to_s)
end
def valid_deploy_token?
diff --git a/ee/app/controllers/concerns/arkose/token_verifiable.rb b/ee/app/controllers/concerns/arkose/token_verifiable.rb
index 3fb3ec7cb668b818a9d6bed32d172787361cf09a..bed6e376b531bccde166b21783dbc3c46079a180 100644
--- a/ee/app/controllers/concerns/arkose/token_verifiable.rb
+++ b/ee/app/controllers/concerns/arkose/token_verifiable.rb
@@ -45,7 +45,7 @@ def log_token_missing
end
def token
- @token ||= params[:arkose_labs_token]
+ @token ||= params[:arkose_labs_token].to_s
end
def arkose_down?
diff --git a/ee/app/controllers/ee/passwords_controller.rb b/ee/app/controllers/ee/passwords_controller.rb
index c6197a879b3ffa23edcd2c5050de230239fb131e..fbf4c5214d98fef5542ea400b74fc8c7164f2839 100644
--- a/ee/app/controllers/ee/passwords_controller.rb
+++ b/ee/app/controllers/ee/passwords_controller.rb
@@ -25,7 +25,7 @@ def log_audit_event
author: current_user || unauth_author,
scope: requester,
target: requester,
- target_details: resource_params[:email],
+ target_details: resource_params[:email].to_s,
message: "Ask for password reset",
ip_address: request.remote_ip
})
diff --git a/ee/app/controllers/ee/sessions_controller.rb b/ee/app/controllers/ee/sessions_controller.rb
index 86ee4eac9a58957680330f3941d72742618f2056..e8b093b95800d6c3dc79f57e893d8597ade2e649 100644
--- a/ee/app/controllers/ee/sessions_controller.rb
+++ b/ee/app/controllers/ee/sessions_controller.rb
@@ -74,7 +74,7 @@ def log_failed_login
end
def complete_identity_verification
- user = ::User.find_by_login(user_params[:login])
+ user = ::User.find_by_login(user_params[:login].to_s)
return if !user || !user.valid_password?(user_params[:password]) || user.access_locked?
return if ::Gitlab::Qa.request?(request.user_agent)
diff --git a/ee/spec/requests/users/registrations_identity_verification_controller_spec.rb b/ee/spec/requests/users/registrations_identity_verification_controller_spec.rb
index 5a3dc9f78eb633d1e2a69b997a7756c6b668fd3c..f54ec42fbe2751a9c713ac069054427a40c2efae 100644
--- a/ee/spec/requests/users/registrations_identity_verification_controller_spec.rb
+++ b/ee/spec/requests/users/registrations_identity_verification_controller_spec.rb
@@ -784,6 +784,17 @@
expect(response.body).to eq({ message: service_response.message, reason: service_response.reason }.to_json)
end
end
+
+ context 'when multiple codes are attempted' do
+ let_it_be(:params) do
+ { registrations_identity_verification: { verification_code: %w[998 999] } }
+ end
+
+ it 'passes no params to VerifyCodeService' do
+ do_request
+ expect(::PhoneVerification::Users::VerifyCodeService).to have_received(:new).with(user, {})
+ end
+ end
end
shared_examples 'it requires a user without an arkose risk_band' do
diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb
index b4ba2a8e50ea98fff521612d68bf599374c3ef5d..319d1121ec8f04a3e82bcd64680f71f20525f46d 100644
--- a/lib/gitlab/auth/auth_finders.rb
+++ b/lib/gitlab/auth/auth_finders.rb
@@ -54,7 +54,7 @@ def find_user_from_static_object_token(request_format)
token = current_request.params[:token].presence || current_request.headers['X-Gitlab-Static-Object-Token'].presence
return unless token
- User.find_by_static_object_token(token) || raise(UnauthorizedError)
+ User.find_by_static_object_token(token.to_s) || raise(UnauthorizedError)
end
def find_user_from_feed_token(request_format)
@@ -83,7 +83,7 @@ def find_user_from_job_token
current_request.env[JOB_TOKEN_HEADER].presence
return unless token
- job = find_valid_running_job_by_token!(token)
+ job = find_valid_running_job_by_token!(token.to_s)
@current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables
job.user
@@ -96,7 +96,7 @@ def find_user_from_basic_auth_job
return unless login.present? && password.present?
return unless ::Gitlab::Auth::CI_JOB_USER == login
- job = find_valid_running_job_by_token!(password)
+ job = find_valid_running_job_by_token!(password.to_s)
@current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables
job.user
@@ -108,16 +108,16 @@ def find_user_from_basic_auth_password
login, password = user_name_and_password(current_request)
return if ::Gitlab::Auth::CI_JOB_USER == login
- Gitlab::Auth.find_with_user_password(login, password)
+ Gitlab::Auth.find_with_user_password(login.to_s, password.to_s)
end
def find_user_from_lfs_token
return unless has_basic_credentials?(current_request)
login, token = user_name_and_password(current_request)
- user = User.find_by_login(login)
+ user = User.find_by_login(login.to_s)
- user if user && Gitlab::LfsToken.new(user).token_valid?(token)
+ user if user && Gitlab::LfsToken.new(user).token_valid?(token.to_s)
end
def find_user_from_personal_access_token
@@ -168,7 +168,7 @@ def deploy_token_from_request
_, token = user_name_and_password(current_request)
end
- deploy_token = DeployToken.active.find_by_token(token)
+ deploy_token = DeployToken.active.find_by_token(token.to_s)
@current_authenticated_deploy_token = deploy_token # rubocop:disable Gitlab/ModuleWithInstanceVariables
deploy_token
@@ -180,7 +180,7 @@ def cluster_agent_token_from_authorization_token
authorization_token, _options = token_and_options(current_request)
- ::Clusters::AgentToken.active.find_by_token(authorization_token)
+ ::Clusters::AgentToken.active.find_by_token(authorization_token.to_s)
end
def find_runner_from_token
@@ -189,7 +189,7 @@ def find_runner_from_token
token = current_request.params[RUNNER_TOKEN_PARAM].presence
return unless token
- ::Ci::Runner.find_by_token(token) || raise(UnauthorizedError)
+ ::Ci::Runner.find_by_token(token.to_s) || raise(UnauthorizedError)
end
def validate_access_token!(scopes: [])
@@ -271,7 +271,7 @@ def find_personal_access_token
return unless token
# Expiration, revocation and scopes are verified in `validate_access_token!`
- PersonalAccessToken.find_by_token(token) || raise(UnauthorizedError)
+ PersonalAccessToken.find_by_token(token.to_s) || raise(UnauthorizedError)
end
def find_oauth_access_token
@@ -294,10 +294,11 @@ def find_personal_access_token_from_http_basic_auth
return unless has_basic_credentials?(current_request)
_username, password = user_name_and_password(current_request)
- PersonalAccessToken.find_by_token(password)
+ PersonalAccessToken.find_by_token(password.to_s)
end
def find_feed_token_user(token)
+ token = token.to_s
find_user_from_path_feed_token(token) || User.find_by_feed_token(token)
end
diff --git a/spec/controllers/oauth/authorized_applications_controller_spec.rb b/spec/controllers/oauth/authorized_applications_controller_spec.rb
index cb047e55752f1f1f307f146d02e3a94329161659..f672552559bddabe96631b6832fbcfd3c9a041de 100644
--- a/spec/controllers/oauth/authorized_applications_controller_spec.rb
+++ b/spec/controllers/oauth/authorized_applications_controller_spec.rb
@@ -24,7 +24,7 @@
let!(:grant) { create(:oauth_access_grant, resource_owner_id: user.id, application: application) }
let!(:access_token) { create(:oauth_access_token, resource_owner: user, application: application) }
- it 'revokes both access grants and tokens' do
+ it 'revokes both access grants and tokens when id is passed' do
expect(grant).not_to be_revoked
expect(access_token).not_to be_revoked
@@ -33,6 +33,18 @@
expect(grant.reload).to be_revoked
expect(access_token.reload).to be_revoked
end
+
+ it 'revokes a specific token when token_id is passed' do
+ expect(grant).not_to be_revoked
+ expect(access_token).not_to be_revoked
+
+ # id is required for this path, but is not used by the
+ # controller
+ delete :destroy, params: { id: 9999999, token_id: access_token.id }
+
+ expect(grant.reload).not_to be_revoked
+ expect(access_token.reload).to be_revoked
+ end
end
it 'includes Two-factor enforcement concern' do
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index a4acdc706996e0972ec6f068dad497fcd3b95b74..9058c5a33d08334fd4a1a384be8dbf7f2d932dce 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -93,6 +93,22 @@
end
end
+ context 'mass assignment' do
+ it 'does not authenticate with multiple usernames' do
+ expect do
+ post(:create, params: { user: { login: ['invalid', user.username], password: user.password } })
+ end.to raise_error(NoMethodError)
+ expect(@request.env['warden']).not_to be_authenticated
+ end
+
+ it 'does not authenticate with multiple passwords' do
+ expect do
+ post(:create, params: { user: { login: user.username, password: ['aaaaaa', user.password] } })
+ end.to raise_error(NoMethodError)
+ expect(@request.env['warden']).not_to be_authenticated
+ end
+ end
+
context 'when user with LDAP identity' do
before do
create(:identity, provider: 'ldapmain', user: user)
@@ -436,6 +452,16 @@ def authenticate_2fa(otp_user_id: user.id, **user_params)
authenticate_2fa(otp_attempt: code)
end
end
+
+ context 'when OTP is an array' do
+ let(:code) { %w[000000 000001] }
+
+ it 'does not authenticate' do
+ authenticate_2fa(otp_attempt: code)
+
+ expect(subject.current_user).not_to eq user
+ end
+ end
end
context 'when the user is on their last attempt' do
@@ -569,6 +595,26 @@ def authenticate_2fa(user_params)
expect(subject.current_user).to eq user
end
+
+ context 'when the verification token is invalid' do
+ let(:user_params) { { verification_token: 'not-the-token' } }
+
+ it 'does not log the user in' do
+ post_action
+
+ expect(subject.current_user).to eq nil
+ end
+ end
+
+ context 'when the verification token is an array' do
+ let(:user_params) { { verification_token: %w[not-the-token still-not-the-token] } }
+
+ it 'does not log the user in' do
+ post_action
+
+ expect(subject.current_user).to eq nil
+ end
+ end
end
end
diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb
index efce440b5281ce9dbb260deb283663e7989dd1c2..26e6cd3661585b9b708d1427f905b874ae0f3618 100644
--- a/spec/lib/gitlab/auth/auth_finders_spec.rb
+++ b/spec/lib/gitlab/auth/auth_finders_spec.rb
@@ -76,6 +76,15 @@ def set_bearer_token(token)
expect(@current_authenticated_job).to be_nil
end
end
+
+ context 'for an array of tokens' do
+ let(:token) { [job.token, 'invalid token'] }
+
+ it "returns an Unauthorized exception" do
+ expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ expect(@current_authenticated_job).to be_nil
+ end
+ end
end
context 'when route is not allowed to be authenticated', :request_store do
@@ -236,6 +245,12 @@ def set_bearer_token(token)
expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
+
+ it 'returns exception if an array is passed' do
+ set_param(:feed_token, [feed_token, 'fake'])
+
+ expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
end
context 'when old format rss_token param is provided' do
@@ -296,12 +311,20 @@ def set_bearer_token(token)
end
context 'when token is incorrect' do
- it 'returns the user' do
+ it 'returns an error' do
request.headers['X-Gitlab-Static-Object-Token'] = 'foobar'
expect { find_user_from_static_object_token(format) }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
end
+
+ context 'when token is an array' do
+ it 'returns an error' do
+ request.headers['X-Gitlab-Static-Object-Token'] = [user.static_object_token, 'foobar']
+
+ expect { find_user_from_static_object_token(format) }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
+ end
end
context 'when token query param is present' do
@@ -314,12 +337,20 @@ def set_bearer_token(token)
end
context 'when token is incorrect' do
- it 'returns the user' do
+ it 'raises an error' do
set_param(:token, 'foobar')
expect { find_user_from_static_object_token(format) }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
end
+
+ context 'when token is an array' do
+ it 'raises an error' do
+ set_param(:token, [user.static_object_token, 'foobar'])
+
+ expect { find_user_from_static_object_token(format) }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
+ end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 25d33bbd7a362026ce4168a9027a9412be51fbd0..51b4d72b13913aac62576a2a4417394893ae5b3f 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -7185,6 +7185,17 @@ def access_levels(groups)
it { is_expected.to eq(false) }
end
+
+ context 'using an array' do
+ let(:user) { create(:user) }
+ let(:password) { [user.password, 'WRONG PASSWORD'] }
+
+ it 'raises an error' do
+ expect do
+ validate_password
+ end.to raise_error(NoMethodError)
+ end
+ end
end
describe '#generate_otp_backup_codes!' do