Commit b355ebc4 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'jej/fix-protected-branch-validations-ce' into 'master'

Fix ProtectedBranch access level validations

See merge request gitlab-org/gitlab-ce!15586
parents 7230a344 96106287
...@@ -21,14 +21,14 @@ module Projects ...@@ -21,14 +21,14 @@ module Projects
def access_levels_options def access_levels_options
{ {
create_access_levels: levels_for_dropdown(ProtectedTag::CreateAccessLevel), create_access_levels: levels_for_dropdown,
push_access_levels: levels_for_dropdown(ProtectedBranch::PushAccessLevel), push_access_levels: levels_for_dropdown,
merge_access_levels: levels_for_dropdown(ProtectedBranch::MergeAccessLevel) merge_access_levels: levels_for_dropdown
} }
end end
def levels_for_dropdown(access_level_type) def levels_for_dropdown
roles = access_level_type.human_access_levels.map do |id, text| roles = ProtectedRefAccess::HUMAN_ACCESS_LEVELS.map do |id, text|
{ id: id, text: text, before_divider: true } { id: id, text: text, before_divider: true }
end end
{ roles: roles } { roles: roles }
......
module ProtectedBranchAccess module ProtectedBranchAccess
extend ActiveSupport::Concern extend ActiveSupport::Concern
ALLOWED_ACCESS_LEVELS ||= [
Gitlab::Access::MASTER,
Gitlab::Access::DEVELOPER,
Gitlab::Access::NO_ACCESS
].freeze
included do included do
include ProtectedRefAccess include ProtectedRefAccess
...@@ -14,18 +8,6 @@ module ProtectedBranchAccess ...@@ -14,18 +8,6 @@ module ProtectedBranchAccess
delegate :project, to: :protected_branch delegate :project, to: :protected_branch
validates :access_level, presence: true, inclusion: {
in: ALLOWED_ACCESS_LEVELS
}
def self.human_access_levels
{
Gitlab::Access::MASTER => "Masters",
Gitlab::Access::DEVELOPER => "Developers + Masters",
Gitlab::Access::NO_ACCESS => "No one"
}.with_indifferent_access
end
def check_access(user) def check_access(user)
return false if access_level == Gitlab::Access::NO_ACCESS return false if access_level == Gitlab::Access::NO_ACCESS
......
module ProtectedRefAccess module ProtectedRefAccess
extend ActiveSupport::Concern extend ActiveSupport::Concern
ALLOWED_ACCESS_LEVELS = [
Gitlab::Access::MASTER,
Gitlab::Access::DEVELOPER,
Gitlab::Access::NO_ACCESS
].freeze
HUMAN_ACCESS_LEVELS = {
Gitlab::Access::MASTER => "Masters".freeze,
Gitlab::Access::DEVELOPER => "Developers + Masters".freeze,
Gitlab::Access::NO_ACCESS => "No one".freeze
}.freeze
included do included do
scope :master, -> { where(access_level: Gitlab::Access::MASTER) } scope :master, -> { where(access_level: Gitlab::Access::MASTER) }
scope :developer, -> { where(access_level: Gitlab::Access::DEVELOPER) } scope :developer, -> { where(access_level: Gitlab::Access::DEVELOPER) }
validates :access_level, presence: true, if: :role?, inclusion: {
in: ALLOWED_ACCESS_LEVELS
}
end end
def humanize def humanize
self.class.human_access_levels[self.access_level] HUMAN_ACCESS_LEVELS[self.access_level]
end
# CE access levels are always role-based,
# where as EE allows groups and users too
def role?
true
end end
def check_access(user) def check_access(user)
......
class ProtectedTag::CreateAccessLevel < ActiveRecord::Base class ProtectedTag::CreateAccessLevel < ActiveRecord::Base
include ProtectedTagAccess include ProtectedTagAccess
validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER,
Gitlab::Access::DEVELOPER,
Gitlab::Access::NO_ACCESS] }
def self.human_access_levels
{
Gitlab::Access::MASTER => "Masters",
Gitlab::Access::DEVELOPER => "Developers + Masters",
Gitlab::Access::NO_ACCESS => "No one"
}.with_indifferent_access
end
def check_access(user) def check_access(user)
return false if access_level == Gitlab::Access::NO_ACCESS return false if access_level == Gitlab::Access::NO_ACCESS
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
**Valid access levels** **Valid access levels**
The access levels are defined in the `ProtectedBranchAccess::ALLOWED_ACCESS_LEVELS` constant. Currently, these levels are recognized: The access levels are defined in the `ProtectedRefAccess::ALLOWED_ACCESS_LEVELS` constant. Currently, these levels are recognized:
``` ```
0 => No access 0 => No access
30 => Developer access 30 => Developer access
......
...@@ -40,10 +40,10 @@ module API ...@@ -40,10 +40,10 @@ module API
params do params do
requires :name, type: String, desc: 'The name of the protected branch' requires :name, type: String, desc: 'The name of the protected branch'
optional :push_access_level, type: Integer, default: Gitlab::Access::MASTER, optional :push_access_level, type: Integer, default: Gitlab::Access::MASTER,
values: ProtectedBranchAccess::ALLOWED_ACCESS_LEVELS, values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS,
desc: 'Access levels allowed to push (defaults: `40`, master access level)' desc: 'Access levels allowed to push (defaults: `40`, master access level)'
optional :merge_access_level, type: Integer, default: Gitlab::Access::MASTER, optional :merge_access_level, type: Integer, default: Gitlab::Access::MASTER,
values: ProtectedBranchAccess::ALLOWED_ACCESS_LEVELS, values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS,
desc: 'Access levels allowed to merge (defaults: `40`, master access level)' desc: 'Access levels allowed to merge (defaults: `40`, master access level)'
end end
post ':id/protected_branches' do post ':id/protected_branches' do
......
RSpec.shared_examples "protected tags > access control > CE" do RSpec.shared_examples "protected tags > access control > CE" do
ProtectedTag::CreateAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| ProtectedRefAccess::HUMAN_ACCESS_LEVELS.each do |(access_type_id, access_type_name)|
it "allows creating protected tags that #{access_type_name} can create" do it "allows creating protected tags that #{access_type_name} can create" do
visit project_protected_tags_path(project) visit project_protected_tags_path(project)
......
shared_examples "protected branches > access control > CE" do shared_examples "protected branches > access control > CE" do
ProtectedBranch::PushAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| ProtectedRefAccess::HUMAN_ACCESS_LEVELS.each do |(access_type_id, access_type_name)|
it "allows creating protected branches that #{access_type_name} can push to" do it "allows creating protected branches that #{access_type_name} can push to" do
visit project_protected_branches_path(project) visit project_protected_branches_path(project)
...@@ -44,7 +44,7 @@ shared_examples "protected branches > access control > CE" do ...@@ -44,7 +44,7 @@ shared_examples "protected branches > access control > CE" do
end end
end end
ProtectedBranch::MergeAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| ProtectedRefAccess::HUMAN_ACCESS_LEVELS.each do |(access_type_id, access_type_name)|
it "allows creating protected branches that #{access_type_name} can merge to" do it "allows creating protected branches that #{access_type_name} can merge to" do
visit project_protected_branches_path(project) visit project_protected_branches_path(project)
......
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