Commit a3cfd7f7 authored by Douwe Maan's avatar Douwe Maan Committed by Winnie Hellmann
Browse files

Merge branch 'bvl-10-0-email-discosure' into 'security-10-0'

(10.0) Avoid partial partial email adresses for matching

See merge request gitlab/gitlabhq!2233

(cherry picked from commit d6c734fcfd72e9ae0c58011c7cb0b16201be1cfa)

0e21b315 Don't allow searching for partial user emails
parent 716962be
......@@ -311,7 +311,8 @@ def filter(filter_name)
#
# Returns an ActiveRecord::Relation.
def search(query)
table = arel_table
table = arel_table
query = query.downcase
pattern = User.to_pattern(query)
order = <<~SQL
......@@ -325,7 +326,7 @@ def search(query)
where(
table[:name].matches(pattern)
.or(table[:email].matches(pattern))
.or(table[:email].eq(query))
.or(table[:username].matches(pattern))
).reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name)
end
......@@ -337,12 +338,13 @@ def search(query)
def search_with_secondary_emails(query)
table = arel_table
email_table = Email.arel_table
pattern = "%#{query}%"
matched_by_emails_user_ids = email_table.project(email_table[:user_id]).where(email_table[:email].matches(pattern))
query = query.downcase
pattern = User.to_pattern(query)
matched_by_emails_user_ids = email_table.project(email_table[:user_id]).where(email_table[:email].eq(query))
where(
table[:name].matches(pattern)
.or(table[:email].matches(pattern))
.or(table[:email].eq(query))
.or(table[:username].matches(pattern))
.or(table[:id].in(matched_by_emails_user_ids))
)
......
---
title: Don't match partial email adresses
merge_request: 2227
author:
type: security
......@@ -38,6 +38,27 @@
end
end
scenario 'do not disclose email addresses', :js do
group.add_owner(user1)
create(:user, email: 'undisclosed_email@gitlab.com', name: "Jane 'invisible' Doe")
visit group_group_members_path(group)
find('.select2-container').click
select_input = find('.select2-input')
select_input.send_keys('@gitlab.com')
wait_for_requests
expect(page).to have_content('No matches found')
select_input.native.clear
select_input.send_keys('undisclosed_email@gitlab.com')
wait_for_requests
expect(page).to have_content("Jane 'invisible' Doe")
end
scenario 'remove user from group', :js do
group.add_owner(user1)
group.add_developer(user2)
......
......@@ -817,11 +817,11 @@
describe 'email matching' do
it 'returns users with a matching Email' do
expect(described_class.search(user.email)).to eq([user, user2])
expect(described_class.search(user.email)).to eq([user])
end
it 'returns users with a partially matching Email' do
expect(described_class.search(user.email[0..2])).to eq([user, user2])
it 'does not return users with a partially matching Email' do
expect(described_class.search(user.email[0..2])).not_to include(user, user2)
end
it 'returns users with a matching Email regardless of the casing' do
......@@ -877,8 +877,8 @@
expect(search_with_secondary_emails(user.email)).to eq([user])
end
it 'returns users with a partially matching email' do
expect(search_with_secondary_emails(user.email[0..2])).to eq([user])
it 'does not return users with a partially matching email' do
expect(search_with_secondary_emails(user.email[0..2])).not_to include([user])
end
it 'returns users with a matching email regardless of the casing' do
......@@ -901,29 +901,8 @@
expect(search_with_secondary_emails(email.email)).to eq([email.user])
end
it 'returns users with a matching part of secondary email' do
expect(search_with_secondary_emails(email.email[1..4])).to eq([email.user])
end
it 'return users with a matching part of secondary email regardless of case' do
expect(search_with_secondary_emails(email.email[1..4].upcase)).to eq([email.user])
expect(search_with_secondary_emails(email.email[1..4].downcase)).to eq([email.user])
expect(search_with_secondary_emails(email.email[1..4].capitalize)).to eq([email.user])
end
it 'returns multiple users with matching secondary emails' do
email1 = create(:email, email: '1_testemail@example.com')
email2 = create(:email, email: '2_testemail@example.com')
email3 = create(:email, email: 'other@email.com')
email3.user.update_attributes!(email: 'another@mail.com')
expect(
search_with_secondary_emails('testemail@example.com').map(&:id)
).to include(email1.user.id, email2.user.id)
expect(
search_with_secondary_emails('testemail@example.com').map(&:id)
).not_to include(email3.user.id)
it 'does not return users with a matching part of secondary email' do
expect(search_with_secondary_emails(email.email[1..4])).not_to include([email.user])
end
end
......
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