From 2d340497aca583d5971d48719b6bc7a81ff1c033 Mon Sep 17 00:00:00 2001 From: Illya Date: Wed, 26 Apr 2023 09:43:04 +0300 Subject: [PATCH 1/3] Add option to allow developer push if there are no branches * Add new access level Changelog: added --- app/models/project.rb | 8 ++- app/models/protected_branch.rb | 10 +++- .../project/repository/branches/default.md | 1 + lib/gitlab/access.rb | 6 +++ lib/gitlab/access/branch_protection.rb | 4 ++ locale/gitlab.pot | 6 +++ .../gitlab/access/branch_protection_spec.rb | 52 +++++++++++++------ spec/models/project_spec.rb | 32 ++++++++++-- spec/models/protected_branch_spec.rb | 38 ++++++++++++++ 9 files changed, 134 insertions(+), 23 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 224193fba08305..8ad45e5b87d2bc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2871,7 +2871,13 @@ def deploy_token_revoke_url_for(token) def default_branch_protected? branch_protection = Gitlab::Access::BranchProtection.new(self.namespace.default_branch_protection) - branch_protection.fully_protected? || branch_protection.developer_can_merge? + branch_protection.fully_protected? || branch_protection.developer_can_merge? || branch_protection.developer_can_initial_push? + end + + def initial_push_to_default_branch_allowed_for_developer? + branch_protection = Gitlab::Access::BranchProtection.new(self.namespace.default_branch_protection) + + !branch_protection.any? || branch_protection.developer_can_push? || branch_protection.developer_can_initial_push? end def environments_for_scope(scope) diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 09a0cfc91dc612..aebce59a040045 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -26,10 +26,16 @@ def self.get_ids_by_name(name) end 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 project.empty_repo? + member_access = project.team.max_member_access(user.id) - if project.empty_repo? && project.default_branch_protected? + # Admins are always allowed to create the default branch return true if user.admin? || user.can?(:admin_project, project) + + # Developers can push if it is allowed by default branch protection settings + if member_access == Gitlab::Access::DEVELOPER && project.initial_push_to_default_branch_allowed_for_developer? + return true + end end super diff --git a/doc/user/project/repository/branches/default.md b/doc/user/project/repository/branches/default.md index e58cc0bf6a4820..b63758986d37cc 100644 --- a/doc/user/project/repository/branches/default.md +++ b/doc/user/project/repository/branches/default.md @@ -108,6 +108,7 @@ at the [instance level](#instance-level-default-branch-protection) and but cannot force push. - **Fully protected** - Developers cannot push new commits, but maintainers can. No one can force push. +- **Fully protected after initial push** - Developers cannot push new commits except initial one, but maintainers can. No one can force push. ### Instance-level default branch protection **(FREE SELF)** diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index bafda11170aa94..d540af05eb91de 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -23,6 +23,7 @@ module Access PROTECTION_DEV_CAN_PUSH = 1 PROTECTION_FULL = 2 PROTECTION_DEV_CAN_MERGE = 3 + PROTECTION_DEV_CAN_INITIAL_PUSH = 4 # Default project creation level NO_ONE_PROJECT_ACCESS = 0 @@ -95,6 +96,11 @@ def protection_options label: s_('DefaultBranchProtection|Fully protected'), help_text: s_('DefaultBranchProtection|Developers cannot push new commits, but maintainers can. No one can force push.'), value: PROTECTION_FULL + }, + { + label: s_('DefaultBranchProtection|Fully protected after initial push'), + help_text: s_('DefaultBranchProtection|Developers cannot push new commits except initial one, but maintainers can. No one can force push.'), + value: PROTECTION_DEV_CAN_INITIAL_PUSH } ] end diff --git a/lib/gitlab/access/branch_protection.rb b/lib/gitlab/access/branch_protection.rb index 339a99eb068685..6ac8de407b0056 100644 --- a/lib/gitlab/access/branch_protection.rb +++ b/lib/gitlab/access/branch_protection.rb @@ -34,6 +34,10 @@ def developer_can_push? level == PROTECTION_DEV_CAN_PUSH end + def developer_can_initial_push? + level == PROTECTION_DEV_CAN_INITIAL_PUSH + end + def developer_can_merge? level == PROTECTION_DEV_CAN_MERGE end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 84729dba6ef8c8..b37718639d9082 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14275,6 +14275,9 @@ msgstr "" msgid "DefaultBranchProtection|Both developers and maintainers can push new commits, force push, or delete the branch." msgstr "" +msgid "DefaultBranchProtection|Developers cannot push new commits except initial one, but maintainers can. No one can force push." +msgstr "" + msgid "DefaultBranchProtection|Developers cannot push new commits, but are allowed to accept merge requests to the branch. Maintainers can push to the branch." msgstr "" @@ -14284,6 +14287,9 @@ msgstr "" msgid "DefaultBranchProtection|Fully protected" msgstr "" +msgid "DefaultBranchProtection|Fully protected after initial push" +msgstr "" + msgid "DefaultBranchProtection|Not protected" msgstr "" diff --git a/spec/lib/gitlab/access/branch_protection_spec.rb b/spec/lib/gitlab/access/branch_protection_spec.rb index 44c30d1f596cb8..5ab610dfc8fd81 100644 --- a/spec/lib/gitlab/access/branch_protection_spec.rb +++ b/spec/lib/gitlab/access/branch_protection_spec.rb @@ -7,10 +7,11 @@ describe '#any?' do where(:level, :result) do - Gitlab::Access::PROTECTION_NONE | false - Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true - Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true - Gitlab::Access::PROTECTION_FULL | true + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true + Gitlab::Access::PROTECTION_FULL | true + Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | true end with_them do @@ -20,10 +21,11 @@ describe '#developer_can_push?' do where(:level, :result) do - Gitlab::Access::PROTECTION_NONE | false - Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true - Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false - Gitlab::Access::PROTECTION_FULL | false + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false + Gitlab::Access::PROTECTION_FULL | false + Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | false end with_them do @@ -35,10 +37,11 @@ describe '#developer_can_merge?' do where(: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 | false + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true + Gitlab::Access::PROTECTION_FULL | false + Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | false end with_them do @@ -50,10 +53,11 @@ describe '#fully_protected?' do 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 + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false + Gitlab::Access::PROTECTION_FULL | true + Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | false end with_them do @@ -62,4 +66,20 @@ end end end + + describe '#developer_can_initial_push?' do + 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 | false + Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | true + end + + with_them do + it do + expect(described_class.new(level).developer_can_initial_push?).to eq(result) + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e9bb01f4b23030..0f6580529e8a1c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2684,10 +2684,34 @@ def has_external_wiki 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 + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true + Gitlab::Access::PROTECTION_FULL | true + Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | 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 'initial_push_to_default_branch_allowed_for_developer?' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, namespace: namespace) } + + subject { project.initial_push_to_default_branch_allowed_for_developer? } + + 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 + Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | true end with_them do diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index d14a7dd1a7e7a0..b8357fc30b891e 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -507,6 +507,44 @@ it { is_expected.to eq(true) } end + + context 'when project is an empty repository' do + before do + allow(project).to receive(:empty_repo?).and_return(true) + end + + context 'when user is an admin' do + let(:current_user) { admin } + + it { is_expected.to eq(true) } + end + + context 'when user is maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to eq(true) } + end + + context 'when user is developer and initial push is allowed' do + let(:current_user) { developer } + + before do + allow(project).to receive(:initial_push_to_default_branch_allowed_for_developer?).and_return(true) + end + + it { is_expected.to eq(true) } + end + + context 'when user is developer and initial push is not allowed' do + let(:current_user) { developer } + + before do + allow(project).to receive(:initial_push_to_default_branch_allowed_for_developer?).and_return(false) + end + + it { is_expected.to eq(false) } + end + end end describe '.by_name' do -- GitLab From f21a4a2b174670c7d120334d73703bbd3a2e428b Mon Sep 17 00:00:00 2001 From: Illya Date: Wed, 26 Apr 2023 22:26:04 +0300 Subject: [PATCH 2/3] Add doc about new option to group API * expose new option --- doc/api/groups.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/api/groups.md b/doc/api/groups.md index 6d295b50a01a30..ea90e67621c5de 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -834,6 +834,7 @@ The `default_branch_protection` attribute determines whether users with the Deve | `1` | Partial protection. Users with the Developer or Maintainer role can:
- Push new commits | | `2` | Full protection. Only users with the Maintainer role can:
- Push new commits | | `3` | Protected against pushes. Users with the Maintainer role can:
- Push new commits
- Force push changes
- Accept merge requests
Users with the Developer role can:
- Accept merge requests| +| `4` | Protected against pushes except inital push. User with the Developer rope can:
- Push commit to empty repository.
Users with the Maintainer role can:
- Push new commits
- Force push changes
- Accept merge requests
Users with the Developer role can:
- Accept merge requests| ## New Subgroup -- GitLab From 4cef29e3e512894feaa9a86fa35ff9bb724b9b8a Mon Sep 17 00:00:00 2001 From: Amy Qualls Date: Fri, 28 Apr 2023 07:02:57 +0000 Subject: [PATCH 3/3] Improve readability of UI * change wording on protection of initial push --- doc/api/groups.md | 2 +- doc/user/project/repository/branches/default.md | 5 ++++- lib/gitlab/access.rb | 2 +- locale/gitlab.pot | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/doc/api/groups.md b/doc/api/groups.md index ea90e67621c5de..be663a6a9c5581 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -834,7 +834,7 @@ The `default_branch_protection` attribute determines whether users with the Deve | `1` | Partial protection. Users with the Developer or Maintainer role can:
- Push new commits | | `2` | Full protection. Only users with the Maintainer role can:
- Push new commits | | `3` | Protected against pushes. Users with the Maintainer role can:
- Push new commits
- Force push changes
- Accept merge requests
Users with the Developer role can:
- Accept merge requests| -| `4` | Protected against pushes except inital push. User with the Developer rope can:
- Push commit to empty repository.
Users with the Maintainer role can:
- Push new commits
- Force push changes
- Accept merge requests
Users with the Developer role can:
- Accept merge requests| +| `4` | Protected against pushes except initial push. User with the Developer rope can:
- Push commit to empty repository.
Users with the Maintainer role can:
- Push new commits
- Force push changes
- Accept merge requests
Users with the Developer role can:
- Accept merge requests| ## New Subgroup diff --git a/doc/user/project/repository/branches/default.md b/doc/user/project/repository/branches/default.md index b63758986d37cc..96f5f6887d9b44 100644 --- a/doc/user/project/repository/branches/default.md +++ b/doc/user/project/repository/branches/default.md @@ -95,6 +95,8 @@ unless a subgroup configuration overrides it. ## Protect initial default branches **(FREE SELF)** +> Full protection after initial push [added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118729) in GitLab 16.0. + GitLab administrators and group owners can define [branch protections](../../../project/protected_branches.md) to apply to every repository's [default branch](#default-branch) at the [instance level](#instance-level-default-branch-protection) and @@ -108,7 +110,8 @@ at the [instance level](#instance-level-default-branch-protection) and but cannot force push. - **Fully protected** - Developers cannot push new commits, but maintainers can. No one can force push. -- **Fully protected after initial push** - Developers cannot push new commits except initial one, but maintainers can. No one can force push. +- **Fully protected after initial push** - Developers can push the initial commit + to a repository, but none afterward. Maintainers can always push. No one can force push. ### Instance-level default branch protection **(FREE SELF)** diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index d540af05eb91de..f1777e059ed04d 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -99,7 +99,7 @@ def protection_options }, { label: s_('DefaultBranchProtection|Fully protected after initial push'), - help_text: s_('DefaultBranchProtection|Developers cannot push new commits except initial one, but maintainers can. No one can force push.'), + help_text: s_('DefaultBranchProtection|Developers can push the initial commit to a repository, but none afterward. Maintainers can always push. No one can force push.'), value: PROTECTION_DEV_CAN_INITIAL_PUSH } ] diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b37718639d9082..8660e77a6628c4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14275,7 +14275,7 @@ msgstr "" msgid "DefaultBranchProtection|Both developers and maintainers can push new commits, force push, or delete the branch." msgstr "" -msgid "DefaultBranchProtection|Developers cannot push new commits except initial one, but maintainers can. No one can force push." +msgid "DefaultBranchProtection|Developers can push the initial commit to a repository, but none afterward. Maintainers can always push. No one can force push." msgstr "" msgid "DefaultBranchProtection|Developers cannot push new commits, but are allowed to accept merge requests to the branch. Maintainers can push to the branch." -- GitLab