Skip to content

G
gitlab-ce
Commit c3ce84f8 authored by Alex Kalderimis's avatar Alex Kalderimis Committed by Douwe Maan
Browse files
Options

Fixes !9930 

This adds support for project maintainers setting the value of
commit_committer_check
parent 9770df1e
Hide whitespace changes
Inline Side-by-side
Showing with 289 additions and 90 deletions
doc/api/projects.md
View file @ c3ce84f8
......@@ -1599,7 +1599,7 @@ GET /projects/:id/push_rule
{
"id": 1,
"project_id": 3,
"commit_message_regex": "Fixes \d +\",
"commit_message_regex": "Fixes \d+\..*",
"branch_name_regex": "",
"deny_delete_tag": false,
"created_at": "2012-10-12T17:04:47Z",
......@@ -1607,10 +1607,16 @@ GET /projects/:id/push_rule
"prevent_secrets": false,
"author_email_regex": "",
"file_name_regex": "",
"max_file_size": 5
"max_file_size": 5,
"commit_committer_check": false
}
```
The following attributes are restricted to certain plans, and will not appear if
you do not have access to those features:
* `commit_committer_check` only available on **[PREMIUM]**
### Add project push rule
Adds a push rule to a specified project.
......@@ -1619,17 +1625,18 @@ Adds a push rule to a specified project.
POST /projects/:id/push_rule
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID of the project or NAMESPACE/PROJECT_NAME |
| `deny_delete_tag` | boolean | no | Deny deleting a tag |
| `member_check` | boolean | no | Restrict commits by author (email) to existing GitLab users |
| `prevent_secrets` | boolean | no | GitLab will reject any files that are likely to contain secrets |
| `commit_message_regex` | string | no | All commit messages must match this, e.g. `Fixed \d+\..*` |
| `branch_name_regex` | string | no | All branch names must match this, e.g. `(feature|hotfix)\/*` |
| `author_email_regex` | string | no | All commit author emails must match this, e.g. `@my-company.com$` |
| `file_name_regex` | string | no | All commited filenames must **not** match this, e.g. `(jar|exe)$` |
| `max_file_size` | integer | no | Maximum file size (MB) |
| Attribute | Type | Required | Description |
| -------------------------------------- | -------------- | -------- | ----------- |
| `id` | integer/string | yes | The ID of the project or NAMESPACE/PROJECT_NAME |
| `deny_delete_tag` **[STARTER]** | boolean | no | Deny deleting a tag |
| `member_check` **[STARTER]** | boolean | no | Restrict commits by author (email) to existing GitLab users |
| `prevent_secrets` **[STARTER]** | boolean | no | GitLab will reject any files that are likely to contain secrets |
| `commit_message_regex` **[STARTER]** | string | no | All commit messages must match this, e.g. `Fixed \d+\..*` |
| `branch_name_regex` **[STARTER]** | string | no | All branch names must match this, e.g. `(feature|hotfix)\/*` |
| `author_email_regex` **[STARTER]** | string | no | All commit author emails must match this, e.g. `@my-company.com$` |
| `file_name_regex` **[STARTER]** | string | no | All commited filenames must **not** match this, e.g. `(jar|exe)$` |
| `max_file_size` **[STARTER]** | integer | no | Maximum file size (MB) |
| `commit_committer_check` **[PREMIUM]** | boolean | no | Users can only push commits to this repository that were committed with one of their own verified emails. |
### Edit project push rule
......@@ -1639,17 +1646,18 @@ Edits a push rule for a specified project.
PUT /projects/:id/push_rule
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID of the project or NAMESPACE/PROJECT_NAME |
| `deny_delete_tag` | boolean | no | Deny deleting a tag |
| `member_check` | boolean | no | Restrict commits by author (email) to existing GitLab users |
| `prevent_secrets` | boolean | no | GitLab will reject any files that are likely to contain secrets |
| `commit_message_regex` | string | no | All commit messages must match this, e.g. `Fixed \d+\..*` |
| `branch_name_regex` | string | no | All branch names must match this, e.g. `(feature|hotfix)\/*` |
| `author_email_regex` | string | no | All commit author emails must match this, e.g. `@my-company.com$` |
| `file_name_regex` | string | no | All commited filenames must **not** match this, e.g. `(jar|exe)$` |
| `max_file_size` | integer | no | Maximum file size (MB) |
| Attribute | Type | Required | Description |
| -------------------------------------- | -------------- | -------- | ----------- |
| `id` | integer/string | yes | The ID of the project or NAMESPACE/PROJECT_NAME |
| `deny_delete_tag` **[STARTER]** | boolean | no | Deny deleting a tag |
| `member_check` **[STARTER]** | boolean | no | Restrict commits by author (email) to existing GitLab users |
| `prevent_secrets` **[STARTER]** | boolean | no | GitLab will reject any files that are likely to contain secrets |
| `commit_message_regex` **[STARTER]** | string | no | All commit messages must match this, e.g. `Fixed \d+\..*` |
| `branch_name_regex` **[STARTER]** | string | no | All branch names must match this, e.g. `(feature|hotfix)\/*` |
| `author_email_regex` **[STARTER]** | string | no | All commit author emails must match this, e.g. `@my-company.com$` |
| `file_name_regex` **[STARTER]** | string | no | All commited filenames must **not** match this, e.g. `(jar|exe)$` |
| `max_file_size` **[STARTER]** | integer | no | Maximum file size (MB) |
| `commit_committer_check` **[PREMIUM]** | boolean | no | Users can only push commits to this repository that were committed with one of their own verified emails. |
### Delete project push rule
......
ee/app/policies/ee/project_policy.rb
View file @ c3ce84f8
......@@ -45,6 +45,16 @@ module EE
!PushRule.global&.commit_committer_check
end
with_scope :subject
condition(:commit_committer_check_available) do
@subject.feature_available?(:commit_committer_check)
end
with_scope :subject
condition(:reject_unsigned_commits_available) do
@subject.feature_available?(:reject_unsigned_commits)
end
with_scope :subject
condition(:pod_logs_enabled) do
@subject.feature_available?(:pod_logs, @user)
......@@ -184,7 +194,19 @@ module EE
rule { admin | (reject_unsigned_commits_disabled_globally & can?(:maintainer_access)) }.enable :change_reject_unsigned_commits
rule { admin | (commit_committer_check_disabled_globally & can?(:maintainer_access)) }.enable :change_commit_committer_check
rule { ~reject_unsigned_commits_available }.prevent :change_reject_unsigned_commits
rule { admin | (commit_committer_check_disabled_globally & can?(:maintainer_access)) }.policy do
enable :change_commit_committer_check
end
rule { commit_committer_check_available }.policy do
enable :read_commit_committer_check
end
rule { ~commit_committer_check_available }.policy do
prevent :change_commit_committer_check
end
rule { owner | reporter }.enable :build_read_project
......
ee/changelogs/unreleased-ee/13174-project-push-rule-commit-committer-check.yml 0 → 100644
View file @ c3ce84f8
title: "Adds commit_committer_check attribute support to project push-rule API endpoint"
merge_request: 13174
type: added
ee/lib/api/project_push_rule.rb
View file @ c3ce84f8
......@@ -5,6 +5,11 @@ module API
before { authenticate! }
before { authorize_admin_project }
before { check_project_feature_available!(:push_rules) }
before do
if params.has_key?(:commit_committer_check)
authorize! :change_commit_committer_check, user_project
end
end
params do
requires :id, type: String, desc: 'The ID of a project'
......@@ -21,9 +26,11 @@ module API
optional :author_email_regex, type: String, desc: 'All commit author emails must match this'
optional :file_name_regex, type: String, desc: 'All commited filenames must not match this'
optional :max_file_size, type: Integer, desc: 'Maximum file size (MB)'
optional :commit_committer_check, type: Boolean, desc: 'Users may only push their own commits'
at_least_one_of :deny_delete_tag, :member_check, :prevent_secrets,
:commit_message_regex, :commit_message_negative_regex, :branch_name_regex, :author_email_regex,
:file_name_regex, :max_file_size
:file_name_regex, :max_file_size,
:commit_committer_check
end
end
......@@ -32,7 +39,7 @@ module API
end
get ":id/push_rule" do
push_rule = user_project.push_rule
present push_rule, with: EE::API::Entities::ProjectPushRule
present push_rule, with: EE::API::Entities::ProjectPushRule, user: current_user
end
desc 'Add a push rule to a project' do
......@@ -46,7 +53,7 @@ module API
error!("Project push rule exists", 422)
else
push_rule = user_project.create_push_rule(declared_params(include_missing: false))
present push_rule, with: EE::API::Entities::ProjectPushRule
present push_rule, with: EE::API::Entities::ProjectPushRule, user: current_user
end
end
......@@ -61,7 +68,7 @@ module API
not_found!('Push Rule') unless push_rule
if push_rule.update(declared_params(include_missing: false))
present push_rule, with: EE::API::Entities::ProjectPushRule
present push_rule, with: EE::API::Entities::ProjectPushRule, user: current_user
else
render_validation_error!(push_rule)
end
......
ee/lib/ee/api/entities.rb
View file @ c3ce84f8
......@@ -191,6 +191,9 @@ module EE
expose :commit_message_regex, :commit_message_negative_regex, :branch_name_regex, :deny_delete_tag
expose :member_check, :prevent_secrets, :author_email_regex
expose :file_name_regex, :max_file_size
expose :commit_committer_check, if: ->(rule, opts) do
Ability.allowed?(opts[:user], :read_commit_committer_check, rule.project)
end
end
class LdapGroupLink < Grape::Entity
......
ee/spec/controllers/projects/push_rules_controller_spec.rb
View file @ c3ce84f8
......@@ -34,54 +34,58 @@ describe Projects::PushRulesController do
end
end
shared_examples 'updateable setting' do |rule_attr, updates, new_value|
it "#{updates ? 'updates' : 'does not update'} the setting" do
patch :update, params: { namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => new_value } }
be_new, be_old = new_value ? [be_truthy, be_falsy] : [be_falsy, be_truthy]
expect(project.reload_push_rule.public_send(rule_attr)).to(updates ? be_new : be_old)
end
end
shared_examples 'a setting with global default' do |rule_attr, updates: true, updates_when_global_enabled: true|
context 'when disabled' do
before do
stub_licensed_features(rule_attr => false)
end
it_behaves_like 'updateable setting', rule_attr, false, true
end
context 'when enabled' do
before do
stub_licensed_features(rule_attr => true)
end
it_behaves_like 'updateable setting', rule_attr, updates, true
end
context 'when global setting is enabled' do
before do
stub_licensed_features(rule_attr => true)
create(:push_rule_sample, rule_attr => true)
end
it_behaves_like 'updateable setting', rule_attr, updates_when_global_enabled, false
end
end
PushRule::SETTINGS_WITH_GLOBAL_DEFAULT.each do |rule_attr|
context "Updating #{rule_attr} rule" do
context 'as an admin' do
let(:user) { create(:admin) }
it 'updates the setting' do
patch :update, params: { namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => true } }
expect(project.reload_push_rule.public_send(rule_attr)).to be_truthy
end
it_behaves_like 'a setting with global default', rule_attr, updates: true
end
context 'as a maintainer user' do
before do
project.add_maintainer(user)
end
context 'when global setting is disabled' do
it 'updates the setting' do
patch :update, params: { namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => true } }
expect(project.reload_push_rule.public_send(rule_attr)).to be_truthy
end
end
context 'when global setting is enabled' do
before do
create(:push_rule_sample, rule_attr => true)
end
it 'does not update the setting' do
patch :update, params: { namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => false } }
expect(project.reload_push_rule.public_send(rule_attr)).to be_truthy
end
end
it_behaves_like 'a setting with global default', rule_attr, updates: true, updates_when_global_enabled: false
end
context 'as a developer user' do
before do
project.add_developer(user)
end
it 'does not update the setting' do
patch :update, params: { namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => true } }
expect(project.reload_push_rule.public_send(rule_attr)).to be_falsy
end
it_behaves_like 'a setting with global default', rule_attr, updates: false, updates_when_global_enabled: false
end
end
end
......
ee/spec/policies/project_policy_spec.rb
View file @ c3ce84f8
......@@ -689,4 +689,35 @@ describe ProjectPolicy do
it { is_expected.to be_allowed(:reporter_access) }
end
context 'commit_committer_check is not enabled by the current license' do
before do
stub_licensed_features(commit_committer_check: false)
end
let(:current_user) { maintainer }
it { is_expected.not_to be_allowed(:change_commit_committer_check) }
it { is_expected.not_to be_allowed(:read_commit_committer_check) }
end
context 'commit_committer_check is enabled by the current license' do
before do
stub_licensed_features(commit_committer_check: true)
end
context 'the user is a maintainer' do
let(:current_user) { maintainer }
it { is_expected.to be_allowed(:change_commit_committer_check) }
it { is_expected.to be_allowed(:read_commit_committer_check) }
end
context 'the user is a developer' do
let(:current_user) { developer }
it { is_expected.not_to be_allowed(:change_commit_committer_check) }
it { is_expected.to be_allowed(:read_commit_committer_check) }
end
end
end
ee/spec/requests/api/project_push_rule_spec.rb
View file @ c3ce84f8
......@@ -7,26 +7,68 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
let!(:project) { create(:project, :repository, creator_id: user.id, namespace: user.namespace) }
before do
stub_licensed_features(push_rules: push_rules_enabled,
commit_committer_check: ccc_enabled)
project.add_maintainer(user)
project.add_developer(user3)
end
let(:push_rules_enabled) { true }
let(:ccc_enabled) { true }
describe "GET /projects/:id/push_rule" do
before do
create(:push_rule, project: project)
create(:push_rule, project: project, **attributes)
end
let(:attributes) do
{ commit_committer_check: true }
end
context "authorized user" do
it "returns project push rule" do
before do
get api("/projects/#{project.id}/push_rule", user)
end
it "returns project push rule" do
expect(response).to have_gitlab_http_status(200)
expect(json_response).to be_an Hash
expect(json_response['project_id']).to eq(project.id)
end
context 'the commit_committer_check feature is enabled' do
let(:ccc_enabled) { true }
it 'returns the commit_committer_check information' do
subset = attributes
.slice(:commit_committer_check)
.transform_keys(&:to_s)
expect(json_response).to include(subset)
end
end
context 'the commit_committer_check feature is not enabled' do
let(:ccc_enabled) { false }
it 'succeeds' do
expect(response).to have_gitlab_http_status(200)
end
it 'does not return the commit_committer_check information' do
expect(json_response).not_to have_key('commit_committer_check')
end
end
context 'push rules are not enabled' do
let(:push_rules_enabled) { false }
it 'is forbidden' do
expect(response).to have_gitlab_http_status(404)
end
end
end
context "unauthorized user" do
context "developer" do
it "does not have access to project push rule" do
get api("/projects/#{project.id}/push_rule", user3)
......@@ -36,21 +78,70 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
end
describe "POST /projects/:id/push_rule" do
context "authorized user" do
it "adds push rule to project" do
post api("/projects/#{project.id}/push_rule", user),
params: { deny_delete_tag: true, member_check: true, prevent_secrets: true, commit_message_regex: 'JIRA\-\d+', branch_name_regex: '(feature|hotfix)\/*', author_email_regex: '[a-zA-Z0-9]+@gitlab.com', file_name_regex: '[a-zA-Z0-9]+.key', max_file_size: 5 }
let(:rules_params) do
{ deny_delete_tag: true,
member_check: true,
prevent_secrets: true,
commit_message_regex: 'JIRA\-\d+',
branch_name_regex: '(feature|hotfix)\/*',
author_email_regex: '[a-zA-Z0-9]+@gitlab.com',
file_name_regex: '[a-zA-Z0-9]+.key',
max_file_size: 5,
commit_committer_check: true }
end
let(:expected_response) do
rules_params.transform_keys(&:to_s)
end
context "maintainer" do
before do
post api("/projects/#{project.id}/push_rule", user), params: rules_params
end
context 'commit_committer_check not allowed by License' do
let(:ccc_enabled) { false }
it "is forbidden to use this service" do
expect(response).to have_gitlab_http_status(403)
end
end
it "is accepted" do
expect(response).to have_gitlab_http_status(201)
end
it "indicates that it belongs to the correct project" do
expect(json_response['project_id']).to eq(project.id)
expect(json_response['deny_delete_tag']).to eq(true)
expect(json_response['member_check']).to eq(true)
expect(json_response['prevent_secrets']).to eq(true)
expect(json_response['commit_message_regex']).to eq('JIRA\-\d+')
expect(json_response['branch_name_regex']).to eq('(feature|hotfix)\/*')
expect(json_response['author_email_regex']).to eq('[a-zA-Z0-9]+@gitlab.com')
expect(json_response['file_name_regex']).to eq('[a-zA-Z0-9]+.key')
expect(json_response['max_file_size']).to eq(5)
end
it "sets all given parameters" do
expect(json_response).to include(expected_response)
end
context 'commit_committer_check is not enabled' do
let(:ccc_enabled) { false }
it "is forbidden to send the the :commit_committer_check parameter" do
expect(response).to have_gitlab_http_status(403)
end
context "without the :commit_committer_check parameter" do
let(:rules_params) do
{ deny_delete_tag: true,
member_check: true,
prevent_secrets: true,
commit_message_regex: 'JIRA\-\d+',
branch_name_regex: '(feature|hotfix)\/*',
author_email_regex: '[a-zA-Z0-9]+@gitlab.com',
file_name_regex: '[a-zA-Z0-9]+.key',
max_file_size: 5 }
end
it "sets all given parameters" do
expect(json_response).to include(expected_response)
end
end
end
end
......@@ -70,9 +161,9 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
expect(response).to have_gitlab_http_status(400)
end
context "unauthorized user" do
context "user with developer_access" do
it "does not add push rule to project" do
post api("/projects/#{project.id}/push_rule", user3), params: { deny_delete_tag: true }
post api("/projects/#{project.id}/push_rule", user3), params: rules_params
expect(response).to have_gitlab_http_status(403)
end
......@@ -95,22 +186,52 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
describe "PUT /projects/:id/push_rule" do
before do
create(:push_rule, project: project)
create(:push_rule, project: project,
deny_delete_tag: true, commit_message_regex: 'Mended')
put api("/projects/#{project.id}/push_rule", user), params: new_settings
end
it "updates an existing project push rule" do
put api("/projects/#{project.id}/push_rule", user),
params: { deny_delete_tag: false, commit_message_regex: 'Fixes \d+\..*' }
context "setting deny_delete_tag and commit_message_regex" do
let(:new_settings) do
{ deny_delete_tag: false, commit_message_regex: 'Fixes \d+\..*' }
end
expect(response).to have_gitlab_http_status(200)
expect(json_response['deny_delete_tag']).to eq(false)
expect(json_response['commit_message_regex']).to eq('Fixes \d+\..*')
it "is successful" do
expect(response).to have_gitlab_http_status(200)
end
it 'includes the expected settings' do
subset = new_settings.transform_keys(&:to_s)
expect(json_response).to include(subset)
end
end
it 'returns 400 if no parameter is given' do
put api("/projects/#{project.id}/push_rule", user)
context "setting commit_committer_check" do
let(:new_settings) { { commit_committer_check: true } }
expect(response).to have_gitlab_http_status(400)
it "is successful" do
expect(response).to have_gitlab_http_status(200)
end
it "sets the commit_committer_check" do
expect(json_response).to include('commit_committer_check' => true)
end
context 'the commit_committer_check feature is not enabled' do
let(:ccc_enabled) { false }
it "is an error to provide the this parameter" do
expect(response).to have_gitlab_http_status(403)
end
end
end
context "not providing parameters" do
let(:new_settings) { {} }
it "is an error" do
expect(response).to have_gitlab_http_status(400)
end
end
end
......@@ -134,7 +255,7 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
create(:push_rule, project: project)
end
context "authorized user" do
context "maintainer" do
it "deletes push rule from project" do
delete api("/projects/#{project.id}/push_rule", user)
......@@ -142,7 +263,7 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
end
end
context "unauthorized user" do
context "user with developer_access" do
it "returns a 403 error" do
delete api("/projects/#{project.id}/push_rule", user3)
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7