Commit 67353ae2 authored by Krasimir Angelov's avatar Krasimir Angelov Committed by Yorick Peterse

Sanitize input when creating/updating protected environments

Remove group ids that are not in the project's invited groups.

Related to https://gitlab.com/gitlab-org/gitlab-ee/issues/11649.
parent 722b0f3f
# frozen_string_literal: true
module ProtectedEnvironments
class BaseService < ::BaseService
include Gitlab::Utils::StrongMemoize
protected
def sanitized_params
params.dup.tap do |sanitized_params|
sanitized_params[:deploy_access_levels_attributes] =
filter_valid_groups(sanitized_params[:deploy_access_levels_attributes])
end
end
private
def filter_valid_groups(attributes)
return unless attributes
attributes.select do |attribute|
attribute[:group_id].nil? || invited_group_ids.include?(attribute[:group_id])
end
end
def invited_group_ids
strong_memoize(:invited_group_ids) do
project.invited_groups.pluck_primary_key.to_set
end
end
end
end
# frozen_string_literal: true
module ProtectedEnvironments
class CreateService < BaseService
class CreateService < ProtectedEnvironments::BaseService
def execute
project.protected_environments.create(params)
project.protected_environments.create(sanitized_params)
end
end
end
# frozen_string_literal: true
module ProtectedEnvironments
class UpdateService < BaseService
class UpdateService < ProtectedEnvironments::BaseService
def execute(protected_environment)
protected_environment.update(params)
protected_environment.update(sanitized_params)
end
end
end
---
title: Prevent IDOR when adding groups to protected environments
merge_request:
author:
type: security
......@@ -32,4 +32,27 @@ describe ProtectedEnvironments::CreateService, '#execute' do
expect(subject.persisted?).to be_falsy
end
end
context 'deploy access level by group' do
let(:params) do
attributes_for(:protected_environment,
deploy_access_levels_attributes: [{ group_id: group.id }])
end
context 'invalid group' do
it_behaves_like 'invalid protected environment group' do
it 'does not create protected environment' do
expect { subject }.not_to change(ProtectedEnvironment, :count)
end
end
end
context 'valid group' do
it_behaves_like 'valid protected environment group' do
it 'creates protected environment' do
expect { subject }.to change(ProtectedEnvironment, :count).by(1)
end
end
end
end
end
......@@ -44,4 +44,16 @@ describe ProtectedEnvironments::UpdateService, '#execute' do
end.not_to change { ProtectedEnvironment::DeployAccessLevel.count }
end
end
context 'deploy access level by group' do
let(:params) { { deploy_access_levels_attributes: [{ group_id: group.id }] } }
context 'invalid group' do
it_behaves_like 'invalid protected environment group'
end
context 'valid group' do
it_behaves_like 'valid protected environment group'
end
end
end
# frozen_string_literal: true
RSpec.shared_examples 'invalid protected environment group' do
let(:group) { create(:group, :private) }
it 'does not create deploy access level' do
expect { subject }.not_to change(ProtectedEnvironment::DeployAccessLevel, :count)
end
end
RSpec.shared_examples 'valid protected environment group' do
let(:project_group_link) { create(:project_group_link) }
let(:group) { project_group_link.group }
let(:project) { project_group_link.project }
it 'creates deploy access level' do
expect { subject }.to change(ProtectedEnvironment::DeployAccessLevel, :count).by(1)
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