Merge branch 'bvl-10-2-email-disclosure' into 'security-10-2'
(10.2) Avoid partial partial email adresses for matching See merge request gitlab/gitlabhq!2232 (cherry picked from commit 081aa1e91a777c9acb31be4a1e76b3dd7032fa9a) There are unresolved conflicts in app/models/user.rb. fa85a3fd Don't allow searching for partial user emails
This commit is contained in:
parent
d332c8c78a
commit
8c0aa7d4a7
4 changed files with 60 additions and 28 deletions
|
@ -315,6 +315,13 @@ class User < ActiveRecord::Base
|
||||||
#
|
#
|
||||||
# Returns an ActiveRecord::Relation.
|
# Returns an ActiveRecord::Relation.
|
||||||
def search(query)
|
def search(query)
|
||||||
|
<<<<<<< HEAD
|
||||||
|
=======
|
||||||
|
table = arel_table
|
||||||
|
query = query.downcase
|
||||||
|
pattern = User.to_pattern(query)
|
||||||
|
|
||||||
|
>>>>>>> f45fc58d84... Merge branch 'bvl-10-2-email-disclosure' into 'security-10-2'
|
||||||
order = <<~SQL
|
order = <<~SQL
|
||||||
CASE
|
CASE
|
||||||
WHEN users.name = %{query} THEN 0
|
WHEN users.name = %{query} THEN 0
|
||||||
|
@ -324,8 +331,16 @@ class User < ActiveRecord::Base
|
||||||
END
|
END
|
||||||
SQL
|
SQL
|
||||||
|
|
||||||
|
<<<<<<< HEAD
|
||||||
fuzzy_search(query, [:name, :email, :username])
|
fuzzy_search(query, [:name, :email, :username])
|
||||||
.reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name)
|
.reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name)
|
||||||
|
=======
|
||||||
|
where(
|
||||||
|
table[:name].matches(pattern)
|
||||||
|
.or(table[:email].eq(query))
|
||||||
|
.or(table[:username].matches(pattern))
|
||||||
|
).reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name)
|
||||||
|
>>>>>>> f45fc58d84... Merge branch 'bvl-10-2-email-disclosure' into 'security-10-2'
|
||||||
end
|
end
|
||||||
|
|
||||||
# searches user by given pattern
|
# searches user by given pattern
|
||||||
|
@ -334,6 +349,7 @@ class User < ActiveRecord::Base
|
||||||
|
|
||||||
def search_with_secondary_emails(query)
|
def search_with_secondary_emails(query)
|
||||||
email_table = Email.arel_table
|
email_table = Email.arel_table
|
||||||
|
<<<<<<< HEAD
|
||||||
matched_by_emails_user_ids = email_table
|
matched_by_emails_user_ids = email_table
|
||||||
.project(email_table[:user_id])
|
.project(email_table[:user_id])
|
||||||
.where(Email.fuzzy_arel_match(:email, query))
|
.where(Email.fuzzy_arel_match(:email, query))
|
||||||
|
@ -343,6 +359,17 @@ class User < ActiveRecord::Base
|
||||||
.or(fuzzy_arel_match(:email, query))
|
.or(fuzzy_arel_match(:email, query))
|
||||||
.or(fuzzy_arel_match(:username, query))
|
.or(fuzzy_arel_match(:username, query))
|
||||||
.or(arel_table[:id].in(matched_by_emails_user_ids))
|
.or(arel_table[:id].in(matched_by_emails_user_ids))
|
||||||
|
=======
|
||||||
|
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].eq(query))
|
||||||
|
.or(table[:username].matches(pattern))
|
||||||
|
.or(table[:id].in(matched_by_emails_user_ids))
|
||||||
|
>>>>>>> f45fc58d84... Merge branch 'bvl-10-2-email-disclosure' into 'security-10-2'
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
5
changelogs/unreleased/bvl-email-disclosure.yml
Normal file
5
changelogs/unreleased/bvl-email-disclosure.yml
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Don't match partial email adresses
|
||||||
|
merge_request: 2227
|
||||||
|
author:
|
||||||
|
type: security
|
|
@ -38,6 +38,27 @@ feature 'Groups > Members > Manage members' do
|
||||||
end
|
end
|
||||||
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
|
scenario 'remove user from group', :js do
|
||||||
group.add_owner(user1)
|
group.add_owner(user1)
|
||||||
group.add_developer(user2)
|
group.add_developer(user2)
|
||||||
|
|
|
@ -913,11 +913,11 @@ describe User do
|
||||||
|
|
||||||
describe 'email matching' do
|
describe 'email matching' do
|
||||||
it 'returns users with a matching Email' 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
|
end
|
||||||
|
|
||||||
it 'returns users with a partially matching Email' do
|
it 'does not return users with a partially matching Email' do
|
||||||
expect(described_class.search(user.email[0..2])).to eq([user, user2])
|
expect(described_class.search(user.email[0..2])).not_to include(user, user2)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns users with a matching Email regardless of the casing' do
|
it 'returns users with a matching Email regardless of the casing' do
|
||||||
|
@ -973,8 +973,8 @@ describe User do
|
||||||
expect(search_with_secondary_emails(user.email)).to eq([user])
|
expect(search_with_secondary_emails(user.email)).to eq([user])
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns users with a partially matching email' do
|
it 'does not return users with a partially matching email' do
|
||||||
expect(search_with_secondary_emails(user.email[0..2])).to eq([user])
|
expect(search_with_secondary_emails(user.email[0..2])).not_to include([user])
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns users with a matching email regardless of the casing' do
|
it 'returns users with a matching email regardless of the casing' do
|
||||||
|
@ -997,29 +997,8 @@ describe User do
|
||||||
expect(search_with_secondary_emails(email.email)).to eq([email.user])
|
expect(search_with_secondary_emails(email.email)).to eq([email.user])
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns users with a matching part of secondary email' do
|
it 'does not return users with a matching part of secondary email' do
|
||||||
expect(search_with_secondary_emails(email.email[1..4])).to eq([email.user])
|
expect(search_with_secondary_emails(email.email[1..4])).not_to include([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)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue