diff options
129 files changed, 2301 insertions, 754 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 65f2bc7045f..19b831e9426 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -475,7 +475,7 @@ migration:path-mysql: <<: *pull-cache stage: test script: - - bundle exec rake db:rollback STEP=120 + - bundle exec rake db:rollback STEP=119 - bundle exec rake db:migrate db:rollback-pg: @@ -578,7 +578,7 @@ codequality: script: - cp .rubocop.yml .rubocop.yml.bak - grep -v "rubocop-gitlab-security" .rubocop.yml.bak > .rubocop.yml - - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate analyze -f json > raw_codeclimate.json + - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate:0.69.0 analyze -f json > raw_codeclimate.json - cat raw_codeclimate.json | docker run -i stedolan/jq -c 'map({check_name,fingerprint,location})' > codeclimate.json - mv .rubocop.yml.bak .rubocop.yml artifacts: diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index d716218d9a4..344b31cf8b7 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -1,6 +1,6 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-arrow-callback, wrap-iife, no-shadow, consistent-return, one-var, one-var-declaration-per-line, camelcase, default-case, no-new, quotes, no-duplicate-case, no-case-declarations, no-fallthrough, max-len */ import { s__ } from './locale'; -/* global ProjectSelect */ +import projectSelect from './project_select'; import IssuableIndex from './issuable_index'; /* global Milestone */ import IssuableForm from './issuable_form'; @@ -26,8 +26,7 @@ import projectAvatar from './project_avatar'; /* global Compare */ /* global CompareAutocomplete */ /* global ProjectFindFile */ -/* global ProjectNew */ -/* global ProjectShow */ +import ProjectNew from './project_new'; import projectImport from './project_import'; import Labels from './labels'; import LabelManager from './label_manager'; @@ -91,6 +90,8 @@ import Members from './members'; import memberExpirationDate from './member_expiration_date'; import DueDateSelectors from './due_date_select'; import Diff from './diff'; +import ProjectLabelSubscription from './project_label_subscription'; +import ProjectVariables from './project_variables'; (function() { var Dispatcher; @@ -187,7 +188,7 @@ import Diff from './diff'; initIssuableSidebar(); break; case 'dashboard:milestones:index': - new ProjectSelect(); + projectSelect(); break; case 'projects:milestones:show': case 'groups:milestones:show': @@ -197,7 +198,7 @@ import Diff from './diff'; break; case 'dashboard:issues': case 'dashboard:merge_requests': - new ProjectSelect(); + projectSelect(); initLegacyFilters(); break; case 'groups:issues': @@ -206,7 +207,7 @@ import Diff from './diff'; const filteredSearchManager = new gl.FilteredSearchManager(page === 'groups:issues' ? 'issues' : 'merge_requests'); filteredSearchManager.setup(); } - new ProjectSelect(); + projectSelect(); break; case 'dashboard:todos:index': new Todos(); @@ -339,7 +340,8 @@ import Diff from './diff'; container: '.js-commit-pipeline-graph', }).bindEvents(); initNotes(); - initChangesDropdown(); + const stickyBarPaddingTop = 16; + initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight - stickyBarPaddingTop); $('.commit-info.branches').load(document.querySelector('.js-commit-box').dataset.commitPath); break; case 'projects:commit:pipelines': @@ -484,7 +486,7 @@ import Diff from './diff'; if ($el.find('.dropdown-group-label').length) { new GroupLabelSubscription($el); } else { - new gl.ProjectLabelSubscription($el); + new ProjectLabelSubscription($el); } }); break; @@ -520,7 +522,7 @@ import Diff from './diff'; // Initialize expandable settings panels initSettingsPanels(); case 'groups:settings:ci_cd:show': - new gl.ProjectVariables(); + new ProjectVariables(); break; case 'ci:lints:create': case 'ci:lints:show': @@ -623,7 +625,6 @@ import Diff from './diff'; case 'show': new Star(); new ProjectNew(); - new ProjectShow(); new NotificationsDropdown(); break; case 'wikis': diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 0035dd23011..cef79eec273 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -71,11 +71,6 @@ import './pager'; import './preview_markdown'; import './project_find_file'; import './project_import'; -import './project_label_subscription'; -import './project_new'; -import './project_select'; -import './project_show'; -import './project_variables'; import './projects_dropdown'; import './projects_list'; import './syntax_highlight'; diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index ddb78aaeea1..36b6a5ed376 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -1,7 +1,7 @@ /* eslint-disable func-names, space-before-function-paren, no-var, consistent-return, no-new, prefer-arrow-callback, no-return-assign, one-var, one-var-declaration-per-line, object-shorthand, no-else-return, newline-per-chained-call, no-shadow, vars-on-top, prefer-template, max-len */ -/* global ProjectSelect */ import Cookies from 'js-cookie'; +import projectSelect from './project_select'; export default class Project { constructor() { @@ -46,7 +46,7 @@ export default class Project { } static projectSelectDropdown () { - new ProjectSelect(); + projectSelect(); $('.project-item-select').on('click', e => Project.changeProject($(e.currentTarget).val())); } diff --git a/app/assets/javascripts/project_label_subscription.js b/app/assets/javascripts/project_label_subscription.js index 0a811627600..b65521b278f 100644 --- a/app/assets/javascripts/project_label_subscription.js +++ b/app/assets/javascripts/project_label_subscription.js @@ -1,55 +1,50 @@ -/* eslint-disable wrap-iife, func-names, space-before-function-paren, object-shorthand, comma-dangle, one-var, one-var-declaration-per-line, no-restricted-syntax, max-len, no-param-reassign */ +export default class ProjectLabelSubscription { + constructor(container) { + this.$container = $(container); + this.$buttons = this.$container.find('.js-subscribe-button'); -(function(global) { - class ProjectLabelSubscription { - constructor(container) { - this.$container = $(container); - this.$buttons = this.$container.find('.js-subscribe-button'); - - this.$buttons.on('click', this.toggleSubscription.bind(this)); - } + this.$buttons.on('click', this.toggleSubscription.bind(this)); + } - toggleSubscription(event) { - event.preventDefault(); + toggleSubscription(event) { + event.preventDefault(); - const $btn = $(event.currentTarget); - const $span = $btn.find('span'); - const url = $btn.attr('data-url'); - const oldStatus = $btn.attr('data-status'); + const $btn = $(event.currentTarget); + const $span = $btn.find('span'); + const url = $btn.attr('data-url'); + const oldStatus = $btn.attr('data-status'); - $btn.addClass('disabled'); - $span.toggleClass('hidden'); + $btn.addClass('disabled'); + $span.toggleClass('hidden'); - $.ajax({ - type: 'POST', - url: url - }).done(() => { - let newStatus, newAction; + $.ajax({ + type: 'POST', + url, + }).done(() => { + let newStatus; + let newAction; - if (oldStatus === 'unsubscribed') { - [newStatus, newAction] = ['subscribed', 'Unsubscribe']; - } else { - [newStatus, newAction] = ['unsubscribed', 'Subscribe']; - } + if (oldStatus === 'unsubscribed') { + [newStatus, newAction] = ['subscribed', 'Unsubscribe']; + } else { + [newStatus, newAction] = ['unsubscribed', 'Subscribe']; + } - $span.toggleClass('hidden'); - $btn.removeClass('disabled'); + $span.toggleClass('hidden'); + $btn.removeClass('disabled'); - this.$buttons.attr('data-status', newStatus); - this.$buttons.find('> span').text(newAction); + this.$buttons.attr('data-status', newStatus); + this.$buttons.find('> span').text(newAction); - this.$buttons.map((button) => { - const $button = $(button); + this.$buttons.map((button) => { + const $button = $(button); - if ($button.attr('data-original-title')) { - $button.tooltip('hide').attr('data-original-title', newAction).tooltip('fixTitle'); - } + if ($button.attr('data-original-title')) { + $button.tooltip('hide').attr('data-original-title', newAction).tooltip('fixTitle'); + } - return button; - }); + return button; }); - } + }); } - - global.ProjectLabelSubscription = ProjectLabelSubscription; -})(window.gl || (window.gl = {})); +} diff --git a/app/assets/javascripts/project_new.js b/app/assets/javascripts/project_new.js index fd89a1a85c3..ca548d011b6 100644 --- a/app/assets/javascripts/project_new.js +++ b/app/assets/javascripts/project_new.js @@ -1,4 +1,4 @@ -/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-unused-vars, one-var, no-underscore-dangle, prefer-template, no-else-return, prefer-arrow-callback, max-len */ +/* eslint-disable func-names, no-var, no-underscore-dangle, prefer-template, prefer-arrow-callback*/ import VisibilitySelect from './visibility_select'; @@ -7,153 +7,145 @@ function highlightChanges($elm) { setTimeout(() => $elm.removeClass('highlight-changes'), 10); } -(function() { - this.ProjectNew = (function() { - function ProjectNew() { - this.toggleSettings = this.toggleSettings.bind(this); - this.$selects = $('.features select'); - this.$repoSelects = this.$selects.filter('.js-repo-select'); - this.$projectSelects = this.$selects.not('.js-repo-select'); - - $('.project-edit-container').on('ajax:before', (function(_this) { - return function() { - $('.project-edit-container').hide(); - return $('.save-project-loader').show(); - }; - })(this)); - - this.initVisibilitySelect(); - - this.toggleSettings(); - this.toggleSettingsOnclick(); - this.toggleRepoVisibility(); - } - - ProjectNew.prototype.initVisibilitySelect = function() { - const visibilityContainer = document.querySelector('.js-visibility-select'); - if (!visibilityContainer) return; - const visibilitySelect = new VisibilitySelect(visibilityContainer); - visibilitySelect.init(); - - const $visibilitySelect = $(visibilityContainer).find('select'); - let projectVisibility = $visibilitySelect.val(); - const PROJECT_VISIBILITY_PRIVATE = '0'; - - $visibilitySelect.on('change', () => { - const newProjectVisibility = $visibilitySelect.val(); - - if (projectVisibility !== newProjectVisibility) { - this.$projectSelects.each((idx, select) => { - const $select = $(select); - const $options = $select.find('option'); - const values = $.map($options, e => e.value); - - // if switched to "private", limit visibility options - if (newProjectVisibility === PROJECT_VISIBILITY_PRIVATE) { - if ($select.val() !== values[0] && $select.val() !== values[1]) { - $select.val(values[1]).trigger('change'); - highlightChanges($select); - } - $options.slice(2).disable(); +export default class ProjectNew { + constructor() { + this.toggleSettings = this.toggleSettings.bind(this); + this.$selects = $('.features select'); + this.$repoSelects = this.$selects.filter('.js-repo-select'); + this.$projectSelects = this.$selects.not('.js-repo-select'); + + $('.project-edit-container').on('ajax:before', () => { + $('.project-edit-container').hide(); + return $('.save-project-loader').show(); + }); + + this.initVisibilitySelect(); + + this.toggleSettings(); + this.toggleSettingsOnclick(); + this.toggleRepoVisibility(); + } + + initVisibilitySelect() { + const visibilityContainer = document.querySelector('.js-visibility-select'); + if (!visibilityContainer) return; + const visibilitySelect = new VisibilitySelect(visibilityContainer); + visibilitySelect.init(); + + const $visibilitySelect = $(visibilityContainer).find('select'); + let projectVisibility = $visibilitySelect.val(); + const PROJECT_VISIBILITY_PRIVATE = '0'; + + $visibilitySelect.on('change', () => { + const newProjectVisibility = $visibilitySelect.val(); + + if (projectVisibility !== newProjectVisibility) { + this.$projectSelects.each((idx, select) => { + const $select = $(select); + const $options = $select.find('option'); + const values = $.map($options, e => e.value); + + // if switched to "private", limit visibility options + if (newProjectVisibility === PROJECT_VISIBILITY_PRIVATE) { + if ($select.val() !== values[0] && $select.val() !== values[1]) { + $select.val(values[1]).trigger('change'); + highlightChanges($select); } + $options.slice(2).disable(); + } - // if switched from "private", increase visibility for non-disabled options - if (projectVisibility === PROJECT_VISIBILITY_PRIVATE) { - $options.enable(); - if ($select.val() !== values[0] && $select.val() !== values[values.length - 1]) { - $select.val(values[values.length - 1]).trigger('change'); - highlightChanges($select); - } + // if switched from "private", increase visibility for non-disabled options + if (projectVisibility === PROJECT_VISIBILITY_PRIVATE) { + $options.enable(); + if ($select.val() !== values[0] && $select.val() !== values[values.length - 1]) { + $select.val(values[values.length - 1]).trigger('change'); + highlightChanges($select); } - }); + } + }); - projectVisibility = newProjectVisibility; - } - }); - }; - - ProjectNew.prototype.toggleSettings = function() { - var self = this; - - this.$selects.each(function () { - var $select = $(this); - var className = $select.data('field') - .replace(/_/g, '-') - .replace('access-level', 'feature'); - self._showOrHide($select, '.' + className); - }); - }; - - ProjectNew.prototype.toggleSettingsOnclick = function() { - this.$selects.on('change', this.toggleSettings); - }; - - ProjectNew.prototype._showOrHide = function(checkElement, container) { - var $container = $(container); - - if ($(checkElement).val() !== '0') { - return $container.show(); - } else { - return $container.hide(); + projectVisibility = newProjectVisibility; } - }; - - ProjectNew.prototype.toggleRepoVisibility = function () { - var $repoAccessLevel = $('.js-repo-access-level select'); - var $lfsEnabledOption = $('.js-lfs-enabled select'); - var containerRegistry = document.querySelectorAll('.js-container-registry')[0]; - var containerRegistryCheckbox = document.getElementById('project_container_registry_enabled'); - var prevSelectedVal = parseInt($repoAccessLevel.val(), 10); - - this.$repoSelects.find("option[value='" + $repoAccessLevel.val() + "']") - .nextAll() - .hide(); - - $repoAccessLevel.off('change') - .on('change', function () { - var selectedVal = parseInt($repoAccessLevel.val(), 10); - - this.$repoSelects.each(function () { - var $this = $(this); - var repoSelectVal = parseInt($this.val(), 10); - - $this.find('option').enable(); - - if (selectedVal < repoSelectVal || repoSelectVal === prevSelectedVal) { - $this.val(selectedVal).trigger('change'); - highlightChanges($this); - } - - $this.find("option[value='" + selectedVal + "']").nextAll().disable(); - }); + }); + } + + toggleSettings() { + this.$selects.each(function () { + var $select = $(this); + var className = $select.data('field') + .replace(/_/g, '-') + .replace('access-level', 'feature'); + ProjectNew._showOrHide($select, '.' + className); + }); + } + + toggleSettingsOnclick() { + this.$selects.on('change', this.toggleSettings); + } + + static _showOrHide(checkElement, container) { + const $container = $(container); + + if ($(checkElement).val() !== '0') { + return $container.show(); + } + return $container.hide(); + } + + toggleRepoVisibility() { + var $repoAccessLevel = $('.js-repo-access-level select'); + var $lfsEnabledOption = $('.js-lfs-enabled select'); + var containerRegistry = document.querySelectorAll('.js-container-registry')[0]; + var containerRegistryCheckbox = document.getElementById('project_container_registry_enabled'); + var prevSelectedVal = parseInt($repoAccessLevel.val(), 10); + + this.$repoSelects.find("option[value='" + $repoAccessLevel.val() + "']") + .nextAll() + .hide(); + + $repoAccessLevel + .off('change') + .on('change', function () { + var selectedVal = parseInt($repoAccessLevel.val(), 10); + + this.$repoSelects.each(function () { + var $this = $(this); + var repoSelectVal = parseInt($this.val(), 10); + + $this.find('option').enable(); + + if (selectedVal < repoSelectVal || repoSelectVal === prevSelectedVal) { + $this.val(selectedVal).trigger('change'); + highlightChanges($this); + } - if (selectedVal) { - this.$repoSelects.removeClass('disabled'); + $this.find("option[value='" + selectedVal + "']").nextAll().disable(); + }); - if ($lfsEnabledOption.length) { - $lfsEnabledOption.removeClass('disabled'); - highlightChanges($lfsEnabledOption); - } - if (containerRegistry) { - containerRegistry.style.display = ''; - } - } else { - this.$repoSelects.addClass('disabled'); + if (selectedVal) { + this.$repoSelects.removeClass('disabled'); - if ($lfsEnabledOption.length) { - $lfsEnabledOption.val('false').addClass('disabled'); - highlightChanges($lfsEnabledOption); - } - if (containerRegistry) { - containerRegistry.style.display = 'none'; - containerRegistryCheckbox.checked = false; - } + if ($lfsEnabledOption.length) { + $lfsEnabledOption.removeClass('disabled'); + highlightChanges($lfsEnabledOption); + } + if (containerRegistry) { + containerRegistry.style.display = ''; } + } else { + this.$repoSelects.addClass('disabled'); - prevSelectedVal = selectedVal; - }.bind(this)); - }; + if ($lfsEnabledOption.length) { + $lfsEnabledOption.val('false').addClass('disabled'); + highlightChanges($lfsEnabledOption); + } + if (containerRegistry) { + containerRegistry.style.display = 'none'; + containerRegistryCheckbox.checked = false; + } + } - return ProjectNew; - })(); -}).call(window); + prevSelectedVal = selectedVal; + }.bind(this)); + } +} diff --git a/app/assets/javascripts/project_select.js b/app/assets/javascripts/project_select.js index bffc85e6315..07a49d1506c 100644 --- a/app/assets/javascripts/project_select.js +++ b/app/assets/javascripts/project_select.js @@ -2,79 +2,73 @@ import Api from './api'; import ProjectSelectComboButton from './project_select_combo_button'; -(function () { - this.ProjectSelect = (function () { - function ProjectSelect() { - $('.ajax-project-select').each(function(i, select) { - var placeholder; - const simpleFilter = $(select).data('simple-filter') || false; - this.groupId = $(select).data('group-id'); - this.includeGroups = $(select).data('include-groups'); - this.allProjects = $(select).data('all-projects') || false; - this.orderBy = $(select).data('order-by') || 'id'; - this.withIssuesEnabled = $(select).data('with-issues-enabled'); - this.withMergeRequestsEnabled = $(select).data('with-merge-requests-enabled'); +export default function projectSelect() { + $('.ajax-project-select').each(function(i, select) { + var placeholder; + const simpleFilter = $(select).data('simple-filter') || false; + this.groupId = $(select).data('group-id'); + this.includeGroups = $(select).data('include-groups'); + this.allProjects = $(select).data('all-projects') || false; + this.orderBy = $(select).data('order-by') || 'id'; + this.withIssuesEnabled = $(select).data('with-issues-enabled'); + this.withMergeRequestsEnabled = $(select).data('with-merge-requests-enabled'); - placeholder = "Search for project"; - if (this.includeGroups) { - placeholder += " or group"; - } + placeholder = "Search for project"; + if (this.includeGroups) { + placeholder += " or group"; + } - $(select).select2({ - placeholder: placeholder, - minimumInputLength: 0, - query: (function (_this) { - return function (query) { - var finalCallback, projectsCallback; - finalCallback = function (projects) { + $(select).select2({ + placeholder: placeholder, + minimumInputLength: 0, + query: (function (_this) { + return function (query) { + var finalCallback, projectsCallback; + finalCallback = function (projects) { + var data; + data = { + results: projects + }; + return query.callback(data); + }; + if (_this.includeGroups) { + projectsCallback = function (projects) { + var groupsCallback; + groupsCallback = function (groups) { var data; - data = { - results: projects - }; - return query.callback(data); + data = groups.concat(projects); + return finalCallback(data); }; - if (_this.includeGroups) { - projectsCallback = function (projects) { - var groupsCallback; - groupsCallback = function (groups) { - var data; - data = groups.concat(projects); - return finalCallback(data); - }; - return Api.groups(query.term, {}, groupsCallback); - }; - } else { - projectsCallback = finalCallback; - } - if (_this.groupId) { - return Api.groupProjects(_this.groupId, query.term, projectsCallback); - } else { - return Api.projects(query.term, { - order_by: _this.orderBy, - with_issues_enabled: _this.withIssuesEnabled, - with_merge_requests_enabled: _this.withMergeRequestsEnabled, - membership: !_this.allProjects, - }, projectsCallback); - } + return Api.groups(query.term, {}, groupsCallback); }; - })(this), - id: function(project) { - if (simpleFilter) return project.id; - return JSON.stringify({ - name: project.name, - url: project.web_url, - }); - }, - text: function (project) { - return project.name_with_namespace || project.name; - }, - dropdownCssClass: "ajax-project-dropdown" + } else { + projectsCallback = finalCallback; + } + if (_this.groupId) { + return Api.groupProjects(_this.groupId, query.term, projectsCallback); + } else { + return Api.projects(query.term, { + order_by: _this.orderBy, + with_issues_enabled: _this.withIssuesEnabled, + with_merge_requests_enabled: _this.withMergeRequestsEnabled, + membership: !_this.allProjects, + }, projectsCallback); + } + }; + })(this), + id: function(project) { + if (simpleFilter) return project.id; + return JSON.stringify({ + name: project.name, + url: project.web_url, }); - if (simpleFilter) return select; - return new ProjectSelectComboButton(select); - }); - } - - return ProjectSelect; - })(); -}).call(window); + }, + text: function (project) { + return project.name_with_namespace || project.name; + }, + dropdownCssClass: "ajax-project-dropdown" + }); + if (simpleFilter) return select; + return new ProjectSelectComboButton(select); + }); +} diff --git a/app/assets/javascripts/project_show.js b/app/assets/javascripts/project_show.js deleted file mode 100644 index 3a51c1f26ac..00000000000 --- a/app/assets/javascripts/project_show.js +++ /dev/null @@ -1,11 +0,0 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife */ - -(function() { - this.ProjectShow = (function() { - function ProjectShow() {} - - return ProjectShow; - })(); -}).call(window); - -// I kept class for future diff --git a/app/assets/javascripts/project_variables.js b/app/assets/javascripts/project_variables.js index 4ee2e49306d..567c311f119 100644 --- a/app/assets/javascripts/project_variables.js +++ b/app/assets/javascripts/project_variables.js @@ -1,43 +1,39 @@ -(() => { - const HIDDEN_VALUE_TEXT = '******'; - class ProjectVariables { - constructor() { - this.$revealBtn = $('.js-btn-toggle-reveal-values'); - this.$revealBtn.on('click', this.toggleRevealState.bind(this)); - } +const HIDDEN_VALUE_TEXT = '******'; + +export default class ProjectVariables { + constructor() { + this.$revealBtn = $('.js-btn-toggle-reveal-values'); + this.$revealBtn.on('click', this.toggleRevealState.bind(this)); + } - toggleRevealState(e) { - e.preventDefault(); + toggleRevealState(e) { + e.preventDefault(); - const oldStatus = this.$revealBtn.attr('data-status'); - let newStatus = 'hidden'; - let newAction = 'Reveal Values'; + const oldStatus = this.$revealBtn.attr('data-status'); + let newStatus = 'hidden'; + let newAction = 'Reveal Values'; - if (oldStatus === 'hidden') { - newStatus = 'revealed'; - newAction = 'Hide Values'; - } + if (oldStatus === 'hidden') { + newStatus = 'revealed'; + newAction = 'Hide Values'; + } - this.$revealBtn.attr('data-status', newStatus); + this.$revealBtn.attr('data-status', newStatus); - const $variables = $('.variable-value'); + const $variables = $('.variable-value'); - $variables.each((_, variable) => { - const $variable = $(variable); - let newText = HIDDEN_VALUE_TEXT; + $variables.each((_, variable) => { + const $variable = $(variable); + let newText = HIDDEN_VALUE_TEXT; - if (newStatus === 'revealed') { - newText = $variable.attr('data-value'); - } + if (newStatus === 'revealed') { + newText = $variable.attr('data-value'); + } - $variable.text(newText); - }); + $variable.text(newText); + }); - this.$revealBtn.text(newAction); - } + this.$revealBtn.text(newAction); } - - window.gl = window.gl || {}; - window.gl.ProjectVariables = ProjectVariables; -})(); +} diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 5f5b5657a2f..bbbb73201be 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -2,7 +2,9 @@ .cgray { color: $common-gray; } .clgray { color: $common-gray-light; } .cred { color: $common-red; } +svg.cred { fill: $common-red; } .cgreen { color: $common-green; } +svg.cgreen { fill: $common-green; } .cdark { color: $common-gray-dark; } .text-secondary { color: $gl-text-color-secondary; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index bce94e09367..848d7f144dc 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -628,21 +628,46 @@ } .diff-file-changes { - width: 450px; + max-width: 560px; + width: 100%; z-index: 150; @media (min-width: $screen-sm-min) { left: $gl-padding; } - a { + .diff-changed-file { + display: flex; padding-top: 8px; padding-bottom: 8px; + min-width: 0; } - .diff-changed-file { + .diff-file-changed-icon { + margin-top: 2px; + } + + .diff-changed-file-content { display: flex; - align-items: center; + flex-direction: column; + min-width: 0; + } + + .diff-changed-file-name, + .diff-changed-file-path { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + .diff-changed-file-path { + direction: rtl; + color: $gl-text-color-tertiary; + } + + .diff-changed-stats { + margin-left: auto; + white-space: nowrap; } } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3be7aee69bc..488b50426fe 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -11,8 +11,7 @@ class ApplicationController < ActionController::Base include EnforcesTwoFactorAuthentication include WithPerformanceBar - before_action :authenticate_user_from_personal_access_token! - before_action :authenticate_user_from_rss_token! + before_action :authenticate_sessionless_user! before_action :authenticate_user! before_action :validate_user_service_ticket! before_action :check_password_expiration @@ -100,27 +99,11 @@ class ApplicationController < ActionController::Base return try(:authenticated_user) end - def authenticate_user_from_personal_access_token! - token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence + # This filter handles personal access tokens, and atom requests with rss tokens + def authenticate_sessionless_user! + user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user - return unless token.present? - - user = User.find_by_personal_access_token(token) - - sessionless_sign_in(user) - end - - # This filter handles authentication for atom request with an rss_token - def authenticate_user_from_rss_token! - return unless request.format.atom? - - token = params[:rss_token].presence - - return unless token.present? - - user = User.find_by_rss_token(token) - - sessionless_sign_in(user) + sessionless_sign_in(user) if user end def log_exception(exception) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 9612b8d8514..56baa19f864 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -54,7 +54,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController if current_user log_audit_event(current_user, with: :saml) # Update SAML identity if data has changed. - identity = current_user.identities.find_by(extern_uid: oauth['uid'], provider: :saml) + identity = current_user.identities.with_extern_uid(:saml, oauth['uid']).take if identity.nil? current_user.identities.create(extern_uid: oauth['uid'], provider: :saml) redirect_to profile_account_path, notice: 'Authentication method updated' @@ -98,7 +98,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController def handle_omniauth if current_user # Add new authentication method - current_user.identities.find_or_create_by(extern_uid: oauth['uid'], provider: oauth['provider']) + current_user.identities + .with_extern_uid(oauth['provider'], oauth['uid']) + .first_or_create(extern_uid: oauth['uid']) log_audit_event(current_user, with: oauth['provider']) redirect_to profile_account_path, notice: 'Authentication method updated' else diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index 28920877635..5f4afd2cdee 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -57,6 +57,7 @@ class Projects::CommitsController < Projects::ApplicationController @repository.commits(@ref, path: @path, limit: @limit, offset: @offset) end + @commits = @commits.with_pipeline_status @commits = prepare_commits_for_rendering(@commits) end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 22de6680511..abe4e5245b1 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -80,7 +80,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo def commits # Get commits from repository # or from cache if already merged - @commits = prepare_commits_for_rendering(@merge_request.commits) + @commits = + prepare_commits_for_rendering(@merge_request.commits.with_pipeline_status) render json: { html: view_to_html_string('projects/merge_requests/_commits') } end diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index f7a9c98629d..b51261a49e0 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -74,7 +74,11 @@ class Projects::WikisController < Projects::ApplicationController def history @page = @project_wiki.find_page(params[:id]) - unless @page + if @page + @page_versions = Kaminari.paginate_array(@page.versions(page: params[:page]), + total_count: @page.count_versions) + .page(params[:page]) + else redirect_to( project_wiki_path(@project, :home), notice: "Page not found" @@ -101,7 +105,7 @@ class Projects::WikisController < Projects::ApplicationController # Call #wiki to make sure the Wiki Repo is initialized @project_wiki.wiki - @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages.first(15)) + @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages(limit: 15)) rescue ProjectWiki::CouldNotCreateWikiError flash[:notice] = "Could not create Wiki Repository at this time. Please try again later." redirect_to project_path(@project) diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index cd1ecaadb85..e5d2693b01e 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -231,6 +231,15 @@ module ApplicationSettingsHelper :sign_in_text, :signup_enabled, :terminal_max_session_time, + :throttle_unauthenticated_enabled, + :throttle_unauthenticated_requests_per_period, + :throttle_unauthenticated_period_in_seconds, + :throttle_authenticated_web_enabled, + :throttle_authenticated_web_requests_per_period, + :throttle_authenticated_web_period_in_seconds, + :throttle_authenticated_api_enabled, + :throttle_authenticated_api_requests_per_period, + :throttle_authenticated_api_period_in_seconds, :two_factor_grace_period, :unique_ips_limit_enabled, :unique_ips_limit_per_user, diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 4dd573c61f1..636316da80a 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -6,11 +6,6 @@ # See 'detailed_status?` method and `Gitlab::Ci::Status` module. # module CiStatusHelper - def ci_status_path(pipeline) - project = pipeline.project - project_pipeline_path(project, pipeline) - end - def ci_label_for_status(status) if detailed_status?(status) return status.label diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 4e4a66e8a02..2b3094918c5 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -152,11 +152,11 @@ module DiffHelper def diff_file_changed_icon(diff_file) if diff_file.deleted_file? || diff_file.renamed_file? - "minus" + "file-deletion" elsif diff_file.new_file? - "plus" + "file-addition" else - "adjust" + "file-modified" end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 5e16badabec..a7e0219b03a 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -295,6 +295,15 @@ class ApplicationSetting < ActiveRecord::Base sign_in_text: nil, signup_enabled: Settings.gitlab['signup_enabled'], terminal_max_session_time: 0, + throttle_unauthenticated_enabled: false, + throttle_unauthenticated_requests_per_period: 3600, + throttle_unauthenticated_period_in_seconds: 3600, + throttle_authenticated_web_enabled: false, + throttle_authenticated_web_requests_per_period: 7200, + throttle_authenticated_web_period_in_seconds: 3600, + throttle_authenticated_api_enabled: false, + throttle_authenticated_api_requests_per_period: 7200, + throttle_authenticated_api_period_in_seconds: 3600, two_factor_grace_period: 48, user_default_external: false, polling_interval_multiplier: 1, diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 19814864e50..c77c837ff3f 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -149,34 +149,70 @@ module Ci end end - # ref can't be HEAD or SHA, can only be branch/tag name - scope :latest, ->(ref = nil) do - max_id = unscope(:select) - .select("max(#{quoted_table_name}.id)") - .group(:ref, :sha) - - if ref - where(ref: ref, id: max_id.where(ref: ref)) - else - where(id: max_id) - end - end scope :internal, -> { where(source: internal_sources) } + # Returns the pipelines in descending order (= newest first), optionally + # limited to a number of references. + # + # ref - The name (or names) of the branch(es)/tag(s) to limit the list of + # pipelines to. + def self.newest_first(ref = nil) + relation = order(id: :desc) + + ref ? relation.where(ref: ref) : relation + end + def self.latest_status(ref = nil) - latest(ref).status + newest_first(ref).pluck(:status).first end def self.latest_successful_for(ref) - success.latest(ref).order(id: :desc).first + newest_first(ref).success.take end def self.latest_successful_for_refs(refs) - success.latest(refs).order(id: :desc).each_with_object({}) do |pipeline, hash| + relation = newest_first(refs).success + + relation.each_with_object({}) do |pipeline, hash| hash[pipeline.ref] ||= pipeline end end + # Returns a Hash containing the latest pipeline status for every given + # commit. + # + # The keys of this Hash are the commit SHAs, the values the statuses. + # + # commits - The list of commit SHAs to get the status for. + # ref - The ref to scope the data to (e.g. "master"). If the ref is not + # given we simply get the latest status for the commits, regardless + # of what refs their pipelines belong to. + def self.latest_status_per_commit(commits, ref = nil) + p1 = arel_table + p2 = arel_table.alias + + # This LEFT JOIN will filter out all but the newest row for every + # combination of (project_id, sha) or (project_id, sha, ref) if a ref is + # given. + cond = p1[:sha].eq(p2[:sha]) + .and(p1[:project_id].eq(p2[:project_id])) + .and(p1[:id].lt(p2[:id])) + + cond = cond.and(p1[:ref].eq(p2[:ref])) if ref + join = p1.join(p2, Arel::Nodes::OuterJoin).on(cond) + + relation = select(:sha, :status) + .where(sha: commits) + .where(p2[:id].eq(nil)) + .joins(join.join_sources) + + relation = relation.where(ref: ref) if ref + + relation.each_with_object({}) do |row, hash| + hash[row[:sha]] = row[:status] + end + end + def self.truncate_sha(sha) sha[0...8] end diff --git a/app/models/commit.rb b/app/models/commit.rb index 6dba154a6ea..a31ebe9cc87 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -80,6 +80,7 @@ class Commit @raw = raw_commit @project = project + @statuses = {} end def id @@ -236,11 +237,13 @@ class Commit end def status(ref = nil) - @statuses ||= {} - return @statuses[ref] if @statuses.key?(ref) - @statuses[ref] = pipelines.latest_status(ref) + @statuses[ref] = project.pipelines.latest_status_per_commit(id, ref)[id] + end + + def set_status_for_ref(ref, status) + @statuses[ref] = status end def signature diff --git a/app/models/commit_collection.rb b/app/models/commit_collection.rb new file mode 100644 index 00000000000..dd93af9df64 --- /dev/null +++ b/app/models/commit_collection.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +# A collection of Commit instances for a specific project and Git reference. +class CommitCollection + include Enumerable + + attr_reader :project, :ref, :commits + + # project - The project the commits belong to. + # commits - The Commit instances to store. + # ref - The name of the ref (e.g. "master"). + def initialize(project, commits, ref = nil) + @project = project + @commits = commits + @ref = ref + end + + def each(&block) + commits.each(&block) + end + + # Sets the pipeline status for every commit. + # + # Setting this status ahead of time removes the need for running a query for + # every commit we're displaying. + def with_pipeline_status + statuses = project.pipelines.latest_status_per_commit(map(&:id), ref) + + each do |commit| + commit.set_status_for_ref(ref, statuses[commit.id]) + end + + self + end + + def respond_to_missing?(message, inc_private = false) + commits.respond_to?(message, inc_private) + end + + # rubocop:disable GitlabSecurity/PublicSend + def method_missing(message, *args, &block) + commits.public_send(message, *args, &block) + end +end diff --git a/app/models/fork_network_member.rb b/app/models/fork_network_member.rb index 6a9b52a1ef8..eb9417dc34f 100644 --- a/app/models/fork_network_member.rb +++ b/app/models/fork_network_member.rb @@ -4,4 +4,14 @@ class ForkNetworkMember < ActiveRecord::Base belongs_to :forked_from_project, class_name: 'Project' validates :fork_network, :project, presence: true + + after_destroy :cleanup_fork_network + + private + + def cleanup_fork_network + # Explicitly using `#count` makes sure we have the correct number if the + # relation was loaded in the fork_network. + fork_network.destroy if fork_network.fork_network_members.count == 0 + end end diff --git a/app/models/identity.rb b/app/models/identity.rb index ac8094b610e..ff811e19f8a 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -1,18 +1,27 @@ class Identity < ActiveRecord::Base include Sortable include CaseSensitivity + belongs_to :user validates :provider, presence: true - validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider } + validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider, case_sensitive: false } validates :user_id, uniqueness: { scope: :provider } + scope :with_provider, ->(provider) { where(provider: provider) } scope :with_extern_uid, ->(provider, extern_uid) do - extern_uid = Gitlab::LDAP::Person.normalize_dn(extern_uid) if provider.starts_with?('ldap') - where(extern_uid: extern_uid, provider: provider) + iwhere(extern_uid: normalize_uid(provider, extern_uid)).with_provider(provider) end def ldap? provider.starts_with?('ldap') end + + def self.normalize_uid(provider, uid) + if provider.to_s.starts_with?('ldap') + Gitlab::LDAP::Person.normalize_dn(uid) + else + uid.to_s + end + end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 1eda0f9cbbd..5382f5cc627 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -284,8 +284,10 @@ class MergeRequestDiff < ActiveRecord::Base def load_commits commits = st_commits.presence || merge_request_diff_commits + commits = commits.map { |commit| Commit.from_hash(commit.to_hash, project) } - commits.map { |commit| Commit.from_hash(commit.to_hash, project) } + CommitCollection + .new(merge_request.source_project, commits, merge_request.source_branch) end def save_diffs diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 3eecbea8cbf..a0af749a93f 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -76,8 +76,8 @@ class ProjectWiki # Returns an Array of Gitlab WikiPage instances or an # empty Array if this Wiki has no pages. - def pages - wiki.pages.map { |page| WikiPage.new(self, page, true) } + def pages(limit: nil) + wiki.pages(limit: limit).map { |page| WikiPage.new(self, page, true) } end # Finds a page within the repository based on a tile diff --git a/app/models/repository.rb b/app/models/repository.rb index 3a89fa9264b..35ee12bdfa1 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -132,7 +132,8 @@ class Repository commits = Gitlab::Git::Commit.where(options) commits = Commit.decorate(commits, @project) if commits.present? - commits + + CommitCollection.new(project, commits, ref) end def commits_between(from, to) @@ -148,11 +149,14 @@ class Repository end raw_repository.gitaly_migrate(:commits_by_message) do |is_enabled| - if is_enabled - find_commits_by_message_by_gitaly(query, ref, path, limit, offset) - else - find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) - end + commits = + if is_enabled + find_commits_by_message_by_gitaly(query, ref, path, limit, offset) + else + find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) + end + + CommitCollection.new(project, commits, ref) end end diff --git a/app/models/user.rb b/app/models/user.rb index be8112749bf..71c34766451 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -269,8 +269,7 @@ class User < ActiveRecord::Base end def for_github_id(id) - joins(:identities) - .where(identities: { provider: :github, extern_uid: id.to_s }) + joins(:identities).merge(Identity.with_extern_uid(:github, id)) end # Find a User by their primary email or any associated secondary email diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 5f710961f95..bdfef677ef3 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -127,19 +127,24 @@ class WikiPage @version ||= @page.version end - # Returns an array of Gitlab Commit instances. - def versions + def versions(options = {}) return [] unless persisted? - wiki.wiki.page_versions(@page.path) + wiki.wiki.page_versions(@page.path, options) end - def commit - versions.first + def count_versions + return [] unless persisted? + + wiki.wiki.count_page_versions(@page.path) + end + + def last_version + @last_version ||= versions(limit: 1).first end def last_commit_sha - commit&.sha + last_version&.sha end # Returns the Date that this latest version was @@ -151,7 +156,7 @@ class WikiPage # Returns boolean True or False if this instance # is an old version of the page. def historical? - @page.historical? && versions.first.sha != version.sha + @page.historical? && last_version.sha != version.sha end # Returns boolean True or False if this instance diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 5957f612e84..e5cd6fcdfe3 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -60,21 +60,14 @@ module Projects # Notifications project.send_move_instructions(@old_path) - # Move main repository - # TODO: check storage type and NOOP when not using Legacy - unless move_repo_folder(@old_path, @new_path) - raise TransferError.new('Cannot move project') - end - - # Move wiki repo also if present - # TODO: check storage type and NOOP when not using Legacy - move_repo_folder("#{@old_path}.wiki", "#{@new_path}.wiki") + # Directories on disk + move_project_folders(project) # Move missing group labels to project Labels::TransferService.new(current_user, @old_group, project).execute # Move uploads - Gitlab::UploadsTransfer.new.move_project(project.path, @old_namespace.full_path, @new_namespace.full_path) + move_project_uploads(project) # Move pages Gitlab::PagesTransfer.new.move_project(project.path, @old_namespace.full_path, @new_namespace.full_path) @@ -131,5 +124,30 @@ module Projects def execute_system_hooks SystemHooksService.new.execute_hooks_for(project, :transfer) end + + def move_project_folders(project) + return if project.hashed_storage?(:repository) + + # Move main repository + unless move_repo_folder(@old_path, @new_path) + raise TransferError.new("Cannot move project") + end + + # Disk path is changed; we need to ensure we reload it + project.reload_repository! + + # Move wiki repo also if present + move_repo_folder("#{@old_path}.wiki", "#{@new_path}.wiki") + end + + def move_project_uploads(project) + return if project.hashed_storage?(:attachments) + + Gitlab::UploadsTransfer.new.move_project( + project.path, + @old_namespace.full_path, + @new_namespace.full_path + ) + end end end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 3a4d5ce0b5c..12658dddc06 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -743,5 +743,56 @@ installations. Set to 0 to completely disable polling. = link_to icon('question-circle'), help_page_path('administration/polling') + %fieldset + %legend User and IP Rate Limits + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :throttle_unauthenticated_enabled do + = f.check_box :throttle_unauthenticated_enabled + Enable unauthenticated request rate limit + %span.help-block + Helps reduce request volume (e.g. from crawlers or abusive bots) + .form-group + = f.label :throttle_unauthenticated_requests_per_period, 'Max requests per period per IP', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :throttle_unauthenticated_requests_per_period, class: 'form-control' + .form-group + = f.label :throttle_unauthenticated_period_in_seconds, 'Rate limit period in seconds', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :throttle_unauthenticated_period_in_seconds, class: 'form-control' + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :throttle_authenticated_api_enabled do + = f.check_box :throttle_authenticated_api_enabled + Enable authenticated API request rate limit + %span.help-block + Helps reduce request volume (e.g. from crawlers or abusive bots) + .form-group + = f.label :throttle_authenticated_api_requests_per_period, 'Max requests per period per user', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :throttle_authenticated_api_requests_per_period, class: 'form-control' + .form-group + = f.label :throttle_authenticated_api_period_in_seconds, 'Rate limit period in seconds', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :throttle_authenticated_api_period_in_seconds, class: 'form-control' + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :throttle_authenticated_web_enabled do + = f.check_box :throttle_authenticated_web_enabled + Enable authenticated web request rate limit + %span.help-block + Helps reduce request volume (e.g. from crawlers or abusive bots) + .form-group + = f.label :throttle_authenticated_web_requests_per_period, 'Max requests per period per user', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :throttle_authenticated_web_requests_per_period, class: 'form-control' + .form-group + = f.label :throttle_authenticated_web_period_in_seconds, 'Rate limit period in seconds', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :throttle_authenticated_web_period_in_seconds, class: 'form-control' + .form-actions = f.submit 'Save', class: 'btn btn-save' diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index df2bf27be9d..6d8fad0eb8d 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -99,7 +99,7 @@ %td.build-link - if project - = link_to ci_status_path(build.pipeline) do + = link_to pipeline_path(build.pipeline) do %strong= build.pipeline.short_sha %td.timestamp diff --git a/app/views/projects/diffs/_stats.html.haml b/app/views/projects/diffs/_stats.html.haml index 2de2cf9e38c..dd473ebe580 100644 --- a/app/views/projects/diffs/_stats.html.haml +++ b/app/views/projects/diffs/_stats.html.haml @@ -22,9 +22,11 @@ - diff_files.each do |diff_file| %li %a.diff-changed-file{ href: "##{hexdigest(diff_file.file_path)}", title: diff_file.new_path } - = icon("#{diff_file_changed_icon(diff_file)} fw", class: "#{diff_file_changed_icon_color(diff_file)} append-right-5") - %span.diff-file-changes-path.append-right-5= diff_file.new_path - .pull-right + = sprite_icon(diff_file_changed_icon(diff_file), size: 16, css_class: "#{diff_file_changed_icon_color(diff_file)} diff-file-changed-icon append-right-8") + %span.diff-changed-file-content.append-right-8 + %strong.diff-changed-file-name= diff_file.blob.name + %span.diff-changed-file-path.prepend-top-5= diff_file.new_path + %span.diff-changed-stats %span.cgreen< +#{diff_file.added_lines} %span.cred< diff --git a/app/views/projects/wikis/_pages_wiki_page.html.haml b/app/views/projects/wikis/_pages_wiki_page.html.haml index 0a1ccbc5f1c..efa16d38f84 100644 --- a/app/views/projects/wikis/_pages_wiki_page.html.haml +++ b/app/views/projects/wikis/_pages_wiki_page.html.haml @@ -2,4 +2,4 @@ = link_to wiki_page.title, project_wiki_path(@project, wiki_page) %small (#{wiki_page.format}) .pull-right - %small= (s_("Last edited %{date}") % { date: time_ago_with_tooltip(wiki_page.commit.authored_date) }).html_safe + %small= (s_("Last edited %{date}") % { date: time_ago_with_tooltip(wiki_page.last_version.authored_date) }).html_safe diff --git a/app/views/projects/wikis/history.html.haml b/app/views/projects/wikis/history.html.haml index 9ee09262324..969a1677d9a 100644 --- a/app/views/projects/wikis/history.html.haml +++ b/app/views/projects/wikis/history.html.haml @@ -21,7 +21,7 @@ %th= _("Last updated") %th= _("Format") %tbody - - @page.versions.each_with_index do |version, index| + - @page_versions.each_with_index do |version, index| - commit = version %tr %td @@ -37,5 +37,6 @@ %td %strong = version.format += paginate @page_versions, theme: 'gitlab' = render 'sidebar' diff --git a/app/views/projects/wikis/show.html.haml b/app/views/projects/wikis/show.html.haml index de15fc99eda..b3b83cee81a 100644 --- a/app/views/projects/wikis/show.html.haml +++ b/app/views/projects/wikis/show.html.haml @@ -11,8 +11,8 @@ .nav-text %h2.wiki-page-title= @page.title.capitalize %span.wiki-last-edit-by - = (_("Last edited by %{name}") % { name: "<strong>#{@page.commit.author_name}</strong>" }).html_safe - #{time_ago_with_tooltip(@page.commit.authored_date)} + = (_("Last edited by %{name}") % { name: "<strong>#{@page.last_version.author_name}</strong>" }).html_safe + #{time_ago_with_tooltip(@page.last_version.authored_date)} .nav-controls = render 'main_links' diff --git a/changelogs/unreleased/34600-performance-wiki-pages.yml b/changelogs/unreleased/34600-performance-wiki-pages.yml new file mode 100644 index 00000000000..541ae8f8e60 --- /dev/null +++ b/changelogs/unreleased/34600-performance-wiki-pages.yml @@ -0,0 +1,5 @@ +--- +title: Performance issues when loading large number of wiki pages +merge_request: 15276 +author: +type: performance diff --git a/changelogs/unreleased/38822-oauth-search-case-insensitive.yml b/changelogs/unreleased/38822-oauth-search-case-insensitive.yml new file mode 100644 index 00000000000..d84360b4c5c --- /dev/null +++ b/changelogs/unreleased/38822-oauth-search-case-insensitive.yml @@ -0,0 +1,5 @@ +--- +title: OAuth identity lookups case-insensitive +merge_request: 15312 +author: +type: fixed diff --git a/changelogs/unreleased/bvl-delete-empty-fork-networks.yml b/changelogs/unreleased/bvl-delete-empty-fork-networks.yml new file mode 100644 index 00000000000..3bbb4cf6e3c --- /dev/null +++ b/changelogs/unreleased/bvl-delete-empty-fork-networks.yml @@ -0,0 +1,5 @@ +--- +title: Clean up empty fork networks +merge_request: 15373 +author: +type: other diff --git a/changelogs/unreleased/bvl-fix-count-with-selects.yml b/changelogs/unreleased/bvl-fix-count-with-selects.yml new file mode 100644 index 00000000000..46a882de524 --- /dev/null +++ b/changelogs/unreleased/bvl-fix-count-with-selects.yml @@ -0,0 +1,6 @@ +--- +title: Fix crash when navigating to second page of the group dashbaord when there + are projects and groups on the first page +merge_request: 15456 +author: +type: fixed diff --git a/changelogs/unreleased/ci-pipeline-status-query.yml b/changelogs/unreleased/ci-pipeline-status-query.yml new file mode 100644 index 00000000000..a464e501418 --- /dev/null +++ b/changelogs/unreleased/ci-pipeline-status-query.yml @@ -0,0 +1,5 @@ +--- +title: Optimise getting the pipeline status of commits +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/improved-changes-dropdown.yml b/changelogs/unreleased/improved-changes-dropdown.yml new file mode 100644 index 00000000000..f305cbe573b --- /dev/null +++ b/changelogs/unreleased/improved-changes-dropdown.yml @@ -0,0 +1,5 @@ +--- +title: Improved diff changed files dropdown design +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/mk-add-user-rate-limits.yml b/changelogs/unreleased/mk-add-user-rate-limits.yml new file mode 100644 index 00000000000..512757da5fc --- /dev/null +++ b/changelogs/unreleased/mk-add-user-rate-limits.yml @@ -0,0 +1,6 @@ +--- +title: Add anonymous rate limit per IP, and authenticated (web or API) rate limits + per user +merge_request: 14708 +author: +type: added diff --git a/changelogs/unreleased/sh-port-hashed-storage-transfer-fix.yml b/changelogs/unreleased/sh-port-hashed-storage-transfer-fix.yml new file mode 100644 index 00000000000..c32afc90f64 --- /dev/null +++ b/changelogs/unreleased/sh-port-hashed-storage-transfer-fix.yml @@ -0,0 +1,5 @@ +--- +title: Fix hashed storage with project transfers to another namespace +merge_request: +author: +type: fixed diff --git a/config/application.rb b/config/application.rb index 5100ec5d2b7..6436f887d14 100644 --- a/config/application.rb +++ b/config/application.rb @@ -113,7 +113,7 @@ module Gitlab config.action_view.sanitized_allowed_protocols = %w(smb) - config.middleware.insert_before Warden::Manager, Rack::Attack + config.middleware.insert_after Warden::Manager, Rack::Attack # Allow access to GitLab API from other domains config.middleware.insert_before Warden::Manager, Rack::Cors do diff --git a/config/initializers/gollum.rb b/config/initializers/gollum.rb index 1ebe3c7a742..2ca32791bb1 100644 --- a/config/initializers/gollum.rb +++ b/config/initializers/gollum.rb @@ -10,4 +10,30 @@ module Gollum index.send(name, *args) end end + + class Wiki + def pages(treeish = nil, limit: nil) + tree_list((treeish || @ref), limit: limit) + end + + def tree_list(ref, limit: nil) + if (sha = @access.ref_to_sha(ref)) + commit = @access.commit(sha) + tree_map_for(sha).inject([]) do |list, entry| + next list unless @page_class.valid_page_name?(entry.name) + list << entry.page(self, commit) + break list if limit && list.size >= limit + list + end + else + [] + end + end + end +end + +Rails.application.configure do + config.after_initialize do + Gollum::Page.per_page = Kaminari.config.default_per_page + end end diff --git a/config/initializers/rack_attack_global.rb b/config/initializers/rack_attack_global.rb new file mode 100644 index 00000000000..9453df2ec5a --- /dev/null +++ b/config/initializers/rack_attack_global.rb @@ -0,0 +1,61 @@ +module Gitlab::Throttle + def self.settings + Gitlab::CurrentSettings.current_application_settings + end + + def self.unauthenticated_options + limit_proc = proc { |req| settings.throttle_unauthenticated_requests_per_period } + period_proc = proc { |req| settings.throttle_unauthenticated_period_in_seconds.seconds } + { limit: limit_proc, period: period_proc } + end + + def self.authenticated_api_options + limit_proc = proc { |req| settings.throttle_authenticated_api_requests_per_period } + period_proc = proc { |req| settings.throttle_authenticated_api_period_in_seconds.seconds } + { limit: limit_proc, period: period_proc } + end + + def self.authenticated_web_options + limit_proc = proc { |req| settings.throttle_authenticated_web_requests_per_period } + period_proc = proc { |req| settings.throttle_authenticated_web_period_in_seconds.seconds } + { limit: limit_proc, period: period_proc } + end +end + +class Rack::Attack + throttle('throttle_unauthenticated', Gitlab::Throttle.unauthenticated_options) do |req| + Gitlab::Throttle.settings.throttle_unauthenticated_enabled && + req.unauthenticated? && + req.ip + end + + throttle('throttle_authenticated_api', Gitlab::Throttle.authenticated_api_options) do |req| + Gitlab::Throttle.settings.throttle_authenticated_api_enabled && + req.api_request? && + req.authenticated_user_id + end + + throttle('throttle_authenticated_web', Gitlab::Throttle.authenticated_web_options) do |req| + Gitlab::Throttle.settings.throttle_authenticated_web_enabled && + req.web_request? && + req.authenticated_user_id + end + + class Request + def unauthenticated? + !authenticated_user_id + end + + def authenticated_user_id + Gitlab::Auth::RequestAuthenticator.new(self).user&.id + end + + def api_request? + path.start_with?('/api') + end + + def web_request? + !api_request? + end + end +end diff --git a/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb b/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb index 22bac46e25c..1716b6e8153 100644 --- a/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb +++ b/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb @@ -1,4 +1,4 @@ -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class AddOnlyAllowMergeIfBuildSucceedsToProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160608195742_add_repository_storage_to_projects.rb b/db/migrate/20160608195742_add_repository_storage_to_projects.rb index 0f3664c13ef..e4febd1614d 100644 --- a/db/migrate/20160608195742_add_repository_storage_to_projects.rb +++ b/db/migrate/20160608195742_add_repository_storage_to_projects.rb @@ -1,4 +1,4 @@ -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class AddRepositoryStorageToProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160615191922_set_missing_stage_on_ci_builds.rb b/db/migrate/20160615191922_set_missing_stage_on_ci_builds.rb index 5336b036bca..c58cb957df4 100644 --- a/db/migrate/20160615191922_set_missing_stage_on_ci_builds.rb +++ b/db/migrate/20160615191922_set_missing_stage_on_ci_builds.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable # rubocop:disable Migration/UpdateColumnInBatches class SetMissingStageOnCiBuilds < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160715154212_add_request_access_enabled_to_projects.rb b/db/migrate/20160715154212_add_request_access_enabled_to_projects.rb index 5dc26f8982a..22c925799a3 100644 --- a/db/migrate/20160715154212_add_request_access_enabled_to_projects.rb +++ b/db/migrate/20160715154212_add_request_access_enabled_to_projects.rb @@ -1,4 +1,4 @@ -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class AddRequestAccessEnabledToProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160715204316_add_request_access_enabled_to_groups.rb b/db/migrate/20160715204316_add_request_access_enabled_to_groups.rb index 4a317646788..4fcb29e1325 100644 --- a/db/migrate/20160715204316_add_request_access_enabled_to_groups.rb +++ b/db/migrate/20160715204316_add_request_access_enabled_to_groups.rb @@ -1,4 +1,4 @@ -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class AddRequestAccessEnabledToGroups < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160721081015_drop_and_readd_has_external_wiki_in_projects.rb b/db/migrate/20160721081015_drop_and_readd_has_external_wiki_in_projects.rb index abe8e701e23..58f7f2a2841 100644 --- a/db/migrate/20160721081015_drop_and_readd_has_external_wiki_in_projects.rb +++ b/db/migrate/20160721081015_drop_and_readd_has_external_wiki_in_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable # rubocop:disable Migration/UpdateColumnInBatches class DropAndReaddHasExternalWikiInProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160831223750_remove_features_enabled_from_projects.rb b/db/migrate/20160831223750_remove_features_enabled_from_projects.rb index 7414a28ac97..aec709aaf59 100644 --- a/db/migrate/20160831223750_remove_features_enabled_from_projects.rb +++ b/db/migrate/20160831223750_remove_features_enabled_from_projects.rb @@ -1,7 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class RemoveFeaturesEnabledFromProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb b/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb index 0100e30a733..df7d922b816 100644 --- a/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb +++ b/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb @@ -1,7 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class RemoveProjectsPushesSinceGc < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb b/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb index ae37da275fd..27ebe0af33b 100644 --- a/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb +++ b/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb @@ -1,4 +1,4 @@ -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class AddTwoFactorColumnsToNamespaces < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170124193205_add_two_factor_columns_to_users.rb b/db/migrate/20170124193205_add_two_factor_columns_to_users.rb index 8d4aefa4365..558a1837c79 100644 --- a/db/migrate/20170124193205_add_two_factor_columns_to_users.rb +++ b/db/migrate/20170124193205_add_two_factor_columns_to_users.rb @@ -1,4 +1,4 @@ -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class AddTwoFactorColumnsToUsers < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb b/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb index 7ad01a04815..6d43f346d4f 100644 --- a/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb +++ b/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb @@ -1,7 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class AddPrintingMergeRequestLinkEnabledToProject < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb b/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb index f335e77fb5e..3c5cd95726a 100644 --- a/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb +++ b/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb @@ -1,4 +1,4 @@ -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class AddAutoCancelPendingPipelinesToProject < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb b/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb index 6c9fe19ca34..807dfcb385d 100644 --- a/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb +++ b/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb @@ -1,4 +1,4 @@ -# rubocop:disable Migration/AddColumnWithDefaultToLargeTable +# rubocop:disable Migration/UpdateLargeTable class RevertAddNotifiedOfOwnActivityToUsers < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20170320173259_migrate_assignees.rb b/db/migrate/20170320173259_migrate_assignees.rb index 7b61e811317..255b5e9c4db 100644 --- a/db/migrate/20170320173259_migrate_assignees.rb +++ b/db/migrate/20170320173259_migrate_assignees.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable # rubocop:disable Migration/UpdateColumnInBatches class MigrateAssignees < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20171006220837_add_global_rate_limits_to_application_settings.rb b/db/migrate/20171006220837_add_global_rate_limits_to_application_settings.rb new file mode 100644 index 00000000000..55e822752af --- /dev/null +++ b/db/migrate/20171006220837_add_global_rate_limits_to_application_settings.rb @@ -0,0 +1,38 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddGlobalRateLimitsToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :application_settings, :throttle_unauthenticated_enabled, :boolean, default: false, allow_null: false + add_column_with_default :application_settings, :throttle_unauthenticated_requests_per_period, :integer, default: 3600, allow_null: false + add_column_with_default :application_settings, :throttle_unauthenticated_period_in_seconds, :integer, default: 3600, allow_null: false + + add_column_with_default :application_settings, :throttle_authenticated_api_enabled, :boolean, default: false, allow_null: false + add_column_with_default :application_settings, :throttle_authenticated_api_requests_per_period, :integer, default: 7200, allow_null: false + add_column_with_default :application_settings, :throttle_authenticated_api_period_in_seconds, :integer, default: 3600, allow_null: false + + add_column_with_default :application_settings, :throttle_authenticated_web_enabled, :boolean, default: false, allow_null: false + add_column_with_default :application_settings, :throttle_authenticated_web_requests_per_period, :integer, default: 7200, allow_null: false + add_column_with_default :application_settings, :throttle_authenticated_web_period_in_seconds, :integer, default: 3600, allow_null: false + end + + def down + remove_column :application_settings, :throttle_authenticated_web_period_in_seconds + remove_column :application_settings, :throttle_authenticated_web_requests_per_period + remove_column :application_settings, :throttle_authenticated_web_enabled + + remove_column :application_settings, :throttle_authenticated_api_period_in_seconds + remove_column :application_settings, :throttle_authenticated_api_requests_per_period + remove_column :application_settings, :throttle_authenticated_api_enabled + + remove_column :application_settings, :throttle_unauthenticated_period_in_seconds + remove_column :application_settings, :throttle_unauthenticated_requests_per_period + remove_column :application_settings, :throttle_unauthenticated_enabled + end +end diff --git a/db/post_migrate/20170131214021_reset_users_authorized_projects_populated.rb b/db/post_migrate/20170131214021_reset_users_authorized_projects_populated.rb index 82f8147547e..f1f81691f81 100644 --- a/db/post_migrate/20170131214021_reset_users_authorized_projects_populated.rb +++ b/db/post_migrate/20170131214021_reset_users_authorized_projects_populated.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable # rubocop:disable Migration/UpdateColumnInBatches class ResetUsersAuthorizedProjectsPopulated < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170309171644_reset_relative_position_for_issue.rb b/db/post_migrate/20170309171644_reset_relative_position_for_issue.rb index 01fff680183..49fd46b0262 100644 --- a/db/post_migrate/20170309171644_reset_relative_position_for_issue.rb +++ b/db/post_migrate/20170309171644_reset_relative_position_for_issue.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable # rubocop:disable Migration/UpdateColumnInBatches class ResetRelativePositionForIssue < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170324160416_migrate_user_activities_to_users_last_activity_on.rb b/db/post_migrate/20170324160416_migrate_user_activities_to_users_last_activity_on.rb index cb1b4f1855d..78413a608f1 100644 --- a/db/post_migrate/20170324160416_migrate_user_activities_to_users_last_activity_on.rb +++ b/db/post_migrate/20170324160416_migrate_user_activities_to_users_last_activity_on.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable class MigrateUserActivitiesToUsersLastActivityOn < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170406142253_migrate_user_project_view.rb b/db/post_migrate/20170406142253_migrate_user_project_view.rb index c4e910b3b44..d6061dd416d 100644 --- a/db/post_migrate/20170406142253_migrate_user_project_view.rb +++ b/db/post_migrate/20170406142253_migrate_user_project_view.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb b/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb index 765daa0a347..bba37e32c01 100644 --- a/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb +++ b/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable # rubocop:disable Migration/UpdateColumnInBatches class EnableAutoCancelPendingPipelinesForAll < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170503004427_update_retried_for_ci_build.rb b/db/post_migrate/20170503004427_update_retried_for_ci_build.rb index 9d9f36550e7..b0b58ab3011 100644 --- a/db/post_migrate/20170503004427_update_retried_for_ci_build.rb +++ b/db/post_migrate/20170503004427_update_retried_for_ci_build.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable class UpdateRetriedForCiBuild < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170508170547_add_head_pipeline_for_each_merge_request.rb b/db/post_migrate/20170508170547_add_head_pipeline_for_each_merge_request.rb index f77078ddd70..81e9d050668 100644 --- a/db/post_migrate/20170508170547_add_head_pipeline_for_each_merge_request.rb +++ b/db/post_migrate/20170508170547_add_head_pipeline_for_each_merge_request.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable class AddHeadPipelineForEachMergeRequest < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170518231126_fix_wrongly_renamed_routes.rb b/db/post_migrate/20170518231126_fix_wrongly_renamed_routes.rb index c78beda9d21..3e952980866 100644 --- a/db/post_migrate/20170518231126_fix_wrongly_renamed_routes.rb +++ b/db/post_migrate/20170518231126_fix_wrongly_renamed_routes.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/post_migrate/20170526190000_migrate_build_stage_reference_again.rb b/db/post_migrate/20170526190000_migrate_build_stage_reference_again.rb index 97cb242415d..31a73bb3b27 100644 --- a/db/post_migrate/20170526190000_migrate_build_stage_reference_again.rb +++ b/db/post_migrate/20170526190000_migrate_build_stage_reference_again.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable class MigrateBuildStageReferenceAgain < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170927112318_update_legacy_diff_notes_type_for_import.rb b/db/post_migrate/20170927112318_update_legacy_diff_notes_type_for_import.rb index a238216253b..b040c81b316 100644 --- a/db/post_migrate/20170927112318_update_legacy_diff_notes_type_for_import.rb +++ b/db/post_migrate/20170927112318_update_legacy_diff_notes_type_for_import.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable class UpdateLegacyDiffNotesTypeForImport < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170927112319_update_notes_type_for_import.rb b/db/post_migrate/20170927112319_update_notes_type_for_import.rb index 1e70acd9868..5a400c71b02 100644 --- a/db/post_migrate/20170927112319_update_notes_type_for_import.rb +++ b/db/post_migrate/20170927112319_update_notes_type_for_import.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/UpdateLargeTable class UpdateNotesTypeForImport < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20171026082505_populate_merge_requests_latest_merge_request_diff_id.rb b/db/post_migrate/20171026082505_populate_merge_requests_latest_merge_request_diff_id.rb deleted file mode 100644 index a7ebbbf34c0..00000000000 --- a/db/post_migrate/20171026082505_populate_merge_requests_latest_merge_request_diff_id.rb +++ /dev/null @@ -1,27 +0,0 @@ -class PopulateMergeRequestsLatestMergeRequestDiffId < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - BATCH_SIZE = 1_000 - - class MergeRequest < ActiveRecord::Base - self.table_name = 'merge_requests' - - include ::EachBatch - end - - disable_ddl_transaction! - - def up - update = ' - latest_merge_request_diff_id = ( - SELECT MAX(id) - FROM merge_request_diffs - WHERE merge_requests.id = merge_request_diffs.merge_request_id - )'.squish - - MergeRequest.where(latest_merge_request_diff_id: nil).each_batch(of: BATCH_SIZE) do |relation| - relation.update_all(update) - end - end -end diff --git a/db/post_migrate/20171026082505_schedule_merge_request_latest_merge_request_diff_id_migrations.rb b/db/post_migrate/20171026082505_schedule_merge_request_latest_merge_request_diff_id_migrations.rb new file mode 100644 index 00000000000..7a63382cc6d --- /dev/null +++ b/db/post_migrate/20171026082505_schedule_merge_request_latest_merge_request_diff_id_migrations.rb @@ -0,0 +1,29 @@ +class ScheduleMergeRequestLatestMergeRequestDiffIdMigrations < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 50_000 + MIGRATION = 'PopulateMergeRequestsLatestMergeRequestDiffId' + + disable_ddl_transaction! + + class MergeRequest < ActiveRecord::Base + self.table_name = 'merge_requests' + + include ::EachBatch + end + + # On GitLab.com, we saw that we generated about 500,000 dead tuples over 5 minutes. + # To keep replication lag from ballooning, we'll aim for 50,000 updates over 5 minutes. + # + # Assuming that there are 5 million rows affected (which is more than on + # GitLab.com), and that each batch of 50,000 rows takes up to 5 minutes, then + # we can migrate all the rows in 8.5 hours. + def up + MergeRequest.where(latest_merge_request_diff_id: nil).each_batch(of: BATCH_SIZE) do |relation, index| + range = relation.pluck('MIN(id)', 'MAX(id)').first + + BackgroundMigrationWorker.perform_in(index * 5.minutes, MIGRATION, range) + end + end +end diff --git a/db/post_migrate/20171114104051_remove_empty_fork_networks.rb b/db/post_migrate/20171114104051_remove_empty_fork_networks.rb new file mode 100644 index 00000000000..2fe99a1b9c1 --- /dev/null +++ b/db/post_migrate/20171114104051_remove_empty_fork_networks.rb @@ -0,0 +1,36 @@ +class RemoveEmptyForkNetworks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 10_000 + + class MigrationForkNetwork < ActiveRecord::Base + include EachBatch + + self.table_name = 'fork_networks' + end + + class MigrationForkNetworkMembers < ActiveRecord::Base + self.table_name = 'fork_network_members' + end + + disable_ddl_transaction! + + def up + say 'Deleting empty ForkNetworks in batches' + + has_members = MigrationForkNetworkMembers + .where('fork_network_members.fork_network_id = fork_networks.id') + .select(1) + MigrationForkNetwork.where('NOT EXISTS (?)', has_members) + .each_batch(of: BATCH_SIZE) do |networks| + deleted = networks.delete_all + + say "Deleted #{deleted} rows in batch" + end + end + + def down + # nothing + end +end diff --git a/db/schema.rb b/db/schema.rb index 37e08d453c8..25b4fa8b888 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171106180641) do +ActiveRecord::Schema.define(version: 20171114104051) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -140,6 +140,15 @@ ActiveRecord::Schema.define(version: 20171106180641) do t.integer "circuitbreaker_storage_timeout", default: 30 t.integer "circuitbreaker_access_retries", default: 3 t.integer "circuitbreaker_backoff_threshold", default: 80 + t.boolean "throttle_unauthenticated_enabled", default: false, null: false + t.integer "throttle_unauthenticated_requests_per_period", default: 3600, null: false + t.integer "throttle_unauthenticated_period_in_seconds", default: 3600, null: false + t.boolean "throttle_authenticated_api_enabled", default: false, null: false + t.integer "throttle_authenticated_api_requests_per_period", default: 7200, null: false + t.integer "throttle_authenticated_api_period_in_seconds", default: 3600, null: false + t.boolean "throttle_authenticated_web_enabled", default: false, null: false + t.integer "throttle_authenticated_web_requests_per_period", default: 7200, null: false + t.integer "throttle_authenticated_web_period_in_seconds", default: 3600, null: false end create_table "audit_events", force: :cascade do |t| diff --git a/doc/administration/high_availability/README.md b/doc/administration/high_availability/README.md index 4d3be0ab8f6..a88e67bfeb5 100644 --- a/doc/administration/high_availability/README.md +++ b/doc/administration/high_availability/README.md @@ -53,7 +53,9 @@ or in different cloud availability zones. > **Note:** GitLab recommends against choosing this HA method because of the complexity of managing DRBD and crafting automatic failover. This is - *compatible* with GitLab, but not officially *supported*. + *compatible* with GitLab, but not officially *supported*. If you are + an EE customer, support will help you with GitLab related problems, but if the + root cause is identified as DRBD, we will not troubleshoot further. Components/Servers Required: 2 servers/virtual machines (one active/one passive) diff --git a/doc/api/groups.md b/doc/api/groups.md index 6a6e94195a7..c1b5737c247 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -82,6 +82,8 @@ GET /groups?custom_attributes[key]=value&custom_attributes[other_key]=other_valu ## List a groups's subgroups +> [Introduced][ce-15142] in GitLab 10.3. + Get a list of visible direct subgroups in this group. When accessed without authentication, only public groups are returned. @@ -513,3 +515,5 @@ And to switch pages add: ``` /groups?per_page=100&page=2 ``` + +[ce-15142]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/15142 diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md index e1c785d85fa..50eb8005b44 100644 --- a/doc/development/database_debugging.md +++ b/doc/development/database_debugging.md @@ -9,17 +9,24 @@ An easy first step is to search for your error in Slack or google "GitLab <my er Available `RAILS_ENV` + - `production` (generally not for your main GDK db, but you may need this for e.g. omnibus) - `development` (this is your main GDK db) - `test` (used for tests like rspec and spinach) ## Nuke everything and start over -If you just want to delete everything and start over, +If you just want to delete everything and start over with an empty DB (~1 minute): - - `bundle exec rake dev:setup RAILS_ENV=development` : Also runs DB specific stuff and seeds dummy data (slow) - - `bundle exec rake db:reset RAILS_ENV=development` : Doesn't do the above (fast) - - `bundle exec rake db:reset RAILS_ENV=test` : Fix the test DB, since it doesn't contain important data. + - `bundle exec rake db:reset RAILS_ENV=development` + +If you just want to delete everything and start over with dummy data (~40 minutes). This also does `db:reset` and runs DB-specific migrations: + + - `bundle exec rake dev:setup RAILS_ENV=development` + +If your test DB is giving you problems, it is safe to nuke it because it doesn't contain important data: + + - `bundle exec rake db:reset RAILS_ENV=test` ## Migration wrangling diff --git a/doc/development/fe_guide/dropdowns.md b/doc/development/fe_guide/dropdowns.md new file mode 100644 index 00000000000..e1660ac5caa --- /dev/null +++ b/doc/development/fe_guide/dropdowns.md @@ -0,0 +1,38 @@ +# Dropdowns + + +## How to style a bootstrap dropdown +1. Use the HTML structure provided by the [docs][bootstrap-dropdowns] +1. Add a specific class to the top level `.dropdown` element + + + ```Haml + .dropdown.my-dropdown + %button{ type: 'button', data: { toggle: 'dropdown' }, 'aria-haspopup': true, 'aria-expanded': false } + %span.dropdown-toggle-text + Toggle Dropdown + = icon('chevron-down') + + %ul.dropdown-menu + %li + %a + item! + ``` + + Or use the helpers + ```Haml + .dropdown.my-dropdown + = dropdown_toggle('Toogle!', { toggle: 'dropdown' }) + = dropdown_content + %li + %a + item! + ``` + +1. Include the mixin in CSS + + ```SCSS + @include new-style-dropdown('.my-dropdown '); + ``` + +[bootstrap-dropdowns]: https://getbootstrap.com/docs/3.3/javascript/#dropdowns diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 8f956681693..73a03c07812 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -77,6 +77,8 @@ Vue resource specific practices and gotchas. ## [Icons](icons.md) How we use SVG for our Icons. +## [Dropdowns](dropdowns.md) +How we use dropdowns. --- ## Style Guides diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 9b8ab5da74e..a235dd74909 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -198,7 +198,43 @@ end Keep in mind that this operation can easily take 10-15 minutes to complete on larger installations (e.g. GitLab.com). As a result you should only add default -values if absolutely necessary. +values if absolutely necessary. There is a RuboCop cop that will fail if this +method is used on some tables that are very large on GitLab.com, which would +cause other issues. + +## Updating an existing column + +To update an existing column to a particular value, you can use +`update_column_in_batches` (`add_column_with_default` uses this internally to +fill in the default value). This will split the updates into batches, so we +don't update too many rows at in a single statement. + +This updates the column `foo` in the `projects` table to 10, where `some_column` +is `'hello'`: + +```ruby +update_column_in_batches(:projects, :foo, 10) do |table, query| + query.where(table[:some_column].eq('hello')) +end +``` + +To perform a computed update, the value can be wrapped in `Arel.sql`, so Arel +treats it as an SQL literal. The below example is the same as the one above, but +the value is set to the product of the `bar` and `baz` columns: + +```ruby +update_value = Arel.sql('bar * baz') + +update_column_in_batches(:projects, :foo, update_value) do |table, query| + query.where(table[:some_column].eq('hello')) +end +``` + +Like `add_column_with_default`, there is a RuboCop cop to detect usage of this +on large tables. In the case of `update_column_in_batches`, it may be acceptable +to run on a large table, as long as it is only updating a small subset of the +rows in the table, but do not ignore that without validating on the GitLab.com +staging environment - or asking someone else to do so for you - beforehand. ## Integer column type diff --git a/doc/user/discussions/img/image_resolved_discussion.png b/doc/user/discussions/img/image_resolved_discussion.png Binary files differindex ed00b5c77fe..ed00b5c77fe 100755..100644 --- a/doc/user/discussions/img/image_resolved_discussion.png +++ b/doc/user/discussions/img/image_resolved_discussion.png diff --git a/doc/user/discussions/img/onion_skin_view.png b/doc/user/discussions/img/onion_skin_view.png Binary files differindex 91c3b396844..91c3b396844 100755..100644 --- a/doc/user/discussions/img/onion_skin_view.png +++ b/doc/user/discussions/img/onion_skin_view.png diff --git a/doc/user/discussions/img/swipe_view.png b/doc/user/discussions/img/swipe_view.png Binary files differindex 82d6e52173c..82d6e52173c 100755..100644 --- a/doc/user/discussions/img/swipe_view.png +++ b/doc/user/discussions/img/swipe_view.png diff --git a/doc/user/discussions/img/two_up_view.png b/doc/user/discussions/img/two_up_view.png Binary files differindex d9e90708e87..d9e90708e87 100755..100644 --- a/doc/user/discussions/img/two_up_view.png +++ b/doc/user/discussions/img/two_up_view.png diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index c1c0d344917..9aeebc34525 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -6,9 +6,6 @@ module API module APIGuard extend ActiveSupport::Concern - PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN".freeze - PRIVATE_TOKEN_PARAM = :private_token - included do |base| # OAuth2 Resource Server Authentication use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| @@ -42,7 +39,7 @@ module API # Helper Methods for Grape Endpoint module HelperMethods - include Gitlab::Utils::StrongMemoize + include Gitlab::Auth::UserAuthFinders def find_current_user! user = find_user_from_access_token || find_user_from_warden @@ -53,76 +50,8 @@ module API user end - def access_token - strong_memoize(:access_token) do - find_oauth_access_token || find_personal_access_token - end - end - - def validate_access_token!(scopes: []) - return unless access_token - - case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes) - when AccessTokenValidationService::INSUFFICIENT_SCOPE - raise InsufficientScopeError.new(scopes) - when AccessTokenValidationService::EXPIRED - raise ExpiredError - when AccessTokenValidationService::REVOKED - raise RevokedError - end - end - private - def find_user_from_access_token - return unless access_token - - validate_access_token! - - access_token.user || raise(UnauthorizedError) - end - - # Check the Rails session for valid authentication details - def find_user_from_warden - warden.try(:authenticate) if verified_request? - end - - def warden - env['warden'] - end - - # Check if the request is GET/HEAD, or if CSRF token is valid. - def verified_request? - Gitlab::RequestForgeryProtection.verified?(env) - end - - def find_oauth_access_token - token = Doorkeeper::OAuth::Token.from_request(doorkeeper_request, *Doorkeeper.configuration.access_token_methods) - return unless token - - # Expiration, revocation and scopes are verified in `find_user_by_access_token` - access_token = OauthAccessToken.by_token(token) - raise UnauthorizedError unless access_token - - access_token.revoke_previous_refresh_token! - access_token - end - - def find_personal_access_token - token = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s - return unless token.present? - - # Expiration, revocation and scopes are verified in `find_user_by_access_token` - access_token = PersonalAccessToken.find_by(token: token) - raise UnauthorizedError unless access_token - - access_token - end - - def doorkeeper_request - @doorkeeper_request ||= ActionDispatch::Request.new(env) - end - # An array of scopes that were registered (using `allow_access_with_scope`) # for the current endpoint class. It also returns scopes registered on # `API::API`, since these are meant to apply to all API routes. @@ -145,8 +74,11 @@ module API private def install_error_responders(base) - error_classes = [MissingTokenError, TokenNotFoundError, - ExpiredError, RevokedError, InsufficientScopeError] + error_classes = [Gitlab::Auth::MissingTokenError, + Gitlab::Auth::TokenNotFoundError, + Gitlab::Auth::ExpiredError, + Gitlab::Auth::RevokedError, + Gitlab::Auth::InsufficientScopeError] base.__send__(:rescue_from, *error_classes, oauth2_bearer_token_error_handler) # rubocop:disable GitlabSecurity/PublicSend end @@ -155,25 +87,25 @@ module API proc do |e| response = case e - when MissingTokenError + when Gitlab::Auth::MissingTokenError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new - when TokenNotFoundError + when Gitlab::Auth::TokenNotFoundError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Bad Access Token.") - when ExpiredError + when Gitlab::Auth::ExpiredError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Token is expired. You can either do re-authorization or token refresh.") - when RevokedError + when Gitlab::Auth::RevokedError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Token was revoked. You have to re-authorize from the user.") - when InsufficientScopeError + when Gitlab::Auth::InsufficientScopeError # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2) # does not include WWW-Authenticate header, which breaks the standard. Rack::OAuth2::Server::Resource::Bearer::Forbidden.new( @@ -186,22 +118,5 @@ module API end end end - - # - # Exceptions - # - - MissingTokenError = Class.new(StandardError) - TokenNotFoundError = Class.new(StandardError) - ExpiredError = Class.new(StandardError) - RevokedError = Class.new(StandardError) - UnauthorizedError = Class.new(StandardError) - - class InsufficientScopeError < StandardError - attr_reader :scopes - def initialize(scopes) - @scopes = scopes.map { |s| s.try(:name) || s } - end - end end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 3c8960cb1ab..b26c61ab8da 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -398,7 +398,7 @@ module API begin @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user! } - rescue APIGuard::UnauthorizedError + rescue Gitlab::Auth::UnauthorizedError unauthorized! end end diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb new file mode 100644 index 00000000000..46ec040ce92 --- /dev/null +++ b/lib/gitlab/auth/request_authenticator.rb @@ -0,0 +1,25 @@ +# Use for authentication only, in particular for Rack::Attack. +# Does not perform authorization of scopes, etc. +module Gitlab + module Auth + class RequestAuthenticator + include UserAuthFinders + + attr_reader :request + + def initialize(request) + @request = request + end + + def user + find_sessionless_user || find_user_from_warden + end + + def find_sessionless_user + find_user_from_access_token || find_user_from_rss_token + rescue Gitlab::Auth::AuthenticationError + nil + end + end + end +end diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb new file mode 100644 index 00000000000..b4114a3ac96 --- /dev/null +++ b/lib/gitlab/auth/user_auth_finders.rb @@ -0,0 +1,109 @@ +module Gitlab + module Auth + # + # Exceptions + # + + AuthenticationError = Class.new(StandardError) + MissingTokenError = Class.new(AuthenticationError) + TokenNotFoundError = Class.new(AuthenticationError) + ExpiredError = Class.new(AuthenticationError) + RevokedError = Class.new(AuthenticationError) + UnauthorizedError = Class.new(AuthenticationError) + + class InsufficientScopeError < AuthenticationError + attr_reader :scopes + def initialize(scopes) + @scopes = scopes.map { |s| s.try(:name) || s } + end + end + + module UserAuthFinders + include Gitlab::Utils::StrongMemoize + + PRIVATE_TOKEN_HEADER = 'HTTP_PRIVATE_TOKEN'.freeze + PRIVATE_TOKEN_PARAM = :private_token + + # Check the Rails session for valid authentication details + def find_user_from_warden + current_request.env['warden']&.authenticate if verified_request? + end + + def find_user_from_rss_token + return unless current_request.path.ends_with?('.atom') || current_request.format.atom? + + token = current_request.params[:rss_token].presence + return unless token + + User.find_by_rss_token(token) || raise(UnauthorizedError) + end + + def find_user_from_access_token + return unless access_token + + validate_access_token! + + access_token.user || raise(UnauthorizedError) + end + + def validate_access_token!(scopes: []) + return unless access_token + + case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes) + when AccessTokenValidationService::INSUFFICIENT_SCOPE + raise InsufficientScopeError.new(scopes) + when AccessTokenValidationService::EXPIRED + raise ExpiredError + when AccessTokenValidationService::REVOKED + raise RevokedError + end + end + + private + + def access_token + strong_memoize(:access_token) do + find_oauth_access_token || find_personal_access_token + end + end + + def find_personal_access_token + token = + current_request.params[PRIVATE_TOKEN_PARAM].presence || + current_request.env[PRIVATE_TOKEN_HEADER].presence + + return unless token + + # Expiration, revocation and scopes are verified in `validate_access_token!` + PersonalAccessToken.find_by(token: token) || raise(UnauthorizedError) + end + + def find_oauth_access_token + token = Doorkeeper::OAuth::Token.from_request(current_request, *Doorkeeper.configuration.access_token_methods) + return unless token + + # Expiration, revocation and scopes are verified in `validate_access_token!` + oauth_token = OauthAccessToken.by_token(token) + raise UnauthorizedError unless oauth_token + + oauth_token.revoke_previous_refresh_token! + oauth_token + end + + # Check if the request is GET/HEAD, or if CSRF token is valid. + def verified_request? + Gitlab::RequestForgeryProtection.verified?(current_request.env) + end + + def ensure_action_dispatch_request(request) + return request if request.is_a?(ActionDispatch::Request) + + ActionDispatch::Request.new(request.env) + end + + def current_request + @current_request ||= ensure_action_dispatch_request(request) + end + end + end +end diff --git a/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id.rb b/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id.rb new file mode 100644 index 00000000000..7e109e96e73 --- /dev/null +++ b/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id.rb @@ -0,0 +1,30 @@ +module Gitlab + module BackgroundMigration + class PopulateMergeRequestsLatestMergeRequestDiffId + BATCH_SIZE = 1_000 + + class MergeRequest < ActiveRecord::Base + self.table_name = 'merge_requests' + + include ::EachBatch + end + + def perform(start_id, stop_id) + update = ' + latest_merge_request_diff_id = ( + SELECT MAX(id) + FROM merge_request_diffs + WHERE merge_requests.id = merge_request_diffs.merge_request_id + )'.squish + + MergeRequest + .where(id: start_id..stop_id) + .where(latest_merge_request_diff_id: nil) + .each_batch(of: BATCH_SIZE) do |relation| + + relation.update_all(update) + end + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 2c35da8f1aa..c276c3566b4 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -220,6 +220,15 @@ module Gitlab # column - The name of the column to update. # value - The value for the column. # + # The `value` argument is typically a literal. To perform a computed + # update, an Arel literal can be used instead: + # + # update_value = Arel.sql('bar * baz') + # + # update_column_in_batches(:projects, :foo, update_value) do |table, query| + # query.where(table[:some_column].eq('hello')) + # end + # # Rubocop's Metrics/AbcSize metric is disabled for this method as Rubocop # determines this method to be too complex while there's no way to make it # less "complex" without introducing extra methods (which actually will diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index 022d1f249a9..d4a53d32c28 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -58,12 +58,12 @@ module Gitlab end end - def pages - @repository.gitaly_migrate(:wiki_get_all_pages) do |is_enabled| + def pages(limit: nil) + @repository.gitaly_migrate(:wiki_get_all_pages, status: Gitlab::GitalyClient::MigrationStatus::DISABLED) do |is_enabled| if is_enabled gitaly_get_all_pages else - gollum_get_all_pages + gollum_get_all_pages(limit: limit) end end end @@ -88,14 +88,23 @@ module Gitlab end end - def page_versions(page_path) + # options: + # :page - The Integer page number. + # :per_page - The number of items per page. + # :limit - Total number of items to return. + def page_versions(page_path, options = {}) current_page = gollum_page_by_path(page_path) - current_page.versions.map do |gollum_git_commit| - gollum_page = gollum_wiki.page(current_page.title, gollum_git_commit.id) - new_version(gollum_page, gollum_git_commit.id) + + commits_from_page(current_page, options).map do |gitlab_git_commit| + gollum_page = gollum_wiki.page(current_page.title, gitlab_git_commit.id) + Gitlab::Git::WikiPageVersion.new(gitlab_git_commit, gollum_page&.format) end end + def count_page_versions(page_path) + @repository.count_commits(ref: 'HEAD', path: page_path) + end + def preview_slug(title, format) # Adapted from gollum gem (Gollum::Wiki#preview_page) to avoid # using Rugged through a Gollum::Wiki instance @@ -110,6 +119,22 @@ module Gitlab private + # options: + # :page - The Integer page number. + # :per_page - The number of items per page. + # :limit - Total number of items to return. + def commits_from_page(gollum_page, options = {}) + unless options[:limit] + options[:offset] = ([1, options.delete(:page).to_i].max - 1) * Gollum::Page.per_page + options[:limit] = (options.delete(:per_page) || Gollum::Page.per_page).to_i + end + + @repository.log(ref: gollum_page.last_version.id, + path: gollum_page.path, + limit: options[:limit], + offset: options[:offset]) + end + def gollum_wiki @gollum_wiki ||= Gollum::Wiki.new(@repository.path) end @@ -126,8 +151,17 @@ module Gitlab end def new_version(gollum_page, commit_id) - commit = Gitlab::Git::Commit.find(@repository, commit_id) - Gitlab::Git::WikiPageVersion.new(commit, gollum_page&.format) + Gitlab::Git::WikiPageVersion.new(version(commit_id), gollum_page&.format) + end + + def version(commit_id) + commit_find_proc = -> { Gitlab::Git::Commit.find(@repository, commit_id) } + + if RequestStore.active? + RequestStore.fetch([:wiki_version_commit, commit_id]) { commit_find_proc.call } + else + commit_find_proc.call + end end def assert_type!(object, klass) @@ -185,8 +219,8 @@ module Gitlab Gitlab::Git::WikiFile.new(gollum_file) end - def gollum_get_all_pages - gollum_wiki.pages.map { |gollum_page| new_page(gollum_page) } + def gollum_get_all_pages(limit: nil) + gollum_wiki.pages(limit: limit).map { |gollum_page| new_page(gollum_page) } end def gitaly_write_page(name, format, content, commit_details) diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index 4d5c67ed892..3945df27eed 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.rb @@ -9,11 +9,8 @@ module Gitlab class User < Gitlab::OAuth::User class << self def find_by_uid_and_provider(uid, provider) - uid = Gitlab::LDAP::Person.normalize_dn(uid) + identity = ::Identity.with_extern_uid(provider, uid).take - identity = ::Identity - .where(provider: provider) - .where(extern_uid: uid).last identity && identity.user end end diff --git a/lib/gitlab/middleware/go.rb b/lib/gitlab/middleware/go.rb index cfc6b2a2029..1edda50e4b0 100644 --- a/lib/gitlab/middleware/go.rb +++ b/lib/gitlab/middleware/go.rb @@ -42,12 +42,11 @@ module Gitlab project_url = URI.join(config.gitlab.url, path) import_prefix = strip_url(project_url.to_s) - repository_url = case current_application_settings.enabled_git_access_protocol - when 'ssh' + repository_url = if current_application_settings.enabled_git_access_protocol == 'ssh' shell = config.gitlab_shell port = ":#{shell.ssh_port}" unless shell.ssh_port == 22 "ssh://#{shell.ssh_user}@#{shell.ssh_host}#{port}/#{path}.git" - when 'http', nil + else "#{project_url}.git" end diff --git a/lib/gitlab/multi_collection_paginator.rb b/lib/gitlab/multi_collection_paginator.rb index eb3c9002710..c22d0a84860 100644 --- a/lib/gitlab/multi_collection_paginator.rb +++ b/lib/gitlab/multi_collection_paginator.rb @@ -55,7 +55,9 @@ module Gitlab def first_collection_last_page_size return @first_collection_last_page_size if defined?(@first_collection_last_page_size) - @first_collection_last_page_size = paginated_first_collection(first_collection_page_count).count + @first_collection_last_page_size = paginated_first_collection(first_collection_page_count) + .except(:select) + .size end end end diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index b4b3b00c84d..552133234a3 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -157,7 +157,7 @@ module Gitlab end def find_by_uid_and_provider - identity = Identity.find_by(provider: auth_hash.provider, extern_uid: auth_hash.uid) + identity = Identity.with_extern_uid(auth_hash.provider, auth_hash.uid).take identity && identity.user end diff --git a/lib/gitlab/routing.rb b/lib/gitlab/routing.rb index 910533076b0..2c994536060 100644 --- a/lib/gitlab/routing.rb +++ b/lib/gitlab/routing.rb @@ -46,10 +46,10 @@ module Gitlab # Only replace the last occurence of `path`. # # `request.fullpath` includes the querystring - path = request.path.sub(%r{/#{path}/*(?!.*#{path})}, "/-/#{path}/") - path << "?#{request.query_string}" if request.query_string.present? + new_path = request.path.sub(%r{/#{path}(/*)(?!.*#{path})}, "/-/#{path}\\1") + new_path << "?#{request.query_string}" if request.query_string.present? - path + new_path end paths.each do |path| diff --git a/qa/qa/scenario/entrypoint.rb b/qa/qa/scenario/entrypoint.rb index ae099fd911e..b9d924651a0 100644 --- a/qa/qa/scenario/entrypoint.rb +++ b/qa/qa/scenario/entrypoint.rb @@ -8,6 +8,7 @@ module QA include Bootable def perform(address, *files) + Specs::Config.act { configure_capybara! } Runtime::Scenario.define(:gitlab_address, address) ## diff --git a/qa/qa/specs/config.rb b/qa/qa/specs/config.rb index 9f9fe9844d2..bce7923e52d 100644 --- a/qa/qa/specs/config.rb +++ b/qa/qa/specs/config.rb @@ -9,6 +9,8 @@ require 'selenium-webdriver' module QA module Specs class Config < Scenario::Template + include Scenario::Actable + def perform configure_rspec! configure_capybara! diff --git a/rubocop/cop/migration/add_column_with_default_to_large_table.rb b/rubocop/cop/migration/update_large_table.rb index fb363f95b56..3ae3fb1b68e 100644 --- a/rubocop/cop/migration/add_column_with_default_to_large_table.rb +++ b/rubocop/cop/migration/update_large_table.rb @@ -12,12 +12,12 @@ module RuboCop # # See https://gitlab.com/gitlab-com/infrastructure/issues/1602 for more # information. - class AddColumnWithDefaultToLargeTable < RuboCop::Cop::Cop + class UpdateLargeTable < RuboCop::Cop::Cop include MigrationHelpers - MSG = 'Using `add_column_with_default` on the `%s` table will take a ' \ - 'long time to complete, and should be avoided unless absolutely ' \ - 'necessary'.freeze + MSG = 'Using `%s` on the `%s` table will take a long time to ' \ + 'complete, and should be avoided unless absolutely ' \ + 'necessary'.freeze LARGE_TABLES = %i[ ci_pipelines @@ -34,20 +34,22 @@ module RuboCop users ].freeze - def_node_matcher :add_column_with_default?, <<~PATTERN - (send nil :add_column_with_default $(sym ...) ...) + def_node_matcher :batch_update?, <<~PATTERN + (send nil ${:add_column_with_default :update_column_in_batches} $(sym ...) ...) PATTERN def on_send(node) return unless in_migration?(node) - matched = add_column_with_default?(node) - return unless matched + matches = batch_update?(node) + return unless matches + + update_method = matches.first + table = matches.last.to_a.first - table = matched.to_a.first return unless LARGE_TABLES.include?(table) - add_offense(node, :expression, format(MSG, table)) + add_offense(node, :expression, format(MSG, update_method, table)) end end end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 4ebbe010e90..a1668749dd9 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -7,7 +7,6 @@ require_relative 'cop/polymorphic_associations' require_relative 'cop/project_path_helper' require_relative 'cop/redirect_with_status' require_relative 'cop/migration/add_column' -require_relative 'cop/migration/add_column_with_default_to_large_table' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' @@ -20,6 +19,7 @@ require_relative 'cop/migration/reversible_add_column_with_default' require_relative 'cop/migration/safer_boolean_column' require_relative 'cop/migration/timestamps' require_relative 'cop/migration/update_column_in_batches' +require_relative 'cop/migration/update_large_table' require_relative 'cop/rspec/env_assignment' require_relative 'cop/rspec/single_line_hook' require_relative 'cop/rspec/verbose_include_metadata' diff --git a/spec/controllers/groups/children_controller_spec.rb b/spec/controllers/groups/children_controller_spec.rb index 4262d474e59..cb1b460fc0e 100644 --- a/spec/controllers/groups/children_controller_spec.rb +++ b/spec/controllers/groups/children_controller_spec.rb @@ -280,6 +280,17 @@ describe Groups::ChildrenController do expect(assigns(:children)).to contain_exactly(other_subgroup, *next_page_projects.take(per_page - 1)) end + + context 'with a mixed first page' do + let!(:first_page_subgroups) { [create(:group, :public, parent: group)] } + let!(:first_page_projects) { create_list(:project, per_page, :public, namespace: group) } + + it 'correctly calculates the counts' do + get :index, group_id: group.to_param, sort: 'id_asc', page: 2, format: :json + + expect(response).to have_gitlab_http_status(200) + end + end end end end diff --git a/spec/factories/fork_network_members.rb b/spec/factories/fork_network_members.rb new file mode 100644 index 00000000000..509c4e1fa1c --- /dev/null +++ b/spec/factories/fork_network_members.rb @@ -0,0 +1,8 @@ +FactoryGirl.define do + factory :fork_network_member do + association :project + association :fork_network + + forked_from_project { fork_network.root_project } + end +end diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 479fb713297..b163ca8dc75 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' describe 'Commits' do - include CiStatusHelper - let(:project) { create(:project, :repository) } let(:user) { create(:user) } @@ -33,7 +31,7 @@ describe 'Commits' do describe 'Commit builds' do before do - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) end it { expect(page).to have_content pipeline.sha[0..7] } @@ -79,7 +77,7 @@ describe 'Commits' do describe 'Commit builds', :js do before do - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) end it 'shows pipeline`s data' do @@ -95,7 +93,7 @@ describe 'Commits' do end it do - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) click_on 'Download artifacts' expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type) end @@ -103,7 +101,7 @@ describe 'Commits' do describe 'Cancel all builds' do it 'cancels commit', :js do - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) click_on 'Cancel running' expect(page).to have_content 'canceled' end @@ -111,7 +109,7 @@ describe 'Commits' do describe 'Cancel build' do it 'cancels build', :js do - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) find('.js-btn-cancel-pipeline').click expect(page).to have_content 'canceled' end @@ -120,13 +118,13 @@ describe 'Commits' do describe '.gitlab-ci.yml not found warning' do context 'ci builds enabled' do it "does not show warning" do - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) expect(page).not_to have_content '.gitlab-ci.yml not found in this commit' end it 'shows warning' do stub_ci_pipeline_yaml_file(nil) - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) expect(page).to have_content '.gitlab-ci.yml not found in this commit' end end @@ -135,7 +133,7 @@ describe 'Commits' do before do stub_ci_builds_disabled stub_ci_pipeline_yaml_file(nil) - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) end it 'does not show warning' do @@ -149,7 +147,7 @@ describe 'Commits' do before do project.team << [user, :reporter] build.update_attributes(artifacts_file: artifacts_file) - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) end it 'Renders header', :js do @@ -171,7 +169,7 @@ describe 'Commits' do visibility_level: Gitlab::VisibilityLevel::INTERNAL, public_builds: false) build.update_attributes(artifacts_file: artifacts_file) - visit ci_status_path(pipeline) + visit pipeline_path(pipeline) end it do diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 0795d0aaa82..1ad7c2d8efa 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -202,7 +202,6 @@ export default { "revert_in_fork_path": "/root/acets-app/forks?continue%5Bnotice%5D=You%27re+not+allowed+to+make+changes+to+this+project+directly.+A+fork+of+this+project+has+been+created+that+you+can+make+changes+in%2C+so+you+can+submit+a+merge+request.+Try+to+cherry-pick+this+commit+again.&continue%5Bnotice_now%5D=You%27re+not+allowed+to+make+changes+to+this+project+directly.+A+fork+of+this+project+is+being+created+that+you+can+make+changes+in%2C+so+you+can+submit+a+merge+request.&continue%5Bto%5D=%2Froot%2Facets-app%2Fmerge_requests%2F22&namespace_key=1", "email_patches_path": "/root/acets-app/merge_requests/22.patch", "plain_diff_path": "/root/acets-app/merge_requests/22.diff", - "ci_status_path": "/root/acets-app/merge_requests/22/ci_status", "status_path": "/root/acets-app/merge_requests/22.json", "merge_check_path": "/root/acets-app/merge_requests/22/merge_check", "ci_environments_status_url": "/root/acets-app/merge_requests/22/ci_environments_status", diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb new file mode 100644 index 00000000000..ffcd90b9fcb --- /dev/null +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe Gitlab::Auth::RequestAuthenticator do + let(:env) do + { + 'rack.input' => '', + 'REQUEST_METHOD' => 'GET' + } + end + let(:request) { ActionDispatch::Request.new(env) } + + subject { described_class.new(request) } + + describe '#user' do + let!(:sessionless_user) { build(:user) } + let!(:session_user) { build(:user) } + + it 'returns sessionless user first' do + allow_any_instance_of(described_class).to receive(:find_sessionless_user).and_return(sessionless_user) + allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user) + + expect(subject.user).to eq sessionless_user + end + + it 'returns session user if no sessionless user found' do + allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user) + + expect(subject.user).to eq session_user + end + + it 'returns nil if no user found' do + expect(subject.user).to be_blank + end + + it 'bubbles up exceptions' do + allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_raise(Gitlab::Auth::UnauthorizedError) + end + end + + describe '#find_sessionless_user' do + let!(:access_token_user) { build(:user) } + let!(:rss_token_user) { build(:user) } + + it 'returns access_token user first' do + allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_return(access_token_user) + allow_any_instance_of(described_class).to receive(:find_user_from_rss_token).and_return(rss_token_user) + + expect(subject.find_sessionless_user).to eq access_token_user + end + + it 'returns rss_token user if no access_token user found' do + allow_any_instance_of(described_class).to receive(:find_user_from_rss_token).and_return(rss_token_user) + + expect(subject.find_sessionless_user).to eq rss_token_user + end + + it 'returns nil if no user found' do + expect(subject.find_sessionless_user).to be_blank + end + + it 'rescue Gitlab::Auth::AuthenticationError exceptions' do + allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_raise(Gitlab::Auth::UnauthorizedError) + + expect(subject.find_sessionless_user).to be_blank + end + end +end diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb new file mode 100644 index 00000000000..4637816570c --- /dev/null +++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb @@ -0,0 +1,194 @@ +require 'spec_helper' + +describe Gitlab::Auth::UserAuthFinders do + include described_class + + let(:user) { create(:user) } + let(:env) do + { + 'rack.input' => '' + } + end + let(:request) { Rack::Request.new(env)} + + def set_param(key, value) + request.update_param(key, value) + end + + describe '#find_user_from_warden' do + context 'with CSRF token' do + before do + allow(Gitlab::RequestForgeryProtection).to receive(:verified?).and_return(true) + end + + context 'with invalid credentials' do + it 'returns nil' do + expect(find_user_from_warden).to be_nil + end + end + + context 'with valid credentials' do + it 'returns the user' do + env['warden'] = double("warden", authenticate: user) + + expect(find_user_from_warden).to eq user + end + end + end + + context 'without CSRF token' do + it 'returns nil' do + allow(Gitlab::RequestForgeryProtection).to receive(:verified?).and_return(false) + env['warden'] = double("warden", authenticate: user) + + expect(find_user_from_warden).to be_nil + end + end + end + + describe '#find_user_from_rss_token' do + context 'when the request format is atom' do + before do + env['HTTP_ACCEPT'] = 'application/atom+xml' + end + + it 'returns user if valid rss_token' do + set_param(:rss_token, user.rss_token) + + expect(find_user_from_rss_token).to eq user + end + + it 'returns nil if rss_token is blank' do + expect(find_user_from_rss_token).to be_nil + end + + it 'returns exception if invalid rss_token' do + set_param(:rss_token, 'invalid_token') + + expect { find_user_from_rss_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + end + + context 'when the request format is not atom' do + it 'returns nil' do + set_param(:rss_token, user.rss_token) + + expect(find_user_from_rss_token).to be_nil + end + end + end + + describe '#find_user_from_access_token' do + let(:personal_access_token) { create(:personal_access_token, user: user) } + + it 'returns nil if no access_token present' do + expect(find_personal_access_token).to be_nil + end + + context 'when validate_access_token! returns valid' do + it 'returns user' do + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token + + expect(find_user_from_access_token).to eq user + end + + it 'returns exception if token has no user' do + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token + allow_any_instance_of(PersonalAccessToken).to receive(:user).and_return(nil) + + expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + end + end + + describe '#find_personal_access_token' do + let(:personal_access_token) { create(:personal_access_token, user: user) } + + context 'passed as header' do + it 'returns token if valid personal_access_token' do + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token + + expect(find_personal_access_token).to eq personal_access_token + end + end + + context 'passed as param' do + it 'returns token if valid personal_access_token' do + set_param(Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_PARAM, personal_access_token.token) + + expect(find_personal_access_token).to eq personal_access_token + end + end + + it 'returns nil if no personal_access_token' do + expect(find_personal_access_token).to be_nil + end + + it 'returns exception if invalid personal_access_token' do + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = 'invalid_token' + + expect { find_personal_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + end + + describe '#find_oauth_access_token' do + let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) } + let(:token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api') } + + context 'passed as header' do + it 'returns token if valid oauth_access_token' do + env['HTTP_AUTHORIZATION'] = "Bearer #{token.token}" + + expect(find_oauth_access_token.token).to eq token.token + end + end + + context 'passed as param' do + it 'returns user if valid oauth_access_token' do + set_param(:access_token, token.token) + + expect(find_oauth_access_token.token).to eq token.token + end + end + + it 'returns nil if no oauth_access_token' do + expect(find_oauth_access_token).to be_nil + end + + it 'returns exception if invalid oauth_access_token' do + env['HTTP_AUTHORIZATION'] = "Bearer invalid_token" + + expect { find_oauth_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + end + + describe '#validate_access_token!' do + let(:personal_access_token) { create(:personal_access_token, user: user) } + + it 'returns nil if no access_token present' do + expect(validate_access_token!).to be_nil + end + + context 'token is not valid' do + before do + allow_any_instance_of(described_class).to receive(:access_token).and_return(personal_access_token) + end + + it 'returns Gitlab::Auth::ExpiredError if token expired' do + personal_access_token.expires_at = 1.day.ago + + expect { validate_access_token! }.to raise_error(Gitlab::Auth::ExpiredError) + end + + it 'returns Gitlab::Auth::RevokedError if token revoked' do + personal_access_token.revoke! + + expect { validate_access_token! }.to raise_error(Gitlab::Auth::RevokedError) + end + + it 'returns Gitlab::Auth::InsufficientScopeError if invalid token scope' do + expect { validate_access_token!(scopes: [:sudo]) }.to raise_error(Gitlab::Auth::InsufficientScopeError) + end + end + end +end diff --git a/spec/migrations/populate_merge_requests_latest_merge_request_diff_id_spec.rb b/spec/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id_spec.rb index 4ea7f441f7c..0cb753c5853 100644 --- a/spec/migrations/populate_merge_requests_latest_merge_request_diff_id_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20171026082505_populate_merge_requests_latest_merge_request_diff_id') -describe PopulateMergeRequestsLatestMergeRequestDiffId, :migration do +describe Gitlab::BackgroundMigration::PopulateMergeRequestsLatestMergeRequestDiffId, :migration, schema: 20171026082505 do let(:projects_table) { table(:projects) } let(:merge_requests_table) { table(:merge_requests) } let(:merge_request_diffs_table) { table(:merge_request_diffs) } @@ -27,30 +26,32 @@ describe PopulateMergeRequestsLatestMergeRequestDiffId, :migration do merge_request_diffs_table.where(merge_request_id: merge_request.id) end - describe '#up' do + describe '#perform' do it 'ignores MRs without diffs' do merge_request_without_diff = create_mr!('without_diff') + mr_id = merge_request_without_diff.id expect(merge_request_without_diff.latest_merge_request_diff_id).to be_nil - expect { migrate! } + expect { subject.perform(mr_id, mr_id) } .not_to change { merge_request_without_diff.reload.latest_merge_request_diff_id } end it 'ignores MRs that have a diff ID already set' do merge_request_with_multiple_diffs = create_mr!('with_multiple_diffs', diffs: 3) diff_id = diffs_for(merge_request_with_multiple_diffs).minimum(:id) + mr_id = merge_request_with_multiple_diffs.id merge_request_with_multiple_diffs.update!(latest_merge_request_diff_id: diff_id) - expect { migrate! } + expect { subject.perform(mr_id, mr_id) } .not_to change { merge_request_with_multiple_diffs.reload.latest_merge_request_diff_id } end it 'migrates multiple MR diffs to the correct values' do merge_requests = Array.new(3).map.with_index { |_, i| create_mr!(i, diffs: 3) } - migrate! + subject.perform(merge_requests.first.id, merge_requests.last.id) merge_requests.each do |merge_request| expect(merge_request.reload.latest_merge_request_diff_id) diff --git a/spec/lib/gitlab/middleware/go_spec.rb b/spec/lib/gitlab/middleware/go_spec.rb index 67121937398..60a134be939 100644 --- a/spec/lib/gitlab/middleware/go_spec.rb +++ b/spec/lib/gitlab/middleware/go_spec.rb @@ -127,6 +127,14 @@ describe Gitlab::Middleware::Go do include_examples 'go-get=1', enabled_protocol: nil end + + context 'with nothing disabled (blank string)' do + before do + stub_application_setting(enabled_git_access_protocol: '') + end + + include_examples 'go-get=1', enabled_protocol: nil + end end def go diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index c7471a21fda..2f19fb7312d 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -662,4 +662,13 @@ describe Gitlab::OAuth::User do end end end + + describe '.find_by_uid_and_provider' do + let!(:existing_user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') } + + it 'normalizes extern_uid' do + allow(oauth_user.auth_hash).to receive(:uid).and_return('MY-UID') + expect(oauth_user.find_user).to eql gl_user + end + end end diff --git a/spec/migrations/remove_empty_fork_networks_spec.rb b/spec/migrations/remove_empty_fork_networks_spec.rb new file mode 100644 index 00000000000..cf6ae5cda74 --- /dev/null +++ b/spec/migrations/remove_empty_fork_networks_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171114104051_remove_empty_fork_networks.rb') + +describe RemoveEmptyForkNetworks, :migration do + let!(:fork_networks) { table(:fork_networks) } + + let(:deleted_project) { create(:project) } + let!(:empty_network) { create(:fork_network, id: 1, root_project_id: deleted_project.id) } + let!(:other_network) { create(:fork_network, id: 2, root_project_id: create(:project).id) } + + before do + deleted_project.destroy! + end + + it 'deletes only the fork network without members' do + expect(fork_networks.count).to eq(2) + + migrate! + + expect(fork_networks.find_by(id: empty_network.id)).to be_nil + expect(fork_networks.find_by(id: other_network.id)).not_to be_nil + expect(fork_networks.count).to eq(1) + end +end diff --git a/spec/migrations/schedule_merge_request_diff_migrations_spec.rb b/spec/migrations/schedule_merge_request_diff_migrations_spec.rb index f95bd6e3511..76afb6c19cf 100644 --- a/spec/migrations/schedule_merge_request_diff_migrations_spec.rb +++ b/spec/migrations/schedule_merge_request_diff_migrations_spec.rb @@ -2,19 +2,6 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20170703130158_schedule_merge_request_diff_migrations') describe ScheduleMergeRequestDiffMigrations, :migration, :sidekiq do - matcher :be_scheduled_migration do |time, *expected| - match do |migration| - BackgroundMigrationWorker.jobs.any? do |job| - job['args'] == [migration, expected] && - job['at'].to_i == time.to_i - end - end - - failure_message do |migration| - "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!" - end - end - let(:merge_request_diffs) { table(:merge_request_diffs) } let(:merge_requests) { table(:merge_requests) } let(:projects) { table(:projects) } @@ -37,9 +24,9 @@ describe ScheduleMergeRequestDiffMigrations, :migration, :sidekiq do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes.from_now, 1, 1) - expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes.from_now, 2, 2) - expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes.from_now, 4, 4) + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 1) + expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 2, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes, 4, 4) expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end diff --git a/spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb b/spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb index 4ab1bb67058..cf323973384 100644 --- a/spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb +++ b/spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb @@ -2,19 +2,6 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20170926150348_schedule_merge_request_diff_migrations_take_two') describe ScheduleMergeRequestDiffMigrationsTakeTwo, :migration, :sidekiq do - matcher :be_scheduled_migration do |time, *expected| - match do |migration| - BackgroundMigrationWorker.jobs.any? do |job| - job['args'] == [migration, expected] && - job['at'].to_i == time.to_i - end - end - - failure_message do |migration| - "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!" - end - end - let(:merge_request_diffs) { table(:merge_request_diffs) } let(:merge_requests) { table(:merge_requests) } let(:projects) { table(:projects) } @@ -37,9 +24,9 @@ describe ScheduleMergeRequestDiffMigrationsTakeTwo, :migration, :sidekiq do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes.from_now, 1, 1) - expect(described_class::MIGRATION).to be_scheduled_migration(20.minutes.from_now, 2, 2) - expect(described_class::MIGRATION).to be_scheduled_migration(30.minutes.from_now, 4, 4) + expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 1, 1) + expect(described_class::MIGRATION).to be_scheduled_migration(20.minutes, 2, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(30.minutes, 4, 4) expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end diff --git a/spec/migrations/schedule_merge_request_latest_merge_request_diff_id_migrations_spec.rb b/spec/migrations/schedule_merge_request_latest_merge_request_diff_id_migrations_spec.rb new file mode 100644 index 00000000000..158d0bc02ed --- /dev/null +++ b/spec/migrations/schedule_merge_request_latest_merge_request_diff_id_migrations_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171026082505_schedule_merge_request_latest_merge_request_diff_id_migrations') + +describe ScheduleMergeRequestLatestMergeRequestDiffIdMigrations, :migration, :sidekiq do + let(:projects_table) { table(:projects) } + let(:merge_requests_table) { table(:merge_requests) } + let(:merge_request_diffs_table) { table(:merge_request_diffs) } + + let(:project) { projects_table.create!(name: 'gitlab', path: 'gitlab-org/gitlab-ce') } + + let!(:merge_request_1) { create_mr!('mr_1', diffs: 1) } + let!(:merge_request_2) { create_mr!('mr_2', diffs: 2) } + let!(:merge_request_migrated) { create_mr!('merge_request_migrated', diffs: 3) } + let!(:merge_request_4) { create_mr!('mr_4', diffs: 3) } + + def create_mr!(name, diffs: 0) + merge_request = + merge_requests_table.create!(target_project_id: project.id, + target_branch: 'master', + source_project_id: project.id, + source_branch: name, + title: name) + + diffs.times do + merge_request_diffs_table.create!(merge_request_id: merge_request.id) + end + + merge_request + end + + def diffs_for(merge_request) + merge_request_diffs_table.where(merge_request_id: merge_request.id) + end + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + + diff_id = diffs_for(merge_request_migrated).minimum(:id) + merge_request_migrated.update!(latest_merge_request_diff_id: diff_id) + end + + it 'correctly schedules background migrations' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, merge_request_1.id, merge_request_1.id) + expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, merge_request_2.id, merge_request_2.id) + expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes, merge_request_4.id, merge_request_4.id) + expect(BackgroundMigrationWorker.jobs.size).to eq 3 + end + end + end + + it 'schedules background migrations' do + Sidekiq::Testing.inline! do + expect(merge_requests_table.where(latest_merge_request_diff_id: nil).count).to eq 3 + + migrate! + + expect(merge_requests_table.where(latest_merge_request_diff_id: nil).count).to eq 0 + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2c9e7013b77..b89b0e555d9 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -625,38 +625,29 @@ describe Ci::Pipeline, :mailer do shared_context 'with some outdated pipelines' do before do - create_pipeline(:canceled, 'ref', 'A') - create_pipeline(:success, 'ref', 'A') - create_pipeline(:failed, 'ref', 'B') - create_pipeline(:skipped, 'feature', 'C') + create_pipeline(:canceled, 'ref', 'A', project) + create_pipeline(:success, 'ref', 'A', project) + create_pipeline(:failed, 'ref', 'B', project) + create_pipeline(:skipped, 'feature', 'C', project) end - def create_pipeline(status, ref, sha) - create(:ci_empty_pipeline, status: status, ref: ref, sha: sha) + def create_pipeline(status, ref, sha, project) + create( + :ci_empty_pipeline, + status: status, + ref: ref, + sha: sha, + project: project + ) end end - describe '.latest' do + describe '.newest_first' do include_context 'with some outdated pipelines' - context 'when no ref is specified' do - let(:pipelines) { described_class.latest.all } - - it 'returns the latest pipeline for the same ref and different sha' do - expect(pipelines.map(&:sha)).to contain_exactly('A', 'B', 'C') - expect(pipelines.map(&:status)) - .to contain_exactly('success', 'failed', 'skipped') - end - end - - context 'when ref is specified' do - let(:pipelines) { described_class.latest('ref').all } - - it 'returns the latest pipeline for ref and different sha' do - expect(pipelines.map(&:sha)).to contain_exactly('A', 'B') - expect(pipelines.map(&:status)) - .to contain_exactly('success', 'failed') - end + it 'returns the pipelines from new to old' do + expect(described_class.newest_first.pluck(:status)) + .to eq(%w[skipped failed success canceled]) end end @@ -664,20 +655,14 @@ describe Ci::Pipeline, :mailer do include_context 'with some outdated pipelines' context 'when no ref is specified' do - let(:latest_status) { described_class.latest_status } - - it 'returns the latest status for the same ref and different sha' do - expect(latest_status).to eq(described_class.latest.status) - expect(latest_status).to eq('failed') + it 'returns the status of the latest pipeline' do + expect(described_class.latest_status).to eq('skipped') end end context 'when ref is specified' do - let(:latest_status) { described_class.latest_status('ref') } - - it 'returns the latest status for ref and different sha' do - expect(latest_status).to eq(described_class.latest_status('ref')) - expect(latest_status).to eq('failed') + it 'returns the status of the latest pipeline for the given ref' do + expect(described_class.latest_status('ref')).to eq('failed') end end end @@ -686,7 +671,7 @@ describe Ci::Pipeline, :mailer do include_context 'with some outdated pipelines' let!(:latest_successful_pipeline) do - create_pipeline(:success, 'ref', 'D') + create_pipeline(:success, 'ref', 'D', project) end it 'returns the latest successful pipeline' do @@ -698,8 +683,13 @@ describe Ci::Pipeline, :mailer do describe '.latest_successful_for_refs' do include_context 'with some outdated pipelines' - let!(:latest_successful_pipeline1) { create_pipeline(:success, 'ref1', 'D') } - let!(:latest_successful_pipeline2) { create_pipeline(:success, 'ref2', 'D') } + let!(:latest_successful_pipeline1) do + create_pipeline(:success, 'ref1', 'D', project) + end + + let!(:latest_successful_pipeline2) do + create_pipeline(:success, 'ref2', 'D', project) + end it 'returns the latest successful pipeline for both refs' do refs = %w(ref1 ref2 ref3) @@ -708,6 +698,62 @@ describe Ci::Pipeline, :mailer do end end + describe '.latest_status_per_commit' do + let(:project) { create(:project) } + + before do + pairs = [ + %w[success ref1 123], + %w[manual master 123], + %w[failed ref 456] + ] + + pairs.each do |(status, ref, sha)| + create( + :ci_empty_pipeline, + status: status, + ref: ref, + sha: sha, + project: project + ) + end + end + + context 'without a ref' do + it 'returns a Hash containing the latest status per commit for all refs' do + expect(described_class.latest_status_per_commit(%w[123 456])) + .to eq({ '123' => 'manual', '456' => 'failed' }) + end + + it 'only includes the status of the given commit SHAs' do + expect(described_class.latest_status_per_commit(%w[123])) + .to eq({ '123' => 'manual' }) + end + + context 'when there are two pipelines for a ref and SHA' do + it 'returns the status of the latest pipeline' do + create( + :ci_empty_pipeline, + status: 'failed', + ref: 'master', + sha: '123', + project: project + ) + + expect(described_class.latest_status_per_commit(%w[123])) + .to eq({ '123' => 'failed' }) + end + end + end + + context 'with a ref' do + it 'only includes the pipelines for the given ref' do + expect(described_class.latest_status_per_commit(%w[123 456], 'master')) + .to eq({ '123' => 'manual' }) + end + end + end + describe '.internal_sources' do subject { described_class.internal_sources } diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb new file mode 100644 index 00000000000..066fe7d154e --- /dev/null +++ b/spec/models/commit_collection_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe CommitCollection do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit } + + describe '#each' do + it 'yields every commit' do + collection = described_class.new(project, [commit]) + + expect { |b| collection.each(&b) }.to yield_with_args(commit) + end + end + + describe '#with_pipeline_status' do + it 'sets the pipeline status for every commit so no additional queries are necessary' do + create( + :ci_empty_pipeline, + ref: 'master', + sha: commit.id, + status: 'success', + project: project + ) + + collection = described_class.new(project, [commit]) + collection.with_pipeline_status + + recorder = ActiveRecord::QueryRecorder.new do + expect(commit.status).to eq('success') + end + + expect(recorder.count).to be_zero + end + end + + describe '#respond_to_missing?' do + it 'returns true when the underlying Array responds to the message' do + collection = described_class.new(project, []) + + expect(collection.respond_to?(:last)).to eq(true) + end + + it 'returns false when the underlying Array does not respond to the message' do + collection = described_class.new(project, []) + + expect(collection.respond_to?(:foo)).to eq(false) + end + end + + describe '#method_missing' do + it 'delegates undefined methods to the underlying Array' do + collection = described_class.new(project, [commit]) + + expect(collection.length).to eq(1) + expect(collection.last).to eq(commit) + expect(collection).not_to be_empty + end + end +end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index e3cfa149e3a..d18a5c9dfa6 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -351,12 +351,19 @@ eos end it 'gives compound status from latest pipelines if ref is nil' do - expect(commit.status(nil)).to eq(Ci::Pipeline.latest_status) - expect(commit.status(nil)).to eq('failed') + expect(commit.status(nil)).to eq(pipeline_from_fix.status) end end end + describe '#set_status_for_ref' do + it 'sets the status for a given reference' do + commit.set_status_for_ref('master', 'failed') + + expect(commit.status('master')).to eq('failed') + end + end + describe '#participants' do let(:user1) { build(:user) } let(:user2) { build(:user) } diff --git a/spec/models/fork_network_member_spec.rb b/spec/models/fork_network_member_spec.rb index 532ca1fca8c..25bf596fddc 100644 --- a/spec/models/fork_network_member_spec.rb +++ b/spec/models/fork_network_member_spec.rb @@ -5,4 +5,22 @@ describe ForkNetworkMember do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:fork_network) } end + + describe 'destroying a ForkNetworkMember' do + let(:fork_network_member) { create(:fork_network_member) } + let(:fork_network) { fork_network_member.fork_network } + + it 'removes the fork network if it was the last member' do + fork_network.fork_network_members.destroy_all + + expect(ForkNetwork.count).to eq(0) + end + + it 'does not destroy the fork network if there are members left' do + fork_network_member.destroy! + + # The root of the fork network is left + expect(ForkNetwork.count).to eq(1) + end + end end diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb index 3ed048744de..a45a6088831 100644 --- a/spec/models/identity_spec.rb +++ b/spec/models/identity_spec.rb @@ -33,5 +33,15 @@ describe Identity do expect(identity).to eq(ldap_identity) end end + + context 'any other provider' do + let!(:test_entity) { create(:identity, provider: 'test_provider', extern_uid: 'test_uid') } + + it 'the extern_uid lookup is case insensitive' do + identity = described_class.with_extern_uid('test_provider', 'TEST_UID').first + + expect(identity).to eq(test_entity) + end + end end end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index a7227b38850..ea75434e399 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -373,7 +373,7 @@ describe WikiPage do end it 'returns commit sha' do - expect(@page.last_commit_sha).to eq @page.commit.sha + expect(@page.last_commit_sha).to eq @page.last_version.sha end it 'is changed after page updated' do diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 6c0996c543d..0462f494e15 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -11,7 +11,6 @@ describe API::Helpers do let(:admin) { create(:admin) } let(:key) { create(:key, user: user) } - let(:params) { {} } let(:csrf_token) { SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH) } let(:env) do { @@ -19,10 +18,13 @@ describe API::Helpers do 'rack.session' => { _csrf_token: csrf_token }, - 'REQUEST_METHOD' => 'GET' + 'REQUEST_METHOD' => 'GET', + 'CONTENT_TYPE' => 'text/plain;charset=utf-8' } end let(:header) { } + let(:request) { Grape::Request.new(env)} + let(:params) { request.params } before do allow_any_instance_of(self.class).to receive(:options).and_return({}) @@ -37,6 +39,10 @@ describe API::Helpers do raise Exception.new("#{status} - #{message}") end + def set_param(key, value) + request.update_param(key, value) + end + describe ".current_user" do subject { current_user } @@ -132,13 +138,13 @@ describe API::Helpers do let(:personal_access_token) { create(:personal_access_token, user: user) } it "returns a 401 response for an invalid token" do - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token' + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = 'invalid token' expect { current_user }.to raise_error /401/ end it "returns a 403 response for a user without access" do - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) expect { current_user }.to raise_error /403/ @@ -146,35 +152,35 @@ describe API::Helpers do it 'returns a 403 response for a user who is blocked' do user.block! - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token expect { current_user }.to raise_error /403/ end it "sets current_user" do - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token expect(current_user).to eq(user) end it "does not allow tokens without the appropriate scope" do personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user']) - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect { current_user }.to raise_error API::APIGuard::InsufficientScopeError + expect { current_user }.to raise_error Gitlab::Auth::InsufficientScopeError end it 'does not allow revoked tokens' do personal_access_token.revoke! - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect { current_user }.to raise_error API::APIGuard::RevokedError + expect { current_user }.to raise_error Gitlab::Auth::RevokedError end it 'does not allow expired tokens' do personal_access_token.update_attributes!(expires_at: 1.day.ago) - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect { current_user }.to raise_error API::APIGuard::ExpiredError + expect { current_user }.to raise_error Gitlab::Auth::ExpiredError end end end @@ -350,7 +356,7 @@ describe API::Helpers do context 'when using param' do context 'when providing username' do before do - params[API::Helpers::SUDO_PARAM] = user.username + set_param(API::Helpers::SUDO_PARAM, user.username) end it_behaves_like 'successful sudo' @@ -358,7 +364,7 @@ describe API::Helpers do context 'when providing user ID' do before do - params[API::Helpers::SUDO_PARAM] = user.id.to_s + set_param(API::Helpers::SUDO_PARAM, user.id.to_s) end it_behaves_like 'successful sudo' @@ -368,7 +374,7 @@ describe API::Helpers do context 'when user does not exist' do before do - params[API::Helpers::SUDO_PARAM] = 'nonexistent' + set_param(API::Helpers::SUDO_PARAM, 'nonexistent') end it 'raises an error' do @@ -382,11 +388,11 @@ describe API::Helpers do token.scopes = %w[api] token.save! - params[API::Helpers::SUDO_PARAM] = user.id.to_s + set_param(API::Helpers::SUDO_PARAM, user.id.to_s) end it 'raises an error' do - expect { current_user }.to raise_error API::APIGuard::InsufficientScopeError + expect { current_user }.to raise_error Gitlab::Auth::InsufficientScopeError end end end @@ -396,7 +402,7 @@ describe API::Helpers do token.user = user token.save! - params[API::Helpers::SUDO_PARAM] = user.id.to_s + set_param(API::Helpers::SUDO_PARAM, user.id.to_s) end it 'raises an error' do @@ -420,7 +426,7 @@ describe API::Helpers do context 'passed as param' do before do - params[API::APIGuard::PRIVATE_TOKEN_PARAM] = token.token + set_param(Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_PARAM, token.token) end it_behaves_like 'sudo' @@ -428,7 +434,7 @@ describe API::Helpers do context 'passed as header' do before do - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = token.token end it_behaves_like 'sudo' diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb new file mode 100644 index 00000000000..0fec14d0cce --- /dev/null +++ b/spec/requests/rack_attack_global_spec.rb @@ -0,0 +1,362 @@ +require 'spec_helper' + +describe 'Rack Attack global throttles' do + let(:settings) { Gitlab::CurrentSettings.current_application_settings } + + # Start with really high limits and override them with low limits to ensure + # the right settings are being exercised + let(:settings_to_set) do + { + throttle_unauthenticated_requests_per_period: 100, + throttle_unauthenticated_period_in_seconds: 1, + throttle_authenticated_api_requests_per_period: 100, + throttle_authenticated_api_period_in_seconds: 1, + throttle_authenticated_web_requests_per_period: 100, + throttle_authenticated_web_period_in_seconds: 1 + } + end + + let(:requests_per_period) { 1 } + let(:period_in_seconds) { 10000 } + let(:period) { period_in_seconds.seconds } + + let(:url_that_does_not_require_authentication) { '/users/sign_in' } + let(:url_that_requires_authentication) { '/dashboard/snippets' } + let(:api_partial_url) { '/todos' } + + around do |example| + # Instead of test environment's :null_store so the throttles can increment + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + + # Make time-dependent tests deterministic + Timecop.freeze { example.run } + + Rack::Attack.cache.store = Rails.cache + end + + # Requires let variables: + # * throttle_setting_prefix (e.g. "throttle_authenticated_api" or "throttle_authenticated_web") + # * get_args + # * other_user_get_args + shared_examples_for 'rate-limited token-authenticated requests' do + before do + # Set low limits + settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period + settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds + end + + context 'when the throttle is enabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + # At first, allow requests under the rate limit. + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + # the last straw + expect_rejection { get(*get_args) } + end + + it 'allows requests after throttling and then waiting for the next period' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_rejection { get(*get_args) } + + Timecop.travel(period.from_now) do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_rejection { get(*get_args) } + end + end + + it 'counts requests from different users separately, even from the same IP' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + # would be over the limit if this wasn't a different user + get(*other_user_get_args) + expect(response).to have_http_status 200 + end + + it 'counts all requests from the same user, even via different IPs' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') + + expect_rejection { get(*get_args) } + end + end + + context 'when the throttle is disabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = false + stub_application_setting(settings_to_set) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + get(*get_args) + expect(response).to have_http_status 200 + end + end + end + end + + describe 'unauthenticated requests' do + before do + # Set low limits + settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period + settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds + end + + context 'when the throttle is enabled' do + before do + settings_to_set[:throttle_unauthenticated_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + # At first, allow requests under the rate limit. + requests_per_period.times do + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + + # the last straw + expect_rejection { get url_that_does_not_require_authentication } + end + + it 'allows requests after throttling and then waiting for the next period' do + requests_per_period.times do + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_does_not_require_authentication } + + Timecop.travel(period.from_now) do + requests_per_period.times do + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_does_not_require_authentication } + end + end + + it 'counts requests from different IPs separately' do + requests_per_period.times do + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + + expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') + + # would be over limit for the same IP + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + end + + context 'when the throttle is disabled' do + before do + settings_to_set[:throttle_unauthenticated_enabled] = false + stub_application_setting(settings_to_set) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + end + end + end + + describe 'API requests authenticated with personal access token', :api do + let(:user) { create(:user) } + let(:token) { create(:personal_access_token, user: user) } + let(:other_user) { create(:user) } + let(:other_user_token) { create(:personal_access_token, user: other_user) } + let(:throttle_setting_prefix) { 'throttle_authenticated_api' } + + context 'with the token in the query string' do + let(:get_args) { [api(api_partial_url, personal_access_token: token)] } + let(:other_user_get_args) { [api(api_partial_url, personal_access_token: other_user_token)] } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'with the token in the headers' do + let(:get_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } + let(:other_user_get_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + end + + describe 'API requests authenticated with OAuth token', :api do + let(:user) { create(:user) } + let(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) } + let(:token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api") } + let(:other_user) { create(:user) } + let(:other_user_application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: other_user) } + let(:other_user_token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: other_user.id, scopes: "api") } + let(:throttle_setting_prefix) { 'throttle_authenticated_api' } + + context 'with the token in the query string' do + let(:get_args) { [api(api_partial_url, oauth_access_token: token)] } + let(:other_user_get_args) { [api(api_partial_url, oauth_access_token: other_user_token)] } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'with the token in the headers' do + let(:get_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } + let(:other_user_get_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + end + + describe '"web" (non-API) requests authenticated with RSS token' do + let(:user) { create(:user) } + let(:other_user) { create(:user) } + let(:throttle_setting_prefix) { 'throttle_authenticated_web' } + + context 'with the token in the query string' do + let(:get_args) { [rss_url(user), nil] } + let(:other_user_get_args) { [rss_url(other_user), nil] } + + it_behaves_like 'rate-limited token-authenticated requests' + end + end + + describe 'web requests authenticated with regular login' do + let(:user) { create(:user) } + + before do + login_as(user) + + # Set low limits + settings_to_set[:throttle_authenticated_web_requests_per_period] = requests_per_period + settings_to_set[:throttle_authenticated_web_period_in_seconds] = period_in_seconds + end + + context 'when the throttle is enabled' do + before do + settings_to_set[:throttle_authenticated_web_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + # At first, allow requests under the rate limit. + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + # the last straw + expect_rejection { get url_that_requires_authentication } + end + + it 'allows requests after throttling and then waiting for the next period' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_requires_authentication } + + Timecop.travel(period.from_now) do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_requires_authentication } + end + end + + it 'counts requests from different users separately, even from the same IP' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + # would be over the limit if this wasn't a different user + login_as(create(:user)) + + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + it 'counts all requests from the same user, even via different IPs' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') + + expect_rejection { get url_that_requires_authentication } + end + end + + context 'when the throttle is disabled' do + before do + settings_to_set[:throttle_authenticated_web_enabled] = false + stub_application_setting(settings_to_set) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + end + end + end + + def api_get_args_with_token_headers(partial_url, token_headers) + ["/api/#{API::API.version}#{partial_url}", nil, token_headers] + end + + def rss_url(user) + "/dashboard/projects.atom?rss_token=#{user.rss_token}" + end + + def private_token_headers(user) + { 'HTTP_PRIVATE_TOKEN' => user.private_token } + end + + def personal_access_token_headers(personal_access_token) + { 'HTTP_PRIVATE_TOKEN' => personal_access_token.token } + end + + def oauth_token_headers(oauth_access_token) + { 'AUTHORIZATION' => "Bearer #{oauth_access_token.token}" } + end + + def expect_rejection(&block) + yield + + expect(response).to have_http_status(429) + end +end diff --git a/spec/routing/group_routing_spec.rb b/spec/routing/group_routing_spec.rb index 7a4c8304e62..71788028cbf 100644 --- a/spec/routing/group_routing_spec.rb +++ b/spec/routing/group_routing_spec.rb @@ -39,13 +39,19 @@ describe "Groups", "routing" do describe 'legacy redirection' do describe 'labels' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels", "/groups/complex.group-namegit/-/labels/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels", "/groups/complex.group-namegit/-/labels" do let(:resource) { create(:group, parent: group, path: 'labels') } end + + context 'when requesting JSON' do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels.json", "/groups/complex.group-namegit/-/labels.json" do + let(:resource) { create(:group, parent: group, path: 'labels') } + end + end end describe 'group_members' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/group_members", "/groups/complex.group-namegit/-/group_members/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/group_members", "/groups/complex.group-namegit/-/group_members" do let(:resource) { create(:group, parent: group, path: 'group_members') } end end @@ -60,7 +66,7 @@ describe "Groups", "routing" do end describe 'milestones' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones", "/groups/complex.group-namegit/-/milestones/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones", "/groups/complex.group-namegit/-/milestones" do let(:resource) { create(:group, parent: group, path: 'milestones') } end @@ -76,18 +82,18 @@ describe "Groups", "routing" do end context 'with a query string' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?hello=world", "/groups/complex.group-namegit/-/milestones/?hello=world" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?hello=world", "/groups/complex.group-namegit/-/milestones?hello=world" do let(:resource) { create(:group, parent: group, path: 'milestones') } end - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?milestones=/milestones", "/groups/complex.group-namegit/-/milestones/?milestones=/milestones" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?milestones=/milestones", "/groups/complex.group-namegit/-/milestones?milestones=/milestones" do let(:resource) { create(:group, parent: group, path: 'milestones') } end end end describe 'edit' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/edit", "/groups/complex.group-namegit/-/edit/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/edit", "/groups/complex.group-namegit/-/edit" do let(:resource) do pending('still rejected because of the wildcard reserved word') create(:group, parent: group, path: 'edit') @@ -96,29 +102,29 @@ describe "Groups", "routing" do end describe 'issues' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/issues", "/groups/complex.group-namegit/-/issues/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/issues", "/groups/complex.group-namegit/-/issues" do let(:resource) { create(:group, parent: group, path: 'issues') } end end describe 'merge_requests' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/merge_requests", "/groups/complex.group-namegit/-/merge_requests/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/merge_requests", "/groups/complex.group-namegit/-/merge_requests" do let(:resource) { create(:group, parent: group, path: 'merge_requests') } end end describe 'projects' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/projects", "/groups/complex.group-namegit/-/projects/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/projects", "/groups/complex.group-namegit/-/projects" do let(:resource) { create(:group, parent: group, path: 'projects') } end end describe 'activity' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/activity", "/groups/complex.group-namegit/-/activity/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/activity", "/groups/complex.group-namegit/-/activity" do let(:resource) { create(:group, parent: group, path: 'activity') } end - it_behaves_like 'redirecting a legacy path', "/groups/activity/activity", "/groups/activity/-/activity/" do + it_behaves_like 'redirecting a legacy path', "/groups/activity/activity", "/groups/activity/-/activity" do let!(:parent) { create(:group, path: 'activity') } let(:resource) { create(:group, parent: parent, path: 'activity') } end diff --git a/spec/rubocop/cop/migration/add_column_with_default_to_large_table_spec.rb b/spec/rubocop/cop/migration/add_column_with_default_to_large_table_spec.rb deleted file mode 100644 index 07cb3fc4a2e..00000000000 --- a/spec/rubocop/cop/migration/add_column_with_default_to_large_table_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'spec_helper' - -require 'rubocop' -require 'rubocop/rspec/support' - -require_relative '../../../../rubocop/cop/migration/add_column_with_default_to_large_table' - -describe RuboCop::Cop::Migration::AddColumnWithDefaultToLargeTable do - include CopHelper - - subject(:cop) { described_class.new } - - context 'in migration' do - before do - allow(cop).to receive(:in_migration?).and_return(true) - end - - described_class::LARGE_TABLES.each do |table| - it "registers an offense for the #{table} table" do - inspect_source(cop, "add_column_with_default :#{table}, :column, default: true") - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end - end - end - - it 'registers no offense for non-blacklisted tables' do - inspect_source(cop, "add_column_with_default :table, :column, default: true") - - expect(cop.offenses).to be_empty - end - end - - context 'outside of migration' do - it 'registers no offense' do - table = described_class::LARGE_TABLES.sample - inspect_source(cop, "add_column_with_default :#{table}, :column, default: true") - - expect(cop.offenses).to be_empty - end - end -end diff --git a/spec/rubocop/cop/migration/update_large_table_spec.rb b/spec/rubocop/cop/migration/update_large_table_spec.rb new file mode 100644 index 00000000000..17b19e139e4 --- /dev/null +++ b/spec/rubocop/cop/migration/update_large_table_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/update_large_table' + +describe RuboCop::Cop::Migration::UpdateLargeTable do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + shared_examples 'large tables' do |update_method| + described_class::LARGE_TABLES.each do |table| + it "registers an offense for the #{table} table" do + inspect_source(cop, "#{update_method} :#{table}, :column, default: true") + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + end + end + + context 'for the add_column_with_default method' do + include_examples 'large tables', 'add_column_with_default' + end + + context 'for the update_column_in_batches method' do + include_examples 'large tables', 'update_column_in_batches' + end + + it 'registers no offense for non-blacklisted tables' do + inspect_source(cop, "add_column_with_default :table, :column, default: true") + + expect(cop.offenses).to be_empty + end + + it 'registers no offense for non-blacklisted methods' do + table = described_class::LARGE_TABLES.sample + + inspect_source(cop, "some_other_method :#{table}, :column, default: true") + + expect(cop.offenses).to be_empty + end + end + + context 'outside of migration' do + let(:table) { described_class::LARGE_TABLES.sample } + + it 'registers no offense for add_column_with_default' do + inspect_source(cop, "add_column_with_default :#{table}, :column, default: true") + + expect(cop.offenses).to be_empty + end + + it 'registers no offense for update_column_in_batches' do + inspect_source(cop, "add_column_with_default :#{table}, :column, default: true") + + expect(cop.offenses).to be_empty + end + end +end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 2459f371a91..2b1337bee7e 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -42,6 +42,18 @@ describe Projects::TransferService do expect(service).to receive(:execute_system_hooks) end end + + it 'disk path has moved' do + old_path = project.repository.disk_path + old_full_path = project.repository.full_path + + transfer_project(project, user, group) + + expect(project.repository.disk_path).not_to eq(old_path) + expect(project.repository.full_path).not_to eq(old_full_path) + expect(project.disk_path).not_to eq(old_path) + expect(project.disk_path).to start_with(group.path) + end end context 'when transfer fails' do @@ -188,6 +200,26 @@ describe Projects::TransferService do end end + context 'when hashed storage in use' do + let(:hashed_project) { create(:project, :repository, :hashed, namespace: user.namespace) } + + before do + group.add_owner(user) + end + + it 'does not move the directory' do + old_path = hashed_project.repository.disk_path + old_full_path = hashed_project.repository.full_path + + transfer_project(hashed_project, user, group) + project.reload + + expect(hashed_project.repository.disk_path).to eq(old_path) + expect(hashed_project.repository.full_path).to eq(old_full_path) + expect(hashed_project.disk_path).to eq(old_path) + end + end + describe 'refreshing project authorizations' do let(:group) { create(:group) } let(:owner) { project.namespace.owner } |
