Commit 19933710 authored by Patrick Bajao's avatar Patrick Bajao

Stop sending emails to users who can't read commit

This is to ensure that only users will be able receive
an email if they can read a commit from the repository
even if they are watching the activity of it.
parent dc884879
......@@ -123,15 +123,19 @@ class NotificationRecipient
return @read_ability if instance_variable_defined?(:@read_ability)
@read_ability =
case @target
when Issuable
:"read_#{@target.to_ability_name}"
when Ci::Pipeline
if @target.is_a?(Ci::Pipeline)
:read_build # We have build trace in pipeline emails
when ActiveRecord::Base
:"read_#{@target.class.model_name.name.underscore}"
else
nil
elsif default_ability_for_target
:"read_#{default_ability_for_target}"
end
end
def default_ability_for_target
@default_ability_for_target ||=
if @target.respond_to?(:to_ability_name)
@target.to_ability_name
elsif @target.class.respond_to?(:model_name)
@target.class.model_name.name.underscore
end
end
......
---
title: Stop sending emails to users who can't read commit
merge_request:
author:
type: security
......@@ -9,11 +9,43 @@ describe NotificationRecipient do
subject(:recipient) { described_class.new(user, :watch, target: target, project: project) }
it 'denies access to a target when cross project access is denied' do
allow(Ability).to receive(:allowed?).and_call_original
expect(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(false)
describe '#has_access?' do
before do
allow(user).to receive(:can?).and_call_original
end
context 'user cannot read cross project' do
it 'returns false' do
expect(user).to receive(:can?).with(:read_cross_project).and_return(false)
expect(recipient.has_access?).to eq false
end
end
context 'user cannot read build' do
let(:target) { build(:ci_pipeline) }
it 'returns false' do
expect(user).to receive(:can?).with(:read_build, target).and_return(false)
expect(recipient.has_access?).to eq false
end
end
context 'user cannot read commit' do
let(:target) { build(:commit) }
it 'returns false' do
expect(user).to receive(:can?).with(:read_commit, target).and_return(false)
expect(recipient.has_access?).to eq false
end
end
context 'target has no policy' do
let(:target) { double.as_null_object }
expect(recipient.has_access?).to be_falsy
it 'returns true' do
expect(recipient.has_access?).to eq true
end
end
end
context '#notification_setting' do
......
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