Commit 907f91f4 authored by Douwe Maan's avatar Douwe Maan

Merge branch '2573-namespace-license-push-rules' into 'master'

Introduce namespace license checks for Push Rules (EES)

Closes #2573

See merge request !2335
parents 07ca0fb9 d3bc16c1
class Admin::PushRulesController < Admin::ApplicationController
before_action :check_push_rules_available!
before_action :push_rule
respond_to :html
......@@ -18,6 +19,10 @@ class Admin::PushRulesController < Admin::ApplicationController
private
def check_push_rules_available!
render_404 unless License.feature_available?(:push_rules)
end
def push_rule_params
params.require(:push_rule).permit(:deny_delete_tag, :delete_branch_regex,
:commit_message_regex, :force_push_regex, :author_email_regex, :member_check,
......
module EE
module Projects
module Settings
module RepositoryController
extend ActiveSupport::Concern
prepended do
before_action :push_rule, only: [:show]
before_action :remote_mirror, only: [:show]
end
private
def push_rule
return unless project.feature_available?(:push_rules)
project.create_push_rule unless project.push_rule
@push_rule = project.push_rule
end
def remote_mirror
@remote_mirror = @project.remote_mirrors.first_or_initialize
end
def acces_levels_options
super.merge(
selected_merge_access_levels: @protected_branch.merge_access_levels.map { |access_level| access_level.user_id || access_level.access_level },
selected_push_access_levels: @protected_branch.push_access_levels.map { |access_level| access_level.user_id || access_level.access_level },
selected_create_access_levels: @protected_tag.create_access_levels.map { |access_level| access_level.user_id || access_level.access_level }
)
end
def load_gon_index
super
gon.push(current_project_id: @project.id) if @project
end
end
end
end
end
......@@ -3,6 +3,7 @@ class Projects::PushRulesController < Projects::ApplicationController
# Authorize
before_action :authorize_admin_project!
before_action :check_push_rules_available!
respond_to :html
......
......@@ -2,23 +2,17 @@ module Projects
module Settings
class RepositoryController < Projects::ApplicationController
before_action :authorize_admin_project!
before_action :remote_mirror, only: [:show]
prepend ::EE::Projects::Settings::RepositoryController
def show
@deploy_keys = DeployKeysPresenter.new(@project, current_user: current_user)
define_protected_refs
project.create_push_rule unless project.push_rule
@push_rule = project.push_rule
end
private
def remote_mirror
@remote_mirror = @project.remote_mirrors.first_or_initialize
end
def define_protected_refs
@protected_branches = @project.protected_branches.order(:name).page(params[:page])
@protected_tags = @project.protected_tags.order(:name).page(params[:page])
......@@ -29,9 +23,6 @@ module Projects
def access_levels_options
{
selected_merge_access_levels: @protected_branch.merge_access_levels.map { |access_level| access_level.user_id || access_level.access_level },
selected_push_access_levels: @protected_branch.push_access_levels.map { |access_level| access_level.user_id || access_level.access_level },
selected_create_access_levels: @protected_tag.create_access_levels.map { |access_level| access_level.user_id || access_level.access_level },
create_access_levels: levels_for_dropdown(ProtectedTag::CreateAccessLevel),
push_access_levels: levels_for_dropdown(ProtectedBranch::PushAccessLevel),
merge_access_levels: levels_for_dropdown(ProtectedBranch::MergeAccessLevel)
......@@ -57,7 +48,6 @@ module Projects
gon.push(protectable_tags_for_dropdown)
gon.push(protectable_branches_for_dropdown)
gon.push(access_levels_options)
gon.push(current_project_id: @project.id) if @project
end
end
end
......
......@@ -25,7 +25,7 @@ module EE
belongs_to :mirror_user, foreign_key: 'mirror_user_id', class_name: 'User'
has_one :mirror_data, dependent: :delete, autosave: true, class_name: 'ProjectMirrorData'
has_one :push_rule, dependent: :destroy
has_one :push_rule, ->(project) { project&.feature_available?(:push_rules) ? all : none }, dependent: :destroy
has_one :index_status, dependent: :destroy
has_one :jenkins_service, dependent: :destroy
has_one :jenkins_deprecated_service, dependent: :destroy
......
......@@ -17,6 +17,7 @@ class License < ActiveRecord::Base
MERGE_REQUEST_REBASE_FEATURE = 'GitLab_MergeRequestRebase'.freeze
MERGE_REQUEST_SQUASH_FEATURE = 'GitLab_MergeRequestSquash'.freeze
OBJECT_STORAGE_FEATURE = 'GitLab_ObjectStorage'.freeze
PUSH_RULES_FEATURE = 'GitLab_PushRules'.freeze
RELATED_ISSUES_FEATURE = 'RelatedIssues'.freeze
SERVICE_DESK_FEATURE = 'GitLab_ServiceDesk'.freeze
......@@ -40,7 +41,8 @@ class License < ActiveRecord::Base
issue_weights: ISSUE_WEIGHTS_FEATURE,
merge_request_approvers: MERGE_REQUEST_APPROVERS_FEATURE,
merge_request_rebase: MERGE_REQUEST_REBASE_FEATURE,
merge_request_squash: MERGE_REQUEST_SQUASH_FEATURE
merge_request_squash: MERGE_REQUEST_SQUASH_FEATURE,
push_rules: PUSH_RULES_FEATURE
}.freeze
STARTER_PLAN = 'starter'.freeze
......@@ -60,6 +62,7 @@ class License < ActiveRecord::Base
{ MERGE_REQUEST_APPROVERS_FEATURE => 1 },
{ MERGE_REQUEST_REBASE_FEATURE => 1 },
{ MERGE_REQUEST_SQUASH_FEATURE => 1 },
{ PUSH_RULES_FEATURE => 1 },
{ RELATED_ISSUES_FEATURE => 1 }
].freeze
......@@ -100,6 +103,7 @@ class License < ActiveRecord::Base
{ MERGE_REQUEST_REBASE_FEATURE => 1 },
{ MERGE_REQUEST_SQUASH_FEATURE => 1 },
{ OBJECT_STORAGE_FEATURE => 1 },
{ PUSH_RULES_FEATURE => 1 },
{ SERVICE_DESK_FEATURE => 1 }
].freeze
......
......@@ -44,6 +44,7 @@ module MergeRequests
@merge_request = merge_request
return true if project.merge_requests_ff_only_enabled
return true unless project.feature_available?(:push_rules)
push_rule = merge_request.project.push_rule
return true unless push_rule
......
......@@ -22,7 +22,7 @@ module Projects
return @project
end
# Repository size limit comes as MB from the view
# EE-only: Repository size limit comes as MB from the view
set_repository_size_limit_as_bytes
set_project_name_from_path
......@@ -103,12 +103,8 @@ module Projects
@project.add_master(owners, current_user: current_user)
end
predefined_push_rule = PushRule.find_by(is_sample: true)
if predefined_push_rule
push_rule = predefined_push_rule.dup.tap{ |gh| gh.is_sample = false }
project.push_rule = push_rule
end
# EE-only
create_predefined_push_rule
@project.group&.refresh_members_authorized_projects
end
......@@ -166,5 +162,16 @@ module Projects
@project.path = @project.name.dup.parameterize
end
end
def create_predefined_push_rule
return unless project.feature_available?(:push_rules)
predefined_push_rule = PushRule.find_by(is_sample: true)
if predefined_push_rule
push_rule = predefined_push_rule.dup.tap{ |gh| gh.is_sample = false }
project.push_rule = push_rule
end
end
end
end
......@@ -34,23 +34,10 @@
Abuse Reports
%span.badge.count= number_with_delimiter(AbuseReport.count(:all))
= nav_link(controller: :licenses) do
= link_to admin_license_path, title: 'License' do
%span
License
- if akismet_enabled?
= nav_link(controller: :spam_logs) do
= link_to admin_spam_logs_path, title: "Spam Logs" do
%span
Spam Logs
= nav_link(controller: :push_rules) do
= link_to admin_push_rule_path, title: 'Push Rules' do
%span
Push Rules
= nav_link(controller: :geo_nodes) do
= link_to admin_geo_nodes_path, title: 'Geo Nodes' do
%span
Geo Nodes
= render 'layouts/nav/admin_ee'
= nav_link(controller: :licenses) do
= link_to admin_license_path, title: 'License' do
%span
License
- if License.feature_available?(:push_rules)
= nav_link(controller: :push_rules) do
= link_to admin_push_rule_path, title: 'Push Rules' do
%span
Push Rules
= nav_link(controller: :geo_nodes) do
= link_to admin_geo_nodes_path, title: 'Geo Nodes' do
%span
Geo Nodes
- return unless @project.feature_available?(:push_rules)
- expanded = Rails.env.test?
%section.settings
.settings-header
......
---
title: Introduce namespace license checks for Push Rules (EES)
merge_request: 2335
author:
......@@ -2,6 +2,7 @@ module API
class ProjectPushRule < Grape::API
before { authenticate! }
before { authorize_admin_project }
before { check_project_feature_available!(:push_rules) }
params do
requires :id, type: String, desc: 'The ID of a project'
......
......@@ -3,6 +3,7 @@ module API
class ProjectGitHook < Grape::API
before { authenticate! }
before { authorize_admin_project }
before { check_project_feature_available!(:push_rules) }
DEPRECATION_MESSAGE = 'This endpoint is deprecated, replaced with push_rules, and will be removed in GitLab 9.0.'.freeze
......
......@@ -3,6 +3,7 @@ module API
class ProjectPushRule < Grape::API
before { authenticate! }
before { authorize_admin_project }
before { check_project_feature_available!(:push_rules) }
params do
requires :id, type: String, desc: 'The ID of a project'
......
......@@ -9,6 +9,10 @@ module EE
user
end
def check_project_feature_available!(feature)
not_found! unless user_project.feature_available?(feature)
end
end
end
end
......@@ -142,7 +142,7 @@ module Gitlab
end
def push_rule_check
return unless @newrev && @oldrev
return unless @newrev && @oldrev && project.feature_available?(:push_rules)
push_rule = project.push_rule
......
......@@ -8,17 +8,52 @@ describe Admin::PushRulesController do
end
describe '#update' do
it 'updates sample push rule' do
params =
{ deny_delete_tag: true, delete_branch_regex: "any", commit_message_regex: "any",
force_push_regex: "any", author_email_regex: "any", member_check: true, file_name_regex: "any",
max_file_size: "0", prevent_secrets: true }
let(:params) do
{
deny_delete_tag: true, delete_branch_regex: "any", commit_message_regex: "any",
force_push_regex: "any", author_email_regex: "any", member_check: true, file_name_regex: "any",
max_file_size: "0", prevent_secrets: true
}
end
it 'updates sample push rule' do
expect_any_instance_of(PushRule).to receive(:update_attributes).with(params)
patch :update, push_rule: params
expect(response).to redirect_to(admin_push_rule_path)
end
context 'push rules unlicensed' do
before do
stub_licensed_features(push_rules: false)
end
it 'returns 404' do
patch :update, push_rule: params
expect(response).to have_http_status(404)
end
end
end
describe '#show' do
it 'returns 200' do
get :show
expect(response).to have_http_status(200)
end
context 'push rules unlicensed' do
before do
stub_licensed_features(push_rules: false)
end
it 'returns 404' do
get :show
expect(response).to have_http_status(404)
end
end
end
end
require 'spec_helper'
describe Projects::PushRulesController do
let(:project) { create(:empty_project, push_rule: create(:push_rule, prevent_secrets: false)) }
let(:user) { create(:user) }
before do
project.add_master(user)
sign_in(user)
end
describe '#update' do
def do_update
patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { prevent_secrets: true }
end
it 'updates the push rule' do
do_update
expect(response).to have_http_status(302)
expect(project.push_rule(true).prevent_secrets).to be_truthy
end
context 'push rules unlicensed' do
before do
stub_licensed_features(push_rules: false)
end
it 'returns 404' do
do_update
expect(response).to have_http_status(404)
end
end
end
end
require 'spec_helper'
describe Projects::Settings::RepositoryController do
let(:project) { create(:project_empty_repo, :public) }
let(:user) { create(:user) }
before do
project.add_master(user)
sign_in(user)
end
describe 'GET show' do
context 'push rule' do
subject(:push_rule) { assigns(:push_rule) }
it 'is created' do
get :show, namespace_id: project.namespace, project_id: project
is_expected.to be_persisted
end
context 'unlicensed' do
before do
stub_licensed_features(push_rules: false)
end
it 'is not created' do
get :show, namespace_id: project.namespace, project_id: project
is_expected.to be_nil
end
end
end
end
end
......@@ -15,19 +15,33 @@ describe 'Project settings > [EE] repository', feature: true do
let(:commit_message) { 'Required part of every message' }
let(:input_id) { 'push_rule_commit_message_regex' }
before do
visit namespace_project_settings_repository_path(project.namespace, project)
context 'push rules licensed' do
before do
visit namespace_project_settings_repository_path(project.namespace, project)
fill_in input_id, with: commit_message
click_button 'Save Push Rules'
end
fill_in input_id, with: commit_message
click_button 'Save Push Rules'
end
it 'displays the new value in the form' do
expect(find("##{input_id}").value).to eq commit_message
end
it 'displays the new value in the form' do
expect(find("##{input_id}").value).to eq commit_message
it 'saves the new value' do
expect(project.push_rule.commit_message_regex).to eq commit_message
end
end
it 'saves the new value' do
expect(project.push_rule.commit_message_regex).to eq commit_message
context 'push rules unlicensed' do
before do
stub_licensed_features(push_rules: false)
visit namespace_project_settings_repository_path(project.namespace, project)
end
it 'hides push rule settings' do
expect(page).not_to have_content('Push Rules')
end
end
end
......
......@@ -165,6 +165,14 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
end
context 'push rules checks' do
shared_examples 'check ignored when push rule unlicensed' do
before do
stub_licensed_features(push_rules: false)
end
it { is_expected.to be_truthy }
end
let(:project) { create(:project, :public, push_rule: push_rule) }
before do
......@@ -183,6 +191,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
project.add_master(user)
end
it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if the rule denies tag deletion' do
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You cannot delete a tag')
end
......@@ -199,6 +209,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
context 'commit message rules' do
let(:push_rule) { create(:push_rule, :commit_message) }
it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if the rule fails' do
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'")
end
......@@ -212,6 +224,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
allow_any_instance_of(Commit).to receive(:author_email).and_return('mike@valid.com')
end
it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if the rule fails for the committer' do
allow_any_instance_of(Commit).to receive(:committer_email).and_return('ana@invalid.com')
......@@ -233,6 +247,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
allow_any_instance_of(Commit).to receive(:author_email).and_return('some@mail.com')
end
it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if the commit author is not a GitLab member' do
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Author 'some@mail.com' is not a member of team")
end
......@@ -243,6 +259,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
context 'file name regex check' do
let(:push_rule) { create(:push_rule, file_name_regex: 'READ*') }
it_behaves_like 'check ignored when push rule unlicensed'
it "returns an error if a new or renamed filed doesn't match the file name regex" do
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "File name README was blacklisted by the pattern READ*.")
end
......@@ -251,6 +269,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
context 'blacklisted files check' do
let(:push_rule) { create(:push_rule, prevent_secrets: true) }
it_behaves_like 'check ignored when push rule unlicensed'
it "returns true if there is no blacklisted files" do
new_rev = nil
......@@ -305,6 +325,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
allow_any_instance_of(Blob).to receive(:size).and_return(2.megabytes)
end
it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if file exceeds the maximum file size' do
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "File \"README\" is larger than the allowed size of 1 MB")
end
......
......@@ -11,6 +11,22 @@ describe Project, models: true do
it { is_expected.to delegate_method(:shared_runners_minutes_used?).to(:namespace) }
end
describe '#push_rule' do
let(:project) { create(:project, push_rule: create(:push_rule)) }
subject(:push_rule) { project.push_rule(true) }
it { is_expected.not_to be_nil }
context 'push rules unlicensed' do
before do
stub_licensed_features(push_rules: false)
end
it { is_expected.to be_nil }
end
end
describe '#feature_available?' do
let(:namespace) { build_stubbed(:namespace) }
let(:project) { build_stubbed(:project, namespace: namespace) }
......
require 'spec_helper'
describe Projects::CreateService, '#execute', services: true do
let(:user) { create :user }
let(:opts) do
{
name: "GitLab",
namespace: user.namespace
}
end
context 'repository_size_limit assignment as Bytes' do
let(:admin_user) { create(:user, admin: true) }
context 'when param present' do
let(:opts) { { repository_size_limit: '100' } }
it 'assign repository_size_limit as Bytes' do
project = create_project(admin_user, opts)
expect(project.repository_size_limit).to eql(100 * 1024 * 1024)
end
end
context 'when param not present' do
let(:opts) { { repository_size_limit: '' } }
it 'assign nil value' do
project = create_project(admin_user, opts)
expect(project.repository_size_limit).to be_nil
end
end
end
context 'git hook sample' do
let!(:sample) { create(:push_rule_sample) }
subject(:push_rule) { create_project(user, opts).push_rule }
it 'creates git hook from sample' do
is_expected.to have_attributes(
force_push_regex: sample.force_push_regex,
deny_delete_tag: sample.deny_delete_tag,
delete_branch_regex: sample.delete_branch_regex,
commit_message_regex: sample.commit_message_regex
)
end
context 'push rules unlicensed' do
before do
stub_licensed_features(push_rules: false)
end
it 'ignores the push rule sample' do
is_expected.to be_nil
end
end
end
def create_project(user, opts)
Projects::CreateService.new(user, opts).execute
end
end
......@@ -242,6 +242,16 @@ describe MergeRequests::MergeService, services: true do
end
describe '#hooks_validation_pass?' do
shared_examples 'hook validations are skipped when push rules unlicensed' do
subject { service.hooks_validation_pass?(merge_request) }
before do
stub_licensed_features(push_rules: false)
end
it { is_expected.to be_truthy }
end
let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
it 'returns true when valid' do
......@@ -253,6 +263,8 @@ describe MergeRequests::MergeService, services: true do
allow(project).to receive(:push_rule) { build(:push_rule, commit_message_regex: 'unmatched pattern .*') }
end
it_behaves_like 'hook validations are skipped when push rules unlicensed'
it 'returns false and saves error when invalid' do
expect(service.hooks_validation_pass?(merge_request)).to be_falsey
expect(merge_request.merge_error).not_to be_empty
......@@ -264,6 +276,8 @@ describe MergeRequests::MergeService, services: true do
allow(project).to receive(:push_rule) { build(:push_rule, author_email_regex: '.*@unmatchedemaildomain.com') }
end
it_behaves_like 'hook validations are skipped when push rules unlicensed'
it 'returns false and saves error when invalid' do
expect(service.hooks_validation_pass?(merge_request)).to be_falsey
expect(merge_request.merge_error).not_to be_empty
......
......@@ -110,30 +110,6 @@ describe Projects::CreateService, '#execute', services: true do
end
end
context 'repository_size_limit assignment as Bytes' do
let(:admin_user) { create(:user, admin: true) }
context 'when param present' do
let(:opts) { { repository_size_limit: '100' } }
it 'assign repository_size_limit as Bytes' do
project = create_project(admin_user, opts)
expect(project.repository_size_limit).to eql(100 * 1024 * 1024)
end
end
context 'when param not present' do
let(:opts) { { repository_size_limit: '' } }
it 'assign nil value' do
project = create_project(admin_user, opts)
expect(project.repository_size_limit).to be_nil
end
end
end
context 'restricted visibility level' do
before do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
......@@ -161,18 +137,6 @@ describe Projects::CreateService, '#execute', services: true do
end
end
context 'git hook sample' do
it 'creates git hook from sample' do
push_rule_sample = create(:push_rule_sample)
push_rule = create_project(user, opts).push_rule
[:force_push_regex, :deny_delete_tag, :delete_branch_regex, :commit_message_regex].each do |attr_name|
expect(push_rule.send(attr_name)).to eq push_rule_sample.send(attr_name)
end
end
end
context 'repository creation' do
it 'synchronously creates the repository' do
expect_any_instance_of(Project).to receive(:create_repository)
......
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