Commit f28f7d11 authored by Patrick Bajao's avatar Patrick Bajao

Set maximum limit for profile events

We are previously allowing to request for profile events with
unlimited `limit`. That can result to possible DoS since a
malicious user can request 1k events and it'll take a while to
respond.

Maximum limit is based on the existing `Kaminari.max_per_page`
setting we have.
parent 038a53e8
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
# WARNING: does not consider project feature visibility! # WARNING: does not consider project feature visibility!
# - user: The user for which to load the events # - user: The user for which to load the events
# - params: # - params:
# - limit: Number of items that to be returned. Defaults to 20 and limited to 100.
# - offset: The page of events to return # - offset: The page of events to return
class UserRecentEventsFinder class UserRecentEventsFinder
prepend FinderWithCrossProjectAccess prepend FinderWithCrossProjectAccess
...@@ -16,7 +17,8 @@ class UserRecentEventsFinder ...@@ -16,7 +17,8 @@ class UserRecentEventsFinder
attr_reader :current_user, :target_user, :params attr_reader :current_user, :target_user, :params
LIMIT = 20 DEFAULT_LIMIT = 20
MAX_LIMIT = 100
def initialize(current_user, target_user, params = {}) def initialize(current_user, target_user, params = {})
@current_user = current_user @current_user = current_user
...@@ -31,7 +33,7 @@ class UserRecentEventsFinder ...@@ -31,7 +33,7 @@ class UserRecentEventsFinder
recent_events(params[:offset] || 0) recent_events(params[:offset] || 0)
.joins(:project) .joins(:project)
.with_associations .with_associations
.limit_recent(params[:limit].presence || LIMIT, params[:offset]) .limit_recent(limit, params[:offset])
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
...@@ -59,4 +61,10 @@ class UserRecentEventsFinder ...@@ -59,4 +61,10 @@ class UserRecentEventsFinder
def projects def projects
target_user.project_interactions.to_sql target_user.project_interactions.to_sql
end end
def limit
return DEFAULT_LIMIT unless params[:limit].present?
[params[:limit].to_i, MAX_LIMIT].min
end
end end
---
title: Set maximum limit for profile events
merge_request:
author:
type: security
...@@ -11,8 +11,10 @@ RSpec.describe UserRecentEventsFinder do ...@@ -11,8 +11,10 @@ RSpec.describe UserRecentEventsFinder do
let!(:private_event) { create(:event, project: private_project, author: project_owner) } let!(:private_event) { create(:event, project: private_project, author: project_owner) }
let!(:internal_event) { create(:event, project: internal_project, author: project_owner) } let!(:internal_event) { create(:event, project: internal_project, author: project_owner) }
let!(:public_event) { create(:event, project: public_project, author: project_owner) } let!(:public_event) { create(:event, project: public_project, author: project_owner) }
let(:limit) { nil }
let(:params) { { limit: limit } }
subject(:finder) { described_class.new(current_user, project_owner) } subject(:finder) { described_class.new(current_user, project_owner, params) }
describe '#execute' do describe '#execute' do
context 'when profile is public' do context 'when profile is public' do
...@@ -48,5 +50,38 @@ RSpec.describe UserRecentEventsFinder do ...@@ -48,5 +50,38 @@ RSpec.describe UserRecentEventsFinder do
expect(events).to include(event_b) expect(events).to include(event_b)
end end
end end
context 'limits' do
before do
stub_const("#{described_class}::DEFAULT_LIMIT", 1)
stub_const("#{described_class}::MAX_LIMIT", 3)
end
context 'when limit is not set' do
it 'returns events limited to DEFAULT_LIMIT' do
expect(finder.execute.size).to eq(described_class::DEFAULT_LIMIT)
end
end
context 'when limit is set' do
let(:limit) { 2 }
it 'returns events limited to specified limit' do
expect(finder.execute.size).to eq(limit)
end
end
context 'when limit is set to a number that exceeds maximum limit' do
let(:limit) { 4 }
before do
create(:event, project: public_project, author: project_owner)
end
it 'returns events limited to MAX_LIMIT' do
expect(finder.execute.size).to eq(described_class::MAX_LIMIT)
end
end
end
end 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