Commit b0088b52 authored by Robert Speicher's avatar Robert Speicher Committed by Rémy Coutable

Merge branch '23403-fix-events-for-private-project-features' into 'security'

Respect project visibility settings in the contributions calendar

This MR fixes a number of bugs relating to access controls and date selection of events for the contributions calendar

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23403

See merge request !2019
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent b0bf9214
...@@ -104,8 +104,7 @@ class UsersController < ApplicationController ...@@ -104,8 +104,7 @@ class UsersController < ApplicationController
end end
def contributions_calendar def contributions_calendar
@contributions_calendar ||= Gitlab::ContributionsCalendar. @contributions_calendar ||= Gitlab::ContributionsCalendar.new(user, current_user)
new(contributed_projects, user)
end end
def load_events def load_events
......
...@@ -49,6 +49,7 @@ class Event < ActiveRecord::Base ...@@ -49,6 +49,7 @@ class Event < ActiveRecord::Base
update_all(updated_at: Time.now) update_all(updated_at: Time.now)
end end
# Update Gitlab::ContributionsCalendar#activity_dates if this changes
def contributions def contributions
where("action = ? OR (target_type in (?) AND action in (?))", where("action = ? OR (target_type in (?) AND action in (?))",
Event::PUSHED, ["MergeRequest", "Issue"], Event::PUSHED, ["MergeRequest", "Issue"],
...@@ -62,7 +63,7 @@ class Event < ActiveRecord::Base ...@@ -62,7 +63,7 @@ class Event < ActiveRecord::Base
def visible_to_user?(user = nil) def visible_to_user?(user = nil)
if push? if push?
true Ability.allowed?(user, :download_code, project)
elsif membership_changed? elsif membership_changed?
true true
elsif created_project? elsif created_project?
......
module Gitlab module Gitlab
class ContributionsCalendar class ContributionsCalendar
attr_reader :activity_dates, :projects, :user attr_reader :contributor
attr_reader :current_user
attr_reader :projects
def initialize(projects, user) def initialize(contributor, current_user = nil)
@projects = projects @contributor = contributor
@user = user @current_user = current_user
@projects = ContributedProjectsFinder.new(contributor).execute(current_user)
end end
def activity_dates def activity_dates
return @activity_dates if @activity_dates.present? return @activity_dates if @activity_dates.present?
@activity_dates = {} # Can't use Event.contributions here because we need to check 3 different
# project_features for the (currently) 3 different contribution types
date_from = 1.year.ago date_from = 1.year.ago
repo_events = event_counts(date_from, :repository).
having(action: Event::PUSHED)
issue_events = event_counts(date_from, :issues).
having(action: [Event::CREATED, Event::CLOSED], target_type: "Issue")
mr_events = event_counts(date_from, :merge_requests).
having(action: [Event::MERGED, Event::CREATED, Event::CLOSED], target_type: "MergeRequest")
events = Event.reorder(nil).contributions.where(author_id: user.id). union = Gitlab::SQL::Union.new([repo_events, issue_events, mr_events])
where("created_at > ?", date_from).where(project_id: projects). events = Event.find_by_sql(union.to_sql).map(&:attributes)
group('date(created_at)').
select('date(created_at) as date, count(id) as total_amount').
map(&:attributes)
activity_dates = (1.year.ago.to_date..Date.today).to_a @activity_events = events.each_with_object(Hash.new {|h, k| h[k] = 0 }) do |event, activities|
activities[event["date"]] += event["total_amount"]
activity_dates.each do |date|
day_events = events.find { |day_events| day_events["date"] == date }
if day_events
@activity_dates[date] = day_events["total_amount"]
end
end end
@activity_dates
end end
def events_by_date(date) def events_by_date(date)
events = Event.contributions.where(author_id: user.id). events = Event.contributions.where(author_id: contributor.id).
where("created_at > ? AND created_at < ?", date.beginning_of_day, date.end_of_day). where(created_at: date.beginning_of_day..date.end_of_day).
where(project_id: projects) where(project_id: projects)
events.select do |event| # Use visible_to_user? instead of the complicated logic in activity_dates
event.push? || event.issue? || event.merge_request? # because we're only viewing the events for a single day.
end events.select {|event| event.visible_to_user?(current_user) }
end end
def starting_year def starting_year
...@@ -49,5 +48,30 @@ module Gitlab ...@@ -49,5 +48,30 @@ module Gitlab
def starting_month def starting_month
Date.today.month Date.today.month
end end
private
def event_counts(date_from, feature)
t = Event.arel_table
# re-running the contributed projects query in each union is expensive, so
# use IN(project_ids...) instead. It's the intersection of two users so
# the list will be (relatively) short
@contributed_project_ids ||= projects.uniq.pluck(:id)
authed_projects = Project.where(id: @contributed_project_ids).
with_feature_available_for_user(feature, current_user).
reorder(nil).
select(:id)
conditions = t[:created_at].gteq(date_from.beginning_of_day).
and(t[:created_at].lteq(Date.today.end_of_day)).
and(t[:author_id].eq(contributor.id))
Event.reorder(nil).
select(t[:project_id], t[:target_type], t[:action], 'date(created_at) AS date', 'count(id) as total_amount').
group(t[:project_id], t[:target_type], t[:action], 'date(created_at)').
where(conditions).
having(t[:project_id].in(Arel::Nodes::SqlLiteral.new(authed_projects.to_sql)))
end
end end
end end
require 'spec_helper'
describe Gitlab::ContributionsCalendar do
let(:contributor) { create(:user) }
let(:user) { create(:user) }
let(:private_project) do
create(:empty_project, :private) do |project|
create(:project_member, user: contributor, project: project)
end
end
let(:public_project) do
create(:empty_project, :public) do |project|
create(:project_member, user: contributor, project: project)
end
end
let(:feature_project) do
create(:empty_project, :public, issues_access_level: ProjectFeature::PRIVATE) do |project|
create(:project_member, user: contributor, project: project).project
end
end
let(:today) { Time.now.to_date }
let(:last_week) { today - 7.days }
let(:last_year) { today - 1.year }
before do
travel_to today
end
after do
travel_back
end
def calendar(current_user = nil)
described_class.new(contributor, current_user)
end
def create_event(project, day)
@targets ||= {}
@targets[project] ||= create(:issue, project: project, author: contributor)
Event.create!(
project: project,
action: Event::CREATED,
target: @targets[project],
author: contributor,
created_at: day,
)
end
describe '#activity_dates' do
it "returns a hash of date => count" do
create_event(public_project, last_week)
create_event(public_project, last_week)
create_event(public_project, today)
expect(calendar.activity_dates).to eq(last_week => 2, today => 1)
end
it "only shows private events to authorized users" do
create_event(private_project, today)
create_event(feature_project, today)
expect(calendar.activity_dates[today]).to eq(0)
expect(calendar(user).activity_dates[today]).to eq(0)
expect(calendar(contributor).activity_dates[today]).to eq(2)
end
end
describe '#events_by_date' do
it "returns all events for a given date" do
e1 = create_event(public_project, today)
e2 = create_event(public_project, today)
create_event(public_project, last_week)
expect(calendar.events_by_date(today)).to contain_exactly(e1, e2)
end
it "only shows private events to authorized users" do
e1 = create_event(public_project, today)
e2 = create_event(private_project, today)
e3 = create_event(feature_project, today)
create_event(public_project, last_week)
expect(calendar.events_by_date(today)).to contain_exactly(e1)
expect(calendar(contributor).events_by_date(today)).to contain_exactly(e1, e2, e3)
end
end
describe '#starting_year' do
it "should be the start of last year" do
expect(calendar.starting_year).to eq(last_year.year)
end
end
describe '#starting_month' do
it "should be the start of this month" do
expect(calendar.starting_month).to eq(today.month)
end
end
end
...@@ -27,13 +27,14 @@ describe Event, models: true do ...@@ -27,13 +27,14 @@ describe Event, models: true do
end end
describe "Push event" do describe "Push event" do
let(:project) { create(:project) } let(:project) { create(:project, :private) }
let(:user) { project.owner } let(:user) { project.owner }
let(:event) { create_event(project, user) } let(:event) { create_event(project, user) }
it do it do
expect(event.push?).to be_truthy expect(event.push?).to be_truthy
expect(event.visible_to_user?).to be_truthy expect(event.visible_to_user?(user)).to be_truthy
expect(event.visible_to_user?(nil)).to be_falsey
expect(event.tag?).to be_falsey expect(event.tag?).to be_falsey
expect(event.branch_name).to eq("master") expect(event.branch_name).to eq("master")
expect(event.author).to eq(user) expect(event.author).to eq(user)
......
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