SECURITY: Add more restrictions on invite emails

They could be filtered and returned in some circumstances where they
shouldn't have been.
This commit is contained in:
Robin Ward 2020-03-04 11:47:09 -05:00
parent 79ce7085c2
commit e01d5e2adc
4 changed files with 51 additions and 32 deletions

View File

@ -286,8 +286,13 @@ class UsersController < ApplicationController
Invite.find_redeemed_invites_from(inviter, offset) Invite.find_redeemed_invites_from(inviter, offset)
end end
invites = invites.filter_by(params[:search]) show_emails = guardian.can_see_invite_emails?(inviter)
render_json_dump invites: serialize_data(invites.to_a, InviteSerializer), if params[:search].present?
filter_sql = '(LOWER(users.username) LIKE :filter)'
filter_sql = '(LOWER(invites.email) LIKE :filter) or (LOWER(users.username) LIKE :filter)' if show_emails
invites = invites.where(filter_sql, filter: "%#{params[:search].downcase}%")
end
render_json_dump invites: serialize_data(invites.to_a, InviteSerializer, show_emails: show_emails),
can_see_invite_details: guardian.can_see_invite_details?(inviter) can_see_invite_details: guardian.can_see_invite_details?(inviter)
end end

View File

@ -5,7 +5,7 @@ class InviteSerializer < ApplicationSerializer
attributes :email, :updated_at, :redeemed_at, :expired, :user attributes :email, :updated_at, :redeemed_at, :expired, :user
def include_email? def include_email?
!object.redeemed? options[:show_emails] && !object.redeemed?
end end
def expired def expired

View File

@ -331,6 +331,10 @@ class Guardian
is_me?(user) is_me?(user)
end end
def can_see_invite_emails?(user)
is_staff? || is_me?(user)
end
def can_invite_to_forum?(groups = nil) def can_invite_to_forum?(groups = nil)
authenticated? && authenticated? &&
(SiteSetting.max_invites_per_day.to_i > 0 || is_staff?) && (SiteSetting.max_invites_per_day.to_i > 0 || is_staff?) &&

View File

@ -1466,23 +1466,13 @@ describe UsersController do
expect(response.status).to eq(200) expect(response.status).to eq(200)
end end
it 'filters by email' do it 'filters by all if viewing self' do
inviter = Fabricate(:user, trust_level: 2) inviter = Fabricate(:user, trust_level: 2)
sign_in(inviter) sign_in(inviter)
invitee = Fabricate(:user) invitee = Fabricate(:user)
Fabricate( Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter, user: invitee)
:invite, Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter, user: invitee)
email: 'billybob@example.com',
invited_by: inviter,
user: invitee
)
Fabricate(
:invite,
email: 'jimtom@example.com',
invited_by: inviter,
user: invitee
)
get "/u/#{inviter.username}/invited.json", params: { search: 'billybob' } get "/u/#{inviter.username}/invited.json", params: { search: 'billybob' }
expect(response.status).to eq(200) expect(response.status).to eq(200)
@ -1490,31 +1480,51 @@ describe UsersController do
invites = JSON.parse(response.body)['invites'] invites = JSON.parse(response.body)['invites']
expect(invites.size).to eq(1) expect(invites.size).to eq(1)
expect(invites.first).to include('email' => 'billybob@example.com') expect(invites.first).to include('email' => 'billybob@example.com')
get "/u/#{inviter.username}/invited.json", params: { search: invitee.username }
expect(response.status).to eq(200)
invites = JSON.parse(response.body)['invites']
expect(invites.size).to eq(2)
expect(invites[0]['email']).to be_present
end end
it 'filters by username' do it "doesn't filter by email if another regular user" do
inviter = Fabricate(:user, trust_level: 2) inviter = Fabricate(:user, trust_level: 2)
sign_in(inviter) sign_in(Fabricate(:user, trust_level: 2))
invitee = Fabricate(:user, username: 'billybob') invitee = Fabricate(:user)
_invite = Fabricate( Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter, user: invitee)
:invite, Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter, user: invitee)
invited_by: inviter,
email: 'billybob@example.com', get "/u/#{inviter.username}/invited.json", params: { search: 'billybob' }
user: invitee expect(response.status).to eq(200)
)
Fabricate( invites = JSON.parse(response.body)['invites']
:invite, expect(invites.size).to eq(0)
invited_by: inviter,
user: Fabricate(:user, username: 'jimtom') get "/u/#{inviter.username}/invited.json", params: { search: invitee.username }
) expect(response.status).to eq(200)
invites = JSON.parse(response.body)['invites']
expect(invites.size).to eq(2)
expect(invites[0]['email']).to be_blank
end
it "filters by email if staff" do
inviter = Fabricate(:user, trust_level: 2)
sign_in(Fabricate(:moderator))
invitee = Fabricate(:user)
Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter, user: invitee)
Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter, user: invitee)
get "/u/#{inviter.username}/invited.json", params: { search: 'billybob' } get "/u/#{inviter.username}/invited.json", params: { search: 'billybob' }
expect(response.status).to eq(200) expect(response.status).to eq(200)
invites = JSON.parse(response.body)['invites'] invites = JSON.parse(response.body)['invites']
expect(invites.size).to eq(1) expect(invites.size).to eq(1)
expect(invites.first).to include('email' => 'billybob@example.com') expect(invites[0]['email']).to be_present
end end
context 'with guest' do context 'with guest' do
@ -1581,7 +1591,7 @@ describe UsersController do
context 'with redeemed invites' do context 'with redeemed invites' do
it 'returns invites' do it 'returns invites' do
sign_in(Fabricate(:user, trust_level: 2)) sign_in(Fabricate(:moderator))
inviter = Fabricate(:user) inviter = Fabricate(:user)
invitee = Fabricate(:user) invitee = Fabricate(:user)
invite = Fabricate(:invite, invited_by: inviter, user: invitee) invite = Fabricate(:invite, invited_by: inviter, user: invitee)