Commit 3851c497 authored by Douwe Maan's avatar Douwe Maan

Merge branch '674-protected-branch-specific-people' into 'master'

Restrict pushes / merges to a protected branch to specific people

# Relevant Issues

- Closes #674 
- Related to #179 

# Screenshots

![2016-08-04_16-19-12](/uploads/c81032fe262949f3084974c4e622e9eb/2016-08-04_16-19-12.png)
![](https://gitlab.com/gitlab-org/gitlab-ee/uploads/342f06078b07b39c2c817b12c81b30fe/Screen_Shot_2016-07-27_at_6.50.56_PM.png)
![](https://gitlab.com/gitlab-org/gitlab-ee/uploads/52fe1ae3027d491270ea4845ae7ce54b/Screen_Shot_2016-07-27_at_6.51.02_PM.png)

# Tasks

- [ ]  ee#674 !581 Restrict merges to specific people
    - [x]  Implementation
        - [x]  Model changes
            - [x]  Protected branch `has_many` access levels
        - [x]  Frontend
            - [x]  How to add new users / roles?
            - [x]  Dropdown should include users
            - [x]  Dropdown shouldn't include users / roles that have already been selected
            - [x]  Allow removing users / roles
        - [x]  Removing a user from the project should remove their access (?)
    - [x]  Test/refactor
        - [x]  Extract common from {Merge,Push}AccessLevel models
        - [x]  Clean up code that's removing users/roles that are already selected
        - [x]  AccessLevelController?
        - [x]  Fix build
        - [x]  Add more tests
    - [x]  Non `:push_code` users can't push even if added to an access level
    - [x]  Remove access levels when a user is removed from a project
    - [x]  Rebase off EE master instead of the "no one can push" feature branch
        - [x]  Fix create for roles
        - [x]  Fix update for roles
        - [x]  Fix delete
        - [x]  Fix create for users
        - [x]  Fix update for users
        - [x]  Fix defaults
    - [x]  Verify
        - [x]  API
            - [x]  For a developer user
                - [x]  When they are granted access specifically to
                    - [x]  Merge
                    - [x]  Push
            - [x]  For a reporter user
                - [x]  When they are granted access specifically to
                    - [x]  Merge
                    - [x]  Push
        - [x]  Default branch protection
    - [x]  CHANGELOG
    - [x]  Screenshots
    - [x]  Only show `:push_code` users in the dropdown
    - [x]  Send email when user is added/removed from a protected branch
    - [x]  Assign to {mini,end}boss
    - [x]  Fix build
    - [x]  Implement @dbalexandre's review comments
    - [x]  EE should _add_ to CE
    - [x]  Wait for [build](https://gitlab.com/gitlab-org/gitlab-ee/commit/61edf43d31452a0b972dc880e277c825d59f764f/builds) to pass
    - [x]  Test by hand
    - [x]  Assign to endboss
    - [x]  Create CE MR to backport changes
    - [x]  Implement @Douwe's comments
        - [x]  access_level#humanize
        - [x]  Blank line in _protected_branch.html.haml
        - [x]  move stuff into shared partials? (_protected_branch_ee.html.haml)
        - [x]  Can we move stuff into shared partials? (_create_protected_branch_ee.html.haml)
        - [x]  The CE version has a bunch of extra attributes here, do we need those?
        - [x]  We can't indent this section just in EE (protected_branches/show.html.haml)
        - [x]  In EE, try not to add stuff to existing views. (protected_branches/show.html.haml)
        - [x]  If we're gonna change multiline blocks to single-line blocks, we need to so in CE too. (factory)
        - [x]  Split up `ProtectedBranches#show`
    - [x]  Wait for [build](https://gitlab.com/gitlab-org/gitlab-ee/commit/3943254d5f92c9be520f685044d23bf75282d42a/builds)
    - [x]  Implement Douwe's suggestions
    - [x]  Add uniqueness validation
    - [x]  Wait for @alfredo's UI enhancements
    - [x]  Remove `show` page access controls
    - [x]  Add more feature specs
    - [ ]  Wait for review for @alfredo's work
    - [ ]  Wait for merge


See merge request !581
parents 53da6aa9 86e098ac
......@@ -10,6 +10,7 @@ v 8.11.0 (unreleased)
- Change LdapGroupSyncWorker to use new LDAP group sync classes
- Allow LDAP `sync_ssh_keys` setting to be set to `true`
- Removed unused GitLab GEO database index
- Restrict protected branch access to specific users !581
- Enable monitoring for ES classes
- [Elastic] Improve code search
- [Elastic] Significant improvement of global search performance
......
/*= require protected_branch_access_dropdown */
(global => {
global.gl = global.gl || {};
class AllowedToMergeDropdown extends gl.ProtectedBranchAccessDropdown {
}
global.gl.AllowedToMergeDropdown = AllowedToMergeDropdown;
})(window);
/*= require protected_branch_access_dropdown */
(global => {
global.gl = global.gl || {};
class AllowedToPushDropdown extends gl.ProtectedBranchAccessDropdown {
}
global.gl.AllowedToPushDropdown = AllowedToPushDropdown;
})(window);
......@@ -470,7 +470,8 @@
} else {
if (!selected) {
value = this.options.id ? this.options.id(data) : data.id;
fieldName = this.options.fieldName;
fieldName = typeof this.options.fieldName === 'function' ? this.options.fieldName() : this.options.fieldName;
field = this.dropdown.parent().find("input[name='" + fieldName + "'][value='" + value + "']");
if (field.length) {
selected = true;
......@@ -533,7 +534,6 @@
GitLabDropdown.prototype.rowClicked = function(el) {
var field, fieldName, groupName, isInput, selectedIndex, selectedObject, value;
fieldName = this.options.fieldName;
isInput = $(this.el).is('input');
if (this.renderedData) {
groupName = el.data('group');
......@@ -545,6 +545,7 @@
selectedObject = this.renderedData[selectedIndex];
}
}
fieldName = typeof this.options.fieldName === 'function' ? this.options.fieldName(selectedObject) : this.options.fieldName;
value = this.options.id ? this.options.id(selectedObject, el) : selectedObject.id;
if (isInput) {
field = $(this.el);
......@@ -559,10 +560,9 @@
field.remove();
}
if (this.options.toggleLabel) {
return this.updateLabel(selectedObject, el, this);
} else {
return selectedObject;
this.updateLabel(selectedObject, el, this);
}
return selectedObject;
} else if (el.hasClass(INDETERMINATE_CLASS)) {
el.addClass(ACTIVE_CLASS);
el.removeClass(INDETERMINATE_CLASS);
......@@ -570,7 +570,7 @@
field.remove();
}
if (!field.length && fieldName) {
this.addInput(fieldName, value);
this.addInput(fieldName, value, selectedObject);
}
return selectedObject;
} else {
......@@ -589,7 +589,7 @@
}
if (value != null) {
if (!field.length && fieldName) {
this.addInput(fieldName, value);
this.addInput(fieldName, value, selectedObject);
} else {
field.val(value).trigger('change');
}
......@@ -598,12 +598,15 @@
}
};
GitLabDropdown.prototype.addInput = function(fieldName, value) {
GitLabDropdown.prototype.addInput = function(fieldName, value, selectedObject) {
var $input;
$input = $('<input>').attr('type', 'hidden').attr('name', fieldName).val(value);
if (this.options.inputId != null) {
$input.attr('id', this.options.inputId);
}
if (selectedObject && selectedObject.type) {
$input.attr('data-type', selectedObject.type);
}
return this.dropdown.before($input);
};
......
......@@ -3,22 +3,177 @@
gl.ProtectedBranchAccessDropdown = class {
constructor(options) {
const { $dropdown, data, onSelect } = options;
const { $dropdown, onSelect, onHide, accessLevel, accessLevelsData } = options;
const self = this;
this.accessLevel = accessLevel;
this.accessLevelsData = accessLevelsData;
this.$dropdown = $dropdown;
this.$wrap = this.$dropdown.closest(`.${this.accessLevel}-container`);
this.usersPath = '/autocomplete/users.json';
this.inputCount = 0;
this.defaultLabel = this.$dropdown.data('defaultLabel');
$dropdown.glDropdown({
data: data,
selectable: true,
inputId: $dropdown.data('input-id'),
fieldName: $dropdown.data('field-name'),
toggleLabel(item) {
return item.text;
filterable: true,
filterRemote: true,
data: this.getData.bind(this),
multiSelect: $dropdown.hasClass('js-multiselect'),
renderRow: this.renderRow.bind(this),
toggleLabel: this.toggleLabel.bind(this),
fieldName: this.fieldName.bind(this),
hidden() {
// Here because last selected item is not considered after first close
this.activeIds = self.getActiveIds();
if (onHide) {
onHide();
}
},
setActiveIds() {
// Needed for pre select options
this.activeIds = self.getActiveIds();
},
clicked(item, $el, e) {
e.preventDefault();
onSelect();
self.inputCount++;
if (onSelect) {
onSelect(item, $el, self);
}
}
});
}
toggleLabel(selectedItem, el) {
let currentItems = this.$dropdown.siblings('.dropdown-menu').find('.is-active');
let types = _.groupBy(currentItems, (item) => { return item.dataset.type; });
let label = [];
if (currentItems.length) {
Object.keys(types).map((type) => {
let numberOfTypes = types[type].length;
let text = numberOfTypes === 1 ? type : `${type}s`;
label.push(`${numberOfTypes} ${text}`);
});
} else {
label.push(this.defaultLabel);
}
return label.join(' and ');
}
getData(query, callback) {
this.getUsers(query).done((response) => {
let data = this.consolidateData(response);
callback(data);
}).error(() => {
new Flash('Failed to load users.');
});
}
consolidateData(response, callback) {
let consolidatedData;
let users = response.map((user) => {
user.type = 'user';
return user;
});
let mergeAccessLevels = this.accessLevelsData.map((level) => {
level.type = 'role';
return level;
});
consolidatedData = mergeAccessLevels;
if (users.length) {
consolidatedData = mergeAccessLevels.concat(['divider'], users);
}
return consolidatedData;
}
getUsers(query) {
return $.ajax({
dataType: 'json',
url: this.buildUrl(this.usersPath),
data: {
search: query,
per_page: 20,
active: true,
project_id: gon.current_project_id,
push_code: true,
}
});
}
buildUrl(url) {
if (gon.relative_url_root != null) {
url = gon.relative_url_root.replace(/\/$/, '') + url;
}
return url;
}
renderRow(item, instance) {
// Dectect if the current item is already saved so we can add
// the `is-active` class so the item looks as marked
const isActive = _.findWhere(instance.activeIds, { id: item.id, type: item.type }) ? 'is-active' : '';
if (item.type === 'user') {
return this.userRowHtml(item, isActive);
} else if (item.type === 'role') {
return this.roleRowHtml(item, isActive);
}
}
userRowHtml(user, isActive) {
const avatarHtml = `<img src='${user.avatar_url}' class='avatar avatar-inline' width='30'>`;
const nameHtml = `<strong class='dropdown-menu-user-full-name'>${user.name}</strong>`;
const usernameHtml = `<span class='dropdown-menu-user-username'>${user.username}</span>`;
return `<li><a href='#' class='${isActive ? 'is-active' : ''}' data-type='${user.type}'>${avatarHtml} ${nameHtml} ${usernameHtml}</a></li>`;
}
roleRowHtml(role, isActive) {
return `<li><a href='#' class='${isActive ? 'is-active' : ''}' data-type='${role.type}'>${role.text}</a></li>`;
}
fieldName(selectedItem) {
let fieldName = '';
let typeToName = {
role: 'access_level',
user: 'user_id',
};
let $input = this.$wrap.find(`input[data-type][value="${selectedItem.id}"]`);
if ($input.length) {
// If input exists return actual name
fieldName = $input.attr('name');
} else {
// If not suggest a name
fieldName = `protected_branch[${this.accessLevel}_attributes][${this.inputCount}][access_level]`; // Role by default
if (selectedItem.type === 'user') {
fieldName = `protected_branch[${this.accessLevel}_attributes][${this.inputCount}][user_id]`;
}
}
return fieldName;
}
getActiveIds() {
let selected = [];
this.$wrap
.find('input[data-type]')
.map((i, el) => {
const $el = $(el);
selected.push({
id: parseInt($el.val()),
type: $el.data('type')
});
});
return selected;
}
}
})(window);
(global => {
global.gl = global.gl || {};
const ACCESS_LEVELS = {
MERGE: 'merge_access_levels',
PUSH: 'push_access_levels',
};
gl.ProtectedBranchCreate = class {
constructor() {
this.$wrap = this.$form = $('#new_protected_branch');
......@@ -15,41 +20,35 @@
this.onSelectCallback = this.onSelect.bind(this);
// Allowed to Merge dropdown
new gl.ProtectedBranchAccessDropdown({
new gl.AllowedToMergeDropdown({
accessLevel: ACCESS_LEVELS.MERGE,
$dropdown: $allowedToMergeDropdown,
data: gon.merge_access_levels,
accessLevelsData: gon.merge_access_levels,
onSelect: this.onSelectCallback
});
// Allowed to Push dropdown
new gl.ProtectedBranchAccessDropdown({
new gl.AllowedToPushDropdown({
accessLevel: ACCESS_LEVELS.PUSH,
$dropdown: $allowedToPushDropdown,
data: gon.push_access_levels,
accessLevelsData: gon.push_access_levels,
onSelect: this.onSelectCallback
});
// Select default
$allowedToPushDropdown.data('glDropdown').selectRowAtIndex(0);
$allowedToMergeDropdown.data('glDropdown').selectRowAtIndex(0);
// Protected branch dropdown
new ProtectedBranchDropdown({
new gl.ProtectedBranchDropdown({
$dropdown: this.$wrap.find('.js-protected-branch-select'),
onSelect: this.onSelectCallback
});
}
// This will run after clicked callback
// Enable submit button after selecting an option
onSelect() {
// Enable submit button
const $branchInput = this.$wrap.find('input[name="protected_branch[name]"]');
const $allowedToMergeInput = this.$wrap.find('input[name="protected_branch[merge_access_levels_attributes][0][access_level]"]');
const $allowedToPushInput = this.$wrap.find('input[name="protected_branch[push_access_levels_attributes][0][access_level]"]');
const $allowedToMergeInputs = this.$wrap.find('input[name^="protected_branch[merge_access_levels_attributes]"]');
const $allowedToPushInputs = this.$wrap.find('input[name^="protected_branch[push_access_levels_attributes]"]');
if ($branchInput.val() && $allowedToMergeInput.val() && $allowedToPushInput.val()){
this.$form.find('input[type="submit"]').removeAttr('disabled');
}
this.$form.find('input[type="submit"]').attr('disabled', !($branchInput.val() && $allowedToMergeInputs.length && $allowedToPushInputs.length));
}
}
......
class ProtectedBranchDropdown {
(global => {
global.gl = global.gl || {};
class ProtectedBranchDropdown {
constructor(options) {
this.onSelect = options.onSelect;
this.$dropdown = options.$dropdown;
......@@ -72,4 +75,7 @@ class ProtectedBranchDropdown {
this.$dropdownFooter.toggleClass('hidden', !branchName);
}
}
}
global.gl.ProtectedBranchDropdown = ProtectedBranchDropdown;
})(window);
(global => {
global.gl = global.gl || {};
const LEVEL_TYPES = {
USER: 'user',
ROLE: 'role',
};
const ACCESS_LEVELS = {
MERGE: 'merge_access_levels',
PUSH: 'push_access_levels',
};
gl.ProtectedBranchEdit = class {
constructor(options) {
this.$wraps = {};
this.hasChanges = false;
this.$wrap = options.$wrap;
this.$allowedToMergeDropdown = this.$wrap.find('.js-allowed-to-merge');
this.$allowedToPushDropdown = this.$wrap.find('.js-allowed-to-push');
this.$wraps[ACCESS_LEVELS.MERGE] = this.$allowedToMergeDropdown.closest(`.${ACCESS_LEVELS.MERGE}-container`);
this.$wraps[ACCESS_LEVELS.PUSH] = this.$allowedToPushDropdown.closest(`.${ACCESS_LEVELS.PUSH}-container`);
this.buildDropdowns();
// Save initial state with existing dropdowns
this.state = {};
for (let ACCESS_LEVEL in ACCESS_LEVELS) {
this.state[`${ACCESS_LEVELS[ACCESS_LEVEL]}_attributes`] = this.getAccessLevelDataFromInputs(ACCESS_LEVEL);
}
}
buildDropdowns() {
// Allowed to merge dropdown
new gl.ProtectedBranchAccessDropdown({
new gl.AllowedToMergeDropdown({
accessLevel: ACCESS_LEVELS.MERGE,
accessLevelsData: gon.merge_access_levels,
$dropdown: this.$allowedToMergeDropdown,
data: gon.merge_access_levels,
onSelect: this.onSelect.bind(this)
onSelect: this.onSelectOption.bind(this),
onHide: this.onDropdownHide.bind(this)
});
// Allowed to push dropdown
new gl.ProtectedBranchAccessDropdown({
new gl.AllowedToPushDropdown({
accessLevel: ACCESS_LEVELS.PUSH,
accessLevelsData: gon.push_access_levels,
$dropdown: this.$allowedToPushDropdown,
data: gon.push_access_levels,
onSelect: this.onSelect.bind(this)
onSelect: this.onSelectOption.bind(this),
onHide: this.onDropdownHide.bind(this)
});
}
onSelect() {
const $allowedToMergeInput = this.$wrap.find(`input[name="${this.$allowedToMergeDropdown.data('fieldName')}"]`);
const $allowedToPushInput = this.$wrap.find(`input[name="${this.$allowedToPushDropdown.data('fieldName')}"]`);
onSelectOption(item, $el, dropdownInstance) {
this.hasChanges = true;
let itemToDestroy;
let accessLevelState = this.state[`${dropdownInstance.accessLevel}_attributes`];
// If element is not active it means it has been active
if (!$el.is('.is-active')) {
// We need to know if the selected item was already saved
// if so we need to append the `_destroy` property
// in order to delete it from the database
// Retrieve the full data of the item we just selected
if (item.type === LEVEL_TYPES.USER) {
itemToDestroy = _.findWhere(accessLevelState, { user_id: item.id });
} else if (item.type === LEVEL_TYPES.ROLE) {
itemToDestroy = _.findWhere(accessLevelState, { access_level: item.id });
}
// State updated by reference
itemToDestroy['_destroy'] = 1;
}
}
onDropdownHide() {
if (!this.hasChanges) return;
$.ajax({
this.hasChanges = true;
this.updatePermissions();
}
updatePermissions() {
let formData = {};
for (let ACCESS_LEVEL in ACCESS_LEVELS) {
formData[`${ACCESS_LEVELS[ACCESS_LEVEL]}_attributes`] = this.consolidateAccessLevelData(ACCESS_LEVEL);
}
return $.ajax({
type: 'POST',
url: this.$wrap.data('url'),
dataType: 'json',
data: {
_method: 'PATCH',
id: this.$wrap.data('banchId'),
protected_branch: {
merge_access_levels_attributes: [{
id: this.$allowedToMergeDropdown.data('access-level-id'),
access_level: $allowedToMergeInput.val()
}],
push_access_levels_attributes: [{
id: this.$allowedToPushDropdown.data('access-level-id'),
access_level: $allowedToPushInput.val()
}]
}
protected_branch: formData
},
success: () => {
success: (response) => {
this.$wrap.effect('highlight');
this.hasChanges = false;
// Update State
for (let ACCESS_LEVEL in ACCESS_LEVELS) {
let accessLevel = ACCESS_LEVELS[ACCESS_LEVEL];
this.state[`${accessLevel}_attributes`] = [];
for (let i = 0; i < response[accessLevel].length; i++) {
let access = response[accessLevel][i];
let accessData = {};
if (access.user_id) {
accessData = {
id: access.id,
user_id: access.user_id,
};
} else {
accessData ={
id: access.id,
access_level: access.access_level,
};
}
this.state[`${accessLevel}_attributes`].push(accessData);
}
}
},
error() {
$.scrollTo(0);
......@@ -58,6 +134,83 @@
}
});
}
consolidateAccessLevelData(accessLevelKey) {
// State takes precedence
let accessLevel = ACCESS_LEVELS[accessLevelKey];
let accessLevelData = [];
let dataFromInputs = this.getAccessLevelDataFromInputs(accessLevelKey);
// Collect and format items that will be sent to the server
for (let i = 0; i < dataFromInputs.length; i++) {
let inState;
let adding;
var userId = parseInt(dataFromInputs[i].user_id);
// Inputs give us the *state* of the dropdown on the frontend before it's persisted
// so we need to compare them with the persisted state which can be get or set on this.state
if (userId) {
adding = LEVEL_TYPES.USER;
inState = _.findWhere(this.state[`${accessLevel}_attributes`], { user_id: userId });
} else {
adding = LEVEL_TYPES.ROLE;
inState = _.findWhere(this.state[`${accessLevel}_attributes`], { access_level: parseInt(dataFromInputs[i].access_level) });
}
if (inState) {
// collect item if it's already saved
accessLevelData.push(inState);
} else {
// format item according the level type
if (adding === LEVEL_TYPES.USER) {
accessLevelData.push({
user_id: parseInt(dataFromInputs[i].user_id)
});
} else if (adding === LEVEL_TYPES.ROLE) {
accessLevelData.push({
access_level: parseInt(dataFromInputs[i].access_level)
});
}
}
}
// Since we didn't considered inputs that were removed
// (because they are not present in the DOM anymore)
// We can get them from the state
this.state[`${accessLevel}_attributes`].forEach((item) => {
if (item._destroy) {
accessLevelData.push(item);
}
});
return accessLevelData;
}
getAccessLevelDataFromInputs(accessLevelKey) {
let accessLevels = [];
let accessLevel = ACCESS_LEVELS[accessLevelKey];
this.$wraps[accessLevel]
.find(`input[name^="protected_branch[${accessLevel}_attributes]"]`)
.map((i, el) => {
const $el = $(el);
const type = $el.data('type');
const value = parseInt($el.val());
const id = parseInt($el.data('id'));
let obj = {};
if (type === LEVEL_TYPES.ROLE) {
obj.access_level = value
} else if (type === LEVEL_TYPES.USER) {
obj.user_id = value;
}
if (id) obj.id = id;
accessLevels.push(obj);
});
return accessLevels;
}
}
})(window);
// Modified version of `UsersSelect` for use with access selection for protected branches.
//
// - Selections are sent via AJAX if `saveOnSelect` is `true`
// - If `saveOnSelect` is `false`, the dropdown element must have a `field-name` data
// attribute. The DOM must contain two fields - "#{field-name}[access_level]" and "#{field_name}[user_id]"
// where the selections will be stored.
class ProtectedBranchesAccessSelect {
constructor(container, saveOnSelect, selectDefault) {
this.container = container;
this.saveOnSelect = saveOnSelect;
this.selectDefault = selectDefault;
this.usersPath = "/autocomplete/users.json";
this.setupDropdown(".allowed-to-merge", gon.merge_access_levels, gon.selected_merge_access_levels);
this.setupDropdown(".allowed-to-push", gon.push_access_levels, gon.selected_push_access_levels);
}
setupDropdown(className, accessLevels, selectedAccessLevels) {
this.container.find(className).each((i, element) => {
var dropdown = $(element).glDropdown({
clicked: _.chain(this.onSelect).partial(element).bind(this).value(),
data: (term, callback) => {
this.getUsers(term, (users) => {
users = _(users).map((user) => _(user).extend({ type: "user" }));
accessLevels = _(accessLevels).map((accessLevel) => _(accessLevel).extend({ type: "role" }));
var accessLevelsWithUsers = accessLevels.concat("divider", users);
callback(_(accessLevelsWithUsers).reject((item) => _.contains(selectedAccessLevels, item.id)));
});
},
filterable: true,
filterRemote: true,
search: { fields: ['name', 'username'] },
selectable: true,
toggleLabel: (selected) => $(element).data('default-label'),
renderRow: (user) => {
if (user.before_divider != null) {
return "<li> <a href='#'>" + user.text + " </a> </li>";
}
var username = user.username ? "@" + user.username : null;
var avatar = user.avatar_url ? user.avatar_url : false;
var img = avatar ? "<img src='" + avatar + "' class='avatar avatar-inline' width='30' />" : '';
var listWithName = "<li> <a href='#' class='dropdown-menu-user-link'> " + img + " <strong class='dropdown-menu-user-full-name'> " + user.name + " </strong>";
var listWithUserName = username ? "<span class='dropdown-menu-user-username'> " + username + " </span>" : '';
var listClosingTags = "</a> </li>";
return listWithName + listWithUserName + listClosingTags;
}
});
if (this.selectDefault) {
$(dropdown).find('.dropdown-toggle-text').text(accessLevels[0].text);
}
});
}
onSelect(dropdown, selected, element, e) {
$(dropdown).find('.dropdown-toggle-text').text(selected.text || selected.name);
var access_level = selected.type == 'user' ? 40 : selected.id;
var user_id = selected.type == 'user' ? selected.id : null;
if (this.saveOnSelect) {
$.ajax({
type: "POST",
url: $(dropdown).data('url'),
dataType: "json",
data: {
_method: 'PATCH',
id: $(dropdown).data('id'),
protected_branch: {
["" + ($(dropdown).data('type')) + "_attributes"]: [{
access_level: access_level,
user_id: user_id
}]
}
},
success: function() {
var row;
row = $(e.target);
row.closest('tr').effect('highlight');
row.closest('td').find('.access-levels-list').append("<li>" + selected.name + "</li>");
location.reload();
},
error: function() {
new Flash("Failed to update branch!", "alert");
}
});
} else {
var fieldName = $(dropdown).data('field-name');
$("input[name='" + fieldName + "[access_level]']").val(access_level);
$("input[name='" + fieldName + "[user_id]']").val(user_id);
}
}
getUsers(query, callback) {
var url = this.buildUrl(this.usersPath);
return $.ajax({
url: url,
data: {
search: query,
per_page: 20,
active: true,
project_id: gon.current_project_id,
push_code: true
},
dataType: "json"
}).done(function(users) {
callback(users);
});
}
buildUrl(url) {
if (gon.relative_url_root != null) {
url = gon.relative_url_root.replace(/\/$/, '') + url;
}
return url;
}
}
......@@ -662,6 +662,15 @@ pre.light-well {
}
}
a.allowed-to-merge, a.allowed-to-push {
cursor: pointer;
cursor: hand;
}
.protected-branch-push-access-list, .protected-branch-merge-access-list {
a { color: #fff; }
}
.protected-branches-list {
a {
color: $gl-gray;
......
......@@ -9,15 +9,7 @@ class AutocompleteController < ApplicationController
@users = @users.where.not(id: params[:skip_users]) if params[:skip_users].present?
@users = @users.active
@users = @users.reorder(:name)
if params[:push_code_to_protected_branches].present? && params[:project_id].present?
project = Project.find_by(id: params[:project_id])
@users = @users.to_a.
select { |user| user.can?(:push_code_to_protected_branches, project) }.
take(Kaminari.config.default_per_page)
else
@users = @users.page(params[:page])
end
@users = load_users_by_ability || @users.page(params[:page])
if params[:search].blank?
# Include current user if available to filter by "Me"
......@@ -56,6 +48,18 @@ class AutocompleteController < ApplicationController
private
def load_users_by_ability
ability = :push_code_to_protected_branches if params[:push_code_to_protected_branches].present?
ability = :push_code if params[:push_code].present?
return if params[:project_id].blank?
return if ability.blank?
@users.to_a.
select { |user| user.can?(ability, @project) }.
take(Kaminari.config.default_per_page)
end
def find_users
@users =
if @project
......
class Projects::ProtectedBranches::ApplicationController < Projects::ApplicationController
protected
def load_protected_branch
@protected_branch = @project.protected_branches.find(params[:protected_branch_id])
end
end
module Projects
module ProtectedBranches
class MergeAccessLevelsController < ProtectedBranches::ApplicationController
before_action :load_protected_branch, only: [:destroy]
def destroy
@merge_access_level = @protected_branch.merge_access_levels.find(params[:id])
@merge_access_level.destroy
redirect_to namespace_project_protected_branch_path(@project.namespace, @project, @protected_branch),
notice: "Successfully deleted. #{@merge_access_level.humanize} will not be able to merge into this protected branch."
end
end
end
end
module Projects
module ProtectedBranches
class PushAccessLevelsController < ProtectedBranches::ApplicationController
before_action :load_protected_branch, only: [:destroy]
def destroy
@push_access_level = @protected_branch.push_access_levels.find(params[:id])
@push_access_level.destroy
redirect_to namespace_project_protected_branch_path(@project.namespace, @project, @protected_branch),
notice: "Successfully deleted. #{@push_access_level.humanize} will not be able to push to this protected branch."
end
end
end
end
......@@ -14,6 +14,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
def create
@protected_branch = ::ProtectedBranches::CreateService.new(@project, current_user, protected_branch_params).execute
if @protected_branch.persisted?
redirect_to namespace_project_protected_branches_path(@project.namespace, @project)
else
......@@ -32,7 +33,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
if @protected_branch.valid?
respond_to do |format|
format.json { render json: @protected_branch, status: :ok }
format.json { render json: @protected_branch, status: :ok, include: [:merge_access_levels, :push_access_levels] }
end
else
respond_to do |format|
......@@ -58,8 +59,8 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
def protected_branch_params
params.require(:protected_branch).permit(:name,
merge_access_levels_attributes: [:access_level, :id],
push_access_levels_attributes: [:access_level, :id])
merge_access_levels_attributes: [:access_level, :id, :user_id, :_destroy],
push_access_levels_attributes: [:access_level, :id, :user_id, :_destroy])
end
def load_protected_branches
......@@ -69,12 +70,15 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
def access_levels_options
{
push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } },
merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } }
merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } },
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 }
}
end
def load_gon_index
params = { open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } } }
params.merge!(current_project_id: @project.id) if @project
gon.push(params.merge(access_levels_options))
end
end
......@@ -9,6 +9,10 @@ module DropdownsHelper
dropdown_output = dropdown_toggle(toggle_text, data_attr, options)
if options.has_key?(:toggle_link)
dropdown_output = dropdown_toggle_link(toggle_text, data_attr, options)
end
dropdown_output << content_tag(:div, class: "dropdown-menu dropdown-select #{options[:dropdown_class] if options.has_key?(:dropdown_class)}") do
output = ""
......@@ -47,6 +51,11 @@ module DropdownsHelper
end
end
def dropdown_toggle_link(toggle_text, data_attr, options = {})
output = content_tag(:a, toggle_text, class: "dropdown-toggle-text #{options[:toggle_class] if options.has_key?(:toggle_class)}", id: (options[:id] if options.has_key?(:id)), data: data_attr)
output.html_safe
end
def dropdown_title(title, back: false)
content_tag :div, class: "dropdown-title" do
title_output = ""
......
module ProtectedBranchAccess
extend ActiveSupport::Concern
included do
validates :user_id, uniqueness: { scope: :protected_branch, allow_nil: true }
validates :access_level, uniqueness: { scope: :protected_branch, unless: :user_id?, conditions: -> { where(user_id: nil) } }
end
def type
if self.user.present?
:user
else
:role
end
end
def humanize
return self.user.name if self.user.present?
self.class.human_access_levels[self.access_level]
end
end
......@@ -14,6 +14,7 @@ class ProjectMember < Member
scope :in_project, ->(project) { where(source_id: project.id) }
before_destroy :delete_member_todos
before_destroy :delete_member_branch_protection
class << self
# Add users to project teams with passed access option
......@@ -105,6 +106,13 @@ class ProjectMember < Member
user.todos.where(project_id: source_id).destroy_all if user
end
def delete_member_branch_protection
if user.present? && project.present?
project.protected_branches.merge_access_by_user(user).destroy_all
project.protected_branches.push_access_by_user(user).destroy_all
end
end
def send_invite
notification_service.invite_project_member(self, @raw_invite_token) unless @skip_notification
......
......@@ -8,11 +8,19 @@ class ProtectedBranch < ActiveRecord::Base
has_many :merge_access_levels, dependent: :destroy
has_many :push_access_levels, dependent: :destroy
validates_length_of :merge_access_levels, is: 1, message: "are restricted to a single instance per protected branch."
validates_length_of :push_access_levels, is: 1, message: "are restricted to a single instance per protected branch."
validates_length_of :merge_access_levels, minimum: 0
validates_length_of :push_access_levels, minimum: 0
accepts_nested_attributes_for :push_access_levels
accepts_nested_attributes_for :merge_access_levels
accepts_nested_attributes_for :push_access_levels, allow_destroy: true
accepts_nested_attributes_for :merge_access_levels, allow_destroy: true
# Returns all merge access levels (for protected branches in scope) that grant merge
# access to the given user.
scope :merge_access_by_user, -> (user) { MergeAccessLevel.joins(:protected_branch).where(protected_branch_id: self.ids).merge(MergeAccessLevel.by_user(user)) }
# Returns all push access levels (for protected branches in scope) that grant push
# access to the given user.
scope :push_access_by_user, -> (user) { PushAccessLevel.joins(:protected_branch).where(protected_branch_id: self.ids).merge(PushAccessLevel.by_user(user)) }
def commit
project.commit(self.name)
......@@ -46,6 +54,24 @@ class ProtectedBranch < ActiveRecord::Base
self.name && self.name.include?('*')
end
# Returns a hash were keys are types of push access levels (user, role), and
# values are the number of access levels of the particular type.
def push_access_level_frequencies
push_access_levels.reduce(Hash.new(0)) do |frequencies, access_level|
frequencies[access_level.type] = frequencies[access_level.type] + 1
frequencies
end
end
# Returns a hash were keys are types of merge access levels (user, role), and
# values are the number of access levels of the particular type.
def merge_access_level_frequencies
merge_access_levels.reduce(Hash.new(0)) do |frequencies, access_level|
frequencies[access_level.type] = frequencies[access_level.type] + 1
frequencies
end
end
protected
def exact_match?(branch_name)
......
......@@ -2,11 +2,15 @@ class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base
include ProtectedBranchAccess
belongs_to :protected_branch
belongs_to :user
delegate :project, to: :protected_branch
validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER,
Gitlab::Access::DEVELOPER] }
scope :by_user, -> (user) { where(user: user ) }
def self.human_access_levels
{
Gitlab::Access::MASTER => "Masters",
......@@ -16,6 +20,7 @@ class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base
def check_access(user)
return true if user.is_admin?
return user.id == self.user_id if self.user.present?
project.team.max_member_access(user.id) >= access_level
end
......
......@@ -2,12 +2,16 @@ class ProtectedBranch::PushAccessLevel < ActiveRecord::Base
include ProtectedBranchAccess
belongs_to :protected_branch
belongs_to :user
delegate :project, to: :protected_branch
validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER,
Gitlab::Access::DEVELOPER,
Gitlab::Access::NO_ACCESS] }
scope :by_user, -> (user) { where(user: user ) }
def self.human_access_levels
{
Gitlab::Access::MASTER => "Masters",
......@@ -19,6 +23,7 @@ class ProtectedBranch::PushAccessLevel < ActiveRecord::Base
def check_access(user)
return false if access_level == Gitlab::Access::NO_ACCESS
return true if user.is_admin?
return user.id == self.user_id if self.user.present?
project.team.max_member_access(user.id) >= access_level
end
......
......@@ -93,6 +93,10 @@ class User < ActiveRecord::Base
has_many :award_emoji, dependent: :destroy
has_many :path_locks, dependent: :destroy
# Protected Branch Access
has_many :protected_branch_merge_access_levels, dependent: :destroy, class_name: ProtectedBranch::MergeAccessLevel
has_many :protected_branch_push_access_levels, dependent: :destroy, class_name: ProtectedBranch::PushAccessLevel
#
# Validations
#
......
- default_label = 'Select'
- dropdown_label = default_label
%div{ class: "#{input_basic_name}-container" }
- if access_levels.present?
- access_levels.map.with_index do |level, i|
- if level.type == :user
- field_key = 'user_id'
- value = level.user_id
- else
- field_key = 'access_level'
- value = level.access_level
%input{ type: 'hidden', name: "protected_branch[#{input_basic_name}_attributes][#{i}][#{field_key}]",
value: value, data: { type: level.type, id: level.id } }
- dropdown_label = [pluralize(level_frequencies[:role], 'role'), pluralize(level_frequencies[:user], 'user')].to_sentence
= dropdown_tag(dropdown_label, options: { toggle_class: "#{toggle_class} js-multiselect", dropdown_class: 'dropdown-menu-user dropdown-menu-selectable', filter: true,
data: { default_label: default_label } })
......@@ -10,10 +10,12 @@
%table.table.table-bordered
%colgroup
%col{ width: "25%" }
%col{ width: "30%" }
%col{ width: "25%" }
%col{ width: "20%" }
%col{ width: "20%" }
%col{ width: "20%" }
%col{ width: "20%" }
- if can_admin_project
%col
%thead
%tr
%th Protected branch (#{@protected_branches.size})
......
......@@ -22,16 +22,18 @@
%label.col-md-2.text-right{ for: 'merge_access_levels_attributes' }
Allowed to merge:
.col-md-10
.js-allowed-to-merge-container
= dropdown_tag('Select',
options: { toggle_class: 'js-allowed-to-merge wide',
data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes' }})
options: { toggle_class: 'js-allowed-to-merge wide js-multiselect', dropdown_class: 'dropdown-menu-user dropdown-menu-selectable', filter: true,
data: { input_id: 'merge_access_levels_attributes', default_label: 'Select' } })
.form-group
%label.col-md-2.text-right{ for: 'push_access_levels_attributes' }
Allowed to push:
.col-md-10
.js-allowed-to-push-container
= dropdown_tag('Select',
options: { toggle_class: 'js-allowed-to-push wide',
data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }})
options: { toggle_class: 'js-allowed-to-push wide js-multiselect', dropdown_class: 'dropdown-menu-user dropdown-menu-selectable', filter: true,
data: { input_id: 'push_access_levels_attributes', default_label: 'Select' } })
.panel-footer
= f.submit 'Protect', class: 'btn-create btn', disabled: true
......@@ -14,7 +14,7 @@
- else
(branch was removed from repository)
= render partial: 'update_protected_branch', locals: { protected_branch: protected_branch }
= render partial: 'protected_branch_access_summary', locals: { protected_branch: protected_branch }
- if can_admin_project
%td
......
%td
= render partial: 'access_level_dropdown', locals: { protected_branch: protected_branch, access_levels: protected_branch.merge_access_levels, level_frequencies: protected_branch.merge_access_level_frequencies, input_basic_name: 'merge_access_levels', toggle_class: 'js-allowed-to-merge' }
%td
= render partial: 'access_level_dropdown', locals: { protected_branch: protected_branch, access_levels: protected_branch.push_access_levels, level_frequencies: protected_branch.push_access_level_frequencies, input_basic_name: 'push_access_levels', toggle_class: 'js-allowed-to-push' }
......@@ -5,7 +5,7 @@
%h4.prepend-top-0
= @protected_branch.name
.col-lg-9
.col-lg-9.edit_protected_branch
%h5 Matching Branches
- if @matching_branches.present?
.table-responsive
......
......@@ -820,7 +820,13 @@ Rails.application.routes.draw do
end
end
resources :protected_branches, only: [:index, :show, :create, :update, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
resources :protected_branches, only: [:index, :show, :create, :update, :destroy, :patch], constraints: { id: Gitlab::Regex.git_reference_regex } do
scope module: :protected_branches do
resources :merge_access_levels, only: [:destroy]
resources :push_access_levels, only: [:destroy]
end
end
resources :variables, only: [:index, :show, :update, :create, :destroy]
resources :triggers, only: [:index, :create, :destroy]
resource :mirror, only: [:show, :update] do
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddColumnUserIdToProtectedBranchesAccessLevels < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = true
DOWNTIME_REASON = "This migrations adds two indexes, and so requires downtime."
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
def change
add_reference :protected_branch_merge_access_levels, :user, foreign_key: true, index: true
add_reference :protected_branch_push_access_levels, :user, foreign_key: true, index: true
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AllowNullsForProtectedBranchAccessLevels < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
def change
change_column_null :protected_branch_merge_access_levels, :access_level, true
change_column_null :protected_branch_push_access_levels, :access_level, true
end
end
......@@ -975,21 +975,25 @@ ActiveRecord::Schema.define(version: 20160817154936) do
create_table "protected_branch_merge_access_levels", force: :cascade do |t|
t.integer "protected_branch_id", null: false
t.integer "access_level", default: 40, null: false
t.integer "access_level", default: 40
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "user_id"
end
add_index "protected_branch_merge_access_levels", ["protected_branch_id"], name: "index_protected_branch_merge_access", using: :btree
add_index "protected_branch_merge_access_levels", ["user_id"], name: "index_protected_branch_merge_access_levels_on_user_id", using: :btree
create_table "protected_branch_push_access_levels", force: :cascade do |t|
t.integer "protected_branch_id", null: false
t.integer "access_level", default: 40, null: false
t.integer "access_level", default: 40
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "user_id"
end
add_index "protected_branch_push_access_levels", ["protected_branch_id"], name: "index_protected_branch_push_access", using: :btree
add_index "protected_branch_push_access_levels", ["user_id"], name: "index_protected_branch_push_access_levels_on_user_id", using: :btree
create_table "protected_branches", force: :cascade do |t|
t.integer "project_id", null: false
......@@ -1307,7 +1311,9 @@ ActiveRecord::Schema.define(version: 20160817154936) do
add_foreign_key "lists", "labels"
add_foreign_key "personal_access_tokens", "users"
add_foreign_key "protected_branch_merge_access_levels", "protected_branches"
add_foreign_key "protected_branch_merge_access_levels", "users"
add_foreign_key "protected_branch_push_access_levels", "protected_branches"
add_foreign_key "remote_mirrors", "projects"
add_foreign_key "protected_branch_push_access_levels", "users"
add_foreign_key "u2f_registrations", "users"
end
......@@ -46,6 +46,21 @@ describe AutocompleteController do
it { expect(body.size).to eq 1 }
it { expect(body.first["username"]).to eq user.username }
end
describe "GET #users that can push code" do
let(:reporter_user) { create(:user) }
before do
project.team << [reporter_user, :reporter]
get(:users, project_id: project.id, push_code: 'true')
end
let(:body) { JSON.parse(response.body) }
it { expect(body).to be_kind_of(Array) }
it { expect(body.size).to eq 2 }
it { expect(body.map { |user| user["username"] }).to match_array([user.username, user2.username]) }
end
end
context 'group members' do
......
......@@ -8,22 +8,45 @@ FactoryGirl.define do
protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER)
end
transient do
authorize_user_to_push nil
authorize_user_to_merge nil
end
trait :remove_default_access_levels do
after(:build) do |protected_branch|
protected_branch.push_access_levels = []
protected_branch.merge_access_levels = []
end
end
trait :developers_can_push do
after(:create) do |protected_branch|
protected_branch.push_access_levels.first.update!(access_level: Gitlab::Access::DEVELOPER)
protected_branch.push_access_levels.create!(access_level: Gitlab::Access::DEVELOPER)
end
end
trait :developers_can_merge do
after(:create) do |protected_branch|
protected_branch.merge_access_levels.first.update!(access_level: Gitlab::Access::DEVELOPER)
protected_branch.merge_access_levels.create!(access_level: Gitlab::Access::DEVELOPER)
end
end
trait :no_one_can_push do
after(:create) do |protected_branch|
protected_branch.push_access_levels.first.update!(access_level: Gitlab::Access::NO_ACCESS)
protected_branch.push_access_levels.create!(access_level: Gitlab::Access::NO_ACCESS)
end
end
trait :masters_can_push do
after(:create) do |protected_branch|
protected_branch.push_access_levels.create!(access_level: Gitlab::Access::MASTER)
end
end
after(:create) do |protected_branch, evaluator|
protected_branch.push_access_levels.create!(user: evaluator.authorize_user_to_push) if evaluator.authorize_user_to_push
protected_branch.merge_access_levels.create!(user: evaluator.authorize_user_to_merge) if evaluator.authorize_user_to_merge
end
end
end
FactoryGirl.define do
factory :protected_branch_merge_access_level, class: ProtectedBranch::MergeAccessLevel do
user nil
protected_branch
access_level { Gitlab::Access::DEVELOPER }
end
end
FactoryGirl.define do
factory :protected_branch_push_access_level, class: ProtectedBranch::PushAccessLevel do
user nil
protected_branch
access_level { Gitlab::Access::DEVELOPER }
end
end
require 'spec_helper'
feature 'Projects > Members > Member is removed from project', feature: true do
let(:user) { create(:user) }
let(:project) { create(:project) }
background do
project.team << [user, :master]
login_as(user)
visit namespace_project_project_members_path(project.namespace, project)
end
scenario 'user is removed from project' do
within(".project_member") { find(".btn-remove").click }
expect(project.users.exists?(user.id)).to be_falsey
end
context 'when the user has been specifically allowed to access a protected branch' do
let(:other_user) { create(:user) }
let!(:matching_protected_branch) { create(:protected_branch, authorize_user_to_push: user, authorize_user_to_merge: user, project: project) }
let!(:non_matching_protected_branch) { create(:protected_branch, authorize_user_to_push: other_user, authorize_user_to_merge: other_user, project: project) }
scenario 'user leaves project' do
within(".project_member") { find(".btn-remove").click }
expect(project.users.exists?(user.id)).to be_falsey
expect(matching_protected_branch.push_access_levels.where(user: user)).not_to exist
expect(matching_protected_branch.merge_access_levels.where(user: user)).not_to exist
expect(non_matching_protected_branch.push_access_levels.where(user: other_user)).to exist
expect(non_matching_protected_branch.merge_access_levels.where(user: other_user)).to exist
end
end
end
......@@ -16,4 +16,22 @@ feature 'Projects > Members > Member leaves project', feature: true do
expect(current_path).to eq(dashboard_projects_path)
expect(project.users.exists?(user.id)).to be_falsey
end
context 'when the user has been specifically allowed to access a protected branch' do
let(:other_user) { create(:user) }
let!(:matching_protected_branch) { create(:protected_branch, authorize_user_to_push: user, authorize_user_to_merge: user, project: project) }
let!(:non_matching_protected_branch) { create(:protected_branch, authorize_user_to_push: other_user, authorize_user_to_merge: other_user, project: project) }
context 'user leaves project' do
it "removes the user's branch permissions" do
click_link 'Leave Project'
expect(current_path).to eq(dashboard_projects_path)
expect(matching_protected_branch.push_access_levels.where(user: user)).not_to exist
expect(matching_protected_branch.merge_access_levels.where(user: user)).not_to exist
expect(non_matching_protected_branch.push_access_levels.where(user: other_user)).to exist
expect(non_matching_protected_branch.merge_access_levels.where(user: other_user)).to exist
end
end
end
end
RSpec.shared_examples "protected branches > access control > CE" do
ProtectedBranch::PushAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
it "allows creating protected branches that #{access_type_name} can push to" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
within('.new_protected_branch') do
allowed_to_push_button = find(".js-allowed-to-push")
unless allowed_to_push_button.text == access_type_name
allowed_to_push_button.click
within(".dropdown.open .dropdown-menu") { click_on access_type_name }
end
end
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to eq([access_type_id])
end
it "allows updating protected branches so that #{access_type_name} can push to them" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
within(".protected-branches-list") do
find(".js-allowed-to-push").click
within('.js-allowed-to-push-container') { click_on access_type_name }
end
wait_for_ajax
expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to include(access_type_id)
end
end
ProtectedBranch::MergeAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
it "allows creating protected branches that #{access_type_name} can merge to" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
within('.new_protected_branch') do
allowed_to_merge_button = find(".js-allowed-to-merge")
unless allowed_to_merge_button.text == access_type_name
allowed_to_merge_button.click
within(".dropdown.open .dropdown-menu") { click_on access_type_name }
end
end
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to eq([access_type_id])
end
it "allows updating protected branches so that #{access_type_name} can merge to them" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
within(".protected-branches-list") do
find(".js-allowed-to-merge").click
within('.js-allowed-to-merge-container') { click_on access_type_name }
end
wait_for_ajax
expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to include(access_type_id)
end
end
end
RSpec.shared_examples "protected branches > access control > EE" do
[['merge', ProtectedBranch::MergeAccessLevel], ['push', ProtectedBranch::PushAccessLevel]].each do |git_operation, access_level_class|
# Need to set a default for the `git_operation` access level that _isn't_ being tested
other_git_operation = git_operation == 'merge' ? 'push' : 'merge'
it "allows creating protected branches that roles and users can #{git_operation} to" do
users = create_list(:user, 5)
users.each { |user| project.team << [user, :developer] }
roles = access_level_class.human_access_levels
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
users.each { |user| set_allowed_to(git_operation, user.name) }
roles.each { |(_, access_type_name)| set_allowed_to(git_operation, access_type_name) }
set_allowed_to(other_git_operation)
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
roles.each { |(access_type_id, _)| expect(ProtectedBranch.last.send("#{git_operation}_access_levels".to_sym).map(&:access_level)).to include(access_type_id) }
users.each { |user| expect(ProtectedBranch.last.send("#{git_operation}_access_levels".to_sym).map(&:user_id)).to include(user.id) }
end
it "allows updating protected branches that roles and users can #{git_operation} to" do
users = create_list(:user, 5)
users.each { |user| project.team << [user, :developer] }
roles = access_level_class.human_access_levels
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
set_allowed_to('merge')
set_allowed_to('push')
click_on "Protect"
within(".js-protected-branch-edit-form") do
users.each { |user| set_allowed_to(git_operation, user.name) }
roles.each { |(_, access_type_name)| set_allowed_to(git_operation, access_type_name) }
end
wait_for_ajax
expect(ProtectedBranch.count).to eq(1)
roles.each { |(access_type_id, _)| expect(ProtectedBranch.last.send("#{git_operation}_access_levels".to_sym).map(&:access_level)).to include(access_type_id) }
users.each { |user| expect(ProtectedBranch.last.send("#{git_operation}_access_levels".to_sym).map(&:user_id)).to include(user.id) }
end
end
end
require 'spec_helper'
Dir["./spec/features/protected_branches/*.rb"].sort.each { |f| require f }
feature 'Projected Branches', feature: true, js: true do
include WaitForAjax
......@@ -8,6 +9,13 @@ feature 'Projected Branches', feature: true, js: true do
before { login_as(user) }
def set_allowed_to(operation, option = 'Masters')
find(".js-allowed-to-#{operation}").click
wait_for_ajax
click_on option
find(".js-allowed-to-#{operation}").click # needed to submit form in some cases
end
def set_protected_branch_name(branch_name)
find(".js-protected-branch-select").click
find(".dropdown-input-field").set(branch_name)
......@@ -18,6 +26,8 @@ feature 'Projected Branches', feature: true, js: true do
it "allows creating explicit protected branches" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('some-branch')
set_allowed_to('merge')
set_allowed_to('push')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('some-branch') }
......@@ -31,6 +41,8 @@ feature 'Projected Branches', feature: true, js: true do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('some-branch')
set_allowed_to('merge')
set_allowed_to('push')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content(commit.id[0..7]) }
......@@ -39,6 +51,8 @@ feature 'Projected Branches', feature: true, js: true do
it "displays an error message if the named branch does not exist" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('some-branch')
set_allowed_to('merge')
set_allowed_to('push')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('branch was removed') }
......@@ -49,6 +63,8 @@ feature 'Projected Branches', feature: true, js: true do
it "allows creating protected branches with a wildcard" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('*-stable')
set_allowed_to('merge')
set_allowed_to('push')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('*-stable') }
......@@ -62,6 +78,8 @@ feature 'Projected Branches', feature: true, js: true do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('*-stable')
set_allowed_to('merge')
set_allowed_to('push')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content("2 matching branches") }
......@@ -74,6 +92,8 @@ feature 'Projected Branches', feature: true, js: true do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('*-stable')
set_allowed_to('merge')
set_allowed_to('push')
click_on "Protect"
visit namespace_project_protected_branches_path(project.namespace, project)
......@@ -88,74 +108,6 @@ feature 'Projected Branches', feature: true, js: true do
end
describe "access control" do
ProtectedBranch::PushAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
it "allows creating protected branches that #{access_type_name} can push to" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
within('.new_protected_branch') do
allowed_to_push_button = find(".js-allowed-to-push")
unless allowed_to_push_button.text == access_type_name
allowed_to_push_button.click
within(".dropdown.open .dropdown-menu") { click_on access_type_name }
end
end
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to eq([access_type_id])
end
it "allows updating protected branches so that #{access_type_name} can push to them" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
within(".protected-branches-list") do
find(".js-allowed-to-push").click
within('.js-allowed-to-push-container') { click_on access_type_name }
end
wait_for_ajax
expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to include(access_type_id)
end
end
ProtectedBranch::MergeAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
it "allows creating protected branches that #{access_type_name} can merge to" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
within('.new_protected_branch') do
allowed_to_merge_button = find(".js-allowed-to-merge")
unless allowed_to_merge_button.text == access_type_name
allowed_to_merge_button.click
within(".dropdown.open .dropdown-menu") { click_on access_type_name }
end
end
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to eq([access_type_id])
end
it "allows updating protected branches so that #{access_type_name} can merge to them" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
click_on "Protect"
expect(ProtectedBranch.count).to eq(1)
within(".protected-branches-list") do
find(".js-allowed-to-merge").click
within('.js-allowed-to-merge-container') { click_on access_type_name }
end
wait_for_ajax
expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to include(access_type_id)
end
end
include_examples "protected branches > access control > EE"
end
end
......@@ -245,19 +245,19 @@ describe Gitlab::GitAccess, lib: true do
[['feature', 'exact'], ['feat*', 'wildcard']].each do |protected_branch_name, protected_branch_type|
context do
before { create(:protected_branch, name: protected_branch_name, project: project) }
before { create(:protected_branch, :remove_default_access_levels, :masters_can_push, name: protected_branch_name, project: project) }
run_permission_checks(permissions_matrix)
end
context "when developers are allowed to push into the #{protected_branch_type} protected branch" do
before { create(:protected_branch, :developers_can_push, name: protected_branch_name, project: project) }
before { create(:protected_branch, :remove_default_access_levels, :masters_can_push, :developers_can_push, name: protected_branch_name, project: project) }
run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
end
context "developers are allowed to merge into the #{protected_branch_type} protected branch" do
before { create(:protected_branch, :developers_can_merge, name: protected_branch_name, project: project) }
before { create(:protected_branch, :remove_default_access_levels, :masters_can_push, :developers_can_merge, name: protected_branch_name, project: project) }
context "when a merge request exists for the given source/target branch" do
context "when the merge request is in progress" do
......@@ -284,13 +284,53 @@ describe Gitlab::GitAccess, lib: true do
end
context "when developers are allowed to push and merge into the #{protected_branch_type} protected branch" do
before { create(:protected_branch, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project) }
before { create(:protected_branch, :remove_default_access_levels, :masters_can_push, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project) }
run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
end
context "when a specific user is allowed to push into the #{protected_branch_type} protected branch" do
let(:user) { create(:user) }
before do
create(:protected_branch, :remove_default_access_levels, authorize_user_to_push: user, name: protected_branch_name, project: project)
end
run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true },
guest: { push_protected_branch: false, merge_into_protected_branch: false },
reporter: { push_protected_branch: false, merge_into_protected_branch: false }))
end
context "when a specific user is allowed to merge into the #{protected_branch_type} protected branch" do
let(:user) { create(:user) }
before do
create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
create(:protected_branch, :remove_default_access_levels, authorize_user_to_merge: user, name: protected_branch_name, project: project)
end
run_permission_checks(permissions_matrix.deep_merge(admin: { push_protected_branch: false, push_all: false, merge_into_protected_branch: true },
master: { push_protected_branch: false, push_all: false, merge_into_protected_branch: true },
developer: { push_protected_branch: false, push_all: false, merge_into_protected_branch: true },
guest: { push_protected_branch: false, merge_into_protected_branch: false },
reporter: { push_protected_branch: false, merge_into_protected_branch: false }))
end
context "when a specific user is allowed to push & merge into the #{protected_branch_type} protected branch" do
let(:user) { create(:user) }
before do
create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
create(:protected_branch, :remove_default_access_levels, authorize_user_to_push: user, authorize_user_to_merge: user, name: protected_branch_name, project: project)
end
run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true },
guest: { push_protected_branch: false, merge_into_protected_branch: false },
reporter: { push_protected_branch: false, merge_into_protected_branch: false }))
end
context "when no one is allowed to push to the #{protected_branch_name} protected branch" do
before { create(:protected_branch, :no_one_can_push, name: protected_branch_name, project: project) }
before { create(:protected_branch, :remove_default_access_levels, :no_one_can_push, name: protected_branch_name, project: project) }
run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false },
master: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false },
......
......@@ -7,6 +7,67 @@ describe ProtectedBranch, models: true do
it { is_expected.to belong_to(:project) }
end
describe "Uniqueness validations" do
[ProtectedBranch::MergeAccessLevel, ProtectedBranch::PushAccessLevel].each do |access_level_class|
let(:user) { create(:user) }
let(:factory_name) { access_level_class.to_s.underscore.sub('/', '_').to_sym }
let(:association_name) { access_level_class.to_s.underscore.sub('protected_branch/', '').pluralize.to_sym }
human_association_name = access_level_class.to_s.underscore.humanize.sub('Protected branch/', '')
context "while checking uniqueness of a role-based #{human_association_name}" do
it "allows a single #{human_association_name} for a role (per protected branch)" do
first_protected_branch = create(:protected_branch, :remove_default_access_levels)
second_protected_branch = create(:protected_branch, :remove_default_access_levels)
first_protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
second_protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
expect(first_protected_branch).to be_valid
expect(second_protected_branch).to be_valid
first_protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
expect(first_protected_branch).to be_invalid
expect(first_protected_branch.errors.full_messages.first).to match("access level has already been taken")
end
it "does not count a user-based #{human_association_name} with an `access_level` set" do
protected_branch = create(:protected_branch, :remove_default_access_levels)
protected_branch.send(association_name) << build(factory_name, user: user, access_level: Gitlab::Access::MASTER)
protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
expect(protected_branch).to be_valid
end
end
context "while checking uniqueness of a user-based #{human_association_name}" do
it "allows a single #{human_association_name} for a user (per protected branch)" do
first_protected_branch = create(:protected_branch, :remove_default_access_levels)
second_protected_branch = create(:protected_branch, :remove_default_access_levels)
first_protected_branch.send(association_name) << build(factory_name, user: user)
second_protected_branch.send(association_name) << build(factory_name, user: user)
expect(first_protected_branch).to be_valid
expect(second_protected_branch).to be_valid
first_protected_branch.send(association_name) << build(factory_name, user: user)
expect(first_protected_branch).to be_invalid
expect(first_protected_branch.errors.full_messages.first).to match("user has already been taken")
end
it "ignores the `access_level` while validating a user-based #{human_association_name}" do
protected_branch = create(:protected_branch, :remove_default_access_levels)
protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
protected_branch.send(association_name) << build(factory_name, user: user, access_level: Gitlab::Access::MASTER)
expect(protected_branch).to be_valid
end
end
end
end
describe "Mass assignment" do
end
......
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