diff options
164 files changed, 2876 insertions, 677 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b4b672b55c6..9bccac90fc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 11.1.4 (2018-07-30) + +- No changes. + ## 11.1.3 (2018-07-27) ### Fixed (8 changes, 1 of them is from the community) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 5fea1768541..18455b77f5e 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.113.0 +0.114.0 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 0062ac97180..831446cbd27 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -5.0.0 +5.1.0 @@ -306,7 +306,7 @@ group :metrics do gem 'influxdb', '~> 0.2', require: false # Prometheus - gem 'prometheus-client-mmap', '~> 0.9.3' + gem 'prometheus-client-mmap', '~> 0.9.4' gem 'raindrops', '~> 0.18' end diff --git a/Gemfile.lock b/Gemfile.lock index d147f81c81e..77c255a7464 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -635,7 +635,7 @@ GEM parser unparser procto (0.0.3) - prometheus-client-mmap (0.9.3) + prometheus-client-mmap (0.9.4) pry (0.10.4) coderay (~> 1.1.0) method_source (~> 0.8.1) @@ -1126,7 +1126,7 @@ DEPENDENCIES peek-sidekiq (~> 1.0.3) pg (~> 0.18.2) premailer-rails (~> 1.9.7) - prometheus-client-mmap (~> 0.9.3) + prometheus-client-mmap (~> 0.9.4) pry-byebug (~> 3.4.1) pry-rails (~> 0.3.4) rack-attack (~> 4.4.1) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index b055b561b61..f7886245504 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -639,7 +639,7 @@ GEM parser unparser procto (0.0.3) - prometheus-client-mmap (0.9.3) + prometheus-client-mmap (0.9.4) pry (0.10.4) coderay (~> 1.1.0) method_source (~> 0.8.1) @@ -1136,7 +1136,7 @@ DEPENDENCIES peek-sidekiq (~> 1.0.3) pg (~> 0.18.2) premailer-rails (~> 1.9.7) - prometheus-client-mmap (~> 0.9.3) + prometheus-client-mmap (~> 0.9.4) pry-byebug (~> 3.4.1) pry-rails (~> 0.3.4) rack-attack (~> 4.4.1) diff --git a/app/assets/javascripts/boards/components/board.js b/app/assets/javascripts/boards/components/board.js index a2355d7fd5c..9ad451fa375 100644 --- a/app/assets/javascripts/boards/components/board.js +++ b/app/assets/javascripts/boards/components/board.js @@ -2,6 +2,9 @@ import Sortable from 'sortablejs'; import Vue from 'vue'; +import { n__ } from '~/locale'; +import Icon from '~/vue_shared/components/icon.vue'; +import Tooltip from '~/vue_shared/directives/tooltip'; import AccessorUtilities from '../../lib/utils/accessor'; import boardList from './board_list.vue'; import BoardBlankState from './board_blank_state.vue'; @@ -17,6 +20,10 @@ gl.issueBoards.Board = Vue.extend({ boardList, 'board-delete': gl.issueBoards.BoardDelete, BoardBlankState, + Icon, + }, + directives: { + Tooltip, }, props: { list: { @@ -46,6 +53,12 @@ gl.issueBoards.Board = Vue.extend({ filter: Store.filter, }; }, + computed: { + counterTooltip() { + const { issuesSize } = this.list; + return `${n__('%d issue', '%d issues', issuesSize)}`; + }, + }, watch: { filter: { handler() { diff --git a/app/assets/javascripts/boards/models/list.js b/app/assets/javascripts/boards/models/list.js index 4f05a0e4282..050cbd8db48 100644 --- a/app/assets/javascripts/boards/models/list.js +++ b/app/assets/javascripts/boards/models/list.js @@ -136,6 +136,8 @@ class List { } this.createIssues(data.issues); + + return data; }); } diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 333338489bc..76467564608 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -125,11 +125,17 @@ gl.issueBoards.BoardsStore = { } else if (listTo.type === 'backlog' && listFrom.type === 'assignee') { issue.removeAssignee(listFrom.assignee); listFrom.removeIssue(issue); - } else if ((listTo.type !== 'label' && listFrom.type === 'assignee') || - (listTo.type !== 'assignee' && listFrom.type === 'label')) { + } else if (this.shouldRemoveIssue(listFrom, listTo)) { listFrom.removeIssue(issue); } }, + shouldRemoveIssue(listFrom, listTo) { + return ( + (listTo.type !== 'label' && listFrom.type === 'assignee') || + (listTo.type !== 'assignee' && listFrom.type === 'label') || + (listFrom.type === 'backlog') + ); + }, moveIssueInList (list, issue, oldIndex, newIndex, idArray) { const beforeId = parseInt(idArray[newIndex - 1], 10) || null; const afterId = parseInt(idArray[newIndex + 1], 10) || null; diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index c494d3bcd3e..d3ffbe0415a 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -50,7 +50,7 @@ export default { }; }, computed: { - ...mapGetters('diffs', ['diffHasExpandedDiscussions']), + ...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']), hasExpandedDiscussions() { return this.diffHasExpandedDiscussions(this.diffFile); }, @@ -108,6 +108,9 @@ export default { false, ); }, + gfmCopyText() { + return `\`${this.diffFile.filePath}\``; + }, }, methods: { ...mapActions('diffs', ['toggleFileDiscussions']), @@ -191,6 +194,7 @@ export default { <clipboard-button :title="__('Copy file path to clipboard')" :text="diffFile.filePath" + :gfm="gfmCopyText" css-class="btn-default btn-transparent btn-clipboard" /> @@ -217,6 +221,7 @@ export default { v-if="diffFile.blob && diffFile.blob.readableText" > <button + :disabled="!diffHasDiscussions(diffFile)" :class="{ active: hasExpandedDiscussions }" :title="s__('MergeRequests|Toggle comments for this file')" class="js-btn-vue-toggle-comments btn" diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index c7b9b1a16e6..fc1d15b8d54 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -48,6 +48,14 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => { }; /** + * Checks if the diff has any discussion + * @param {Boolean} diff + * @returns {Boolean} + */ +export const diffHasDiscussions = (state, getters) => diff => + getters.getDiffFileDiscussions(diff).length > 0; + +/** * Returns an array with the discussions of the given diff * @param {Object} diff * @returns {Array} diff --git a/app/assets/javascripts/ide/components/ide.vue b/app/assets/javascripts/ide/components/ide.vue index 257a7432c20..2c8305aa0cc 100644 --- a/app/assets/javascripts/ide/components/ide.vue +++ b/app/assets/javascripts/ide/components/ide.vue @@ -1,6 +1,7 @@ <script> import Mousetrap from 'mousetrap'; import { mapActions, mapState, mapGetters } from 'vuex'; +import { __ } from '~/locale'; import NewModal from './new_dropdown/modal.vue'; import IdeSidebar from './ide_side_bar.vue'; import RepoTabs from './repo_tabs.vue'; @@ -25,7 +26,6 @@ export default { }, computed: { ...mapState([ - 'changedFiles', 'openFiles', 'viewer', 'currentMergeRequestId', @@ -34,18 +34,10 @@ export default { 'currentProjectId', 'errorMessage', ]), - ...mapGetters(['activeFile', 'hasChanges']), + ...mapGetters(['activeFile', 'hasChanges', 'someUncommitedChanges']), }, mounted() { - const returnValue = 'Are you sure you want to lose unsaved changes?'; - window.onbeforeunload = e => { - if (!this.changedFiles.length) return undefined; - - Object.assign(e, { - returnValue, - }); - return returnValue; - }; + window.onbeforeunload = e => this.onBeforeUnload(e); Mousetrap.bind(['t', 'command+p', 'ctrl+p'], e => { if (e.preventDefault) { @@ -59,6 +51,16 @@ export default { }, methods: { ...mapActions(['toggleFileFinder']), + onBeforeUnload(e = {}) { + const returnValue = __('Are you sure you want to lose unsaved changes?'); + + if (!this.someUncommitedChanges) return undefined; + + Object.assign(e, { + returnValue, + }); + return returnValue; + }, mousetrapStopCallback(e, el, combo) { if ( (combo === 't' && el.classList.contains('dropdown-input-field')) || diff --git a/app/assets/javascripts/lib/utils/http_status.js b/app/assets/javascripts/lib/utils/http_status.js index 229d53b18b0..e4852c85378 100644 --- a/app/assets/javascripts/lib/utils/http_status.js +++ b/app/assets/javascripts/lib/utils/http_status.js @@ -2,11 +2,34 @@ * exports HTTP status codes */ -export default { +const httpStatusCodes = { ABORTED: 0, - NO_CONTENT: 204, OK: 200, + CREATED: 201, + ACCEPTED: 202, + NON_AUTHORITATIVE_INFORMATION: 203, + NO_CONTENT: 204, + RESET_CONTENT: 205, + PARTIAL_CONTENT: 206, + MULTI_STATUS: 207, + ALREADY_REPORTED: 208, + IM_USED: 226, MULTIPLE_CHOICES: 300, BAD_REQUEST: 400, NOT_FOUND: 404, }; + +export const successCodes = [ + httpStatusCodes.OK, + httpStatusCodes.CREATED, + httpStatusCodes.ACCEPTED, + httpStatusCodes.NON_AUTHORITATIVE_INFORMATION, + httpStatusCodes.NO_CONTENT, + httpStatusCodes.RESET_CONTENT, + httpStatusCodes.PARTIAL_CONTENT, + httpStatusCodes.MULTI_STATUS, + httpStatusCodes.ALREADY_REPORTED, + httpStatusCodes.IM_USED, +]; + +export default httpStatusCodes; diff --git a/app/assets/javascripts/lib/utils/poll.js b/app/assets/javascripts/lib/utils/poll.js index 91d8c30744f..04a6948f1f1 100644 --- a/app/assets/javascripts/lib/utils/poll.js +++ b/app/assets/javascripts/lib/utils/poll.js @@ -1,4 +1,4 @@ -import httpStatusCodes from './http_status'; +import httpStatusCodes, { successCodes } from './http_status'; import { normalizeHeaders } from './common_utils'; /** @@ -62,7 +62,7 @@ export default class Poll { checkConditions(response) { const headers = normalizeHeaders(response.headers); const pollInterval = parseInt(headers[this.intervalHeader], 10); - if (pollInterval > 0 && response.status === httpStatusCodes.OK && this.canPoll) { + if (pollInterval > 0 && successCodes.indexOf(response.status) !== -1 && this.canPoll) { clearTimeout(this.timeoutID); this.timeoutID = setTimeout(() => { this.makeRequest(); diff --git a/app/assets/javascripts/vue_shared/components/clipboard_button.vue b/app/assets/javascripts/vue_shared/components/clipboard_button.vue index d272bf3f55f..945a33d9622 100644 --- a/app/assets/javascripts/vue_shared/components/clipboard_button.vue +++ b/app/assets/javascripts/vue_shared/components/clipboard_button.vue @@ -31,6 +31,11 @@ export default { type: String, required: true, }, + gfm: { + type: String, + required: false, + default: null, + }, title: { type: String, required: true, @@ -51,6 +56,14 @@ export default { default: 'btn-default', }, }, + computed: { + clipboardText() { + if (this.gfm !== null) { + return JSON.stringify({ text: this.text, gfm: this.gfm }); + } + return this.text; + }, + }, }; </script> @@ -59,7 +72,7 @@ export default { v-tooltip :class="cssClass" :title="title" - :data-clipboard-text="text" + :data-clipboard-text="clipboardText" :data-container="tooltipContainer" :data-placement="tooltipPlacement" type="button" diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index a2789021ab4..473ca408c04 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -444,3 +444,5 @@ textarea { color: $placeholder-text-color; } } + +.lh-100 { line-height: 1; } diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 7347da2ae61..a68b47b1d02 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -205,7 +205,7 @@ .board-title { margin: 0; - padding: 12px $gl-padding; + padding: $gl-padding-8 $gl-padding; font-size: 1em; border-bottom: 1px solid $border-color; display: flex; diff --git a/app/assets/stylesheets/pages/issues/issue_count_badge.scss b/app/assets/stylesheets/pages/issues/issue_count_badge.scss index ccb62bfed18..4fba89e956b 100644 --- a/app/assets/stylesheets/pages/issues/issue_count_badge.scss +++ b/app/assets/stylesheets/pages/issues/issue_count_badge.scss @@ -1,29 +1,11 @@ .issue-count-badge { display: inline-flex; - align-items: stretch; - height: 24px; -} - -.issue-count-badge-count { - display: flex; - align-items: center; - padding-right: 10px; - padding-left: 10px; - border: 1px solid $border-color; border-radius: $border-radius-base; - line-height: 1; - - &.has-btn { - border-right: 0; - border-top-right-radius: 0; - border-bottom-right-radius: 0; - } + border: 1px solid $border-color; + padding: 5px $gl-padding-8; } -.issue-count-badge-add-button { - display: flex; +.issue-count-badge-count { + display: inline-flex; align-items: center; - border: 1px solid $border-color; - border-radius: 0 $border-radius-base $border-radius-base 0; - line-height: 1; } diff --git a/app/controllers/admin/jobs_controller.rb b/app/controllers/admin/jobs_controller.rb index ac1ae0f16b3..e355d5fdea7 100644 --- a/app/controllers/admin/jobs_controller.rb +++ b/app/controllers/admin/jobs_controller.rb @@ -2,7 +2,7 @@ class Admin::JobsController < Admin::ApplicationController def index @scope = params[:scope] @all_builds = Ci::Build - @builds = @all_builds.order('created_at DESC') + @builds = @all_builds.order('id DESC') @builds = case @scope when 'pending' diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eeceb99c8d2..73d7b8cb9cf 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -397,7 +397,7 @@ class ApplicationController < ActionController::Base # actually stored in the session and a token is needed # for every request. If you want the token to work as a # sign in token, you can simply remove store: false. - sign_in user, store: false + sign_in(user, store: false, message: :sessionless_sign_in) end end diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index 09e143c23e8..7dd19f87ef5 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -12,8 +12,9 @@ module Boards skip_before_action :authenticate_user!, only: [:index] def index - issues = Boards::Issues::ListService.new(board_parent, current_user, filter_params).execute - issues = issues.page(params[:page]).per(params[:per] || 20) + list_service = Boards::Issues::ListService.new(board_parent, current_user, filter_params) + issues = list_service.execute + issues = issues.page(params[:page]).per(params[:per] || 20).without_count make_sure_position_is_set(issues) if Gitlab::Database.read_write? issues = issues.preload(:project, :milestone, @@ -22,10 +23,7 @@ module Boards notes: [:award_emoji, :author] ) - render json: { - issues: serialize_as_json(issues), - size: issues.total_count - } + render_issues(issues, list_service.metadata) end def create @@ -51,6 +49,13 @@ module Boards private + def render_issues(issues, metadata) + data = { issues: serialize_as_json(issues) } + data.merge!(metadata) + + render json: data + end + def make_sure_position_is_set(issues) issues.each do |issue| issue.move_to_end && issue.save unless issue.relative_position diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 69a053d4246..dfa1da7872c 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -60,7 +60,7 @@ module AuthenticatesWithTwoFactor remember_me(user) if user_params[:remember_me] == '1' user.save! - sign_in(user) + sign_in(user, message: :two_factor_authenticated) else user.increment_failed_attempts! Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=OTP") @@ -77,7 +77,7 @@ module AuthenticatesWithTwoFactor session.delete(:challenge) remember_me(user) if user_params[:remember_me] == '1' - sign_in(user) + sign_in(user, message: :two_factor_authenticated) else user.increment_failed_attempts! Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=U2F") diff --git a/app/controllers/projects/mirrors_controller.rb b/app/controllers/projects/mirrors_controller.rb index 3b24d231f3d..da209c3ba61 100644 --- a/app/controllers/projects/mirrors_controller.rb +++ b/app/controllers/projects/mirrors_controller.rb @@ -13,7 +13,9 @@ class Projects::MirrorsController < Projects::ApplicationController end def update - if project.update(mirror_params) + result = ::Projects::UpdateService.new(project, current_user, mirror_params).execute + + if result[:status] == :success flash[:notice] = 'Mirroring settings were successfully updated.' else flash[:alert] = project.errors.full_messages.join(', ').html_safe diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index b7c656246ef..da7aeb26a75 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -1,10 +1,16 @@ class Projects::WikisController < Projects::ApplicationController include PreviewMarkdown + include Gitlab::Utils::StrongMemoize before_action :authorize_read_wiki! before_action :authorize_create_wiki!, only: [:edit, :create, :history] before_action :authorize_admin_wiki!, only: :destroy before_action :load_project_wiki + before_action :load_page, only: [:show, :edit, :update, :history, :destroy] + before_action :valid_encoding?, only: [:show, :edit, :update], if: :load_page + before_action only: [:edit, :update], unless: :valid_encoding? do + redirect_to(project_wiki_path(@project, @page)) + end def pages @wiki_pages = Kaminari.paginate_array(@project_wiki.pages).page(params[:page]) @@ -12,11 +18,11 @@ class Projects::WikisController < Projects::ApplicationController end def show - @page = @project_wiki.find_page(params[:id], params[:version_id]) - view_param = @project_wiki.empty? ? params[:view] : 'create' if @page + set_encoding_error unless valid_encoding? + render 'show' elsif file = @project_wiki.find_file(params[:id], params[:version_id]) response.headers['Content-Security-Policy'] = "default-src 'none'" @@ -38,13 +44,11 @@ class Projects::WikisController < Projects::ApplicationController end def edit - @page = @project_wiki.find_page(params[:id]) end def update return render('empty') unless can?(current_user, :create_wiki, @project) - @page = @project_wiki.find_page(params[:id]) @page = WikiPages::UpdateService.new(@project, current_user, wiki_params).execute(@page) if @page.valid? @@ -79,8 +83,6 @@ class Projects::WikisController < Projects::ApplicationController end def history - @page = @project_wiki.find_page(params[:id]) - if @page @page_versions = Kaminari.paginate_array(@page.versions(page: params[:page].to_i), total_count: @page.count_versions) @@ -94,8 +96,6 @@ class Projects::WikisController < Projects::ApplicationController end def destroy - @page = @project_wiki.find_page(params[:id]) - WikiPages::DestroyService.new(@project, current_user).execute(@page) redirect_to project_wiki_path(@project, :home), @@ -141,4 +141,25 @@ class Projects::WikisController < Projects::ApplicationController page.update_attributes(args) # rubocop:disable Rails/ActiveRecordAliases end end + + def load_page + @page ||= @project_wiki.find_page(*page_params) + end + + def page_params + keys = [:id] + keys << :version_id if params[:action] == 'show' + + params.values_at(*keys) + end + + def valid_encoding? + strong_memoize(:valid_encoding) do + @page.content.encoding == Encoding::UTF_8 + end + end + + def set_encoding_error + flash.now[:notice] = "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository." + end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 4ca42e2d4a2..ab8e2e35b98 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -89,6 +89,14 @@ class SessionsController < Devise::SessionsController ).increment end + ## + # We do have some duplication between lib/gitlab/auth/activity.rb here, but + # leaving this method here because of backwards compatibility. + # + def login_counter + @login_counter ||= Gitlab::Metrics.counter(:user_session_logins_total, 'User sign in count') + end + def log_failed_login Gitlab::AppLogger.info("Failed Login: username=#{user_params[:login]} ip=#{request.remote_ip}") end @@ -97,10 +105,6 @@ class SessionsController < Devise::SessionsController (options = env["warden.options"]) && options[:action] == "unauthenticated" end - def login_counter - @login_counter ||= Gitlab::Metrics.counter(:user_session_logins_total, 'User sign in count') - end - # Handle an "initial setup" state, where there's only one user, it's an admin, # and they require a password change. def check_initial_setup diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 6fdfd964fca..372e2a96c2c 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -130,7 +130,7 @@ class IssuableFinder counts[:all] = counts.values.sum - counts + counts.with_indifferent_access end def group diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 9e2346177a4..9c1cac22acf 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -148,6 +148,7 @@ module ApplicationSettingsHelper :after_sign_up_text, :akismet_api_key, :akismet_enabled, + :allow_local_requests_from_hooks_and_services, :authorized_keys_enabled, :auto_devops_enabled, :auto_devops_domain, @@ -174,6 +175,7 @@ module ApplicationSettingsHelper :ed25519_key_restriction, :email_author_in_body, :enabled_git_access_protocol, + :enforce_terms, :gitaly_timeout_default, :gitaly_timeout_medium, :gitaly_timeout_fast, @@ -182,6 +184,7 @@ module ApplicationSettingsHelper :help_page_hide_commercial_content, :help_page_support_url, :help_page_text, + :hide_third_party_offers, :home_page_url, :housekeeping_bitmaps_enabled, :housekeeping_enabled, @@ -203,6 +206,7 @@ module ApplicationSettingsHelper :metrics_port, :metrics_sample_interval, :metrics_timeout, + :mirror_available, :pages_domain_verification_enabled, :password_authentication_enabled_for_web, :password_authentication_enabled_for_git, @@ -233,6 +237,7 @@ module ApplicationSettingsHelper :sign_in_text, :signup_enabled, :terminal_max_session_time, + :terms, :throttle_unauthenticated_enabled, :throttle_unauthenticated_requests_per_period, :throttle_unauthenticated_period_in_seconds, @@ -250,12 +255,7 @@ module ApplicationSettingsHelper :instance_statistics_visibility_private, :user_default_external, :user_oauth_applications, - :version_check_enabled, - :allow_local_requests_from_hooks_and_services, - :hide_third_party_offers, - :enforce_terms, - :terms, - :mirror_available + :version_check_enabled ] end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 01a59039cff..4c901c1d67f 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -227,26 +227,28 @@ class ApplicationSetting < ActiveRecord::Base def self.defaults { after_sign_up_text: nil, + allow_local_requests_from_hooks_and_services: false, akismet_enabled: false, authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand container_registry_token_expire_delay: 5, default_artifacts_expire_in: '30 days', default_branch_protection: Settings.gitlab['default_branch_protection'], + default_group_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_projects_limit: Settings.gitlab['default_projects_limit'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], - default_group_visibility: Settings.gitlab.default_projects_features['visibility_level'], disabled_oauth_sign_in_sources: [], domain_whitelist: Settings.gitlab['domain_whitelist'], dsa_key_restriction: 0, ecdsa_key_restriction: 0, ed25519_key_restriction: 0, + gitaly_timeout_default: 55, + gitaly_timeout_fast: 10, + gitaly_timeout_medium: 30, gravatar_enabled: Settings.gravatar['enabled'], - help_page_text: nil, help_page_hide_commercial_content: false, - unique_ips_limit_per_user: 10, - unique_ips_limit_time_window: 3600, - unique_ips_limit_enabled: false, + help_page_text: nil, + hide_third_party_offers: false, housekeeping_bitmaps_enabled: true, housekeeping_enabled: true, housekeeping_full_repack_period: 50, @@ -257,12 +259,14 @@ class ApplicationSetting < ActiveRecord::Base koding_url: nil, max_artifacts_size: Settings.artifacts['max_size'], max_attachment_size: Settings.gitlab['max_attachment_size'], - password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'], + mirror_available: true, password_authentication_enabled_for_git: true, + password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'], performance_bar_allowed_group_id: nil, rsa_key_restriction: 0, plantuml_enabled: false, plantuml_url: nil, + polling_interval_multiplier: 1, project_export_enabled: true, recaptcha_enabled: false, repository_checks_enabled: true, @@ -277,18 +281,19 @@ 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, + throttle_authenticated_api_requests_per_period: 7200, + throttle_authenticated_web_enabled: false, + throttle_authenticated_web_period_in_seconds: 3600, + throttle_authenticated_web_requests_per_period: 7200, + throttle_unauthenticated_enabled: false, + throttle_unauthenticated_period_in_seconds: 3600, + throttle_unauthenticated_requests_per_period: 3600, two_factor_grace_period: 48, - user_default_external: false, - polling_interval_multiplier: 1, + unique_ips_limit_enabled: false, + unique_ips_limit_per_user: 10, + unique_ips_limit_time_window: 3600, usage_ping_enabled: Settings.gitlab['usage_ping_enabled'], instance_statistics_visibility_private: false, gitaly_timeout_fast: 10, diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index db86400128c..35b20bc1e0b 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -22,9 +22,10 @@ module Ci has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy, inverse_of: :job # rubocop:disable Cop/ActiveRecordDependent - has_one :job_artifacts_archive, -> { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id - has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id - has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id + + Ci::JobArtifact.file_types.each do |key, value| + has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id + end has_one :metadata, class_name: 'Ci::BuildMetadata' has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build @@ -386,6 +387,10 @@ module Ci trace.exist? end + def has_test_reports? + job_artifacts.test_reports.any? + end + def has_old_trace? old_trace.present? end @@ -453,16 +458,22 @@ module Ci save end + def erase_test_reports! + # TODO: Use fast_destroy_all in the context of https://gitlab.com/gitlab-org/gitlab-ce/issues/35240 + job_artifacts_junit&.destroy + end + def erase(opts = {}) return false unless erasable? erase_artifacts! + erase_test_reports! erase_trace! update_erased!(opts[:erased_by]) end def erasable? - complete? && (artifacts? || has_trace?) + complete? && (artifacts? || has_test_reports? || has_trace?) end def erased? @@ -539,10 +550,6 @@ module Ci Gitlab::Ci::Build::Image.from_services(self) end - def artifacts - [options[:artifacts]] - end - def cache cache = options[:cache] diff --git a/app/models/ci/build_runner_session.rb b/app/models/ci/build_runner_session.rb index 6f3be31d8e1..869dc0ccadf 100644 --- a/app/models/ci/build_runner_session.rb +++ b/app/models/ci/build_runner_session.rb @@ -17,7 +17,7 @@ module Ci { subprotocols: ['terminal.gitlab.com'].freeze, url: "#{url}/exec".sub("https://", "wss://"), - headers: { Authorization: authorization.presence }.compact, + headers: { Authorization: [authorization.presence] }.compact, ca_pem: certificate.presence } end diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 3b952391b7e..054b714f8ac 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -4,11 +4,17 @@ module Ci include ObjectStorage::BackgroundMove extend Gitlab::Ci::Model + TEST_REPORT_FILE_TYPES = %w[junit].freeze + DEFAULT_FILE_NAMES = { junit: 'junit.xml' }.freeze + TYPE_AND_FORMAT_PAIRS = { archive: :zip, metadata: :gzip, trace: :raw, junit: :gzip }.freeze + belongs_to :project belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id mount_uploader :file, JobArtifactUploader + validates :file_format, presence: true, unless: :trace?, on: :create + validate :valid_file_format?, unless: :trace?, on: :create before_save :set_size, if: :file_changed? after_save :update_project_statistics_after_save, if: :size_changed? after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed? @@ -17,14 +23,33 @@ module Ci scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) } + scope :test_reports, -> do + types = self.file_types.select { |file_type| TEST_REPORT_FILE_TYPES.include?(file_type) }.values + + where(file_type: types) + end + delegate :exists?, :open, to: :file enum file_type: { archive: 1, metadata: 2, - trace: 3 + trace: 3, + junit: 4 } + enum file_format: { + raw: 1, + zip: 2, + gzip: 3 + } + + def valid_file_format? + unless TYPE_AND_FORMAT_PAIRS[self.file_type&.to_sym] == self.file_format&.to_sym + errors.add(:file_format, 'Invalid file format with specified file type') + end + end + def update_file_store # The file.object_store is set during `uploader.store!` # which happens after object is inserted/updated diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb index 164c704260e..4fef615e6e3 100644 --- a/app/models/concerns/atomic_internal_id.rb +++ b/app/models/concerns/atomic_internal_id.rb @@ -26,6 +26,10 @@ module AtomicInternalId module ClassMethods def has_internal_id(column, scope:, init:, presence: true) # rubocop:disable Naming/PredicateName + # We require init here to retain the ability to recalculate in the absence of a + # InternaLId record (we may delete records in `internal_ids` for example). + raise "has_internal_id requires a init block, none given." unless init + before_validation :"ensure_#{scope}_#{column}!", on: :create validates column, presence: presence diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 14cc12b38a5..d0f7f9c3593 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -149,6 +149,10 @@ class Milestone < ActiveRecord::Base reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) when 'due_date_desc' reorder(Gitlab::Database.nulls_last_order('due_date', 'DESC')) + when 'name_asc' + reorder(Arel::Nodes::Ascending.new(arel_table[:title].lower)) + when 'name_desc' + reorder(Arel::Nodes::Descending.new(arel_table[:title].lower)) when 'start_date_asc' reorder(Gitlab::Database.nulls_last_order('start_date', 'ASC')) when 'start_date_desc' diff --git a/app/models/project.rb b/app/models/project.rb index b876270116e..da30d2fbb4f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -548,6 +548,10 @@ class Project < ActiveRecord::Base repository.commit_by(oid: oid) end + def commits_by(oids:) + repository.commits_by(oids: oids) + end + # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) latest_pipeline = pipelines.latest_successful_for(ref) diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index 5d4e3c34b39..ef4a26f0e7f 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -5,7 +5,7 @@ class ProjectStatistics < ActiveRecord::Base before_save :update_storage_size COLUMNS_TO_REFRESH = [:repository_size, :lfs_objects_size, :commit_count].freeze - INCREMENTABLE_COLUMNS = [:build_artifacts_size].freeze + INCREMENTABLE_COLUMNS = { build_artifacts_size: %i[storage_size] }.freeze def total_repository_size repository_size + lfs_objects_size @@ -38,11 +38,28 @@ class ProjectStatistics < ActiveRecord::Base self.storage_size = repository_size + lfs_objects_size + build_artifacts_size end + # Since this incremental update method does not call update_storage_size above, + # we have to update the storage_size here as additional column. + # Additional columns are updated depending on key => [columns], which allows + # to update statistics which are and also those which aren't included in storage_size + # or any other additional summary column in the future. def self.increment_statistic(project_id, key, amount) - raise ArgumentError, "Cannot increment attribute: #{key}" unless key.in?(INCREMENTABLE_COLUMNS) + raise ArgumentError, "Cannot increment attribute: #{key}" unless INCREMENTABLE_COLUMNS.key?(key) return if amount == 0 where(project_id: project_id) - .update_all(["#{key} = COALESCE(#{key}, 0) + (?)", amount]) + .columns_to_increment(key, amount) + end + + def self.columns_to_increment(key, amount) + updates = ["#{key} = COALESCE(#{key}, 0) + (#{amount})"] + + if (additional = INCREMENTABLE_COLUMNS[key]) + additional.each do |column| + updates << "#{column} = COALESCE(#{column}, 0) + (#{amount})" + end + end + + update_all(updates.join(', ')) end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 9873d9a6327..50f42be661b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -312,6 +312,8 @@ class Repository # types - An Array of file types (e.g. `:readme`) used to refresh extra # caches. def refresh_method_caches(types) + return if types.empty? + to_refresh = [] types.each do |type| diff --git a/app/models/user.rb b/app/models/user.rb index 58429f8d607..03549872924 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -248,6 +248,7 @@ class User < ActiveRecord::Base scope :todo_authors, ->(user_id, state) { where(id: Todo.where(user_id: user_id, state: state).select(:author_id)) } scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) } scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) } + scope :confirmed, -> { where.not(confirmed_at: nil) } def self.with_two_factor_indistinct joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id") @@ -293,14 +294,17 @@ class User < ActiveRecord::Base end # Find a User by their primary email or any associated secondary email - def find_by_any_email(email) - by_any_email(email).take + def find_by_any_email(email, confirmed: false) + by_any_email(email, confirmed: confirmed).take end # Returns a relation containing all the users for the given Email address - def by_any_email(email) + def by_any_email(email, confirmed: false) users = where(email: email) + users = users.confirmed if confirmed + emails = joins(:emails).where(emails: { email: email }) + emails = emails.confirmed if confirmed union = Gitlab::SQL::Union.new([users, emails]) from("(#{union.to_sql}) #{table_name}") diff --git a/app/presenters/ci/build_runner_presenter.rb b/app/presenters/ci/build_runner_presenter.rb new file mode 100644 index 00000000000..02f6c5bdf81 --- /dev/null +++ b/app/presenters/ci/build_runner_presenter.rb @@ -0,0 +1,43 @@ +module Ci + class BuildRunnerPresenter < SimpleDelegator + def artifacts + return unless options[:artifacts] + + list = [] + list << create_archive(options[:artifacts]) + list << create_reports(options[:artifacts][:reports], expire_in: options[:artifacts][:expire_in]) + list.flatten.compact + end + + private + + def create_archive(artifacts) + return unless artifacts[:untracked] || artifacts[:paths] + + { + artifact_type: :archive, + artifact_format: :zip, + name: artifacts[:name], + untracked: artifacts[:untracked], + paths: artifacts[:paths], + when: artifacts[:when], + expire_in: artifacts[:expire_in] + } + end + + def create_reports(reports, expire_in:) + return unless reports&.any? + + reports.map do |k, v| + { + artifact_type: k.to_sym, + artifact_format: :gzip, + name: ::Ci::JobArtifact::DEFAULT_FILE_NAMES[k.to_sym], + paths: v, + when: 'always', + expire_in: expire_in + } + end + end + end +end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 4fe04e4b206..63fd9d63ec4 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -132,6 +132,10 @@ class MergeRequestWidgetEntity < IssuableEntity can?(request.current_user, :create_note, merge_request) end + expose :can_create_issue do |merge_request| + can?(current_user, :create_issue, merge_request.project) + end + expose :can_update do |merge_request| can?(request.current_user, :update_merge_request, merge_request) end diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 50c11be0d15..0db1418b37a 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -3,14 +3,35 @@ module Boards module Issues class ListService < Boards::BaseService + include Gitlab::Utils::StrongMemoize + def execute - issues = IssuesFinder.new(current_user, filter_params).execute - issues = filter(issues) - issues.order_by_position_and_priority + fetch_issues.order_by_position_and_priority + end + + def metadata + keys = metadata_fields.keys + columns = metadata_fields.values_at(*keys).join(', ') + results = Issue.where(id: fetch_issues.select('issues.id')).pluck(columns) + + Hash[keys.zip(results.flatten)] end private + def metadata_fields + { size: 'COUNT(*)' } + end + + # We memoize the query here since the finder methods we use are quite complex. This does not memoize the result of the query. + def fetch_issues + strong_memoize(:fetch_issues) do + issues = IssuesFinder.new(current_user, filter_params).execute + + filter(issues).reorder(nil) + end + end + def filter(issues) issues = without_board_labels(issues) unless list&.movable? || list&.closed? issues = with_list_label(issues) if list&.label? diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 29c8ce5fea3..e3640e38bfa 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -3,6 +3,7 @@ class GitPushService < BaseService attr_accessor :push_data, :push_commits include Gitlab::Access + include Gitlab::Utils::StrongMemoize # The N most recent commits to process in a single push payload. PROCESS_COMMIT_LIMIT = 100 @@ -21,14 +22,14 @@ class GitPushService < BaseService # 6. Checks if the project's main language has changed # def execute - @project.repository.after_create if @project.empty_repo? - @project.repository.after_push_commit(branch_name) + project.repository.after_create if project.empty_repo? + project.repository.after_push_commit(branch_name) if push_remove_branch? - @project.repository.after_remove_branch + project.repository.after_remove_branch @push_commits = [] elsif push_to_new_branch? - @project.repository.after_create_branch + project.repository.after_create_branch # Re-find the pushed commits. if default_branch? @@ -38,14 +39,14 @@ class GitPushService < BaseService # Use the pushed commits that aren't reachable by the default branch # as a heuristic. This may include more commits than are actually pushed, but # that shouldn't matter because we check for existing cross-references later. - @push_commits = @project.repository.commits_between(@project.default_branch, params[:newrev]) + @push_commits = project.repository.commits_between(project.default_branch, params[:newrev]) # don't process commits for the initial push to the default branch process_commit_messages end elsif push_to_existing_branch? # Collect data for this git push - @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev]) + @push_commits = project.repository.commits_between(params[:oldrev], params[:newrev]) process_commit_messages @@ -64,7 +65,7 @@ class GitPushService < BaseService end def update_gitattributes - @project.repository.copy_gitattributes(params[:ref]) + project.repository.copy_gitattributes(params[:ref]) end def update_caches @@ -76,7 +77,7 @@ class GitPushService < BaseService else paths = Set.new - @push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| + last_pushed_commits.each do |commit| commit.raw_deltas.each do |diff| paths << diff.new_path end @@ -88,11 +89,11 @@ class GitPushService < BaseService types = [] end - ProjectCacheWorker.perform_async(@project.id, types, [:commit_count, :repository_size]) + ProjectCacheWorker.perform_async(project.id, types, [:commit_count, :repository_size]) end def update_signatures - commit_shas = @push_commits.last(PROCESS_COMMIT_LIMIT).map(&:sha) + commit_shas = last_pushed_commits.map(&:sha) return if commit_shas.empty? @@ -103,16 +104,14 @@ class GitPushService < BaseService commit_shas = Gitlab::Git::Commit.shas_with_signatures(project.repository, commit_shas) - commit_shas.each do |sha| - CreateGpgSignatureWorker.perform_async(sha, project.id) - end + CreateGpgSignatureWorker.perform_async(commit_shas, project.id) end # Schedules processing of commit messages. def process_commit_messages default = default_branch? - @push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| + last_pushed_commits.each do |commit| if commit.matches_cross_reference_regex? ProcessCommitWorker .perform_async(project.id, current_user.id, commit.to_hash, default) @@ -123,10 +122,10 @@ class GitPushService < BaseService protected def update_remote_mirrors - return unless @project.has_remote_mirror? + return unless project.has_remote_mirror? - @project.mark_stuck_remote_mirrors_as_failed! - @project.update_remote_mirrors + project.mark_stuck_remote_mirrors_as_failed! + project.update_remote_mirrors end def execute_related_hooks @@ -134,14 +133,14 @@ class GitPushService < BaseService # could cause the last commit of a merge request to change. # UpdateMergeRequestsWorker - .perform_async(@project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) + .perform_async(project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) - EventCreateService.new.push(@project, current_user, build_push_data) - Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push) + EventCreateService.new.push(project, current_user, build_push_data) + Ci::CreatePipelineService.new(project, current_user, build_push_data).execute(:push) SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks) - @project.execute_hooks(build_push_data.dup, :push_hooks) - @project.execute_services(build_push_data.dup, :push_hooks) + project.execute_hooks(build_push_data.dup, :push_hooks) + project.execute_services(build_push_data.dup, :push_hooks) if push_remove_branch? AfterBranchDeleteService @@ -151,52 +150,50 @@ class GitPushService < BaseService end def perform_housekeeping - housekeeping = Projects::HousekeepingService.new(@project) + housekeeping = Projects::HousekeepingService.new(project) housekeeping.increment! housekeeping.execute if housekeeping.needed? rescue Projects::HousekeepingService::LeaseTaken end def process_default_branch - @push_commits_count = project.repository.commit_count_for_ref(params[:ref]) - - offset = [@push_commits_count - PROCESS_COMMIT_LIMIT, 0].max + offset = [push_commits_count - PROCESS_COMMIT_LIMIT, 0].max @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) - @project.after_create_default_branch + project.after_create_default_branch end def build_push_data @push_data ||= Gitlab::DataBuilder::Push.build( - @project, + project, current_user, params[:oldrev], params[:newrev], params[:ref], @push_commits, - commits_count: @push_commits_count) + commits_count: push_commits_count) end def push_to_existing_branch? # Return if this is not a push to a branch (e.g. new commits) - Gitlab::Git.branch_ref?(params[:ref]) && !Gitlab::Git.blank_ref?(params[:oldrev]) + branch_ref? && !Gitlab::Git.blank_ref?(params[:oldrev]) end def push_to_new_branch? - Gitlab::Git.branch_ref?(params[:ref]) && Gitlab::Git.blank_ref?(params[:oldrev]) + strong_memoize(:push_to_new_branch) do + branch_ref? && Gitlab::Git.blank_ref?(params[:oldrev]) + end end def push_remove_branch? - Gitlab::Git.branch_ref?(params[:ref]) && Gitlab::Git.blank_ref?(params[:newrev]) - end - - def push_to_branch? - Gitlab::Git.branch_ref?(params[:ref]) + strong_memoize(:push_remove_branch) do + branch_ref? && Gitlab::Git.blank_ref?(params[:newrev]) + end end def default_branch? - Gitlab::Git.branch_ref?(params[:ref]) && - (Gitlab::Git.ref_name(params[:ref]) == project.default_branch || project.default_branch.nil?) + branch_ref? && + (branch_name == project.default_branch || project.default_branch.nil?) end def commit_user(commit) @@ -204,6 +201,24 @@ class GitPushService < BaseService end def branch_name - @branch_name ||= Gitlab::Git.ref_name(params[:ref]) + strong_memoize(:branch_name) do + Gitlab::Git.ref_name(params[:ref]) + end + end + + def branch_ref? + strong_memoize(:branch_ref) do + Gitlab::Git.branch_ref?(params[:ref]) + end + end + + def push_commits_count + strong_memoize(:push_commits_count) do + project.repository.commit_count_for_ref(params[:ref]) + end + end + + def last_pushed_commits + @last_pushed_commits ||= @push_commits.last(PROCESS_COMMIT_LIMIT) end end diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 3ff2d1d107d..dbadafc0f52 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -9,12 +9,12 @@ class GitTagPushService < BaseService @push_data = build_push_data - EventCreateService.new.push(project, current_user, @push_data) - Ci::CreatePipelineService.new(project, current_user, @push_data).execute(:push) + EventCreateService.new.push(project, current_user, push_data) + Ci::CreatePipelineService.new(project, current_user, push_data).execute(:push) - SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks) - project.execute_hooks(@push_data.dup, :tag_push_hooks) - project.execute_services(@push_data.dup, :tag_push_hooks) + SystemHooksService.new.execute_hooks(build_system_push_data, :tag_push_hooks) + project.execute_hooks(push_data.dup, :tag_push_hooks) + project.execute_services(push_data.dup, :tag_push_hooks) ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size]) diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index c02dddf67b2..faa4c8a5a4f 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -37,6 +37,8 @@ module Issues end if issue.previous_changes.include?('confidential') + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::ConfidentialIssueWorker.perform_in(1.hour, issue.id) if issue.confidential? create_confidentiality_note(issue) end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index aca0ba66646..c186a5971dc 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -15,6 +15,8 @@ module Members notification_service.decline_access_request(member) end + enqeue_delete_todos(member) + after_execute(member: member) member @@ -22,6 +24,12 @@ module Members private + def enqeue_delete_todos(member) + type = member.is_a?(GroupMember) ? 'Group' : 'Project' + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::EntityLeaveWorker.perform_in(1.hour, member.user_id, member.source_id, type) + end + def can_destroy_member?(member) can?(current_user, destroy_member_permission(member), member) end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index d3dc11435fe..31ab4fbe49e 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -25,13 +25,7 @@ module Projects return validation_failed! if project.errors.any? if project.update(params.except(:default_branch)) - if project.previous_changes.include?('path') - project.rename_repo - else - system_hook_service.execute_hooks_for(project, :update) - end - - update_pages_config if changing_pages_https_only? + after_update success else @@ -47,6 +41,30 @@ module Projects private + def after_update + todos_features_changes = %w( + issues_access_level + merge_requests_access_level + repository_access_level + ) + project_changed_feature_keys = project.project_feature.previous_changes.keys + + if project.previous_changes.include?(:visibility_level) && project.private? + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::ProjectPrivateWorker.perform_in(1.hour, project.id) + elsif (project_changed_feature_keys & todos_features_changes).present? + TodosDestroyer::PrivateFeaturesWorker.perform_in(1.hour, project.id) + end + + if project.previous_changes.include?('path') + project.rename_repo + else + system_hook_service.execute_hooks_for(project, :update) + end + + update_pages_config if changing_pages_https_only? + end + def validation_failed! model_errors = project.errors.full_messages.to_sentence error_message = model_errors.presence || 'Project could not be updated!' diff --git a/app/services/todos/destroy/base_service.rb b/app/services/todos/destroy/base_service.rb new file mode 100644 index 00000000000..dff5e1f30e5 --- /dev/null +++ b/app/services/todos/destroy/base_service.rb @@ -0,0 +1,33 @@ +module Todos + module Destroy + class BaseService + def execute + return unless todos_to_remove? + + without_authorized(todos).delete_all + end + + private + + def without_authorized(items) + items.where('user_id NOT IN (?)', authorized_users) + end + + def authorized_users + ProjectAuthorization.select(:user_id).where(project_id: project_ids) + end + + def todos + raise NotImplementedError + end + + def project_ids + raise NotImplementedError + end + + def todos_to_remove? + raise NotImplementedError + end + end + end +end diff --git a/app/services/todos/destroy/confidential_issue_service.rb b/app/services/todos/destroy/confidential_issue_service.rb new file mode 100644 index 00000000000..c5b66df057a --- /dev/null +++ b/app/services/todos/destroy/confidential_issue_service.rb @@ -0,0 +1,39 @@ +module Todos + module Destroy + class ConfidentialIssueService < ::Todos::Destroy::BaseService + extend ::Gitlab::Utils::Override + + attr_reader :issue + + def initialize(issue_id) + @issue = Issue.find_by(id: issue_id) + end + + private + + override :todos + def todos + Todo.where(target: issue) + .where('user_id != ?', issue.author_id) + .where('user_id NOT IN (?)', issue.assignees.select(:id)) + end + + override :todos_to_remove? + def todos_to_remove? + issue&.confidential? + end + + override :project_ids + def project_ids + issue.project_id + end + + override :authorized_users + def authorized_users + ProjectAuthorization.select(:user_id) + .where(project_id: project_ids) + .where('access_level >= ?', Gitlab::Access::REPORTER) + end + end + end +end diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb new file mode 100644 index 00000000000..2ff9f94b718 --- /dev/null +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -0,0 +1,59 @@ +module Todos + module Destroy + class EntityLeaveService < ::Todos::Destroy::BaseService + extend ::Gitlab::Utils::Override + + attr_reader :user_id, :entity + + def initialize(user_id, entity_id, entity_type) + unless %w(Group Project).include?(entity_type) + raise ArgumentError.new("#{entity_type} is not an entity user can leave") + end + + @user_id = user_id + @entity = entity_type.constantize.find_by(id: entity_id) + end + + private + + override :todos + def todos + if entity.private? + Todo.where(project_id: project_ids, user_id: user_id) + else + project_ids.each do |project_id| + TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user_id) + end + + Todo.where( + target_id: confidential_issues.select(:id), target_type: Issue, user_id: user_id + ) + end + end + + override :project_ids + def project_ids + case entity + when Project + [entity.id] + when Namespace + Project.select(:id).where(namespace_id: entity.self_and_descendants.select(:id)) + end + end + + override :todos_to_remove? + def todos_to_remove? + # if an entity is provided we want to check always at least private features + !!entity + end + + def confidential_issues + assigned_ids = IssueAssignee.select(:issue_id).where(user_id: user_id) + + Issue.where(project_id: project_ids, confidential: true) + .where('author_id != ?', user_id) + .where('id NOT IN (?)', assigned_ids) + end + end + end +end diff --git a/app/services/todos/destroy/private_features_service.rb b/app/services/todos/destroy/private_features_service.rb new file mode 100644 index 00000000000..4d8e2877bfb --- /dev/null +++ b/app/services/todos/destroy/private_features_service.rb @@ -0,0 +1,40 @@ +module Todos + module Destroy + class PrivateFeaturesService < ::Todos::Destroy::BaseService + attr_reader :project_ids, :user_id + + def initialize(project_ids, user_id = nil) + @project_ids = project_ids + @user_id = user_id + end + + def execute + ProjectFeature.where(project_id: project_ids).each do |project_features| + target_types = [] + target_types << Issue if private?(project_features.issues_access_level) + target_types << MergeRequest if private?(project_features.merge_requests_access_level) + target_types << Commit if private?(project_features.repository_access_level) + + next if target_types.empty? + + remove_todos(project_features.project_id, target_types) + end + end + + private + + def private?(feature_level) + feature_level == ProjectFeature::PRIVATE + end + + def remove_todos(project_id, target_types) + items = Todo.where(project_id: project_id) + items = items.where(user_id: user_id) if user_id + + items.where('user_id NOT IN (?)', authorized_users) + .where(target_type: target_types) + .delete_all + end + end + end +end diff --git a/app/services/todos/destroy/project_private_service.rb b/app/services/todos/destroy/project_private_service.rb new file mode 100644 index 00000000000..171933e7cbc --- /dev/null +++ b/app/services/todos/destroy/project_private_service.rb @@ -0,0 +1,30 @@ +module Todos + module Destroy + class ProjectPrivateService < ::Todos::Destroy::BaseService + extend ::Gitlab::Utils::Override + + attr_reader :project + + def initialize(project_id) + @project = Project.find_by(id: project_id) + end + + private + + override :todos + def todos + Todo.where(project_id: project_ids) + end + + override :project_ids + def project_ids + project.id + end + + override :todos_to_remove? + def todos_to_remove? + project&.private? + end + end + end +end diff --git a/app/views/projects/wikis/_main_links.html.haml b/app/views/projects/wikis/_main_links.html.haml index cadda0a33c2..8d91f411f89 100644 --- a/app/views/projects/wikis/_main_links.html.haml +++ b/app/views/projects/wikis/_main_links.html.haml @@ -4,6 +4,6 @@ = s_("Wiki|New page") = link_to project_wiki_history_path(@project, @page), class: "btn" do = s_("Wiki|Page history") - - if can?(current_user, :create_wiki, @project) && @page.latest? + - if can?(current_user, :create_wiki, @project) && @page.latest? && @valid_encoding = link_to project_wiki_edit_path(@project, @page), class: "btn js-wiki-edit" do = _("Edit") diff --git a/app/views/shared/boards/components/_board.html.haml b/app/views/shared/boards/components/_board.html.haml index 03e008f5fa0..b35877e5518 100644 --- a/app/views/shared/boards/components/_board.html.haml +++ b/app/views/shared/boards/components/_board.html.haml @@ -32,17 +32,21 @@ "v-if" => "!list.preset && list.id" } %button.board-delete.has-tooltip.float-right{ type: "button", title: _("Delete list"), "aria-label" => _("Delete list"), data: { placement: "bottom" }, "@click.stop" => "deleteBoard" } = icon("trash") - .issue-count-badge.clearfix{ "v-if" => 'list.type !== "blank" && list.type !== "promotion"' } - %span.issue-count-badge-count.float-left{ ":class" => '{ "has-btn": list.type !== "closed" && !disabled }' } + .issue-count-badge.text-secondary{ "v-if" => 'list.type !== "blank" && list.type !== "promotion"', ":title": "counterTooltip", "v-tooltip": true, data: { placement: "top" } } + %span.issue-count-badge-count + %icon.mr-1{ name: "issues" } {{ list.issuesSize }} - - if can?(current_user, :admin_list, current_board_parent) - %button.issue-count-badge-add-button.btn.btn-sm.btn-default.has-tooltip.js-no-trigger-collapse{ type: "button", - "@click" => "showNewIssueForm", - "v-if" => 'list.type !== "closed"', - "aria-label" => _("New issue"), - "title" => _("New issue"), - data: { placement: "top", container: "body" } } - = icon("plus", class: "js-no-trigger-collapse") + = render_if_exists "shared/boards/components/list_weight" + + - if can?(current_user, :admin_list, current_board_parent) + %button.issue-count-badge-add-button.btn.btn-sm.btn-default.ml-1.has-tooltip.js-no-trigger-collapse{ type: "button", + "@click" => "showNewIssueForm", + "v-if" => 'list.type !== "closed"', + "aria-label" => _("New issue"), + "title" => _("New issue"), + data: { placement: "top", container: "body" } } + = icon("plus", class: "js-no-trigger-collapse") + %board-list{ "v-if" => 'list.type !== "blank" && list.type !== "promotion"', ":list" => "list", ":issues" => "list.issues", diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 4de35b9bd06..f2651cb54ec 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -73,6 +73,11 @@ - repository_check:repository_check_batch - repository_check:repository_check_single_repository +- todos_destroyer:todos_destroyer_confidential_issue +- todos_destroyer:todos_destroyer_entity_leave +- todos_destroyer:todos_destroyer_project_private +- todos_destroyer:todos_destroyer_private_features + - default - mailers # ActionMailer::DeliveryJob.queue_name diff --git a/app/workers/concerns/todos_destroyer_queue.rb b/app/workers/concerns/todos_destroyer_queue.rb new file mode 100644 index 00000000000..8e2b1d30579 --- /dev/null +++ b/app/workers/concerns/todos_destroyer_queue.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +## +# Concern for setting Sidekiq settings for the various Todos Destroyers. +# +module TodosDestroyerQueue + extend ActiveSupport::Concern + + included do + queue_namespace :todos_destroyer + end +end diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb index a2da1bda11f..91833941070 100644 --- a/app/workers/create_gpg_signature_worker.rb +++ b/app/workers/create_gpg_signature_worker.rb @@ -3,15 +3,23 @@ class CreateGpgSignatureWorker include ApplicationWorker - def perform(commit_sha, project_id) + def perform(commit_shas, project_id) + return if commit_shas.empty? + project = Project.find_by(id: project_id) return unless project - commit = project.commit(commit_sha) + commits = project.commits_by(oids: commit_shas) - return unless commit + return if commits.empty? # This calculates and caches the signature in the database - Gitlab::Gpg::Commit.new(commit).signature + commits.each do |commit| + begin + Gitlab::Gpg::Commit.new(commit).signature + rescue => e + Rails.logger.error("Failed to create signature for commit #{commit.id}. Error: #{e.message}") + end + end end end diff --git a/app/workers/todos_destroyer/confidential_issue_worker.rb b/app/workers/todos_destroyer/confidential_issue_worker.rb new file mode 100644 index 00000000000..9d640c14963 --- /dev/null +++ b/app/workers/todos_destroyer/confidential_issue_worker.rb @@ -0,0 +1,10 @@ +module TodosDestroyer + class ConfidentialIssueWorker + include ApplicationWorker + include TodosDestroyerQueue + + def perform(issue_id) + ::Todos::Destroy::ConfidentialIssueService.new(issue_id).execute + end + end +end diff --git a/app/workers/todos_destroyer/entity_leave_worker.rb b/app/workers/todos_destroyer/entity_leave_worker.rb new file mode 100644 index 00000000000..e62d9876f4a --- /dev/null +++ b/app/workers/todos_destroyer/entity_leave_worker.rb @@ -0,0 +1,10 @@ +module TodosDestroyer + class EntityLeaveWorker + include ApplicationWorker + include TodosDestroyerQueue + + def perform(user_id, entity_id, entity_type) + ::Todos::Destroy::EntityLeaveService.new(user_id, entity_id, entity_type).execute + end + end +end diff --git a/app/workers/todos_destroyer/private_features_worker.rb b/app/workers/todos_destroyer/private_features_worker.rb new file mode 100644 index 00000000000..f457d5e0471 --- /dev/null +++ b/app/workers/todos_destroyer/private_features_worker.rb @@ -0,0 +1,10 @@ +module TodosDestroyer + class PrivateFeaturesWorker + include ApplicationWorker + include TodosDestroyerQueue + + def perform(project_id, user_id = nil) + ::Todos::Destroy::PrivateFeaturesService.new(project_id, user_id).execute + end + end +end diff --git a/app/workers/todos_destroyer/project_private_worker.rb b/app/workers/todos_destroyer/project_private_worker.rb new file mode 100644 index 00000000000..7a853c36370 --- /dev/null +++ b/app/workers/todos_destroyer/project_private_worker.rb @@ -0,0 +1,10 @@ +module TodosDestroyer + class ProjectPrivateWorker + include ApplicationWorker + include TodosDestroyerQueue + + def perform(project_id) + ::Todos::Destroy::ProjectPrivateService.new(project_id).execute + end + end +end diff --git a/changelogs/unreleased/25990-interactive-web-terminals-authorization.yml b/changelogs/unreleased/25990-interactive-web-terminals-authorization.yml new file mode 100644 index 00000000000..0a2853c20c6 --- /dev/null +++ b/changelogs/unreleased/25990-interactive-web-terminals-authorization.yml @@ -0,0 +1,5 @@ +--- +title: Fix authorization for interactive web terminals +merge_request: 20811 +author: +type: fixed diff --git a/changelogs/unreleased/32821-better-error-message-add-invalid-user-to-project.yml b/changelogs/unreleased/32821-better-error-message-add-invalid-user-to-project.yml new file mode 100644 index 00000000000..587d7209c2f --- /dev/null +++ b/changelogs/unreleased/32821-better-error-message-add-invalid-user-to-project.yml @@ -0,0 +1,5 @@ +--- +title: Improve error message when adding invalid user to a project +merge_request: 20885 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/changelogs/unreleased/48823-copy-gfm.yml b/changelogs/unreleased/48823-copy-gfm.yml new file mode 100644 index 00000000000..b6137e2e3f9 --- /dev/null +++ b/changelogs/unreleased/48823-copy-gfm.yml @@ -0,0 +1,5 @@ +--- +title: Resolve Copy diff file path as GFM is broken +merge_request: 20725 +author: +type: fixed diff --git a/changelogs/unreleased/49161-disable-toggle-comments.yml b/changelogs/unreleased/49161-disable-toggle-comments.yml new file mode 100644 index 00000000000..5ec16191f07 --- /dev/null +++ b/changelogs/unreleased/49161-disable-toggle-comments.yml @@ -0,0 +1,5 @@ +--- +title: Disables toggle comments button if diff has no discussions +merge_request: +author: +type: other diff --git a/changelogs/unreleased/49701-sorting-by-name-on-milestones-page-error.yml b/changelogs/unreleased/49701-sorting-by-name-on-milestones-page-error.yml new file mode 100644 index 00000000000..7eb73110d60 --- /dev/null +++ b/changelogs/unreleased/49701-sorting-by-name-on-milestones-page-error.yml @@ -0,0 +1,5 @@ +--- +title: Fix sorting by name on milestones page +merge_request: 20881 +author: +type: fixed diff --git a/changelogs/unreleased/49747-update-poll-2xx.yml b/changelogs/unreleased/49747-update-poll-2xx.yml new file mode 100644 index 00000000000..359d1b80447 --- /dev/null +++ b/changelogs/unreleased/49747-update-poll-2xx.yml @@ -0,0 +1,5 @@ +--- +title: Changes poll.js to keep polling on any 2xx http status code +merge_request: 20904 +author: +type: other diff --git a/changelogs/unreleased/artifact-format-v2.yml b/changelogs/unreleased/artifact-format-v2.yml new file mode 100644 index 00000000000..e264e0a9fa1 --- /dev/null +++ b/changelogs/unreleased/artifact-format-v2.yml @@ -0,0 +1,5 @@ +--- +title: Extend gitlab-ci.yml to request junit.xml test reports +merge_request: 20390 +author: +type: added diff --git a/changelogs/unreleased/feature-gb-login-activity-metrics.yml b/changelogs/unreleased/feature-gb-login-activity-metrics.yml new file mode 100644 index 00000000000..5d687b984eb --- /dev/null +++ b/changelogs/unreleased/feature-gb-login-activity-metrics.yml @@ -0,0 +1,5 @@ +--- +title: Add more comprehensive metrics tracking authentication activity +merge_request: 20668 +author: +type: added diff --git a/changelogs/unreleased/fix-storage-size-for-artifacts-change.yml b/changelogs/unreleased/fix-storage-size-for-artifacts-change.yml new file mode 100644 index 00000000000..6a3e1420726 --- /dev/null +++ b/changelogs/unreleased/fix-storage-size-for-artifacts-change.yml @@ -0,0 +1,5 @@ +--- +title: Update total storage size when changing size of artifacts +merge_request: 20697 +author: Peter Marko +type: fixed diff --git a/changelogs/unreleased/fj-37736-improve-performance-post-receive-create-gpg-siganture-worker.yml b/changelogs/unreleased/fj-37736-improve-performance-post-receive-create-gpg-siganture-worker.yml new file mode 100644 index 00000000000..0b35c5c6786 --- /dev/null +++ b/changelogs/unreleased/fj-37736-improve-performance-post-receive-create-gpg-siganture-worker.yml @@ -0,0 +1,5 @@ +--- +title: Performing Commit GPG signature calculation in bulk +merge_request: 20870 +author: +type: performance diff --git a/changelogs/unreleased/fj-49512-fix-gitlab-git-pages-encoding.yml b/changelogs/unreleased/fj-49512-fix-gitlab-git-pages-encoding.yml new file mode 100644 index 00000000000..3af90fff3f6 --- /dev/null +++ b/changelogs/unreleased/fj-49512-fix-gitlab-git-pages-encoding.yml @@ -0,0 +1,5 @@ +--- +title: Prevent editing and updating wiki pages with non UTF-8 encoding via web interface +merge_request: 20906 +author: +type: fixed diff --git a/changelogs/unreleased/ide-warn-staged-files.yml b/changelogs/unreleased/ide-warn-staged-files.yml new file mode 100644 index 00000000000..ae3c4f392c0 --- /dev/null +++ b/changelogs/unreleased/ide-warn-staged-files.yml @@ -0,0 +1,5 @@ +--- +title: Warn user when reload IDE with staged changes +merge_request: 20857 +author: +type: added diff --git a/changelogs/unreleased/rails5-gpg-permit-concurrent.yml b/changelogs/unreleased/rails5-gpg-permit-concurrent.yml new file mode 100644 index 00000000000..cf1b0023f86 --- /dev/null +++ b/changelogs/unreleased/rails5-gpg-permit-concurrent.yml @@ -0,0 +1,5 @@ +--- +title: Permit concurrent loads in gpg keychain mutex +merge_request: 20894 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/sh-fix-admin-jobs-controller-timing-out.yml b/changelogs/unreleased/sh-fix-admin-jobs-controller-timing-out.yml new file mode 100644 index 00000000000..e1adebbf076 --- /dev/null +++ b/changelogs/unreleased/sh-fix-admin-jobs-controller-timing-out.yml @@ -0,0 +1,5 @@ +--- +title: Fix /admin/jobs failing to load due to statement timeout +merge_request: 20909 +author: +type: performance diff --git a/changelogs/unreleased/sh-simplify-liveness-check.yml b/changelogs/unreleased/sh-simplify-liveness-check.yml new file mode 100644 index 00000000000..225e3dc1378 --- /dev/null +++ b/changelogs/unreleased/sh-simplify-liveness-check.yml @@ -0,0 +1,5 @@ +--- +title: Add /-/health basic health check endpoint +merge_request: 20456 +author: +type: added diff --git a/changelogs/unreleased/sh-support-users-find-by-confirmed-emails.yml b/changelogs/unreleased/sh-support-users-find-by-confirmed-emails.yml new file mode 100644 index 00000000000..4b0c8117b3e --- /dev/null +++ b/changelogs/unreleased/sh-support-users-find-by-confirmed-emails.yml @@ -0,0 +1,5 @@ +--- +title: Add support for searching users by confirmed e-mails +merge_request: 20893 +author: +type: other diff --git a/changelogs/unreleased/todos-visibility-change.yml b/changelogs/unreleased/todos-visibility-change.yml new file mode 100644 index 00000000000..b7632b94771 --- /dev/null +++ b/changelogs/unreleased/todos-visibility-change.yml @@ -0,0 +1,5 @@ +--- +title: Delete todos when user loses access to read the target +merge_request: 20665 +author: +type: other diff --git a/changelogs/unreleased/zj-remove-git-rake-tasks.yml b/changelogs/unreleased/zj-remove-git-rake-tasks.yml new file mode 100644 index 00000000000..8c90fc7d0fe --- /dev/null +++ b/changelogs/unreleased/zj-remove-git-rake-tasks.yml @@ -0,0 +1,5 @@ +--- +title: Remove gitlab:user:check_repos, gitlab:check_repo, gitlab:git:prune, gitlab:git:gc, and gitlab:git:repack +merge_request: 20806 +author: +type: removed diff --git a/config/application.rb b/config/application.rb index b9d4f6765e3..a086e860e16 100644 --- a/config/application.rb +++ b/config/application.rb @@ -154,6 +154,10 @@ module Gitlab config.action_view.sanitized_allowed_protocols = %w(smb) + # This middleware needs to precede ActiveRecord::QueryCache and other middlewares that + # connect to the database. + config.middleware.insert_after "Rails::Rack::Logger", "Gitlab::Middleware::BasicHealthCheck" + config.middleware.insert_after Warden::Manager, Rack::Attack # Allow access to GitLab API from other domains diff --git a/config/initializers/rbtrace.rb b/config/initializers/rbtrace.rb deleted file mode 100644 index 3a076c99ad0..00000000000 --- a/config/initializers/rbtrace.rb +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -require 'rbtrace' if ENV['ENABLE_RBTRACE'] diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index f6803eb0b5a..6f54bee4713 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -8,6 +8,8 @@ Sidekiq.default_worker_options = { retry: 3 } enable_json_logs = Gitlab.config.sidekiq.log_format == 'json' Sidekiq.configure_server do |config| + require 'rbtrace' if ENV['ENABLE_RBTRACE'] + config.redis = queues_config_hash config.server_middleware do |chain| diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index 8cc36820d3c..d64b659c6d7 100644 --- a/config/initializers/warden.rb +++ b/config/initializers/warden.rb @@ -1,10 +1,20 @@ Rails.application.configure do |config| Warden::Manager.after_set_user(scope: :user) do |user, auth, opts| Gitlab::Auth::UniqueIpsLimiter.limit_user!(user) - end - Warden::Manager.before_failure(scope: :user) do |env, opts| - Gitlab::Auth::BlockedUserTracker.log_if_user_blocked(env) + activity = Gitlab::Auth::Activity.new(user, opts) + + case opts[:event] + when :authentication + activity.user_authenticated! + when :set_user + activity.user_authenticated! + activity.user_session_override! + when :fetch # rubocop:disable Lint/EmptyWhen + # We ignore session fetch events + else + activity.user_session_override! + end end Warden::Manager.after_authentication(scope: :user) do |user, auth, opts| @@ -15,7 +25,17 @@ Rails.application.configure do |config| ActiveSession.set(user, auth.request) end - Warden::Manager.before_logout(scope: :user) do |user, auth, opts| - ActiveSession.destroy(user || auth.user, auth.request.session.id) + Warden::Manager.before_failure(scope: :user) do |env, opts| + tracker = Gitlab::Auth::BlockedUserTracker.new(env) + tracker.log_blocked_user_activity! if tracker.user_blocked? + + Gitlab::Auth::Activity.new(tracker.user, opts).user_authentication_failed! + end + + Warden::Manager.before_logout(scope: :user) do |user_warden, auth, opts| + user = user_warden || auth.user + + ActiveSession.destroy(user, auth.request.session.id) + Gitlab::Auth::Activity.new(user, opts).user_session_destroyed! end end diff --git a/config/routes.rb b/config/routes.rb index 63e40a31b76..d16a587c5ee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,6 +46,7 @@ Rails.application.routes.draw do get 'health_check(/:checks)' => 'health_check#index', as: :health_check scope path: '-' do + # '/-/health' implemented by BasicHealthMiddleware get 'liveness' => 'health#liveness' get 'readiness' => 'health#readiness' post 'storage_check' => 'health#storage_check' diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 70b584ff9e9..3c85cd07d46 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -45,6 +45,7 @@ - [github_import_advance_stage, 1] - [project_service, 1] - [delete_user, 1] + - [todos_destroyer, 1] - [delete_merged_branches, 1] - [authorized_projects, 1] - [expire_build_instance_artifacts, 1] diff --git a/config/unicorn.rb.example b/config/unicorn.rb.example index 220a0191160..8f2d842e5b6 100644 --- a/config/unicorn.rb.example +++ b/config/unicorn.rb.example @@ -124,6 +124,10 @@ before_fork do |server, worker| end after_fork do |server, worker| + # Unicorn clears out signals before it forks, so rbtrace won't work + # unless it is enabled after the fork. + require 'rbtrace' if ENV['ENABLE_RBTRACE'] + # per-process listener ports for debugging/admin/migrations # addr = "127.0.0.1:#{9293 + worker.nr}" # server.listen(addr, :tries => -1, :delay => 5, :tcp_nopush => true) diff --git a/db/migrate/20180705160945_add_file_format_to_ci_job_artifacts.rb b/db/migrate/20180705160945_add_file_format_to_ci_job_artifacts.rb new file mode 100644 index 00000000000..63c188693f3 --- /dev/null +++ b/db/migrate/20180705160945_add_file_format_to_ci_job_artifacts.rb @@ -0,0 +1,7 @@ +class AddFileFormatToCiJobArtifacts < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column :ci_job_artifacts, :file_format, :integer, limit: 2 + end +end diff --git a/db/schema.rb b/db/schema.rb index fbacff19591..1d6896b7669 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -394,6 +394,7 @@ ActiveRecord::Schema.define(version: 20180722103201) do t.datetime_with_timezone "expire_at" t.string "file" t.binary "file_sha256" + t.integer "file_format", limit: 2 end add_index "ci_job_artifacts", ["expire_at", "job_id"], name: "index_ci_job_artifacts_on_expire_at_and_job_id", using: :btree diff --git a/doc/README.md b/doc/README.md index 32924942497..a814c787f94 100644 --- a/doc/README.md +++ b/doc/README.md @@ -177,7 +177,8 @@ instant how code changes impact your production environment. - [Prometheus metrics](user/project/integrations/prometheus_library/metrics.md): Let Prometheus collect metrics from various services, like Kubernetes, NGINX, NGINX ingress controller, HAProxy, and Amazon Cloud Watch. - [GitLab Performance Monitoring](administration/monitoring/performance/index.md): Use InfluxDB and Grafana to monitor the performance of your GitLab instance (will be eventually replaced by Prometheus). - [Health check](user/admin_area/monitoring/health_check.md): GitLab provides liveness and readiness probes to indicate service health and reachability to required services. -- [GitLab Cycle Analytics](user/project/cycle_analytics.md): Cycle Analytics measures the time it takes to go from an [idea to production](https://about.gitlab.com/2016/08/05/continuous-integration-delivery-and-deployment-with-gitlab/#from-idea-to-production-with-gitlab) for each project you have. +- [GitLab Cycle Analytics](user/project/cycle_analytics.md): Cycle Analytics measures the time it takes to go from an + [idea to production](https://about.gitlab.com/2016/08/05/continuous-integration-delivery-and-deployment-with-gitlab/#from-idea-to-production-with-gitlab) for each project you have. ## Getting started with GitLab diff --git a/doc/administration/high_availability/nfs.md b/doc/administration/high_availability/nfs.md index 87e96b71dd4..387c3fb6a5b 100644 --- a/doc/administration/high_availability/nfs.md +++ b/doc/administration/high_availability/nfs.md @@ -39,23 +39,11 @@ Our support team will not be able to assist on performance issues related to file system access. Customers and users have reported that AWS EFS does not perform well for GitLab's -use-case. There are several issues that can cause problems. For these reasons -GitLab does not recommend using EFS with GitLab. - -- EFS bases allowed IOPS on volume size. The larger the volume, the more IOPS - are allocated. For smaller volumes, users may experience decent performance - for a period of time due to 'Burst Credits'. Over a period of weeks to months - credits may run out and performance will bottom out. -- To keep "Burst Credits" available, it may be necessary to provision more space - with 'dummy data'. However, this may get expensive. -- Another option to maintain "Burst Credits" is to use FS Cache on the server so - that AWS doesn't always have to go into EFS to access files. -- For larger volumes, allocated IOPS may not be the problem. Workloads where - many small files are written in a serialized manner are not well-suited for EFS. - EBS with an NFS server on top will perform much better. - -In addition, avoid storing GitLab log files (e.g. those in `/var/log/gitlab`) -because this will also affect performance. We recommend that the log files be +use-case. Workloads where many small files are written in a serialized manner, like `git`, +are not well-suited for EFS. EBS with an NFS server on top will perform much better. + +If you do choose to use EFS, avoid storing GitLab log files (e.g. those in `/var/log/gitlab`) +there because this will also affect performance. We recommend that the log files be stored on a local volume. For more details on another person's experience with EFS, see diff --git a/doc/administration/img/raketasks/check_repos_output.png b/doc/administration/img/raketasks/check_repos_output.png Binary files differdeleted file mode 100644 index 7fda2ba0c0f..00000000000 --- a/doc/administration/img/raketasks/check_repos_output.png +++ /dev/null diff --git a/doc/administration/raketasks/check.md b/doc/administration/raketasks/check.md index 0c145830f02..0ca1d77f1d0 100644 --- a/doc/administration/raketasks/check.md +++ b/doc/administration/raketasks/check.md @@ -1,4 +1,4 @@ -# Check Rake Tasks +# Integrity Check Rake Task ## Repository Integrity @@ -28,14 +28,8 @@ exactly which repositories are causing the trouble. ### Check all GitLab repositories ->**Note:** -> -> - `gitlab:repo:check` has been deprecated in favor of `gitlab:git:fsck` -> - [Deprecated][ce-15931] in GitLab 10.4. -> - `gitlab:repo:check` will be removed in the future. [Removal issue][ce-41699] - This task loops through all repositories on the GitLab server and runs the -3 integrity checks described previously. +integrity check described previously. **Omnibus Installation** @@ -49,33 +43,6 @@ sudo gitlab-rake gitlab:git:fsck sudo -u git -H bundle exec rake gitlab:git:fsck RAILS_ENV=production ``` -### Check repositories for a specific user - -This task checks all repositories that a specific user has access to. This is important -because sometimes you know which user is experiencing trouble but you don't know -which project might be the cause. - -If the rake task is executed without brackets at the end, you will be prompted -to enter a username. - -**Omnibus Installation** - -```bash -sudo gitlab-rake gitlab:user:check_repos -sudo gitlab-rake gitlab:user:check_repos[<username>] -``` - -**Source Installation** - -```bash -sudo -u git -H bundle exec rake gitlab:user:check_repos RAILS_ENV=production -sudo -u git -H bundle exec rake gitlab:user:check_repos[<username>] RAILS_ENV=production -``` - -Example output: - - - ## Uploaded Files Integrity Various types of files can be uploaded to a GitLab installation by users. @@ -167,5 +134,4 @@ The LDAP check Rake task will test the bind_dn and password credentials executed as part of the `gitlab:check` task, but can run independently. See [LDAP Rake Tasks - LDAP Check](ldap.md#check) for details. -[ce-15931]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/15931 -[ce-41699]: https://gitlab.com/gitlab-org/gitlab-ce/issues/41699 +[git-fsck]: https://git-scm.com/docs/git-fsck diff --git a/doc/administration/troubleshooting/debug.md b/doc/administration/troubleshooting/debug.md index 7ae4f7c1515..2902af8c782 100644 --- a/doc/administration/troubleshooting/debug.md +++ b/doc/administration/troubleshooting/debug.md @@ -77,7 +77,12 @@ and more. However, this is not enabled by default. To enable it, define the gitlab_rails['env'] = {"ENABLE_RBTRACE" => "1"} ``` -Then reconfigure the system and restart Unicorn and Sidekiq. +Then reconfigure the system and restart Unicorn and Sidekiq. To run this +in Omnibus, run as root: + +```ruby +/opt/gitlab/embedded/bin/ruby /opt/gitlab/embedded/bin/rbtrace +``` ## Common Problems diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index 1320efaf767..da836a0e82e 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -15,7 +15,7 @@ Use the following rules when creating realtime solutions. Use that as your polling interval. This way it is [easy for system administrators to change the polling rate](../../administration/polling.md). A `Poll-Interval: -1` means you should disable polling, and this must be implemented. -1. A response with HTTP status `4XX` or `5XX` should disable polling as well. +1. A response with HTTP status different from 2XX should disable polling as well. 1. Use a common library for polling. 1. Poll on active tabs only. Please use [Visibility](https://github.com/ai/visibilityjs). 1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval will be @@ -25,15 +25,15 @@ controlled by the server. ### Lazy Loading Images -To improve the time to first render we are using lazy loading for images. This works by setting -the actual image source on the `data-src` attribute. After the HTML is rendered and JavaScript is loaded, +To improve the time to first render we are using lazy loading for images. This works by setting +the actual image source on the `data-src` attribute. After the HTML is rendered and JavaScript is loaded, the value of `data-src` will be moved to `src` automatically if the image is in the current viewport. * Prepare images in HTML for lazy loading by renaming the `src` attribute to `data-src` AND adding the class `lazy` * If you are using the Rails `image_tag` helper, all images will be lazy-loaded by default unless `lazy: false` is provided. If you are asynchronously adding content which contains lazy images then you need to call the function -`gl.lazyLoader.searchLazyImages()` which will search for lazy images and load them if needed. +`gl.lazyLoader.searchLazyImages()` which will search for lazy images and load them if needed. But in general it should be handled automatically through a `MutationObserver` in the lazy loading function. ### Animations @@ -97,19 +97,19 @@ bundle and included on the page. ```javascript import initMyWidget from './my_widget'; - + document.addEventListener('DOMContentLoaded', () => { initMyWidget(); }); ``` -- **Supporting Module Placement:** +- **Supporting Module Placement:** - If a class or a module is _specific to a particular route_, try to locate it close to the entry point it will be used. For instance, if `my_widget.js` is only imported within `pages/widget/show/index.js`, you should place the module at `pages/widget/show/my_widget.js` and import it with a relative path (e.g. `import initMyWidget from './my_widget';`). - + - If a class or module is _used by multiple routes_, place it within a shared directory at the closest common parent directory for the entry points that import it. For example, if `my_widget.js` is imported within diff --git a/doc/university/high-availability/aws/README.md b/doc/university/high-availability/aws/README.md index dc045961ed7..8f7bb8636c5 100644 --- a/doc/university/high-availability/aws/README.md +++ b/doc/university/high-availability/aws/README.md @@ -2,10 +2,8 @@ comments: false --- -DANGER: This guide exists for reference of how an AWS deployment could work. -We are currently seeing very slow EFS access performance which causes GitLab to -be 5-10x slower than using NFS or Local disk. We _do not_ recommend follow this -guide at this time. +> **Note**: We **do not** recommend using the AWS Elastic File System (EFS), as it can result +in [significantly degraded performance](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/doc/administration/high_availability/nfs.md#aws-elastic-file-system). # High Availability on AWS diff --git a/doc/user/admin_area/monitoring/health_check.md b/doc/user/admin_area/monitoring/health_check.md index 843fb4ce26b..1b676bfb383 100644 --- a/doc/user/admin_area/monitoring/health_check.md +++ b/doc/user/admin_area/monitoring/health_check.md @@ -20,14 +20,24 @@ To access monitoring resources, the client IP needs to be included in a whitelis [Read how to add IPs to a whitelist for the monitoring endpoints][admin]. -## Using the endpoint +## Using the endpoints With default whitelist settings, the probes can be accessed from localhost: +- `http://localhost/-/health` - `http://localhost/-/readiness` - `http://localhost/-/liveness` -which will then provide a report of system health in JSON format. + +The first endpoint, `/-/health/`, only checks whether the application server is running. It does +-not verify the database or other services are running. A successful response will return +a 200 status code with the following message: + +``` +GitLab OK +``` + +The readiness and liveness probes will provide a report of system health in JSON format. Readiness example output: @@ -42,12 +52,6 @@ Readiness example output: "shared_state_check" : { "status" : "ok" }, - "fs_shards_check" : { - "labels" : { - "shard" : "default" - }, - "status" : "ok" - }, "db_check" : { "status" : "ok" }, @@ -61,9 +65,6 @@ Liveness example output: ``` { - "fs_shards_check" : { - "status" : "ok" - }, "cache_check" : { "status" : "ok" }, diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e883687f2db..4be7707d3e7 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1236,7 +1236,13 @@ module API end class Artifacts < Grape::Entity - expose :name, :untracked, :paths, :when, :expire_in + expose :name + expose :untracked + expose :paths + expose :when + expose :expire_in + expose :artifact_type + expose :artifact_format end class Cache < Grape::Entity diff --git a/lib/api/members.rb b/lib/api/members.rb index 3d2220fed96..d23dd834c69 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -75,7 +75,10 @@ module API member = source.members.find_by(user_id: params[:user_id]) conflict!('Member already exists') if member - member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) + user = User.find_by_id(params[:user_id]) + not_found!('User') unless user + + member = source.add_user(user, params[:access_level], current_user: current_user, expires_at: params[:expires_at]) if !member not_allowed! # This currently can only be reached in EE diff --git a/lib/api/projects.rb b/lib/api/projects.rb index eadde7b17bb..7adde79d6c3 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -13,6 +13,10 @@ module API # EE::API::Projects would override this helper end + params :optional_update_params_ee do + # EE::API::Projects would override this helper + end + # EE::API::Projects would override this method def apply_filters(projects) projects = projects.with_issues_available_for_user(current_user) if params[:with_issues_enabled] @@ -21,6 +25,37 @@ module API projects end + + def verify_update_project_attrs!(project, attrs) + end + end + + def self.update_params_at_least_one_of + [ + :jobs_enabled, + :resolve_outdated_diff_discussions, + :ci_config_path, + :container_registry_enabled, + :default_branch, + :description, + :issues_enabled, + :lfs_enabled, + :merge_requests_enabled, + :merge_method, + :name, + :only_allow_merge_if_all_discussions_are_resolved, + :only_allow_merge_if_pipeline_succeeds, + :path, + :printing_merge_request_link_enabled, + :public_builds, + :request_access_enabled, + :shared_runners_enabled, + :snippets_enabled, + :tag_list, + :visibility, + :wiki_enabled, + :avatar + ] end helpers do @@ -252,39 +287,13 @@ module API success Entities::Project end params do - # CE - at_least_one_of_ce = - [ - :jobs_enabled, - :resolve_outdated_diff_discussions, - :ci_config_path, - :container_registry_enabled, - :default_branch, - :description, - :issues_enabled, - :lfs_enabled, - :merge_requests_enabled, - :merge_method, - :name, - :only_allow_merge_if_all_discussions_are_resolved, - :only_allow_merge_if_pipeline_succeeds, - :path, - :printing_merge_request_link_enabled, - :public_builds, - :request_access_enabled, - :shared_runners_enabled, - :snippets_enabled, - :tag_list, - :visibility, - :wiki_enabled, - :avatar - ] optional :name, type: String, desc: 'The name of the project' optional :default_branch, type: String, desc: 'The default branch of the project' optional :path, type: String, desc: 'The path of the repository' use :optional_project_params - at_least_one_of(*at_least_one_of_ce) + + at_least_one_of(*::API::Projects.update_params_at_least_one_of) end put ':id' do authorize_admin_project @@ -294,6 +303,8 @@ module API attrs = translate_params_for_compatibility(attrs) + verify_update_project_attrs!(user_project, attrs) + result = ::Projects::UpdateService.new(user_project, current_user, attrs).execute if result[:status] == :success diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 06c034444a1..c7595493e11 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -109,7 +109,7 @@ module API if result.valid? if result.build Gitlab::Metrics.add_event(:build_found) - present result.build, with: Entities::JobRequest::Response + present Ci::BuildRunnerPresenter.new(result.build), with: Entities::JobRequest::Response else Gitlab::Metrics.add_event(:build_not_found) header 'X-GitLab-Last-Update', new_update @@ -231,6 +231,10 @@ module API requires :id, type: Integer, desc: %q(Job's ID) optional :token, type: String, desc: %q(Job's authentication token) optional :expire_in, type: String, desc: %q(Specify when artifacts should expire) + optional :artifact_type, type: String, desc: %q(The type of artifact), + default: 'archive', values: Ci::JobArtifact.file_types.keys + optional :artifact_format, type: String, desc: %q(The format of artifact), + default: 'zip', values: Ci::JobArtifact.file_formats.keys optional 'file.path', type: String, desc: %q(path to locally stored body (generated by Workhorse)) optional 'file.name', type: String, desc: %q(real filename as send in Content-Disposition (generated by Workhorse)) optional 'file.type', type: String, desc: %q(real content type as send in Content-Type (generated by Workhorse)) @@ -254,29 +258,29 @@ module API bad_request!('Missing artifacts file!') unless artifacts file_to_large! unless artifacts.size < max_artifacts_size - bad_request!("Already uploaded") if job.job_artifacts_archive - expire_in = params['expire_in'] || Gitlab::CurrentSettings.current_application_settings.default_artifacts_expire_in - job.build_job_artifacts_archive( + job.job_artifacts.build( project: job.project, file: artifacts, - file_type: :archive, + file_type: params['artifact_type'], + file_format: params['artifact_format'], file_sha256: artifacts.sha256, expire_in: expire_in) if metadata - job.build_job_artifacts_metadata( + job.job_artifacts.build( project: job.project, file: metadata, file_type: :metadata, + file_format: :gzip, file_sha256: metadata.sha256, expire_in: expire_in) end if job.update(artifacts_expire_in: expire_in) - present job, with: Entities::JobRequest::Response + present Ci::BuildRunnerPresenter.new(job), with: Entities::JobRequest::Response else render_validation_error!(job) end diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 19d5e66c77e..8d586f3594c 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -20,115 +20,112 @@ module API success Entities::ApplicationSetting end params do + optional :admin_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.' + optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' + optional :after_sign_out_path, type: String, desc: 'We will redirect users to this page after they sign out' + optional :akismet_enabled, type: Boolean, desc: 'Helps prevent bots from creating issues' + given akismet_enabled: ->(val) { val } do + requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com' + end + optional :clientside_sentry_enabled, type: Boolean, desc: 'Sentry can also be used for reporting and logging clientside exceptions. https://sentry.io/for/javascript/' + given clientside_sentry_enabled: ->(val) { val } do + requires :clientside_sentry_dsn, type: String, desc: 'Clientside Sentry Data Source Name' + end + optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)' + optional :default_artifacts_expire_in, type: String, desc: "Set the default expiration time for each job's artifacts" optional :default_branch_protection, type: Integer, values: [0, 1, 2], desc: 'Determine if developers can push to master' + optional :default_group_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default group visibility' optional :default_project_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default project visibility' + optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects' optional :default_snippet_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default snippet visibility' - optional :default_group_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default group visibility' - optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.' - optional :import_sources, type: Array[String], values: %w[github bitbucket gitlab google_code fogbugz git gitlab_project manifest], - desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com' optional :disabled_oauth_sign_in_sources, type: Array[String], desc: 'Disable certain OAuth sign-in sources' - optional :enabled_git_access_protocol, type: String, values: %w[ssh http nil], desc: 'Allow only the selected protocols to be used for Git access.' - optional :project_export_enabled, type: Boolean, desc: 'Enable project export' - optional :gravatar_enabled, type: Boolean, desc: 'Flag indicating if the Gravatar service is enabled' - optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects' - optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB' - optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.' - optional :user_oauth_applications, type: Boolean, desc: 'Allow users to register any application to use GitLab as an OAuth provider' - optional :user_default_external, type: Boolean, desc: 'Newly registered users will by default be external' - optional :signup_enabled, type: Boolean, desc: 'Flag indicating if sign up is enabled' - optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up' - optional :domain_whitelist, type: String, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' optional :domain_blacklist_enabled, type: Boolean, desc: 'Enable domain blacklist for sign ups' given domain_blacklist_enabled: ->(val) { val } do requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' end - optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' - optional :password_authentication_enabled_for_web, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' - optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 - optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 - mutually_exclusive :password_authentication_enabled_for_web, :password_authentication_enabled, :signin_enabled - optional :password_authentication_enabled_for_git, type: Boolean, desc: 'Flag indicating if password authentication is enabled for Git over HTTP(S)' - optional :performance_bar_allowed_group_path, type: String, desc: 'Path of the group that is allowed to toggle the performance bar.' - optional :performance_bar_allowed_group_id, type: String, desc: 'Depreated: Use :performance_bar_allowed_group_path instead. Path of the group that is allowed to toggle the performance bar.' # support legacy names, can be removed in v6 - optional :performance_bar_enabled, type: String, desc: 'Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance.' # support legacy names, can be removed in v6 - optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' - given require_two_factor_authentication: ->(val) { val } do - requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' - end - optional :home_page_url, type: String, desc: 'We will redirect non-logged in users to this page' - optional :after_sign_out_path, type: String, desc: 'We will redirect users to this page after they sign out' - optional :sign_in_text, type: String, desc: 'The sign in text of the GitLab application' + optional :domain_whitelist, type: String, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' + optional :email_author_in_body, type: Boolean, desc: 'Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead.' + optional :enabled_git_access_protocol, type: String, values: %w[ssh http nil], desc: 'Allow only the selected protocols to be used for Git access.' + optional :gitaly_timeout_default, type: Integer, desc: 'Default Gitaly timeout, in seconds. Set to 0 to disable timeouts.' + optional :gitaly_timeout_fast, type: Integer, desc: 'Gitaly fast operation timeout, in seconds. Set to 0 to disable timeouts.' + optional :gitaly_timeout_medium, type: Integer, desc: 'Medium Gitaly timeout, in seconds. Set to 0 to disable timeouts.' + optional :gravatar_enabled, type: Boolean, desc: 'Flag indicating if the Gravatar service is enabled' optional :help_page_hide_commercial_content, type: Boolean, desc: 'Hide marketing-related entries from help' - optional :help_page_text, type: String, desc: 'Custom text displayed on the help page' optional :help_page_support_url, type: String, desc: 'Alternate support URL for help page' - optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects' - given shared_runners_enabled: ->(val) { val } do - requires :shared_runners_text, type: String, desc: 'Shared runners text ' + optional :help_page_text, type: String, desc: 'Custom text displayed on the help page' + optional :home_page_url, type: String, desc: 'We will redirect non-logged in users to this page' + optional :housekeeping_enabled, type: Boolean, desc: 'Enable automatic repository housekeeping (git repack, git gc)' + given housekeeping_enabled: ->(val) { val } do + requires :housekeeping_bitmaps_enabled, type: Boolean, desc: "Creating pack file bitmaps makes housekeeping take a little longer but bitmaps should accelerate 'git clone' performance." + requires :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run." + requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run." + requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." + end + optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.' + optional :import_sources, type: Array[String], values: %w[github bitbucket gitlab google_code fogbugz git gitlab_project], + desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com' + optional :koding_enabled, type: Boolean, desc: 'Enable Koding' + given koding_enabled: ->(val) { val } do + requires :koding_url, type: String, desc: 'The Koding team URL' end optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size for each job's artifacts" - optional :default_artifacts_expire_in, type: String, desc: "Set the default expiration time for each job's artifacts" + optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB' optional :max_pages_size, type: Integer, desc: 'Maximum size of pages in MB' - optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)' - optional :prometheus_metrics_enabled, type: Boolean, desc: 'Enable Prometheus metrics' optional :metrics_enabled, type: Boolean, desc: 'Enable the InfluxDB metrics' given metrics_enabled: ->(val) { val } do requires :metrics_host, type: String, desc: 'The InfluxDB host' + requires :metrics_method_call_threshold, type: Integer, desc: 'A method call is only tracked when it takes longer to complete than the given amount of milliseconds.' + requires :metrics_packet_size, type: Integer, desc: 'The amount of points to store in a single UDP packet' requires :metrics_port, type: Integer, desc: 'The UDP port to use for connecting to InfluxDB' requires :metrics_pool_size, type: Integer, desc: 'The amount of InfluxDB connections to open' - requires :metrics_timeout, type: Integer, desc: 'The amount of seconds after which an InfluxDB connection will time out' - requires :metrics_method_call_threshold, type: Integer, desc: 'A method call is only tracked when it takes longer to complete than the given amount of milliseconds.' requires :metrics_sample_interval, type: Integer, desc: 'The sampling interval in seconds' - requires :metrics_packet_size, type: Integer, desc: 'The amount of points to store in a single UDP packet' + requires :metrics_timeout, type: Integer, desc: 'The amount of seconds after which an InfluxDB connection will time out' end - optional :sidekiq_throttling_enabled, type: Boolean, desc: 'Enable Sidekiq Job Throttling' - given sidekiq_throttling_enabled: ->(val) { val } do - requires :sidekiq_throttling_queus, type: Array[String], desc: 'Choose which queues you wish to throttle' - requires :sidekiq_throttling_factor, type: Float, desc: 'The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.' + optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 + optional :password_authentication_enabled_for_web, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' + mutually_exclusive :password_authentication_enabled_for_web, :password_authentication_enabled, :signin_enabled + optional :password_authentication_enabled_for_git, type: Boolean, desc: 'Flag indicating if password authentication is enabled for Git over HTTP(S)' + optional :performance_bar_allowed_group_id, type: String, desc: 'Deprecated: Use :performance_bar_allowed_group_path instead. Path of the group that is allowed to toggle the performance bar.' # support legacy names, can be removed in v6 + optional :performance_bar_allowed_group_path, type: String, desc: 'Path of the group that is allowed to toggle the performance bar.' + optional :performance_bar_enabled, type: String, desc: 'Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance.' # support legacy names, can be removed in v6 + optional :plantuml_enabled, type: Boolean, desc: 'Enable PlantUML' + given plantuml_enabled: ->(val) { val } do + requires :plantuml_url, type: String, desc: 'The PlantUML server URL' end + optional :polling_interval_multiplier, type: BigDecimal, desc: 'Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.' + optional :project_export_enabled, type: Boolean, desc: 'Enable project export' + optional :prometheus_metrics_enabled, type: Boolean, desc: 'Enable Prometheus metrics' optional :recaptcha_enabled, type: Boolean, desc: 'Helps prevent bots from creating accounts' given recaptcha_enabled: ->(val) { val } do requires :recaptcha_site_key, type: String, desc: 'Generate site key at http://www.google.com/recaptcha' requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha' end - optional :akismet_enabled, type: Boolean, desc: 'Helps prevent bots from creating issues' - given akismet_enabled: ->(val) { val } do - requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com' + optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues." + optional :repository_storages, type: Array[String], desc: 'Storage paths for new projects' + optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' + given require_two_factor_authentication: ->(val) { val } do + requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' end - optional :admin_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.' + optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.' + optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up' optional :sentry_enabled, type: Boolean, desc: 'Sentry is an error reporting and logging tool which is currently not shipped with GitLab, get it here: https://getsentry.com' given sentry_enabled: ->(val) { val } do requires :sentry_dsn, type: String, desc: 'Sentry Data Source Name' end - optional :clientside_sentry_enabled, type: Boolean, desc: 'Sentry can also be used for reporting and logging clientside exceptions. https://sentry.io/for/javascript/' - given clientside_sentry_enabled: ->(val) { val } do - requires :clientside_sentry_dsn, type: String, desc: 'Clientside Sentry Data Source Name' - end - optional :repository_storages, type: Array[String], desc: 'Storage paths for new projects' - optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues." - optional :koding_enabled, type: Boolean, desc: 'Enable Koding' - given koding_enabled: ->(val) { val } do - requires :koding_url, type: String, desc: 'The Koding team URL' - end - optional :plantuml_enabled, type: Boolean, desc: 'Enable PlantUML' - given plantuml_enabled: ->(val) { val } do - requires :plantuml_url, type: String, desc: 'The PlantUML server URL' + optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.' + optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects' + given shared_runners_enabled: ->(val) { val } do + requires :shared_runners_text, type: String, desc: 'Shared runners text ' end - optional :version_check_enabled, type: Boolean, desc: 'Let GitLab inform you when an update is available.' - optional :email_author_in_body, type: Boolean, desc: 'Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead.' - optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.' - optional :housekeeping_enabled, type: Boolean, desc: 'Enable automatic repository housekeeping (git repack, git gc)' - given housekeeping_enabled: ->(val) { val } do - requires :housekeeping_bitmaps_enabled, type: Boolean, desc: "Creating pack file bitmaps makes housekeeping take a little longer but bitmaps should accelerate 'git clone' performance." - requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." - requires :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run." - requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run." + optional :sidekiq_throttling_enabled, type: Boolean, desc: 'Enable Sidekiq Job Throttling' + given sidekiq_throttling_enabled: ->(val) { val } do + requires :sidekiq_throttling_factor, type: Float, desc: 'The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.' + requires :sidekiq_throttling_queues, type: Array[String], desc: 'Choose which queues you wish to throttle' end + optional :sign_in_text, type: String, desc: 'The sign in text of the GitLab application' + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 + optional :signup_enabled, type: Boolean, desc: 'Flag indicating if sign up is enabled' optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.' - optional :polling_interval_multiplier, type: BigDecimal, desc: 'Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.' - optional :gitaly_timeout_default, type: Integer, desc: 'Default Gitaly timeout, in seconds. Set to 0 to disable timeouts.' - optional :gitaly_timeout_medium, type: Integer, desc: 'Medium Gitaly timeout, in seconds. Set to 0 to disable timeouts.' - optional :gitaly_timeout_fast, type: Integer, desc: 'Gitaly fast operation timeout, in seconds. Set to 0 to disable timeouts.' optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.' optional :instance_statistics_visibility_private, type: Boolean, desc: 'When set to `true` Instance statistics will only be available to admins' diff --git a/lib/gitlab/auth/activity.rb b/lib/gitlab/auth/activity.rb new file mode 100644 index 00000000000..9f84c578d4f --- /dev/null +++ b/lib/gitlab/auth/activity.rb @@ -0,0 +1,77 @@ +module Gitlab + module Auth + ## + # Metrics and logging for user authentication activity. + # + class Activity + extend Gitlab::Utils::StrongMemoize + + COUNTERS = { + user_authenticated: 'Counter of successful authentication events', + user_unauthenticated: 'Counter of authentication failures', + user_not_found: 'Counter of failed log-ins when user is unknown', + user_password_invalid: 'Counter of failed log-ins with invalid password', + user_session_override: 'Counter of manual log-ins and sessions overrides', + user_session_destroyed: 'Counter of user sessions being destroyed', + user_two_factor_authenticated: 'Counter of two factor authentications', + user_sessionless_authentication: 'Counter of sessionless authentications', + user_blocked: 'Counter of sign in attempts when user is blocked' + }.freeze + + def initialize(user, opts) + @user = user + @opts = opts + end + + def user_authentication_failed! + self.class.user_unauthenticated_counter_increment! + + case @opts[:message] + when :not_found_in_database + self.class.user_not_found_counter_increment! + when :invalid + self.class.user_password_invalid_counter_increment! + end + + self.class.user_blocked_counter_increment! if @user&.blocked? + end + + def user_authenticated! + self.class.user_authenticated_counter_increment! + end + + def user_session_override! + self.class.user_session_override_counter_increment! + + case @opts[:message] + when :two_factor_authenticated + self.class.user_two_factor_authenticated_counter_increment! + when :sessionless_sign_in + self.class.user_sessionless_authentication_counter_increment! + end + end + + def user_session_destroyed! + self.class.user_session_destroyed_counter_increment! + end + + def self.each_counter + COUNTERS.each_pair do |metric, description| + yield "#{metric}_counter", metric, description + end + end + + each_counter do |counter, metric, description| + define_singleton_method(counter) do + strong_memoize(counter) do + Gitlab::Metrics.counter("gitlab_auth_#{metric}_total".to_sym, description) + end + end + + define_singleton_method("#{counter}_increment!") do + public_send(counter).increment # rubocop:disable GitlabSecurity/PublicSend + end + end + end + end +end diff --git a/lib/gitlab/auth/blocked_user_tracker.rb b/lib/gitlab/auth/blocked_user_tracker.rb index 7609a7b04f6..b6d2adc834b 100644 --- a/lib/gitlab/auth/blocked_user_tracker.rb +++ b/lib/gitlab/auth/blocked_user_tracker.rb @@ -2,37 +2,58 @@ module Gitlab module Auth class BlockedUserTracker + include Gitlab::Utils::StrongMemoize + ACTIVE_RECORD_REQUEST_PARAMS = 'action_dispatch.request.request_parameters' - def self.log_if_user_blocked(env) - message = env.dig('warden.options', :message) + def initialize(env) + @env = env + end - # Devise calls User#active_for_authentication? on the User model and then - # throws an exception to Warden with User#inactive_message: - # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8 - # - # Since Warden doesn't pass the user record to the failure handler, we - # need to do a database lookup with the username. We can limit the - # lookups to happen when the user was blocked by checking the inactive - # message passed along by Warden. - return unless message == User::BLOCKED_MESSAGE + def user_blocked? + user&.blocked? + end - # Check for either LDAP or regular GitLab account logins - login = env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'username') || - env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login') + def user + return unless has_user_blocked_message? - return unless login.present? + strong_memoize(:user) do + # Check for either LDAP or regular GitLab account logins + login = @env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'username') || + @env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login') - user = User.by_login(login) + User.by_login(login) if login.present? + end + rescue TypeError + end - return unless user&.blocked? + def log_blocked_user_activity! + return unless user_blocked? - Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{env['REMOTE_ADDR']}") + Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{@env['REMOTE_ADDR']}") SystemHooksService.new.execute_hooks_for(user, :failed_login) - true rescue TypeError end + + private + + ## + # Devise calls User#active_for_authentication? on the User model and then + # throws an exception to Warden with User#inactive_message: + # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8 + # + # Since Warden doesn't pass the user record to the failure handler, we + # need to do a database lookup with the username. We can limit the + # lookups to happen when the user was blocked by checking the inactive + # message passed along by Warden. + # + def has_user_blocked_message? + strong_memoize(:user_blocked_message) do + message = @env.dig('warden.options', :message) + message == User::BLOCKED_MESSAGE + end + end end end end diff --git a/lib/gitlab/ci/config/entry/artifacts.rb b/lib/gitlab/ci/config/entry/artifacts.rb index 8275aacee9b..e80f9d2e452 100644 --- a/lib/gitlab/ci/config/entry/artifacts.rb +++ b/lib/gitlab/ci/config/entry/artifacts.rb @@ -6,13 +6,16 @@ module Gitlab # Entry that represents a configuration of job artifacts. # class Artifacts < Node + include Configurable include Validatable include Attributable - ALLOWED_KEYS = %i[name untracked paths when expire_in].freeze + ALLOWED_KEYS = %i[name untracked paths reports when expire_in].freeze attributes ALLOWED_KEYS + entry :reports, Entry::Reports, description: 'Report-type artifacts.' + validations do validates :config, type: Hash validates :config, allowed_keys: ALLOWED_KEYS @@ -21,6 +24,7 @@ module Gitlab validates :name, type: String validates :untracked, boolean: true validates :paths, array_of_strings: true + validates :reports, type: Hash validates :when, inclusion: { in: %w[on_success on_failure always], message: 'should be on_success, on_failure ' \ @@ -28,6 +32,13 @@ module Gitlab validates :expire_in, duration: true end end + + helpers :reports + + def value + @config[:reports] = reports_value if @config.key?(:reports) + @config + end end end end diff --git a/lib/gitlab/ci/config/entry/commands.rb b/lib/gitlab/ci/config/entry/commands.rb index 65d19db249c..9f66f11be9b 100644 --- a/lib/gitlab/ci/config/entry/commands.rb +++ b/lib/gitlab/ci/config/entry/commands.rb @@ -9,18 +9,7 @@ module Gitlab include Validatable validations do - include LegacyValidationHelpers - - validate do - unless string_or_array_of_strings?(config) - errors.add(:config, - 'should be a string or an array of strings') - end - end - - def string_or_array_of_strings?(field) - validate_string(field) || validate_array_of_strings(field) - end + validates :config, array_of_strings_or_string: true end def value diff --git a/lib/gitlab/ci/config/entry/reports.rb b/lib/gitlab/ci/config/entry/reports.rb new file mode 100644 index 00000000000..5963f3eb90c --- /dev/null +++ b/lib/gitlab/ci/config/entry/reports.rb @@ -0,0 +1,32 @@ +module Gitlab + module Ci + class Config + module Entry + ## + # Entry that represents a configuration of job artifacts. + # + class Reports < Node + include Validatable + include Attributable + + ALLOWED_KEYS = %i[junit].freeze + + attributes ALLOWED_KEYS + + validations do + validates :config, type: Hash + validates :config, allowed_keys: ALLOWED_KEYS + + with_options allow_nil: true do + validates :junit, array_of_strings_or_string: true + end + end + + def value + @config.transform_values { |v| Array(v) } + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb index 55658900628..b3c889ee92f 100644 --- a/lib/gitlab/ci/config/entry/validators.rb +++ b/lib/gitlab/ci/config/entry/validators.rb @@ -130,6 +130,20 @@ module Gitlab end end + class ArrayOfStringsOrStringValidator < RegexpValidator + def validate_each(record, attribute, value) + unless validate_array_of_strings_or_string(value) + record.errors.add(attribute, 'should be an array of strings or a string') + end + end + + private + + def validate_array_of_strings_or_string(values) + validate_array_of_strings(values) || validate_string(values) + end + end + class TypeValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) type = options[:with] diff --git a/lib/gitlab/ci/trace.rb b/lib/gitlab/ci/trace.rb index ee54b893598..93e219a21f9 100644 --- a/lib/gitlab/ci/trace.rb +++ b/lib/gitlab/ci/trace.rb @@ -164,6 +164,8 @@ module Gitlab def create_build_trace!(job, path) File.open(path) do |stream| + # TODO: Set `file_format: :raw` after we've cleaned up legacy traces migration + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20307 job.create_job_artifacts_trace!( project: job.project, file_type: :trace, diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index 742118b76a8..e731e654f3c 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Gitlab class GitPostReceive include Gitlab::Identifier @@ -14,10 +16,11 @@ module Gitlab end def changes_refs - return enum_for(:changes_refs) unless block_given? + return changes unless block_given? changes.each do |change| - oldrev, newrev, ref = change.strip.split(' ') + change.strip! + oldrev, newrev, ref = change.split(' ') yield oldrev, newrev, ref end @@ -26,13 +29,10 @@ module Gitlab private def deserialize_changes(changes) - changes = utf8_encode_changes(changes) - changes.lines + utf8_encode_changes(changes).each_line end def utf8_encode_changes(changes) - changes = changes.dup - changes.force_encoding('UTF-8') return changes if changes.valid_encoding? diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index a4263369269..8a91e034377 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -71,8 +71,16 @@ module Gitlab if MUTEX.locked? && MUTEX.owned? optimistic_using_tmp_keychain(&block) else - MUTEX.synchronize do - optimistic_using_tmp_keychain(&block) + if Gitlab.rails5? + ActiveSupport::Dependencies.interlock.permit_concurrent_loads do + MUTEX.synchronize do + optimistic_using_tmp_keychain(&block) + end + end + else + MUTEX.synchronize do + optimistic_using_tmp_keychain(&block) + end end end end diff --git a/lib/gitlab/middleware/basic_health_check.rb b/lib/gitlab/middleware/basic_health_check.rb new file mode 100644 index 00000000000..f2a03217098 --- /dev/null +++ b/lib/gitlab/middleware/basic_health_check.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +# This middleware provides a health check that does not hit the database. Its purpose +# is to notify the prober that the application server is handling requests, but a 200 +# response does not signify that the database or other services are ready. +# +# See https://thisdata.com/blog/making-a-rails-health-check-that-doesnt-hit-the-database/ for +# more details. + +module Gitlab + module Middleware + class BasicHealthCheck + # This can't be frozen because Rails::Rack::Logger wraps the body + # rubocop:disable Style/MutableConstant + OK_RESPONSE = [200, { 'Content-Type' => 'text/plain' }, ["GitLab OK"]] + EMPTY_RESPONSE = [404, { 'Content-Type' => 'text/plain' }, [""]] + # rubocop:enable Style/MutableConstant + HEALTH_PATH = '/-/health' + + def initialize(app) + @app = app + end + + def call(env) + return @app.call(env) unless env['PATH_INFO'] == HEALTH_PATH + + request = Rack::Request.new(env) + + return OK_RESPONSE if client_ip_whitelisted?(request) + + EMPTY_RESPONSE + end + + def client_ip_whitelisted?(request) + ip_whitelist.any? { |e| e.include?(request.ip) } + end + + def ip_whitelist + @ip_whitelist ||= Settings.monitoring.ip_whitelist.map(&IPAddr.method(:new)) + end + end + end +end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index a8acafa9cd9..e5b5f3548e4 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -385,14 +385,6 @@ namespace :gitlab do end end - namespace :repo do - desc "GitLab | Check the integrity of the repositories managed by GitLab" - task check: :gitlab_environment do - puts "This task is deprecated. Please use gitlab:git:fsck instead".color(:red) - Rake::Task["gitlab:git:fsck"].execute - end - end - namespace :orphans do desc 'Gitlab | Check for orphaned namespaces and repositories' task check: :gitlab_environment do @@ -422,23 +414,6 @@ namespace :gitlab do end end - namespace :user do - desc "GitLab | Check the integrity of a specific user's repositories" - task :check_repos, [:username] => :gitlab_environment do |t, args| - username = args[:username] || prompt("Check repository integrity for username? ".color(:blue)) - user = User.find_by(username: username) - if user - repo_dirs = user.authorized_projects.map do |p| - p.repository.path_to_repo - end - - repo_dirs.each { |repo_dir| check_repo_integrity(repo_dir) } - else - puts "\nUser '#{username}' not found".color(:red) - end - end - end - # Helper methods ########################## diff --git a/lib/tasks/gitlab/git.rake b/lib/tasks/gitlab/git.rake index cb4f7e5c8a8..8a53b51d4fe 100644 --- a/lib/tasks/gitlab/git.rake +++ b/lib/tasks/gitlab/git.rake @@ -1,87 +1,24 @@ namespace :gitlab do namespace :git do - desc "GitLab | Git | Repack" - task repack: :gitlab_environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} repack -a --quiet), "Repacking repo") - if failures.empty? - puts "Done".color(:green) - else - output_failures(failures) - end - end - - desc "GitLab | Git | Run garbage collection on all repos" - task gc: :gitlab_environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} gc --auto --quiet), "Garbage Collecting") - if failures.empty? - puts "Done".color(:green) - else - output_failures(failures) - end - end - - desc "GitLab | Git | Prune all repos" - task prune: :gitlab_environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} prune), "Git Prune") - if failures.empty? - puts "Done".color(:green) - else - output_failures(failures) - end - end - desc 'GitLab | Git | Check all repos integrity' task fsck: :gitlab_environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} fsck --name-objects --no-progress), "Checking integrity") do |repo| - check_config_lock(repo) - check_ref_locks(repo) - end - - if failures.empty? - puts "Done".color(:green) - else - output_failures(failures) - end - end - - def perform_git_cmd(cmd, message) - puts "Starting #{message} on all repositories" - failures = [] - all_repos do |repo| - if system(*cmd, chdir: repo) - puts "Performed #{message} at #{repo}" - else - failures << repo + Project.find_each(batch_size: 100) do |project| + begin + project.repository.fsck + + rescue => e + failures << "#{project.full_path} on #{project.repository_storage}: #{e}" end - yield(repo) if block_given? + puts "Performed integrity check for #{project.repository.full_path}" end - failures - end - - def output_failures(failures) - puts "The following repositories reported errors:".color(:red) - failures.each { |f| puts "- #{f}" } - end - - def check_config_lock(repo_dir) - config_exists = File.exist?(File.join(repo_dir, 'config.lock')) - config_output = config_exists ? 'yes'.color(:red) : 'no'.color(:green) - - puts "'config.lock' file exists?".color(:yellow) + " ... #{config_output}" - end - - def check_ref_locks(repo_dir) - lock_files = Dir.glob(File.join(repo_dir, 'refs/heads/*.lock')) - - if lock_files.present? - puts "Ref lock files exist:".color(:red) - - lock_files.each { |lock_file| puts " #{lock_file}" } + if failures.empty? + puts "Done".color(:green) else - puts "No ref lock files exist".color(:green) + puts "The following repositories reported errors:".color(:red) + failures.each { |f| puts "- #{f}" } end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5ae2b0002d1..1bc2093433f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -558,6 +558,9 @@ msgstr "" msgid "Are you sure you want to delete this pipeline schedule?" msgstr "" +msgid "Are you sure you want to lose unsaved changes?" +msgstr "" + msgid "Are you sure you want to remove %{group_name}?" msgstr "" diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f1165c73847..bad7a28556c 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -57,6 +57,10 @@ describe ApplicationController do end describe "#authenticate_user_from_personal_access_token!" do + before do + stub_authentication_activity_metrics(debug: false) + end + controller(described_class) do def index render text: 'authenticated' @@ -67,7 +71,13 @@ describe ApplicationController do context "when the 'personal_access_token' param is populated with the personal access token" do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, private_token: personal_access_token.token + expect(response).to have_gitlab_http_status(200) expect(response.body).to eq('authenticated') end @@ -75,15 +85,25 @@ describe ApplicationController do context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + @request.headers["PRIVATE-TOKEN"] = personal_access_token.token get :index + expect(response).to have_gitlab_http_status(200) expect(response.body).to eq('authenticated') end end it "doesn't log the user in otherwise" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, private_token: "token" + expect(response.status).not_to eq(200) expect(response.body).not_to eq('authenticated') end @@ -174,6 +194,10 @@ describe ApplicationController do end describe '#authenticate_sessionless_user!' do + before do + stub_authentication_activity_metrics(debug: false) + end + describe 'authenticating a user from a feed token' do controller(described_class) do def index @@ -184,7 +208,13 @@ describe ApplicationController do context "when the 'feed_token' param is populated with the feed token" do context 'when the request format is atom' do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, feed_token: user.feed_token, format: :atom + expect(response).to have_gitlab_http_status 200 expect(response.body).to eq 'authenticated' end @@ -192,7 +222,13 @@ describe ApplicationController do context 'when the request format is ics' do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, feed_token: user.feed_token, format: :ics + expect(response).to have_gitlab_http_status 200 expect(response.body).to eq 'authenticated' end @@ -200,7 +236,11 @@ describe ApplicationController do context 'when the request format is neither atom nor ics' do it "doesn't log the user in" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, feed_token: user.feed_token + expect(response.status).not_to have_gitlab_http_status 200 expect(response.body).not_to eq 'authenticated' end @@ -209,7 +249,11 @@ describe ApplicationController do context "when the 'feed_token' param is populated with an invalid feed token" do it "doesn't log the user" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, feed_token: 'token', format: :atom + expect(response.status).not_to eq 200 expect(response.body).not_to eq 'authenticated' end diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index ce7762691c9..d98e6ff0df8 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -42,7 +42,7 @@ describe Boards::IssuesController do parsed_response = JSON.parse(response.body) expect(response).to match_response_schema('issues') - expect(parsed_response.length).to eq 2 + expect(parsed_response['issues'].length).to eq 2 expect(development.issues.map(&:relative_position)).not_to include(nil) end @@ -80,7 +80,7 @@ describe Boards::IssuesController do parsed_response = JSON.parse(response.body) expect(response).to match_response_schema('issues') - expect(parsed_response.length).to eq 2 + expect(parsed_response['issues'].length).to eq 2 end end diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index fed6677935e..30623b6cb3d 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -2,50 +2,131 @@ require 'spec_helper' describe Projects::WikisController do let(:project) { create(:project, :public, :repository) } - let(:user) { create(:user) } - let(:wiki) { ProjectWiki.new(project, user) } + let(:user) { project.owner } + let(:project_wiki) { ProjectWiki.new(project, user) } + let(:wiki) { project_wiki.wiki } + let(:wiki_title) { 'page-title-test' } - describe 'GET #show' do - let(:wiki_title) { 'page-title-test' } + before do + create_page(wiki_title, 'hello world') + + sign_in(user) + end + + after do + destroy_page(wiki_title) + end + describe 'GET #show' do render_views - before do - create_page(wiki_title, 'hello world') - end + subject { get :show, namespace_id: project.namespace, project_id: project, id: wiki_title } - it 'limits the retrieved pages for the sidebar' do - sign_in(user) + context 'when page content encoding is invalid' do + it 'limits the retrieved pages for the sidebar' do + expect(controller).to receive(:load_wiki).and_return(project_wiki) - expect(controller).to receive(:load_wiki).and_return(wiki) + # empty? call + expect(project_wiki).to receive(:pages).with(limit: 1).and_call_original + # Sidebar entries + expect(project_wiki).to receive(:pages).with(limit: 15).and_call_original - # empty? call - expect(wiki).to receive(:pages).with(limit: 1).and_call_original - # Sidebar entries - expect(wiki).to receive(:pages).with(limit: 15).and_call_original + subject + + expect(response).to have_http_status(:ok) + expect(response.body).to include(wiki_title) + end + end - get :show, namespace_id: project.namespace, project_id: project, id: wiki_title + context 'when page content encoding is invalid' do + it 'sets flash error' do + allow(controller).to receive(:valid_encoding?).and_return(false) - expect(response).to have_http_status(:ok) - expect(response.body).to include(wiki_title) + subject + + expect(response).to have_http_status(:ok) + expect(flash[:notice]).to eq 'The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.' + end end end describe 'POST #preview_markdown' do it 'renders json in a correct format' do - sign_in(user) - post :preview_markdown, namespace_id: project.namespace, project_id: project, id: 'page/path', text: '*Markdown* text' expect(JSON.parse(response.body).keys).to match_array(%w(body references)) end end + describe 'GET #edit' do + subject { get(:edit, namespace_id: project.namespace, project_id: project, id: wiki_title) } + + context 'when page content encoding is invalid' do + it 'redirects to show' do + allow(controller).to receive(:valid_encoding?).and_return(false) + + subject + + expect(response).to redirect_to(project_wiki_path(project, project_wiki.pages.first)) + end + end + + context 'when page content encoding is valid' do + render_views + + it 'shows the edit page' do + subject + + expect(response).to have_http_status(:ok) + expect(response.body).to include('Edit Page') + end + end + end + + describe 'PATCH #update' do + let(:new_title) { 'New title' } + let(:new_content) { 'New content' } + subject do + patch(:update, + namespace_id: project.namespace, + project_id: project, + id: wiki_title, + wiki: { title: new_title, content: new_content }) + end + + context 'when page content encoding is invalid' do + it 'redirects to show' do + allow(controller).to receive(:valid_encoding?).and_return(false) + + subject + expect(response).to redirect_to(project_wiki_path(project, project_wiki.pages.first)) + end + end + + context 'when page content encoding is valid' do + render_views + + it 'updates the page' do + subject + + wiki_page = project_wiki.pages.first + + expect(wiki_page.title).to eq new_title + expect(wiki_page.content).to eq new_content + end + end + end + def create_page(name, content) - project.wiki.wiki.write_page(name, :markdown, content, commit_details(name)) + wiki.write_page(name, :markdown, content, commit_details(name)) end def commit_details(name) Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "created page #{name}") end + + def destroy_page(title, dir = '') + page = wiki.page(title: title, dir: dir) + project_wiki.delete_page(page, "test commit") + end end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 83cb4750741..8bd1f1ae4e0 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -187,6 +187,13 @@ FactoryBot.define do end end + trait :test_reports do + after(:create) do |build| + create(:ci_job_artifact, :junit, job: build) + build.reload + end + end + trait :expired do artifacts_expire_at 1.minute.ago end diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb index 3d3287d8168..a6a87782fe7 100644 --- a/spec/factories/ci/job_artifacts.rb +++ b/spec/factories/ci/job_artifacts.rb @@ -4,6 +4,7 @@ FactoryBot.define do factory :ci_job_artifact, class: Ci::JobArtifact do job factory: :ci_build file_type :archive + file_format :zip trait :remote_store do file_store JobArtifactUploader::Store::REMOTE @@ -15,6 +16,7 @@ FactoryBot.define do trait :archive do file_type :archive + file_format :zip after(:build) do |artifact, _| artifact.file = fixture_file_upload( @@ -24,6 +26,7 @@ FactoryBot.define do trait :metadata do file_type :metadata + file_format :gzip after(:build) do |artifact, _| artifact.file = fixture_file_upload( @@ -33,6 +36,7 @@ FactoryBot.define do trait :trace do file_type :trace + file_format :raw after(:build) do |artifact, evaluator| artifact.file = fixture_file_upload( @@ -40,6 +44,16 @@ FactoryBot.define do end end + trait :junit do + file_type :junit + file_format :gzip + + after(:build) do |artifact, evaluator| + artifact.file = fixture_file_upload( + Rails.root.join('spec/fixtures/junit.xml.gz'), 'application/x-gzip') + end + end + trait :correct_checksum do after(:build) do |artifact, evaluator| artifact.file_sha256 = Digest::SHA256.file(artifact.file.path).hexdigest diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 0ef7f35f64a..760324adacc 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -137,6 +137,26 @@ describe 'User views a wiki page' do end end + context 'when page has invalid content encoding' do + let(:content) { 'whatever'.force_encoding('ISO-8859-1') } + + before do + allow(Gitlab::EncodingHelper).to receive(:encode!).and_return(content) + + visit(project_wiki_path(project, wiki_page)) + end + + it 'does not show "Edit" button' do + expect(page).not_to have_selector('a.btn', text: 'Edit') + end + + it 'shows error' do + page.within(:css, '.flash-notice') do + expect(page).to have_content('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.') + end + end + end + it 'opens a default wiki page', :js do visit(project_path(project)) diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 21891b9ccda..44758f862a8 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -3,28 +3,40 @@ require 'spec_helper' describe 'Login' do include TermsHelper - it 'Successful user signin invalidates password reset token' do - user = create(:user) + before do + stub_authentication_activity_metrics(debug: true) + end + + describe 'password reset token after successful sign in' do + it 'invalidates password reset token' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + user = create(:user) - expect(user.reset_password_token).to be_nil + expect(user.reset_password_token).to be_nil - visit new_user_password_path - fill_in 'user_email', with: user.email - click_button 'Reset password' + visit new_user_password_path + fill_in 'user_email', with: user.email + click_button 'Reset password' - user.reload - expect(user.reset_password_token).not_to be_nil + user.reload + expect(user.reset_password_token).not_to be_nil - find('a[href="#login-pane"]').click - gitlab_sign_in(user) - expect(current_path).to eq root_path + find('a[href="#login-pane"]').click + gitlab_sign_in(user) + expect(current_path).to eq root_path - user.reload - expect(user.reset_password_token).to be_nil + user.reload + expect(user.reset_password_token).to be_nil + end end describe 'initial login after setup' do it 'allows the initial admin to create a password' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + # This behavior is dependent on there only being one user User.delete_all @@ -56,6 +68,11 @@ describe 'Login' do describe 'with a blocked account' do it 'prevents the user from logging in' do + expect(authentication_metrics) + .to increment(:user_blocked_counter) + .and increment(:user_unauthenticated_counter) + .and increment(:user_session_destroyed_counter).twice + user = create(:user, :blocked) gitlab_sign_in(user) @@ -64,6 +81,11 @@ describe 'Login' do end it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do + expect(authentication_metrics) + .to increment(:user_blocked_counter) + .and increment(:user_unauthenticated_counter) + .and increment(:user_session_destroyed_counter).twice + user = create(:user, :blocked) expect { gitlab_sign_in(user) }.not_to change { user.reload.sign_in_count } @@ -72,13 +94,22 @@ describe 'Login' do describe 'with the ghost user' do it 'disallows login' do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + gitlab_sign_in(User.ghost) expect(page).to have_content('Invalid Login or password.') end it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do - expect { gitlab_sign_in(User.ghost) }.not_to change { User.ghost.reload.sign_in_count } + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + + expect { gitlab_sign_in(User.ghost) } + .not_to change { User.ghost.reload.sign_in_count } end end @@ -93,17 +124,30 @@ describe 'Login' do before do gitlab_sign_in(user, remember: true) + expect(page).to have_content('Two-Factor Authentication') end it 'does not show a "You are already signed in." error message' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(user.current_otp) + expect(page).not_to have_content('You are already signed in.') end context 'using one-time code' do it 'allows login with valid code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(user.current_otp) + expect(current_path).to eq root_path end @@ -114,11 +158,20 @@ describe 'Login' do end it 'blocks login with invalid code' do + # TODO invalid 2FA code does not generate any events + # See gitlab-org/gitlab-ce#49785 + enter_code('foo') + expect(page).to have_content('Invalid two-factor code') end it 'allows login with invalid code, then valid code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code('foo') expect(page).to have_content('Invalid two-factor code') @@ -139,16 +192,33 @@ describe 'Login' do context 'with valid code' do it 'allows login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(codes.sample) + expect(current_path).to eq root_path end it 'invalidates the used code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + expect { enter_code(codes.sample) } .to change { user.reload.otp_backup_codes.size }.by(-1) end it 'invalidates backup codes twice in a row' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter).twice + .and increment(:user_session_override_counter).twice + .and increment(:user_two_factor_authenticated_counter).twice + .and increment(:user_session_destroyed_counter) + random_code = codes.delete(codes.sample) expect { enter_code(random_code) } .to change { user.reload.otp_backup_codes.size }.by(-1) @@ -163,6 +233,9 @@ describe 'Login' do context 'with invalid code' do it 'blocks login' do + # TODO, invalid two factor authentication does not increment + # metrics / counters, see gitlab-org/gitlab-ce#49785 + code = codes.sample expect(user.invalidate_otp_backup_code!(code)).to eq true @@ -176,7 +249,7 @@ describe 'Login' do end end - context 'logging in via OAuth' do + context 'when logging in via OAuth' do let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')} let(:mock_saml_response) do File.read('spec/fixtures/authentication/saml_response.xml') @@ -185,49 +258,80 @@ describe 'Login' do before do stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config_with_upstream_two_factor_authn_contexts]) - gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response) end context 'when authn_context is worth two factors' do let(:mock_saml_response) do File.read('spec/fixtures/authentication/saml_response.xml') - .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', + 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') end it 'signs user in without prompting for second factor' do + # TODO, OAuth authentication does not fire events, + # see gitlab-org/gitlab-ce#49786 + + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + + sign_in_using_saml! + expect(page).not_to have_content('Two-Factor Authentication') expect(current_path).to eq root_path end end - context 'when authn_context is not worth two factors' do + context 'when two factor authentication is required' do it 'shows 2FA prompt after OAuth login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + + sign_in_using_saml! + expect(page).to have_content('Two-Factor Authentication') + enter_code(user.current_otp) + expect(current_path).to eq root_path end end + + def sign_in_using_saml! + gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response) + end end end describe 'without two-factor authentication' do - let(:user) { create(:user) } + context 'with correct username and password' do + let(:user) { create(:user) } - it 'allows basic login' do - gitlab_sign_in(user) - expect(current_path).to eq root_path - end + it 'allows basic login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) - it 'does not show a "You are already signed in." error message' do - gitlab_sign_in(user) - expect(page).not_to have_content('You are already signed in.') + gitlab_sign_in(user) + + expect(current_path).to eq root_path + expect(page).not_to have_content('You are already signed in.') + end end - it 'blocks invalid login' do - user = create(:user, password: 'not-the-default') + context 'with invalid username and password' do + let(:user) { create(:user, password: 'not-the-default') } - gitlab_sign_in(user) - expect(page).to have_content('Invalid Login or password.') + it 'blocks invalid login' do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + + gitlab_sign_in(user) + + expect(page).to have_content('Invalid Login or password.') + end end end @@ -243,18 +347,26 @@ describe 'Login' do context 'with grace period defined' do before do stub_application_setting(two_factor_grace_period: 48) - gitlab_sign_in(user) end context 'within the grace period' do it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content('The global settings require you to enable Two-Factor Authentication for your account. You need to do this before ') end it 'allows skipping two-factor configuration', :js do - expect(current_path).to eq profile_two_factor_auth_path + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path click_link 'Configure it later' expect(current_path).to eq root_path end @@ -264,6 +376,11 @@ describe 'Login' do let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The global settings require you to enable Two-Factor Authentication for your account.' @@ -271,6 +388,11 @@ describe 'Login' do end it 'disallows skipping two-factor configuration', :js do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).not_to have_link('Configure it later') end @@ -280,10 +402,14 @@ describe 'Login' do context 'without grace period defined' do before do stub_application_setting(two_factor_grace_period: 0) - gitlab_sign_in(user) end it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The global settings require you to enable Two-Factor Authentication for your account.' @@ -303,11 +429,15 @@ describe 'Login' do context 'with grace period defined' do before do stub_application_setting(two_factor_grace_period: 48) - gitlab_sign_in(user) end context 'within the grace period' do it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -316,8 +446,12 @@ describe 'Login' do end it 'allows skipping two-factor configuration', :js do - expect(current_path).to eq profile_two_factor_auth_path + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + gitlab_sign_in(user) + + expect(current_path).to eq profile_two_factor_auth_path click_link 'Configure it later' expect(current_path).to eq root_path end @@ -327,6 +461,11 @@ describe 'Login' do let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -335,6 +474,11 @@ describe 'Login' do end it 'disallows skipping two-factor configuration', :js do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).not_to have_link('Configure it later') end @@ -344,10 +488,14 @@ describe 'Login' do context 'without grace period defined' do before do stub_application_setting(two_factor_grace_period: 0) - gitlab_sign_in(user) end it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -431,6 +579,9 @@ describe 'Login' do end it 'asks to accept the terms on first login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -447,6 +598,9 @@ describe 'Login' do end it 'does not ask for terms when the user already accepted them' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + accept_terms(user) visit new_user_session_path @@ -467,6 +621,9 @@ describe 'Login' do context 'when the user did not enable 2FA' do it 'asks to set 2FA before asking to accept the terms' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -495,6 +652,11 @@ describe 'Login' do end it 'asks the user to accept the terms' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -518,6 +680,9 @@ describe 'Login' do end it 'asks the user to accept the terms before setting a new password' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -546,6 +711,10 @@ describe 'Login' do end it 'asks the user to accept the terms before setting an email' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + gitlab_sign_in_via('saml', user, 'my-uid') expect_to_be_on_terms_page diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 3b741d51598..a2ac4d238c7 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -81,6 +81,7 @@ "can_revert_on_current_merge_request": { "type": ["boolean", "null"] }, "can_cherry_pick_on_current_merge_request": { "type": ["boolean", "null"] }, "can_create_note": { "type": "boolean" }, + "can_create_issue": { "type": "boolean" }, "can_update": { "type": "boolean" } }, "additionalProperties": false diff --git a/spec/fixtures/junit.xml.gz b/spec/fixtures/junit.xml.gz Binary files differnew file mode 100644 index 00000000000..88b7de6fa61 --- /dev/null +++ b/spec/fixtures/junit.xml.gz diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index f7af099b3bf..1ee6f4cf680 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -161,6 +161,28 @@ describe('Store', () => { }, 0); }); + it('moves an issue from backlog to a list', (done) => { + const backlog = gl.issueBoards.BoardsStore.addList({ + ...listObj, + list_type: 'backlog', + }); + const listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate); + + expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2); + + setTimeout(() => { + expect(backlog.issues.length).toBe(1); + expect(listTwo.issues.length).toBe(1); + + gl.issueBoards.BoardsStore.moveIssueToList(backlog, listTwo, backlog.findIssue(1)); + + expect(backlog.issues.length).toBe(0); + expect(listTwo.issues.length).toBe(1); + + done(); + }, 0); + }); + it('moves issue to top of another list', (done) => { const listOne = gl.issueBoards.BoardsStore.addList(listObj); const listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate); diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index 241ff07026e..92b2004c4d7 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -11,7 +11,9 @@ const discussionFixture = 'merge_requests/diff_discussion.json'; describe('diff_file_header', () => { let vm; let props; + const diffDiscussionMock = getJSONFixture(discussionFixture)[0]; const Component = Vue.extend(DiffFileHeader); + const store = new Vuex.Store({ modules: { diffs: diffsModule, @@ -20,7 +22,6 @@ describe('diff_file_header', () => { }); beforeEach(() => { - const diffDiscussionMock = getJSONFixture(discussionFixture)[0]; const diffFile = convertObjectPropsToCamelCase(diffDiscussionMock.diff_file, { deep: true }); props = { diffFile, @@ -303,7 +304,7 @@ describe('diff_file_header', () => { const button = vm.$el.querySelector('.btn-clipboard'); expect(button).not.toBe(null); - expect(button.dataset.clipboardText).toBe(props.diffFile.filePath); + expect(button.dataset.clipboardText).toBe('{"text":"files/ruby/popen.rb","gfm":"`files/ruby/popen.rb`"}'); }); describe('file mode', () => { @@ -409,7 +410,7 @@ describe('diff_file_header', () => { }); describe('handles toggle discussions', () => { - it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => { + it('renders a disabled button when diff has no discussions', () => { const propsCopy = Object.assign({}, props); propsCopy.diffFile.submodule = false; propsCopy.diffFile.blob = { @@ -428,11 +429,44 @@ describe('diff_file_header', () => { store, }); - spyOn(vm, 'toggleFileDiscussions'); - - vm.$el.querySelector('.js-btn-vue-toggle-comments').click(); - - expect(vm.toggleFileDiscussions).toHaveBeenCalled(); + expect( + vm.$el.querySelector('.js-btn-vue-toggle-comments').getAttribute('disabled'), + ).toEqual('disabled'); + }); + + describe('with discussions', () => { + it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => { + const propsCopy = Object.assign({}, props); + propsCopy.diffFile.submodule = false; + propsCopy.diffFile.blob = { + id: '848ed9407c6730ff16edb3dd24485a0eea24292a', + path: 'lib/base.js', + name: 'base.js', + mode: '100644', + readableText: true, + icon: 'file-text-o', + }; + propsCopy.addMergeRequestButtons = true; + propsCopy.diffFile.deletedFile = true; + + const discussionGetter = () => [diffDiscussionMock]; + notesModule.getters.discussions = discussionGetter; + vm = mountComponentWithStore(Component, { + props: propsCopy, + store: new Vuex.Store({ + modules: { + diffs: diffsModule, + notes: notesModule, + }, + }), + }); + + spyOn(vm, 'toggleFileDiscussions'); + + vm.$el.querySelector('.js-btn-vue-toggle-comments').click(); + + expect(vm.toggleFileDiscussions).toHaveBeenCalled(); + }); }); }); }); diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index f5628a01a55..987c4dbcb26 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -167,6 +167,24 @@ describe('Diffs Module Getters', () => { }); }); + describe('diffHasDiscussions', () => { + it('returns true when getDiffFileDiscussions returns discussions', () => { + expect( + getters.diffHasDiscussions(localState, { + getDiffFileDiscussions: () => [discussionMock], + })(diffFileMock), + ).toEqual(true); + }); + + it('returns false when getDiffFileDiscussions returns no discussions', () => { + expect( + getters.diffHasDiscussions(localState, { + getDiffFileDiscussions: () => [], + })(diffFileMock), + ).toEqual(false); + }); + }); + describe('getDiffFileDiscussions', () => { it('returns an array with discussions when fileHash matches and the discussion belongs to a diff', () => { discussionMock.diff_file.file_hash = diffFileMock.fileHash; diff --git a/spec/javascripts/ide/components/ide_spec.js b/spec/javascripts/ide/components/ide_spec.js index 708c9fe69af..49b8e934cdd 100644 --- a/spec/javascripts/ide/components/ide_spec.js +++ b/spec/javascripts/ide/components/ide_spec.js @@ -45,6 +45,33 @@ describe('ide component', () => { }); }); + describe('onBeforeUnload', () => { + it('returns undefined when no staged files or changed files', () => { + expect(vm.onBeforeUnload()).toBe(undefined); + }); + + it('returns warning text when their are changed files', () => { + vm.$store.state.changedFiles.push(file()); + + expect(vm.onBeforeUnload()).toBe('Are you sure you want to lose unsaved changes?'); + }); + + it('returns warning text when their are staged files', () => { + vm.$store.state.stagedFiles.push(file()); + + expect(vm.onBeforeUnload()).toBe('Are you sure you want to lose unsaved changes?'); + }); + + it('updates event object', () => { + const event = {}; + vm.$store.state.stagedFiles.push(file()); + + vm.onBeforeUnload(event); + + expect(event.returnValue).toBe('Are you sure you want to lose unsaved changes?'); + }); + }); + describe('file finder', () => { beforeEach(done => { spyOn(vm, 'toggleFileFinder'); diff --git a/spec/javascripts/lib/utils/poll_spec.js b/spec/javascripts/lib/utils/poll_spec.js index 9b8f68f1676..523f4997bc0 100644 --- a/spec/javascripts/lib/utils/poll_spec.js +++ b/spec/javascripts/lib/utils/poll_spec.js @@ -1,4 +1,5 @@ import Poll from '~/lib/utils/poll'; +import { successCodes } from '~/lib/utils/http_status'; const waitForAllCallsToFinish = (service, waitForCount, successCallback) => { const timer = () => { @@ -91,28 +92,32 @@ describe('Poll', () => { }).catch(done.fail); }); - it('starts polling when http status is 200 and interval header is provided', (done) => { - mockServiceCall(service, { status: 200, headers: { 'poll-interval': 1 } }); + describe('for 2xx status code', () => { + successCodes.forEach(httpCode => { + it(`starts polling when http status is ${httpCode} and interval header is provided`, (done) => { + mockServiceCall(service, { status: httpCode, headers: { 'poll-interval': 1 } }); - const Polling = new Poll({ - resource: service, - method: 'fetch', - data: { page: 1 }, - successCallback: callbacks.success, - errorCallback: callbacks.error, - }); + const Polling = new Poll({ + resource: service, + method: 'fetch', + data: { page: 1 }, + successCallback: callbacks.success, + errorCallback: callbacks.error, + }); - Polling.makeRequest(); + Polling.makeRequest(); - waitForAllCallsToFinish(service, 2, () => { - Polling.stop(); + waitForAllCallsToFinish(service, 2, () => { + Polling.stop(); - expect(service.fetch.calls.count()).toEqual(2); - expect(service.fetch).toHaveBeenCalledWith({ page: 1 }); - expect(callbacks.success).toHaveBeenCalled(); - expect(callbacks.error).not.toHaveBeenCalled(); + expect(service.fetch.calls.count()).toEqual(2); + expect(service.fetch).toHaveBeenCalledWith({ page: 1 }); + expect(callbacks.success).toHaveBeenCalled(); + expect(callbacks.error).not.toHaveBeenCalled(); - done(); + done(); + }); + }); }); }); diff --git a/spec/javascripts/vue_shared/components/clipboard_button_spec.js b/spec/javascripts/vue_shared/components/clipboard_button_spec.js index e135690349e..ea525b1e44f 100644 --- a/spec/javascripts/vue_shared/components/clipboard_button_spec.js +++ b/spec/javascripts/vue_shared/components/clipboard_button_spec.js @@ -6,31 +6,47 @@ describe('clipboard button', () => { const Component = Vue.extend(clipboardButton); let vm; - beforeEach(() => { - vm = mountComponent(Component, { - text: 'copy me', - title: 'Copy this value into Clipboard!', - cssClass: 'btn-danger', - }); - }); - afterEach(() => { vm.$destroy(); }); - it('renders a button for clipboard', () => { - expect(vm.$el.tagName).toEqual('BUTTON'); - expect(vm.$el.getAttribute('data-clipboard-text')).toEqual('copy me'); - expect(vm.$el).toHaveSpriteIcon('duplicate'); - }); + describe('without gfm', () => { + beforeEach(() => { + vm = mountComponent(Component, { + text: 'copy me', + title: 'Copy this value into Clipboard!', + cssClass: 'btn-danger', + }); + }); - it('should have a tooltip with default values', () => { - expect(vm.$el.getAttribute('data-original-title')).toEqual('Copy this value into Clipboard!'); - expect(vm.$el.getAttribute('data-placement')).toEqual('top'); - expect(vm.$el.getAttribute('data-container')).toEqual(null); + it('renders a button for clipboard', () => { + expect(vm.$el.tagName).toEqual('BUTTON'); + expect(vm.$el.getAttribute('data-clipboard-text')).toEqual('copy me'); + expect(vm.$el).toHaveSpriteIcon('duplicate'); + }); + + it('should have a tooltip with default values', () => { + expect(vm.$el.getAttribute('data-original-title')).toEqual('Copy this value into Clipboard!'); + expect(vm.$el.getAttribute('data-placement')).toEqual('top'); + expect(vm.$el.getAttribute('data-container')).toEqual(null); + }); + + it('should render provided classname', () => { + expect(vm.$el.classList).toContain('btn-danger'); + }); }); - it('should render provided classname', () => { - expect(vm.$el.classList).toContain('btn-danger'); + describe('with gfm', () => { + it('sets data-clipboard-text with gfm', () => { + vm = mountComponent(Component, { + text: 'copy me', + gfm: '`path/to/file`', + title: 'Copy this value into Clipboard!', + cssClass: 'btn-danger', + }); + expect(vm.$el.getAttribute('data-clipboard-text')).toEqual( + '{"text":"copy me","gfm":"`path/to/file`"}', + ); + }); }); }); diff --git a/spec/lib/gitlab/auth/activity_spec.rb b/spec/lib/gitlab/auth/activity_spec.rb new file mode 100644 index 00000000000..07854cb1eba --- /dev/null +++ b/spec/lib/gitlab/auth/activity_spec.rb @@ -0,0 +1,30 @@ +require 'fast_spec_helper' + +describe Gitlab::Auth::Activity do + describe '.each_counter' do + it 'has all static counters defined' do + described_class.each_counter do |counter| + expect(described_class).to respond_to(counter) + end + end + + it 'has all static incrementers defined' do + described_class.each_counter do |counter| + expect(described_class).to respond_to("#{counter}_increment!") + end + end + + it 'has all counters starting with `user_`' do + described_class.each_counter do |counter| + expect(counter).to start_with('user_') + end + end + + it 'yields counter method, name and description' do + described_class.each_counter do |method, name, description| + expect(method).to eq "#{name}_counter" + expect(description).to start_with('Counter of') + end + end + end +end diff --git a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb index 43b68e69131..13c09b9cb9b 100644 --- a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb +++ b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb @@ -3,24 +3,30 @@ require 'spec_helper' describe Gitlab::Auth::BlockedUserTracker do set(:user) { create(:user) } - describe '.log_if_user_blocked' do + describe '#log_blocked_user_activity!' do it 'does not log if user failed to login due to undefined reason' do expect_any_instance_of(SystemHooksService).not_to receive(:execute_hooks_for) - expect(described_class.log_if_user_blocked({})).to be_nil + tracker = described_class.new({}) + + expect(tracker.user).to be_nil + expect(tracker.user_blocked?).to be_falsey + expect(tracker.log_blocked_user_activity!).to be_nil end it 'gracefully handles malformed environment variables' do - env = { 'warden.options' => 'test' } + tracker = described_class.new({ 'warden.options' => 'test' }) - expect(described_class.log_if_user_blocked(env)).to be_nil + expect(tracker.user).to be_nil + expect(tracker.user_blocked?).to be_falsey + expect(tracker.log_blocked_user_activity!).to be_nil end context 'failed login due to blocked user' do let(:base_env) { { 'warden.options' => { message: User::BLOCKED_MESSAGE } } } let(:env) { base_env.merge(request_env) } - subject { described_class.log_if_user_blocked(env) } + subject { described_class.new(env) } before do expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login) @@ -32,14 +38,17 @@ describe Gitlab::Auth::BlockedUserTracker do it 'logs a blocked user' do user.block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end it 'logs a blocked user by e-mail' do user.block! env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.log_blocked_user_activity!).to be_truthy end end @@ -49,13 +58,17 @@ describe Gitlab::Auth::BlockedUserTracker do it 'logs a blocked user' do user.block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end it 'logs a LDAP blocked user' do user.ldap_block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end end end diff --git a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb index 5c31423fdee..d48aac15f28 100644 --- a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb @@ -18,6 +18,14 @@ describe Gitlab::Ci::Config::Entry::Artifacts do expect(entry).to be_valid end end + + context "when value includes 'reports' keyword" do + let(:config) { { paths: %w[public/], reports: { junit: 'junit.xml' } } } + + it 'returns general artifact and report-type artifacts configuration' do + expect(entry.value).to eq config + end + end end context 'when entry value is not correct' do @@ -39,6 +47,15 @@ describe Gitlab::Ci::Config::Entry::Artifacts do .to include 'artifacts config contains unknown keys: test' end end + + context "when 'reports' keyword is not hash" do + let(:config) { { paths: %w[public/], reports: 'junit.xml' } } + + it 'reports error' do + expect(entry.errors) + .to include 'artifacts reports should be a hash' + end + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/commands_spec.rb b/spec/lib/gitlab/ci/config/entry/commands_spec.rb index afa4a089418..8934aeb83db 100644 --- a/spec/lib/gitlab/ci/config/entry/commands_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/commands_spec.rb @@ -41,8 +41,7 @@ describe Gitlab::Ci::Config::Entry::Commands do describe '#errors' do it 'saves errors' do expect(entry.errors) - .to include 'commands config should be a ' \ - 'string or an array of strings' + .to include 'commands config should be an array of strings or a string' end end end diff --git a/spec/lib/gitlab/ci/config/entry/reports_spec.rb b/spec/lib/gitlab/ci/config/entry/reports_spec.rb new file mode 100644 index 00000000000..b3a3a6bee1d --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/reports_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Entry::Reports do + let(:entry) { described_class.new(config) } + + describe 'validation' do + context 'when entry config value is correct' do + let(:config) { { junit: %w[junit.xml] } } + + describe '#value' do + it 'returns artifacs configuration' do + expect(entry.value).to eq config + end + end + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + + context 'when value is not array' do + let(:config) { { junit: 'junit.xml' } } + + it 'converts to array' do + expect(entry.value).to eq({ junit: ['junit.xml'] } ) + end + end + end + + context 'when entry value is not correct' do + describe '#errors' do + context 'when value of attribute is invalid' do + let(:config) { { junit: 10 } } + + it 'reports error' do + expect(entry.errors) + .to include 'reports junit should be an array of strings or a string' + end + end + + context 'when there is an unknown key present' do + let(:config) { { codeclimate: 'codeclimate.json' } } + + it 'reports error' do + expect(entry.errors) + .to include 'reports config contains unknown keys: codeclimate' + end + end + end + end + end +end diff --git a/spec/lib/gitlab/middleware/basic_health_check_spec.rb b/spec/lib/gitlab/middleware/basic_health_check_spec.rb new file mode 100644 index 00000000000..187d903a5e1 --- /dev/null +++ b/spec/lib/gitlab/middleware/basic_health_check_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Gitlab::Middleware::BasicHealthCheck do + let(:app) { double(:app) } + let(:middleware) { described_class.new(app) } + let(:env) { {} } + + describe '#call' do + context 'outside IP' do + before do + env['REMOTE_ADDR'] = '8.8.8.8' + end + + it 'returns a 404' do + env['PATH_INFO'] = described_class::HEALTH_PATH + + response = middleware.call(env) + + expect(response[0]).to eq(404) + end + + it 'forwards the call for other paths' do + env['PATH_INFO'] = '/' + + expect(app).to receive(:call) + + middleware.call(env) + end + end + + context 'whitelisted IP' do + before do + env['REMOTE_ADDR'] = '127.0.0.1' + end + + it 'returns 200 response when endpoint is hit' do + env['PATH_INFO'] = described_class::HEALTH_PATH + + expect(app).not_to receive(:call) + + response = middleware.call(env) + + expect(response[0]).to eq(200) + expect(response[1]).to eq({ 'Content-Type' => 'text/plain' }) + expect(response[2]).to eq(['GitLab OK']) + end + + it 'forwards the call for other paths' do + env['PATH_INFO'] = '/-/readiness' + + expect(app).to receive(:call) + + middleware.call(env) + end + end + end +end diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb index 7183957aa50..35622366829 100644 --- a/spec/models/ci/build_runner_session_spec.rb +++ b/spec/models/ci/build_runner_session_spec.rb @@ -29,7 +29,7 @@ describe Ci::BuildRunnerSession, model: true do it 'adds Authorization header if authorization is present' do subject.authorization = 'whatever' - expect(terminal_specification[:headers]).to include(Authorization: 'whatever') + expect(terminal_specification[:headers]).to include(Authorization: ['whatever']) end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 67199eb6d26..e4fa04baae6 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -514,6 +514,44 @@ describe Ci::Build do end end + describe '#has_test_reports?' do + subject { build.has_test_reports? } + + context 'when build has a test report' do + let(:build) { create(:ci_build, :test_reports) } + + it { is_expected.to be_truthy } + end + + context 'when build does not have test reports' do + let(:build) { create(:ci_build, :artifacts) } + + it { is_expected.to be_falsy } + end + end + + describe '#erase_test_reports!' do + subject { build.erase_test_reports! } + + context 'when build has a test report' do + let!(:build) { create(:ci_build, :test_reports) } + + it 'removes a test report' do + subject + + expect(build.has_test_reports?).to be_falsy + end + end + + context 'when build does not have test reports' do + let!(:build) { create(:ci_build, :artifacts) } + + it 'does not erase anything' do + expect { subject }.not_to change { Ci::JobArtifact.count } + end + end + end + describe '#has_old_trace?' do subject { build.has_old_trace? } @@ -776,6 +814,10 @@ describe Ci::Build do expect(build.artifacts_metadata.exists?).to be_falsy end + it 'removes test reports' do + expect(build.job_artifacts.test_reports.count).to eq(0) + end + it 'erases build trace in trace file' do expect(build).not_to have_trace end @@ -807,7 +849,7 @@ describe Ci::Build do context 'build is erasable' do context 'new artifacts' do - let!(:build) { create(:ci_build, :trace_artifact, :success, :artifacts) } + let!(:build) { create(:ci_build, :test_reports, :trace_artifact, :success, :artifacts) } describe '#erase' do before do diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 0fd7612c011..4f34c2e81f8 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -15,6 +15,22 @@ describe Ci::JobArtifact do it { is_expected.to delegate_method(:open).to(:file) } it { is_expected.to delegate_method(:exists?).to(:file) } + describe '.test_reports' do + subject { described_class.test_reports } + + context 'when there is a test report' do + let!(:artifact) { create(:ci_job_artifact, :junit) } + + it { is_expected.to eq([artifact]) } + end + + context 'when there are no test reports' do + let!(:artifact) { create(:ci_job_artifact, :archive) } + + it { is_expected.to be_empty } + end + end + describe 'callbacks' do subject { create(:ci_job_artifact, :archive) } @@ -87,6 +103,40 @@ describe Ci::JobArtifact do end end + describe 'validates file format' do + subject { artifact } + + context 'when archive type with zip format' do + let(:artifact) { build(:ci_job_artifact, :archive, file_format: :zip) } + + it { is_expected.to be_valid } + end + + context 'when archive type with gzip format' do + let(:artifact) { build(:ci_job_artifact, :archive, file_format: :gzip) } + + it { is_expected.not_to be_valid } + end + + context 'when archive type without format specification' do + let(:artifact) { build(:ci_job_artifact, :archive, file_format: nil) } + + it { is_expected.not_to be_valid } + end + + context 'when junit type with zip format' do + let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) } + + it { is_expected.not_to be_valid } + end + + context 'when junit type with gzip format' do + let(:artifact) { build(:ci_job_artifact, :junit, file_format: :gzip) } + + it { is_expected.to be_valid } + end + end + describe '#file' do subject { artifact.file } diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 204d6b47832..55b984faecf 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -310,4 +310,24 @@ describe Milestone do expect(milestone.participants).to eq [user] end end + + describe '.sort_by_attribute' do + set(:milestone_1) { create(:milestone, title: 'Foo') } + set(:milestone_2) { create(:milestone, title: 'Bar') } + set(:milestone_3) { create(:milestone, title: 'Zoo') } + + context 'ordering by name ascending' do + it 'sorts by title ascending' do + expect(described_class.sort_by_attribute('name_asc')) + .to eq([milestone_2, milestone_1, milestone_3]) + end + end + + context 'ordering by name descending' do + it 'sorts by title descending' do + expect(described_class.sort_by_attribute('name_desc')) + .to eq([milestone_3, milestone_1, milestone_2]) + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b75ca91b007..ef4167a3912 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3877,6 +3877,16 @@ describe Project do end end + context '#commits_by' do + let(:project) { create(:project, :repository) } + let(:commits) { project.repository.commits('HEAD', limit: 3).commits } + let(:commit_shas) { commits.map(&:id) } + + it 'retrieves several commits from the repository by oid' do + expect(project.commits_by(oids: commit_shas)).to eq commits + end + end + def rugged_config Gitlab::GitalyClient::StorageSettings.allow_disk_access do project.repository.rugged.config diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 38a3590ad12..64c39f09e33 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -128,6 +128,12 @@ describe ProjectStatistics do .by(13) end + it 'increases also storage size by that amount' do + expect { described_class.increment_statistic(project.id, :build_artifacts_size, 20) } + .to change { statistics.reload.storage_size } + .by(20) + end + context 'when the amount is 0' do it 'does not execute a query' do project diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fc46551c3be..982d24e7eab 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -949,6 +949,7 @@ describe User do user = create(:user, email: 'foo@example.com') expect(described_class.find_by_any_email(user.email)).to eq user + expect(described_class.find_by_any_email(user.email, confirmed: true)).to eq user end it 'finds by secondary email' do @@ -956,11 +957,19 @@ describe User do user = email.user expect(described_class.find_by_any_email(email.email)).to eq user + expect(described_class.find_by_any_email(email.email, confirmed: true)).to eq user end it 'returns nil when nothing found' do expect(described_class.find_by_any_email('')).to be_nil end + + it 'returns nil when user is not confirmed' do + user = create(:user, email: 'foo@example.com', confirmed_at: nil) + + expect(described_class.find_by_any_email(user.email, confirmed: false)).to eq(user) + expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil + end end describe '.by_any_email' do @@ -974,6 +983,12 @@ describe User do expect(described_class.by_any_email(user.email)).to eq([user]) end + + it 'returns a relation of users for confirmed users' do + user = create(:user) + + expect(described_class.by_any_email(user.email, confirmed: true)).to eq([user]) + end end describe '.search' do diff --git a/spec/presenters/ci/build_runner_presenter_spec.rb b/spec/presenters/ci/build_runner_presenter_spec.rb new file mode 100644 index 00000000000..e7019b990dd --- /dev/null +++ b/spec/presenters/ci/build_runner_presenter_spec.rb @@ -0,0 +1,86 @@ +require 'spec_helper' + +describe Ci::BuildRunnerPresenter do + let(:presenter) { described_class.new(build) } + let(:archive) { { paths: ['sample.txt'] } } + let(:junit) { { junit: ['junit.xml'] } } + + let(:archive_expectation) do + { + artifact_type: :archive, + artifact_format: :zip, + paths: archive[:paths], + untracked: archive[:untracked] + } + end + + let(:junit_expectation) do + { + name: 'junit.xml', + artifact_type: :junit, + artifact_format: :gzip, + paths: ['junit.xml'], + when: 'always' + } + end + + describe '#artifacts' do + context "when option contains archive-type artifacts" do + let(:build) { create(:ci_build, options: { artifacts: archive } ) } + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(archive_expectation) + end + + context "when untracked is specified" do + let(:archive) { { untracked: true } } + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(archive_expectation) + end + end + + context "when untracked and paths are missing" do + let(:archive) { { when: 'always' } } + + it 'does not present hash' do + expect(presenter.artifacts).to be_empty + end + end + end + + context "when option has 'junit' keyword" do + let(:build) { create(:ci_build, options: { artifacts: { reports: junit } } ) } + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(junit_expectation) + end + end + + context "when option has both archive and reports specification" do + let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: junit } } ) } + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(archive_expectation) + expect(presenter.artifacts.second).to include(junit_expectation) + end + + context "when archive specifies 'expire_in' keyword" do + let(:archive) { { paths: ['sample.txt'], expire_in: '3 mins 4 sec' } } + + it 'inherits expire_in from archive' do + expect(presenter.artifacts.first).to include({ **archive_expectation, expire_in: '3 mins 4 sec' }) + expect(presenter.artifacts.second).to include({ **junit_expectation, expire_in: '3 mins 4 sec' }) + end + end + end + + context "when option has no artifact keywords" do + let(:build) { create(:ci_build, :no_options) } + + it 'does not present hash' do + expect(presenter.artifacts).to be_nil + end + end + end +end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 8412d0383f7..5814d834572 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -655,13 +655,15 @@ describe API::Jobs do end context 'job is erasable' do - let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline) } + let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) } it 'erases job content' do expect(response).to have_gitlab_http_status(201) + expect(job.job_artifacts.count).to eq(0) expect(job.trace.exist?).to be_falsy expect(job.artifacts_file.exists?).to be_falsy expect(job.artifacts_metadata.exists?).to be_falsy + expect(job.has_test_reports?).to be_falsy end it 'updates job' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index c621760b6c4..93e1c3a2294 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -231,6 +231,14 @@ describe API::Members do expect(response).to have_gitlab_http_status(409) end + it 'returns 404 when the user_id is not valid' do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + user_id: 0, access_level: Member::MAINTAINER + + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + it 'returns 400 when user_id is not given' do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), access_level: Member::MAINTAINER diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index d57993ab454..0f41e46cdd5 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -424,7 +424,9 @@ describe API::Runner, :clean_gitlab_redis_shared_state do 'untracked' => false, 'paths' => %w(out/), 'when' => 'always', - 'expire_in' => '7d' }] + 'expire_in' => '7d', + "artifact_type" => "archive", + "artifact_format" => "zip" }] end let(:expected_cache) do @@ -1420,6 +1422,56 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end end + + context 'when artifact_type is archive' do + context 'when artifact_format is zip' do + let(:params) { { artifact_type: :archive, artifact_format: :zip } } + + it 'stores junit test report' do + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(201) + expect(job.reload.job_artifacts_archive).not_to be_nil + end + end + + context 'when artifact_format is gzip' do + let(:params) { { artifact_type: :archive, artifact_format: :gzip } } + + it 'returns an error' do + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(400) + expect(job.reload.job_artifacts_archive).to be_nil + end + end + end + + context 'when artifact_type is junit' do + context 'when artifact_format is gzip' do + let(:file_upload) { fixture_file_upload('spec/fixtures/junit.xml.gz') } + let(:params) { { artifact_type: :junit, artifact_format: :gzip } } + + it 'stores junit test report' do + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(201) + expect(job.reload.job_artifacts_junit).not_to be_nil + end + end + + context 'when artifact_format is raw' do + let(:file_upload) { fixture_file_upload('spec/fixtures/junit.xml.gz') } + let(:params) { { artifact_type: :junit, artifact_format: :raw } } + + it 'returns an error' do + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(400) + expect(job.reload.job_artifacts_junit).to be_nil + end + end + end end context 'when artifacts are being stored outside of tmp path' do diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index ecf5d849d3f..18d52082399 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -24,7 +24,7 @@ describe Ci::RetryBuildService do artifacts_file artifacts_metadata artifacts_size created_at updated_at started_at finished_at queued_at erased_by erased_at auto_canceled_by job_artifacts job_artifacts_archive - job_artifacts_metadata job_artifacts_trace].freeze + job_artifacts_metadata job_artifacts_trace job_artifacts_junit].freeze IGNORE_ACCESSORS = %i[type lock_version target_url base_tags trace_sections @@ -38,7 +38,7 @@ describe Ci::RetryBuildService do let(:another_pipeline) { create(:ci_empty_pipeline, project: project) } let(:build) do - create(:ci_build, :failed, :artifacts, :expired, :erased, + create(:ci_build, :failed, :artifacts, :test_reports, :expired, :erased, :queued, :coverage, :tags, :allowed_to_fail, :on_tag, :triggered, :trace_artifact, :teardown_environment, description: 'my-job', stage: 'test', stage_id: stage.id, diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 3f62e7e61e1..657afb2b2b5 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -761,7 +761,7 @@ describe GitPushService, services: true do end it 'does not queue a CreateGpgSignatureWorker' do - expect(CreateGpgSignatureWorker).not_to receive(:perform_async).with(sample_commit.id, project.id) + expect(CreateGpgSignatureWorker).not_to receive(:perform_async) execute_service(project, user, oldrev, newrev, ref) end @@ -769,7 +769,15 @@ describe GitPushService, services: true do context 'when the signature is not yet cached' do it 'queues a CreateGpgSignatureWorker' do - expect(CreateGpgSignatureWorker).to receive(:perform_async).with(sample_commit.id, project.id) + expect(CreateGpgSignatureWorker).to receive(:perform_async).with([sample_commit.id], project.id) + + execute_service(project, user, oldrev, newrev, ref) + end + + it 'can queue several commits to create the gpg signature' do + allow(Gitlab::Git::Commit).to receive(:shas_with_signatures).and_return([sample_commit.id, another_sample_commit.id]) + + expect(CreateGpgSignatureWorker).to receive(:perform_async).with([sample_commit.id, another_sample_commit.id], project.id) execute_service(project, user, oldrev, newrev, ref) end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index fa98d05c61b..5bcfef46b75 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -55,6 +55,8 @@ describe Issues::UpdateService, :mailer do end it 'updates the issue with the given params' do + expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in) + update_issue(opts) expect(issue).to be_valid @@ -74,6 +76,21 @@ describe Issues::UpdateService, :mailer do .to change { project.open_issues_count }.from(1).to(0) end + it 'enqueues ConfidentialIssueWorker when an issue is made confidential' do + expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(1.hour, issue.id) + + update_issue(confidential: true) + end + + it 'does not enqueue ConfidentialIssueWorker when an issue is made non confidential' do + # set confidentiality to true before the actual update + issue.update!(confidential: true) + + expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in) + + update_issue(confidential: false) + end + it 'updates open issue counter for assignees when issue is reassigned' do update_issue(assignee_ids: [user2.id]) diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index ef47b0a450b..0a5220c7c61 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -20,6 +20,11 @@ describe Members::DestroyService do end shared_examples 'a service destroying a member' do + before do + type = member.is_a?(GroupMember) ? 'Group' : 'Project' + expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(1.hour, member.user_id, member.source_id, type) + end + it 'destroys the member' do expect { described_class.new(current_user).execute(member, opts) }.to change { member.source.members_and_requesters.count }.by(-1) end diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index a4c103e6f30..36b619ba9be 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -79,7 +79,7 @@ describe Projects::UpdatePagesService do context "for a valid job" do before do create(:ci_job_artifact, file: file, job: build) - create(:ci_job_artifact, file_type: :metadata, file: metadata, job: build) + create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) build.reload end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index ecf1ba05618..e6871545a0b 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -15,6 +15,8 @@ describe Projects::UpdateService do context 'when changing visibility level' do context 'when visibility_level is INTERNAL' do it 'updates the project to internal' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in) + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) expect(result).to eq({ status: :success }) @@ -24,12 +26,30 @@ describe Projects::UpdateService do context 'when visibility_level is PUBLIC' do it 'updates the project to public' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in) + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + expect(result).to eq({ status: :success }) expect(project).to be_public end end + context 'when visibility_level is PRIVATE' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'updates the project to private' do + expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(1.hour, project.id) + + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + expect(result).to eq({ status: :success }) + expect(project).to be_private + end + end + context 'when visibility levels are restricted to PUBLIC only' do before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) @@ -38,6 +58,7 @@ describe Projects::UpdateService do context 'when visibility_level is INTERNAL' do it 'updates the project to internal' do result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) + expect(result).to eq({ status: :success }) expect(project).to be_internal end @@ -54,6 +75,7 @@ describe Projects::UpdateService do context 'when updated by an admin' do it 'updates the project to public' do result = update_project(project, admin, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + expect(result).to eq({ status: :success }) expect(project).to be_public end @@ -166,6 +188,20 @@ describe Projects::UpdateService do end end + context 'when changing feature visibility to private' do + it 'updates the visibility correctly' do + expect(TodosDestroyer::PrivateFeaturesWorker) + .to receive(:perform_in).with(1.hour, project.id) + + result = update_project(project, user, project_feature_attributes: + { issues_access_level: ProjectFeature::PRIVATE } + ) + + expect(result).to eq({ status: :success }) + expect(project.project_feature.issues_access_level).to be(ProjectFeature::PRIVATE) + end + end + context 'when updating a project that contains container images' do before do stub_container_registry_config(enabled: true) diff --git a/spec/services/todos/destroy/confidential_issue_service_spec.rb b/spec/services/todos/destroy/confidential_issue_service_spec.rb new file mode 100644 index 00000000000..54d1d7e83f1 --- /dev/null +++ b/spec/services/todos/destroy/confidential_issue_service_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Todos::Destroy::ConfidentialIssueService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:guest) { create(:user) } + let(:project_member) { create(:user) } + let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) } + + let!(:todo_issue_non_member) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, target: issue, project: project) } + let!(:todo_issue_author) { create(:todo, user: author, target: issue, project: project) } + let!(:todo_issue_asignee) { create(:todo, user: assignee, target: issue, project: project) } + let!(:todo_issue_guest) { create(:todo, user: guest, target: issue, project: project) } + let!(:todo_another_non_member) { create(:todo, user: user, project: project) } + + describe '#execute' do + before do + project.add_developer(project_member) + project.add_guest(guest) + end + + subject { described_class.new(issue.id).execute } + + context 'when provided issue is confidential' do + before do + issue.update!(confidential: true) + end + + it 'removes issue todos for a user who is not a project member' do + expect { subject }.to change { Todo.count }.from(6).to(4) + + expect(user.todos).to match_array([todo_another_non_member]) + expect(author.todos).to match_array([todo_issue_author]) + expect(project_member.todos).to match_array([todo_issue_member]) + end + end + + context 'when provided issue is not confidential' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + end +end diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb new file mode 100644 index 00000000000..bad408a314e --- /dev/null +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -0,0 +1,135 @@ +require 'spec_helper' + +describe Todos::Destroy::EntityLeaveService do + let(:group) { create(:group, :private) } + let(:project) { create(:project, group: group) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:mr) { create(:merge_request, source_project: project) } + + let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_user2) { create(:todo, user: user2, target: issue, project: project) } + + describe '#execute' do + context 'when a user leaves a project' do + subject { described_class.new(user.id, project.id, 'Project').execute } + + context 'when project is private' do + it 'removes todos for the provided user' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(user2.todos).to match_array([todo_issue_user2]) + end + end + + context 'when project is not private' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + context 'when a user is not an author of confidential issue' do + before do + issue.update!(confidential: true) + end + + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + + context 'when a user is an author of confidential issue' do + before do + issue.update!(author: user, confidential: true) + end + + it 'removes only confidential issues todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when a user is an assignee of confidential issue' do + before do + issue.update!(confidential: true) + issue.assignees << user + end + + it 'removes only confidential issues todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'feature visibility check' do + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only users issue todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + end + end + end + + context 'when a user leaves a group' do + subject { described_class.new(user.id, group.id, 'Group').execute } + + context 'when group is private' do + it 'removes todos for the user' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(user2.todos).to match_array([todo_issue_user2]) + end + + context 'with nested groups', :nested_groups do + let(:subgroup) { create(:group, :private, parent: group) } + let(:subproject) { create(:project, group: subgroup) } + + let!(:todo_subproject_user) { create(:todo, user: user, project: subproject) } + let!(:todo_subproject_user2) { create(:todo, user: user2, project: subproject) } + + it 'removes todos for the user including subprojects todos' do + expect { subject }.to change { Todo.count }.from(5).to(2) + + expect(user.todos).to be_empty + expect(user2.todos) + .to match_array([todo_issue_user2, todo_subproject_user2]) + end + end + end + + context 'when group is not private' do + before do + issue.update!(confidential: true) + + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + end + + context 'when entity type is not valid' do + it 'raises an exception' do + expect { described_class.new(user.id, group.id, 'GroupWrongly').execute } + .to raise_error(ArgumentError) + end + end + + context 'when entity was not found' do + it 'does not remove any todos' do + expect { described_class.new(user.id, 999999, 'Group').execute } + .not_to change { Todo.count } + end + end + end +end diff --git a/spec/services/todos/destroy/private_features_service_spec.rb b/spec/services/todos/destroy/private_features_service_spec.rb new file mode 100644 index 00000000000..be8b5bb3979 --- /dev/null +++ b/spec/services/todos/destroy/private_features_service_spec.rb @@ -0,0 +1,143 @@ +require 'spec_helper' + +describe Todos::Destroy::PrivateFeaturesService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:another_user) { create(:user) } + let(:project_member) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:mr) { create(:merge_request, source_project: project) } + + let!(:todo_mr_non_member) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_mr_non_member2) { create(:todo, user: another_user, target: mr, project: project) } + let!(:todo_mr_member) { create(:todo, user: project_member, target: mr, project: project) } + let!(:todo_issue_non_member) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_non_member2) { create(:todo, user: another_user, target: issue, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, target: issue, project: project) } + let!(:commit_todo_non_member) { create(:on_commit_todo, user: user, project: project) } + let!(:commit_todo_non_member2) { create(:on_commit_todo, user: another_user, project: project) } + let!(:commit_todo_member) { create(:on_commit_todo, user: project_member, project: project) } + + before do + project.add_developer(project_member) + end + + context 'when user_id is provided' do + subject { described_class.new(project.id, user.id).execute } + + context 'when all feaures have same visibility as the project' do + it 'removes only user issue todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members but the user is a member' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(8) + end + end + + context 'when mrs, builds and repository are visible only to project members' do + before do + # builds and merge requests cannot have higher visibility than repository + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(repository_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user mr and commit todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs are visible only to project members' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user merge request todo' do + expect { subject }.to change { Todo.count }.from(9).to(8) + end + end + + context 'when mrs and issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user merge request and issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + end + + context 'when user_id is not provided' do + subject { described_class.new(project.id).execute } + + context 'when all feaures have same visibility as the project' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs, builds and repository are visible only to project members' do + before do + # builds and merge requests cannot have higher visibility than repository + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(repository_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members mr and commit todos' do + expect { subject }.to change { Todo.count }.from(9).to(5) + end + end + + context 'when mrs are visible only to project members' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members merge request todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs and issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members merge request and issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(5) + end + end + end +end diff --git a/spec/services/todos/destroy/project_private_service_spec.rb b/spec/services/todos/destroy/project_private_service_spec.rb new file mode 100644 index 00000000000..badf3f913a5 --- /dev/null +++ b/spec/services/todos/destroy/project_private_service_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Todos::Destroy::ProjectPrivateService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:project_member) { create(:user) } + + let!(:todo_issue_non_member) { create(:todo, user: user, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, project: project) } + let!(:todo_another_non_member) { create(:todo, user: user, project: project) } + + describe '#execute' do + before do + project.add_developer(project_member) + end + + subject { described_class.new(project.id).execute } + + context 'when a project set to private' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'removes issue todos for a user who is not a member' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(project_member.todos).to match_array([todo_issue_member]) + end + end + + context 'when project is not private' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + end +end diff --git a/spec/support/helpers/stub_metrics.rb b/spec/support/helpers/stub_metrics.rb new file mode 100644 index 00000000000..64983fdf222 --- /dev/null +++ b/spec/support/helpers/stub_metrics.rb @@ -0,0 +1,27 @@ +module StubMetrics + def authentication_metrics + Gitlab::Auth::Activity + end + + def stub_authentication_activity_metrics(debug: false) + authentication_metrics.each_counter do |name, metric, description| + allow(authentication_metrics).to receive(name) + .and_return(double("#{metric} - #{description}")) + end + + debug_authentication_activity_metrics if debug + end + + def debug_authentication_activity_metrics + authentication_metrics.tap do |metrics| + metrics.each_counter do |name, metric| + "#{name}_increment!".tap do |incrementer| + allow(metrics).to receive(incrementer).and_wrap_original do |method| + puts "Authentication activity metric incremented: #{name}" + method.call + end + end + end + end + end +end diff --git a/spec/support/matchers/metric_counter_matcher.rb b/spec/support/matchers/metric_counter_matcher.rb new file mode 100644 index 00000000000..22d5cd17e3f --- /dev/null +++ b/spec/support/matchers/metric_counter_matcher.rb @@ -0,0 +1,11 @@ +RSpec::Matchers.define :increment do |counter| + match do |adapter| + expect(adapter.send(counter)) + .to receive(:increment) + .exactly(@exactly || :once) + end + + chain :twice do + @exactly = :twice + end +end diff --git a/spec/support/rspec.rb b/spec/support/rspec.rb index 54b8df7aa19..9b8bcebcb3a 100644 --- a/spec/support/rspec.rb +++ b/spec/support/rspec.rb @@ -1,4 +1,5 @@ require_relative "helpers/stub_configuration" +require_relative "helpers/stub_metrics" require_relative "helpers/stub_object_storage" require_relative "helpers/stub_env" @@ -7,6 +8,7 @@ RSpec.configure do |config| config.raise_errors_for_deprecations! config.include StubConfiguration + config.include StubMetrics config.include StubObjectStorage config.include StubENV diff --git a/spec/support/shared_examples/services/boards/issues_list_service.rb b/spec/support/shared_examples/services/boards/issues_list_service.rb index 3e744323cea..8b879cef084 100644 --- a/spec/support/shared_examples/services/boards/issues_list_service.rb +++ b/spec/support/shared_examples/services/boards/issues_list_service.rb @@ -7,6 +7,16 @@ shared_examples 'issues list service' do described_class.new(parent, user, params).execute end + context '#metadata' do + it 'returns issues count for list' do + params = { board_id: board.id, id: list1.id } + + metadata = described_class.new(parent, user, params).metadata + + expect(metadata[:size]).to eq(3) + end + end + context 'issues are ordered by priority' do it 'returns opened issues when list_id is missing' do params = { board_id: board.id } diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb index d0263ad9a37..57b006e1a39 100644 --- a/spec/tasks/gitlab/git_rake_spec.rb +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -2,45 +2,19 @@ require 'rake_helper' describe 'gitlab:git rake tasks' do let(:base_path) { 'tmp/tests/default_storage' } - - before(:all) do - @default_storage_hash = Gitlab.config.repositories.storages.default.to_h - end + let!(:project) { create(:project, :repository) } before do Rake.application.rake_require 'tasks/gitlab/git' - storages = { 'default' => Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => base_path)) } - - path = Settings.absolute("#{base_path}/@hashed/1/2/test.git") - FileUtils.mkdir_p(path) - Gitlab::Popen.popen(%W[git -C #{path} init --bare]) - allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) allow_any_instance_of(String).to receive(:color) { |string, _color| string } stub_warn_user_is_not_gitlab end - after do - FileUtils.rm_rf(Settings.absolute(base_path)) - end - describe 'fsck' do it 'outputs the integrity check for a repo' do - expect { run_rake_task('gitlab:git:fsck') }.to output(%r{Performed Checking integrity at .*@hashed/1/2/test.git}).to_stdout - end - - it 'errors out about config.lock issues' do - FileUtils.touch(Settings.absolute("#{base_path}/@hashed/1/2/test.git/config.lock")) - - expect { run_rake_task('gitlab:git:fsck') }.to output(/file exists\? ... yes/).to_stdout - end - - it 'errors out about ref lock issues' do - FileUtils.mkdir_p(Settings.absolute("#{base_path}/@hashed/1/2/test.git/refs/heads")) - FileUtils.touch(Settings.absolute("#{base_path}/@hashed/1/2/test.git/refs/heads/blah.lock")) - - expect { run_rake_task('gitlab:git:fsck') }.to output(/Ref lock files exist:/).to_stdout + expect { run_rake_task('gitlab:git:fsck') }.to output(/Performed integrity check for/).to_stdout end end end diff --git a/spec/workers/create_gpg_signature_worker_spec.rb b/spec/workers/create_gpg_signature_worker_spec.rb index aa6c347d738..502765e9786 100644 --- a/spec/workers/create_gpg_signature_worker_spec.rb +++ b/spec/workers/create_gpg_signature_worker_spec.rb @@ -2,21 +2,37 @@ require 'spec_helper' describe CreateGpgSignatureWorker do let(:project) { create(:project, :repository) } + let(:commits) { project.repository.commits('HEAD', limit: 3).commits } + let(:commit_shas) { commits.map(&:id) } context 'when GpgKey is found' do - let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } + let(:gpg_commit) { instance_double(Gitlab::Gpg::Commit) } + + before do + allow(Project).to receive(:find_by).with(id: project.id).and_return(project) + allow(project).to receive(:commits_by).with(oids: commit_shas).and_return(commits) + end + + subject { described_class.new.perform(commit_shas, project.id) } it 'calls Gitlab::Gpg::Commit#signature' do - commit = instance_double(Commit) - gpg_commit = instance_double(Gitlab::Gpg::Commit) + commits.each do |commit| + expect(Gitlab::Gpg::Commit).to receive(:new).with(commit).and_return(gpg_commit).once + end - allow(Project).to receive(:find_by).with(id: project.id).and_return(project) - allow(project).to receive(:commit).with(commit_sha).and_return(commit) + expect(gpg_commit).to receive(:signature).exactly(commits.size).times + + subject + end + + it 'can recover from exception and continue the signature process' do + allow(gpg_commit).to receive(:signature) + allow(Gitlab::Gpg::Commit).to receive(:new).and_return(gpg_commit) + allow(Gitlab::Gpg::Commit).to receive(:new).with(commits.first).and_raise(StandardError) - expect(Gitlab::Gpg::Commit).to receive(:new).with(commit).and_return(gpg_commit) - expect(gpg_commit).to receive(:signature) + expect(gpg_commit).to receive(:signature).exactly(2).times - described_class.new.perform(commit_sha, project.id) + subject end end @@ -24,7 +40,7 @@ describe CreateGpgSignatureWorker do let(:nonexisting_commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a34' } it 'does not raise errors' do - expect { described_class.new.perform(nonexisting_commit_sha, project.id) }.not_to raise_error + expect { described_class.new.perform([nonexisting_commit_sha], project.id) }.not_to raise_error end end @@ -32,13 +48,13 @@ describe CreateGpgSignatureWorker do let(:nonexisting_project_id) { -1 } it 'does not raise errors' do - expect { described_class.new.perform(anything, nonexisting_project_id) }.not_to raise_error + expect { described_class.new.perform(commit_shas, nonexisting_project_id) }.not_to raise_error end it 'does not call Gitlab::Gpg::Commit#signature' do expect_any_instance_of(Gitlab::Gpg::Commit).not_to receive(:signature) - described_class.new.perform(anything, nonexisting_project_id) + described_class.new.perform(commit_shas, nonexisting_project_id) end end end diff --git a/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb b/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb new file mode 100644 index 00000000000..9d7c0b8f560 --- /dev/null +++ b/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::ConfidentialIssueWorker do + it "calls the Todos::Destroy::ConfidentialIssueService with the params it was given" do + service = double + + expect(::Todos::Destroy::ConfidentialIssueService).to receive(:new).with(100).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end diff --git a/spec/workers/todos_destroyer/entity_leave_worker_spec.rb b/spec/workers/todos_destroyer/entity_leave_worker_spec.rb new file mode 100644 index 00000000000..955447906aa --- /dev/null +++ b/spec/workers/todos_destroyer/entity_leave_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::EntityLeaveWorker do + it "calls the Todos::Destroy::EntityLeaveService with the params it was given" do + service = double + + expect(::Todos::Destroy::EntityLeaveService).to receive(:new).with(100, 5, 'Group').and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100, 5, 'Group') + end +end diff --git a/spec/workers/todos_destroyer/private_features_worker_spec.rb b/spec/workers/todos_destroyer/private_features_worker_spec.rb new file mode 100644 index 00000000000..9599f5ee071 --- /dev/null +++ b/spec/workers/todos_destroyer/private_features_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::PrivateFeaturesWorker do + it "calls the Todos::Destroy::PrivateFeaturesService with the params it was given" do + service = double + + expect(::Todos::Destroy::PrivateFeaturesService).to receive(:new).with(100, nil).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end diff --git a/spec/workers/todos_destroyer/project_private_worker_spec.rb b/spec/workers/todos_destroyer/project_private_worker_spec.rb new file mode 100644 index 00000000000..15d926fa9d5 --- /dev/null +++ b/spec/workers/todos_destroyer/project_private_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::ProjectPrivateWorker do + it "calls the Todos::Destroy::ProjectPrivateService with the params it was given" do + service = double + + expect(::Todos::Destroy::ProjectPrivateService).to receive(:new).with(100).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end |
