Commit e5ba7aae authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge branch 'security-cross-reference-fix-ce-12-3' into '12-3-stable'

Filter not accessible label events

See merge request gitlab/gitlabhq!3440
parents 3440d0f6 bc22ef7b
# frozen_string_literal: true
class ResourceLabelEventFinder
include FinderMethods
MAX_PER_PAGE = 100
attr_reader :params, :current_user, :eventable
def initialize(current_user, eventable, params = {})
@current_user = current_user
@eventable = eventable
@params = params
end
def execute
events = eventable.resource_label_events.inc_relations
events = events.page(page).per(per_page)
events = visible_to_user(events)
Kaminari.paginate_array(events)
end
private
def visible_to_user(events)
ResourceLabelEvent.preload_label_subjects(events)
events.select do |event|
Ability.allowed?(current_user, :read_label, event)
end
end
def per_page
[params[:per_page], MAX_PER_PAGE].compact.min
end
def page
params[:page] || 1
end
end
...@@ -13,6 +13,7 @@ class ResourceLabelEvent < ApplicationRecord ...@@ -13,6 +13,7 @@ class ResourceLabelEvent < ApplicationRecord
belongs_to :label belongs_to :label
scope :created_after, ->(time) { where('created_at > ?', time) } scope :created_after, ->(time) { where('created_at > ?', time) }
scope :inc_relations, -> { includes(:label, :user) }
validates :user, presence: { unless: :importing? }, on: :create validates :user, presence: { unless: :importing? }, on: :create
validates :label, presence: { unless: :importing? }, on: :create validates :label, presence: { unless: :importing? }, on: :create
...@@ -30,6 +31,15 @@ class ResourceLabelEvent < ApplicationRecord ...@@ -30,6 +31,15 @@ class ResourceLabelEvent < ApplicationRecord
%i(issue merge_request).freeze %i(issue merge_request).freeze
end end
def self.preload_label_subjects(events)
labels = events.map(&:label).compact
project_labels, group_labels = labels.partition { |label| label.is_a? ProjectLabel }
preloader = ActiveRecord::Associations::Preloader.new
preloader.preload(project_labels, { project: :project_feature })
preloader.preload(group_labels, :group)
end
def issuable def issuable
issue || merge_request issue || merge_request
end end
......
# frozen_string_literal: true
class ResourceLabelEventPolicy < BasePolicy
condition(:can_read_label) { @subject.label_id.nil? || can?(:read_label, @subject.label) }
condition(:can_read_issuable) { can?(:"read_#{@subject.issuable.to_ability_name}", @subject.issuable) }
rule { can_read_label }.policy do
enable :read_label
end
rule { can_read_label & can_read_issuable }.policy do
enable :read_resource_label_event
end
end
---
title: Do not show resource label events referencing not accessible labels.
merge_request:
author:
type: security
...@@ -24,14 +24,14 @@ module API ...@@ -24,14 +24,14 @@ module API
use :pagination use :pagination
end end
# rubocop: disable CodeReuse/ActiveRecord
get ":id/#{eventables_str}/:eventable_id/resource_label_events" do get ":id/#{eventables_str}/:eventable_id/resource_label_events" do
eventable = find_noteable(parent_type, params[:id], eventable_type, params[:eventable_id]) eventable = find_noteable(parent_type, params[:id], eventable_type, params[:eventable_id])
events = eventable.resource_label_events.includes(:label, :user)
opts = { page: params[:page], per_page: params[:per_page] }
events = ResourceLabelEventFinder.new(current_user, eventable, opts).execute
present paginate(events), with: Entities::ResourceLabelEvent present paginate(events), with: Entities::ResourceLabelEvent
end end
# rubocop: enable CodeReuse/ActiveRecord
desc "Get a single #{eventable_type.to_s.downcase} resource label event" do desc "Get a single #{eventable_type.to_s.downcase} resource label event" do
success Entities::ResourceLabelEvent success Entities::ResourceLabelEvent
...@@ -45,6 +45,8 @@ module API ...@@ -45,6 +45,8 @@ module API
eventable = find_noteable(parent_type, params[:id], eventable_type, params[:eventable_id]) eventable = find_noteable(parent_type, params[:id], eventable_type, params[:eventable_id])
event = eventable.resource_label_events.find(params[:event_id]) event = eventable.resource_label_events.find(params[:event_id])
not_found!('ResourceLabelEvent') unless can?(current_user, :read_resource_label_event, event)
present event, with: Entities::ResourceLabelEvent present event, with: Entities::ResourceLabelEvent
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe ResourceLabelEventFinder do
set(:user) { create(:user) }
set(:issue_project) { create(:project) }
set(:issue) { create(:issue, project: issue_project) }
describe '#execute' do
subject { described_class.new(user, issue).execute }
it 'returns events with labels accessible by user' do
label = create(:label, project: issue_project)
event = create_event(label)
issue_project.add_guest(user)
expect(subject).to eq [event]
end
it 'filters events with public project labels if issues and MRs are private' do
project = create(:project, :public, :issues_private, :merge_requests_private)
label = create(:label, project: project)
create_event(label)
expect(subject).to be_empty
end
it 'filters events with project labels not accessible by user' do
project = create(:project, :private)
label = create(:label, project: project)
create_event(label)
expect(subject).to be_empty
end
it 'filters events with group labels not accessible by user' do
group = create(:group, :private)
label = create(:group_label, group: group)
create_event(label)
expect(subject).to be_empty
end
it 'paginates results' do
label = create(:label, project: issue_project)
create_event(label)
create_event(label)
issue_project.add_guest(user)
paginated = described_class.new(user, issue, per_page: 1).execute
expect(subject.count).to eq 2
expect(paginated.count).to eq 1
end
def create_event(label)
create(:resource_label_event, issue: issue, label: label)
end
end
end
require 'spec_helper'
describe ResourceLabelEventPolicy do
set(:user) { create(:user) }
set(:project) { create(:project, :private) }
set(:issue) { create(:issue, project: project) }
set(:private_project) { create(:project, :private) }
describe '#read_resource_label_event' do
context 'with non-member user' do
it 'does not allow to read event' do
event = build_event(project)
expect(permissions(user, event)).to be_disallowed(:read_resource_label_event)
end
end
context 'with member user' do
before do
project.add_guest(user)
end
it 'allows to read event for accessible label' do
event = build_event(project)
expect(permissions(user, event)).to be_allowed(:read_resource_label_event)
end
it 'does not allow to read event for not accessible label' do
event = build_event(private_project)
expect(permissions(user, event)).to be_disallowed(:read_resource_label_event)
end
end
end
describe '#read_label' do
it 'allows to read deleted label' do
event = build(:resource_label_event, issue: issue, label: nil)
expect(permissions(user, event)).to be_allowed(:read_label)
end
it 'allows to read accessible label' do
project.add_guest(user)
event = build_event(project)
expect(permissions(user, event)).to be_allowed(:read_label)
end
it 'does not allow to read not accessible label' do
event = build_event(private_project)
expect(permissions(user, event)).to be_disallowed(:read_label)
end
end
def build_event(label_project)
label = create(:label, project: label_project)
build(:resource_label_event, issue: issue, label: label)
end
def permissions(user, issue)
described_class.new(user, issue)
end
end
...@@ -5,28 +5,23 @@ require 'spec_helper' ...@@ -5,28 +5,23 @@ require 'spec_helper'
describe API::ResourceLabelEvents do describe API::ResourceLabelEvents do
set(:user) { create(:user) } set(:user) { create(:user) }
set(:project) { create(:project, :public, namespace: user.namespace) } set(:project) { create(:project, :public, namespace: user.namespace) }
set(:label) { create(:label, project: project) }
before do before do
project.add_developer(user) project.add_developer(user)
end end
context 'when eventable is an Issue' do context 'when eventable is an Issue' do
let(:issue) { create(:issue, project: project, author: user) }
it_behaves_like 'resource_label_events API', 'projects', 'issues', 'iid' do it_behaves_like 'resource_label_events API', 'projects', 'issues', 'iid' do
let(:parent) { project } let(:parent) { project }
let(:eventable) { issue } let(:eventable) { create(:issue, project: project, author: user) }
let!(:event) { create(:resource_label_event, issue: issue) }
end end
end end
context 'when eventable is a Merge Request' do context 'when eventable is a Merge Request' do
let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) }
it_behaves_like 'resource_label_events API', 'projects', 'merge_requests', 'iid' do it_behaves_like 'resource_label_events API', 'projects', 'merge_requests', 'iid' do
let(:parent) { project } let(:parent) { project }
let(:eventable) { merge_request } let(:eventable) { create(:merge_request, source_project: project, target_project: project, author: user) }
let!(:event) { create(:resource_label_event, merge_request: merge_request) }
end end
end end
end end
...@@ -2,43 +2,98 @@ ...@@ -2,43 +2,98 @@
shared_examples 'resource_label_events API' do |parent_type, eventable_type, id_name| shared_examples 'resource_label_events API' do |parent_type, eventable_type, id_name|
describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events" do describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events" do
it "returns an array of resource label events" do context "with local label reference" do
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", user) let!(:event) { create_event(label) }
expect(response).to have_gitlab_http_status(200) it "returns an array of resource label events" do
expect(response).to include_pagination_headers get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", user)
expect(json_response).to be_an Array
expect(json_response.first['id']).to eq(event.id) expect(response).to have_gitlab_http_status(200)
end expect(response).to include_pagination_headers
expect(json_response).to be_an Array
expect(json_response.first['id']).to eq(event.id)
end
it "returns a 404 error when eventable id not found" do
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/12345/resource_label_events", user)
expect(response).to have_gitlab_http_status(404)
end
it "returns 404 when not authorized" do
parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
private_user = create(:user)
it "returns a 404 error when eventable id not found" do get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", private_user)
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/12345/resource_label_events", user)
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end
end end
it "returns 404 when not authorized" do context "with cross-project label reference" do
parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) let(:private_project) { create(:project, :private) }
private_user = create(:user) let(:project_label) { create(:label, project: private_project) }
let!(:event) { create_event(project_label) }
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", private_user) it "returns cross references accessible by user" do
private_project.add_guest(user)
expect(response).to have_gitlab_http_status(404) get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", user)
expect(json_response).to be_an Array
expect(json_response.first['id']).to eq(event.id)
end
it "does not return cross references not accessible by user" do
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", user)
expect(json_response).to be_an Array
expect(json_response).to eq []
end
end end
end end
describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events/:event_id" do describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events/:event_id" do
it "returns a resource label event by id" do context "with local label reference" do
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/#{event.id}", user) let!(:event) { create_event(label) }
it "returns a resource label event by id" do
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/#{event.id}", user)
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response['id']).to eq(event.id) expect(json_response['id']).to eq(event.id)
end
it "returns 404 when not authorized" do
parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
private_user = create(:user)
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/#{event.id}", private_user)
expect(response).to have_gitlab_http_status(404)
end
it "returns a 404 error if resource label event not found" do
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/12345", user)
expect(response).to have_gitlab_http_status(404)
end
end end
it "returns a 404 error if resource label event not found" do context "with cross-project label reference" do
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/12345", user) let(:private_project) { create(:project, :private) }
let(:project_label) { create(:label, project: private_project) }
let!(:event) { create_event(project_label) }
it "returns a 404 error if cross-reference project is not accessible" do
get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/#{event.id}", user)
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end
end end
end end
def create_event(label)
create(:resource_label_event, eventable.class.name.underscore => eventable, label: label)
end
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