From 8410e6f8c1d2a9f8276152480eadf8edcd9ac48a Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 4 Mar 2020 11:47:09 -0500 Subject: [PATCH] SECURITY: Add more restrictions on invite emails They could be filtered and returned in some circumstances where they shouldn't have been. --- app/controllers/users_controller.rb | 9 +++- app/serializers/invite_serializer.rb | 2 +- lib/guardian.rb | 4 ++ spec/requests/users_controller_spec.rb | 68 +++++++++++++++----------- 4 files changed, 51 insertions(+), 32 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 3da6474c22..e96a79c953 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -286,8 +286,13 @@ class UsersController < ApplicationController Invite.find_redeemed_invites_from(inviter, offset) end - invites = invites.filter_by(params[:search]) - render_json_dump invites: serialize_data(invites.to_a, InviteSerializer), + show_emails = guardian.can_see_invite_emails?(inviter) + 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) end diff --git a/app/serializers/invite_serializer.rb b/app/serializers/invite_serializer.rb index befa52fc21..9c03c813fb 100644 --- a/app/serializers/invite_serializer.rb +++ b/app/serializers/invite_serializer.rb @@ -5,7 +5,7 @@ class InviteSerializer < ApplicationSerializer attributes :email, :updated_at, :redeemed_at, :expired, :user def include_email? - !object.redeemed? + options[:show_emails] && !object.redeemed? end def expired diff --git a/lib/guardian.rb b/lib/guardian.rb index 080faf78a9..fae144f2b6 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -331,6 +331,10 @@ class Guardian is_me?(user) end + def can_see_invite_emails?(user) + is_staff? || is_me?(user) + end + def can_invite_to_forum?(groups = nil) authenticated? && (SiteSetting.max_invites_per_day.to_i > 0 || is_staff?) && diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 18727eeca6..ed28cab650 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1466,23 +1466,13 @@ describe UsersController do expect(response.status).to eq(200) end - it 'filters by email' do + it 'filters by all if viewing self' do inviter = Fabricate(:user, trust_level: 2) sign_in(inviter) 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 - ) + 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' } expect(response.status).to eq(200) @@ -1490,31 +1480,51 @@ describe UsersController do invites = JSON.parse(response.body)['invites'] expect(invites.size).to eq(1) 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 - it 'filters by username' do + it "doesn't filter by email if another regular user" do inviter = Fabricate(:user, trust_level: 2) - sign_in(inviter) + sign_in(Fabricate(:user, trust_level: 2)) - invitee = Fabricate(:user, username: 'billybob') - _invite = Fabricate( - :invite, - invited_by: inviter, - email: 'billybob@example.com', - user: invitee - ) - Fabricate( - :invite, - invited_by: inviter, - user: Fabricate(:user, username: 'jimtom') - ) + 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' } + expect(response.status).to eq(200) + + invites = JSON.parse(response.body)['invites'] + expect(invites.size).to eq(0) + + 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' } expect(response.status).to eq(200) invites = JSON.parse(response.body)['invites'] expect(invites.size).to eq(1) - expect(invites.first).to include('email' => 'billybob@example.com') + expect(invites[0]['email']).to be_present end context 'with guest' do @@ -1581,7 +1591,7 @@ describe UsersController do context 'with redeemed invites' do it 'returns invites' do - sign_in(Fabricate(:user, trust_level: 2)) + sign_in(Fabricate(:moderator)) inviter = Fabricate(:user) invitee = Fabricate(:user) invite = Fabricate(:invite, invited_by: inviter, user: invitee)