diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 80c7a803392f55e89d41eaf2c79ac12aef6b5720..7175eefcde7e378fb357efa167882b15842d5ad9 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -195,7 +195,8 @@ def group_params_attributes :require_two_factor_authentication, :two_factor_grace_period, :project_creation_level, - :subgroup_creation_level + :subgroup_creation_level, + :default_branch_protection ] end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 99212d09b8ee959f549399d48b3c9470eb0771b0..f06e9da3b2aff482af193fa93298c62430546ae7 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -139,6 +139,10 @@ def reset_ci_minutes!(namespace_id) end end + def default_branch_protection + super || Gitlab::CurrentSettings.default_branch_protection + end + def visibility_level_field :visibility_level end diff --git a/app/models/project.rb b/app/models/project.rb index f72e777c00419f09577a1a82006bb71abdc35a42..fdf7452d14332010fb23c32b0554b6416f090792 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2359,6 +2359,12 @@ def deploy_token_revoke_url_for(token) Gitlab::Routing.url_helpers.revoke_project_deploy_token_path(self, token) end + def default_branch_protected? + branch_protection = Gitlab::Access::BranchProtection.new(self.namespace.default_branch_protection) + + branch_protection.fully_protected? || branch_protection.developer_can_merge? + end + private def closest_namespace_setting(name) diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 94c3b83564f54dd043768a94ce54aca2e817a721..594c822c18f68a289b0ad1623a5370ecde1733ff 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -11,7 +11,8 @@ class ProtectedBranch < ApplicationRecord def self.protected_ref_accessible_to?(ref, user, project:, action:, protected_refs: nil) # Maintainers, owners and admins are allowed to create the default branch - if default_branch_protected? && project.empty_repo? + + if project.empty_repo? && project.default_branch_protected? return true if user.admin? || project.team.max_member_access(user.id) > Gitlab::Access::DEVELOPER end @@ -20,7 +21,7 @@ def self.protected_ref_accessible_to?(ref, user, project:, action:, protected_re # Check if branch name is marked as protected in the system def self.protected?(project, ref_name) - return true if project.empty_repo? && default_branch_protected? + return true if project.empty_repo? && project.default_branch_protected? self.matching(ref_name, protected_refs: protected_refs(project)).present? end @@ -33,11 +34,6 @@ def self.any_protected?(project, ref_names) end end - def self.default_branch_protected? - Gitlab::CurrentSettings.default_branch_protection == Gitlab::Access::PROTECTION_FULL || - Gitlab::CurrentSettings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE - end - def self.protected_refs(project) project.protected_branches end diff --git a/app/services/projects/protect_default_branch_service.rb b/app/services/projects/protect_default_branch_service.rb index 245490791bf605444b3b8cc3908bd9ee3193f644..1d3fb523448720287cfd76cb93f692493ad89c69 100644 --- a/app/services/projects/protect_default_branch_service.rb +++ b/app/services/projects/protect_default_branch_service.rb @@ -11,7 +11,7 @@ def initialize(project) @project = project @default_branch_protection = Gitlab::Access::BranchProtection - .new(Gitlab::CurrentSettings.default_branch_protection) + .new(project.namespace.default_branch_protection) end def execute diff --git a/app/views/admin/application_settings/_visibility_and_access.html.haml b/app/views/admin/application_settings/_visibility_and_access.html.haml index ae90ffd9efc90ba1d0743427f804cee756954b49..a4acbe6c885583e332d22becc4568422e5b613b8 100644 --- a/app/views/admin/application_settings/_visibility_and_access.html.haml +++ b/app/views/admin/application_settings/_visibility_and_access.html.haml @@ -2,9 +2,8 @@ = form_errors(@application_setting) %fieldset - .form-group - = f.label :default_branch_protection, class: 'label-bold' - = f.select :default_branch_protection, options_for_select(Gitlab::Access.protection_options, @application_setting.default_branch_protection), {}, class: 'form-control' + = render 'shared/default_branch_protection', f: f, selected_level: @application_setting.default_branch_protection + .form-group = f.label s_('ProjectCreationLevel|Default project creation protection'), class: 'label-bold' = f.select :default_project_creation, options_for_select(Gitlab::Access.project_creation_options, @application_setting.default_project_creation), {}, class: 'form-control' diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml index 618cfe57be48199243b46be1f8983ba5c6e0e6af..016a9c8e0540868051015d55706df8b18dfd2b5f 100644 --- a/app/views/groups/settings/_permissions.html.haml +++ b/app/views/groups/settings/_permissions.html.haml @@ -33,6 +33,7 @@ = render_if_exists 'groups/settings/ip_restriction', f: f, group: @group = render_if_exists 'groups/settings/allowed_email_domain', f: f, group: @group = render 'groups/settings/lfs', f: f + = render 'shared/default_branch_protection', f: f, selected_level: @group.default_branch_protection = render 'groups/settings/project_creation_level', f: f, group: @group = render 'groups/settings/subgroup_creation_level', f: f, group: @group = render 'groups/settings/two_factor_auth', f: f diff --git a/app/views/shared/_default_branch_protection.html.haml b/app/views/shared/_default_branch_protection.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..d7ae21debd86817c3c9725a517f02a514e967253 --- /dev/null +++ b/app/views/shared/_default_branch_protection.html.haml @@ -0,0 +1,3 @@ +.form-group + = f.label :default_branch_protection, class: 'label-bold' + = f.select :default_branch_protection, options_for_select(Gitlab::Access.protection_options, selected_level), {}, class: 'form-control' diff --git a/changelogs/unreleased/7583-developer-cannot-push-to-projects-they-create-in-groups.yml b/changelogs/unreleased/7583-developer-cannot-push-to-projects-they-create-in-groups.yml new file mode 100644 index 0000000000000000000000000000000000000000..2055abc45516afccfc6ec33ac51e74607950dd55 --- /dev/null +++ b/changelogs/unreleased/7583-developer-cannot-push-to-projects-they-create-in-groups.yml @@ -0,0 +1,5 @@ +--- +title: Introduce default branch protection at the group level +merge_request: 24426 +author: +type: added diff --git a/db/migrate/20200207062728_add_default_branch_protection_to_namespaces.rb b/db/migrate/20200207062728_add_default_branch_protection_to_namespaces.rb new file mode 100644 index 0000000000000000000000000000000000000000..c21cac307f96e02428c9d225ad77722c4fec8442 --- /dev/null +++ b/db/migrate/20200207062728_add_default_branch_protection_to_namespaces.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddDefaultBranchProtectionToNamespaces < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + with_lock_retries do + add_column :namespaces, :default_branch_protection, :integer, limit: 2 + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 149582f94bf46bd27bd046e0bd94935775d92a66..fa251f657410742a42a9d7769bbf5c4b299ab196 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2762,6 +2762,7 @@ t.integer "max_pages_size" t.integer "max_artifacts_size" t.boolean "mentions_disabled" + t.integer "default_branch_protection", limit: 2 t.index ["created_at"], name: "index_namespaces_on_created_at" t.index ["custom_project_templates_group_id", "type"], name: "index_namespaces_on_custom_project_templates_group_id_and_type", where: "(custom_project_templates_group_id IS NOT NULL)" t.index ["file_template_project_id"], name: "index_namespaces_on_file_template_project_id" diff --git a/doc/api/groups.md b/doc/api/groups.md index 3a488ca65469d1c4f270b522be0a8b5268a69237..e48cb78b2cc904d25132e5e712b1a8cd0a32fd22 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -42,6 +42,7 @@ GET /groups "emails_disabled": null, "mentions_disabled": null, "lfs_enabled": true, + "default_branch_protection": 2, "avatar_url": "http://localhost:3000/uploads/group/avatar/1/foo.jpg", "web_url": "http://localhost:3000/groups/foo-bar", "request_access_enabled": false, @@ -76,6 +77,7 @@ GET /groups?statistics=true "emails_disabled": null, "mentions_disabled": null, "lfs_enabled": true, + "default_branch_protection": 2, "avatar_url": "http://localhost:3000/uploads/group/avatar/1/foo.jpg", "web_url": "http://localhost:3000/groups/foo-bar", "request_access_enabled": false, @@ -148,6 +150,7 @@ GET /groups/:id/subgroups "emails_disabled": null, "mentions_disabled": null, "lfs_enabled": true, + "default_branch_protection": 2, "avatar_url": "http://gitlab.example.com/uploads/group/avatar/1/foo.jpg", "web_url": "http://gitlab.example.com/groups/foo-bar", "request_access_enabled": false, @@ -493,9 +496,20 @@ Parameters: | `lfs_enabled` | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group. | | `request_access_enabled` | boolean | no | Allow users to request member access. | | `parent_id` | integer | no | The parent group ID for creating nested group. | +| `default_branch_protection` | integer | no | See [Options for `default_branch_protection`](#options-for-default_branch_protection). Default to the global level default branch protection setting. | | `shared_runners_minutes_limit` | integer | no | **(STARTER ONLY)** Pipeline minutes quota for this group. | | `extra_shared_runners_minutes_limit` | integer | no | **(STARTER ONLY)** Extra pipeline minutes quota for this group. | +### Options for `default_branch_protection` + +The `default_branch_protection` attribute determines whether developers and maintainers can push to the applicable master branch, as described in the following table: + +| Value | Description | +|-------|-------------------------------------------------------------------------------------------------------------| +| `0` | No protection. Developers and maintainers can:
- Push new commits
- Force push changes
- Delete the branch | +| `1` | Partial protection. Developers and maintainers can:
- Push new commits | +| `2` | Full protection. Only maintainers can:
- Push new commits | + ## Transfer project to group Transfer a project to the Group namespace. Available only to instance administrators, although an [alternative API endpoint](projects.md#transfer-a-project-to-a-new-namespace) is available which does not require instance administrator access. Transferring projects may fail when tagged packages exist in the project's repository. @@ -542,6 +556,7 @@ PUT /groups/:id | `mentions_disabled` | boolean | no | Disable the capability of a group from getting mentioned | | `lfs_enabled` (optional) | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group. | | `request_access_enabled` | boolean | no | Allow users to request member access. | +| `default_branch_protection` | integer | no | See [Options for `default_branch_protection`](#options-for-default_branch_protection). | | `file_template_project_id` | integer | no | **(PREMIUM)** The ID of a project to load custom file templates from. | | `shared_runners_minutes_limit` | integer | no | **(STARTER ONLY)** Pipeline minutes quota for this group. | | `extra_shared_runners_minutes_limit` | integer | no | **(STARTER ONLY)** Extra pipeline minutes quota for this group. | diff --git a/doc/user/admin_area/settings/visibility_and_access_controls.md b/doc/user/admin_area/settings/visibility_and_access_controls.md index 8f64e9207b522414baf2fea99e5f6dd1af5dd1a2..704dd89ede255391c6206538aa93c8e559120037 100644 --- a/doc/user/admin_area/settings/visibility_and_access_controls.md +++ b/doc/user/admin_area/settings/visibility_and_access_controls.md @@ -26,6 +26,8 @@ To change the default branch protection: For more details, see [Protected branches](../../project/protected_branches.md). +To change this setting for a specific group, see [Default branch protection for groups](../../group/index.md#changing-the-default-branch-protection-of-a-group) + ## Default project creation protection Project creation protection specifies which roles can create projects. diff --git a/doc/user/group/index.md b/doc/user/group/index.md index 6b76e070c4181056821122df796a4dd351980dfd..cca82f6a4fb3cee5c1cdd2f47285f31b3e703be1 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -181,6 +181,21 @@ of a group: 1. Give a different member **Owner** permissions. 1. Have the new owner sign in and remove **Owner** permissions from you. +## Changing the default branch protection of a group + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/7583) in GitLab 12.9. + +By default, every group inherits the branch protection set at the global level. + +To change this setting for a specific group: + +1. Go to the group's **{settings}** **Settings > General** page. +1. Expand the **Permissions, LFS, 2FA** section. +1. Select the desired option in the **Default branch protection** dropdown list. +1. Click **Save changes**. + +To change this setting globally, see [Default branch protection](../admin_area/settings/visibility_and_access_controls.md#default-branch-protection). + ## Add projects to a group There are two different ways to add a new project to a group: diff --git a/lib/api/entities/group.rb b/lib/api/entities/group.rb index ae5ee4784edf6f58ab76fa1b771c5c7787e21a22..10e10e52d9f3d84d7984e83f8a317f15b67a138d 100644 --- a/lib/api/entities/group.rb +++ b/lib/api/entities/group.rb @@ -13,6 +13,7 @@ class Group < BasicGroupDetails expose :emails_disabled expose :mentions_disabled expose :lfs_enabled?, as: :lfs_enabled + expose :default_branch_protection expose :avatar_url do |group, options| group.avatar_url(only_path: false) end diff --git a/lib/api/helpers/groups_helpers.rb b/lib/api/helpers/groups_helpers.rb index e0fea4c7c9609d62303b7fa1f44650d43bf24cf8..25701ff683dee8e71629c234743631694d2f6a37 100644 --- a/lib/api/helpers/groups_helpers.rb +++ b/lib/api/helpers/groups_helpers.rb @@ -21,6 +21,7 @@ module GroupsHelpers optional :mentions_disabled, type: Boolean, desc: 'Disable a group from getting mentioned' optional :lfs_enabled, type: Boolean, desc: 'Enable/disable LFS for the projects in this group' optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access' + optional :default_branch_protection, type: Integer, values: ::Gitlab::Access.protection_values, desc: 'Determine if developers can push to master' end params :optional_params_ee do diff --git a/lib/gitlab/access/branch_protection.rb b/lib/gitlab/access/branch_protection.rb index f039e5c011f92417a95561a4aa7900993a3f246d..339a99eb06868505cf9cacc3c1764c8c7043f3da 100644 --- a/lib/gitlab/access/branch_protection.rb +++ b/lib/gitlab/access/branch_protection.rb @@ -37,6 +37,10 @@ def developer_can_push? def developer_can_merge? level == PROTECTION_DEV_CAN_MERGE end + + def fully_protected? + level == PROTECTION_FULL + end end end end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 1c58c2b5c972609354beabceef66a53be5dc02ed..11c70d3aeca05812d9c1c330f4e5bbc31f3fa4ce 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -411,6 +411,13 @@ expect(group.reload.project_creation_level).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) end + it 'updates the default_branch_protection successfully' do + post :update, params: { id: group.to_param, group: { default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE } } + + expect(response).to have_gitlab_http_status(:found) + expect(group.reload.default_branch_protection).to eq(::Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + end + context 'when a project inside the group has container repositories' do before do stub_container_registry_config(enabled: true) diff --git a/spec/lib/gitlab/access/branch_protection_spec.rb b/spec/lib/gitlab/access/branch_protection_spec.rb index 7f2979e8e28d426b131e6c3f15b7f95e6e74053b..e4b763357c43b6c6f1bce280cd0a802cd4d13ba6 100644 --- a/spec/lib/gitlab/access/branch_protection_spec.rb +++ b/spec/lib/gitlab/access/branch_protection_spec.rb @@ -51,4 +51,21 @@ end end end + + describe '#fully_protected?' do + using RSpec::Parameterized::TableSyntax + + where(:level, :result) do + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false + Gitlab::Access::PROTECTION_FULL | true + end + + with_them do + it do + expect(described_class.new(level).fully_protected?).to eq(result) + end + end + end end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 2f4ab2e71dbb5d3b048ce970bb9c82f9f5167fba..8d13f3776771a7b20ea54ed21a2dd321b3efb0a6 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -46,32 +46,27 @@ expect(project_access.can_push_to_branch?('master')).to be_truthy end - it 'returns false if user is developer and project is fully protected' do - empty_project.add_developer(user) - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) - - expect(project_access.can_push_to_branch?('master')).to be_falsey - end - - it 'returns false if user is developer and it is not allowed to push new commits but can merge into branch' do - empty_project.add_developer(user) - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(project_access.can_push_to_branch?('master')).to be_falsey - end - - it 'returns true if user is developer and project is unprotected' do - empty_project.add_developer(user) - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - - expect(project_access.can_push_to_branch?('master')).to be_truthy - end - - it 'returns true if user is developer and project grants developers permission' do - empty_project.add_developer(user) - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - expect(project_access.can_push_to_branch?('master')).to be_truthy + context 'when the user is a developer' do + using RSpec::Parameterized::TableSyntax + + before do + empty_project.add_developer(user) + end + + where(:default_branch_protection_level, :result) do + Gitlab::Access::PROTECTION_NONE | true + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false + Gitlab::Access::PROTECTION_FULL | false + end + + with_them do + it do + expect(empty_project.namespace).to receive(:default_branch_protection).and_return(default_branch_protection_level).at_least(:once) + + expect(project_access.can_push_to_branch?('master')).to eq(result) + end + end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 54e54366c5a7cb7fa6c192adb63f303492454ab9..276fbc2cb54c8a063fb1e31cce6c74ec96b35aaf 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -531,6 +531,41 @@ def project_rugged(project) end end + describe "#default_branch_protection" do + let(:namespace) { create(:namespace) } + let(:default_branch_protection) { nil } + let(:group) { create(:group, default_branch_protection: default_branch_protection) } + + before do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + end + + context 'for a namespace' do + # Unlike a group, the settings of a namespace cannot be altered + # via the UI or the API. + + it 'returns the instance level setting' do + expect(namespace.default_branch_protection).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + end + end + + context 'for a group' do + context 'that has not altered the default value' do + it 'returns the instance level setting' do + expect(group.default_branch_protection).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + end + end + + context 'that has altered the default value' do + let(:default_branch_protection) { Gitlab::Access::PROTECTION_FULL } + + it 'returns the group level setting' do + expect(group.default_branch_protection).to eq(default_branch_protection) + end + end + end + end + describe '#self_and_hierarchy' do let!(:group) { create(:group, path: 'git_lab') } let!(:nested_group) { create(:group, parent: group) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2b4a832634f0d0bb35f873f9d8d8381662b123a7..4885d4b63ff4a6f44b991381bbb62a0767fcfd36 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1623,6 +1623,29 @@ end end + describe '#default_branch_protected?' do + using RSpec::Parameterized::TableSyntax + + let_it_be(:project) { create(:project) } + + subject { project.default_branch_protected? } + + where(:default_branch_protection_level, :result) do + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true + Gitlab::Access::PROTECTION_FULL | true + end + + with_them do + before do + expect(project.namespace).to receive(:default_branch_protection).and_return(default_branch_protection_level) + end + + it { is_expected.to eq(result) } + end + end + describe '#pages_url' do let(:group) { create(:group, name: group_name) } let(:project) { create(:project, namespace: group, name: project_name) } @@ -4576,7 +4599,7 @@ def enable_lfs end it 'does not protect when branch protection is disabled' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_NONE) project.after_import @@ -4584,7 +4607,7 @@ def enable_lfs end it "gives developer access to push when branch protection is set to 'developers can push'" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) project.after_import @@ -4594,7 +4617,7 @@ def enable_lfs end it "gives developer access to merge when branch protection is set to 'developers can merge'" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) project.after_import diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 7f8a60dafa8cce34c48e96a2020f127010dedbe9..30fce1cd5c4cdf4a371567f9e8ec1cf046224308 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -164,31 +164,45 @@ end end - context "new project" do - let(:project) { create(:project) } + context 'new project' do + using RSpec::Parameterized::TableSyntax - it 'returns false when default_protected_branch is unprotected' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + let(:project) { create(:project) } - expect(described_class.protected?(project, 'master')).to be false - end + context 'when the group has set their own default_branch_protection level' do + where(:default_branch_protection_level, :result) do + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true + Gitlab::Access::PROTECTION_FULL | true + end - it 'returns false when default_protected_branch lets developers push' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + with_them do + it 'protects the default branch based on the default branch protection setting of the group' do + expect(project.namespace).to receive(:default_branch_protection).and_return(default_branch_protection_level) - expect(described_class.protected?(project, 'master')).to be false + expect(described_class.protected?(project, 'master')).to eq(result) + end + end end - it 'returns true when default_branch_protection does not let developers push but let developer merge branches' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(described_class.protected?(project, 'master')).to be true - end + context 'when the group has not set their own default_branch_protection level' do + where(:default_branch_protection_level, :result) do + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true + Gitlab::Access::PROTECTION_FULL | true + end - it 'returns true when default_branch_protection is in full protection' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) + with_them do + before do + stub_application_setting(default_branch_protection: default_branch_protection_level) + end - expect(described_class.protected?(project, 'master')).to be true + it 'protects the default branch based on the instance level default branch protection setting' do + expect(described_class.protected?(project, 'master')).to eq(result) + end + end end end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index e849924684095a7f5eab082c431a8654d3fe8289..fb564bb398b70cedcf685720d270343372cc0cdf 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -545,7 +545,8 @@ def response_project_ids(json_response, key) name: new_group_name, request_access_enabled: true, project_creation_level: "noone", - subgroup_creation_level: "maintainer" + subgroup_creation_level: "maintainer", + default_branch_protection: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } expect(response).to have_gitlab_http_status(:ok) @@ -566,6 +567,7 @@ def response_project_ids(json_response, key) expect(json_response['projects'].length).to eq(2) expect(json_response['shared_projects']).to be_an Array expect(json_response['shared_projects'].length).to eq(0) + expect(json_response['default_branch_protection']).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) end it 'returns 404 for a non existing group' do diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb index 8b4f45010ed777a85c3eb4bfcd223781e6a021bc..d7357cf4d0b2cd706b979140cb105a668ba00a9a 100644 --- a/spec/services/git/branch_push_service_spec.rb +++ b/spec/services/git/branch_push_service_spec.rb @@ -186,7 +186,7 @@ end it "when pushing a branch for the first time with default branch protection disabled" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_NONE) expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") @@ -195,7 +195,7 @@ end it "when pushing a branch for the first time with default branch protection set to 'developers can push'" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") @@ -208,7 +208,7 @@ end it "when pushing a branch for the first time with an existing branch permission configured" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) create(:protected_branch, :no_one_can_push, :developers_can_merge, project: project, name: 'master') expect(project).to receive(:execute_hooks) @@ -223,7 +223,7 @@ end it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") diff --git a/spec/services/projects/protect_default_branch_service_spec.rb b/spec/services/projects/protect_default_branch_service_spec.rb index c145b2c06c633c5c92fd2dc17faaeba993abff03..c0b819ab17b505153edfcd7585f2804f648ee88c 100644 --- a/spec/services/projects/protect_default_branch_service_spec.rb +++ b/spec/services/projects/protect_default_branch_service_spec.rb @@ -4,7 +4,7 @@ describe Projects::ProtectDefaultBranchService do let(:service) { described_class.new(project) } - let(:project) { instance_spy(Project) } + let(:project) { create(:project) } describe '#execute' do before do @@ -147,7 +147,7 @@ describe '#protect_branch?' do context 'when default branch protection is disabled' do it 'returns false' do - allow(Gitlab::CurrentSettings) + allow(project.namespace) .to receive(:default_branch_protection) .and_return(Gitlab::Access::PROTECTION_NONE) @@ -157,7 +157,7 @@ context 'when default branch protection is enabled' do before do - allow(Gitlab::CurrentSettings) + allow(project.namespace) .to receive(:default_branch_protection) .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) @@ -199,7 +199,7 @@ describe '#push_access_level' do context 'when developers can push' do it 'returns the DEVELOPER access level' do - allow(Gitlab::CurrentSettings) + allow(project.namespace) .to receive(:default_branch_protection) .and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) @@ -209,7 +209,7 @@ context 'when developers can not push' do it 'returns the MAINTAINER access level' do - allow(Gitlab::CurrentSettings) + allow(project.namespace) .to receive(:default_branch_protection) .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) @@ -221,7 +221,7 @@ describe '#merge_access_level' do context 'when developers can merge' do it 'returns the DEVELOPER access level' do - allow(Gitlab::CurrentSettings) + allow(project.namespace) .to receive(:default_branch_protection) .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) @@ -231,7 +231,7 @@ context 'when developers can not merge' do it 'returns the MAINTAINER access level' do - allow(Gitlab::CurrentSettings) + allow(project.namespace) .to receive(:default_branch_protection) .and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH)