From a5ab3467a705b62911feacc3cf627fdbb00aa198 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 13 Jan 2020 15:07:53 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../pages/groups/group_members/index/index.js | 3 + .../registry/settings/components/settings_form.vue | 84 ++++++++----- .../javascripts/registry/settings/store/getters.js | 8 ++ .../javascripts/registry/settings/store/index.js | 2 + .../registry/settings/store/mutations.js | 5 + .../javascripts/registry/settings/store/state.js | 4 + app/assets/javascripts/registry/settings/utils.js | 6 + app/assets/javascripts/vuex_shared/bindings.js | 26 +++- app/assets/stylesheets/pages/members.scss | 2 +- .../admin/application_settings_controller.rb | 65 ++++++++-- .../requires_whitelisted_monitoring_client.rb | 2 +- app/controllers/groups/group_links_controller.rb | 24 ++++ app/controllers/groups/group_members_controller.rb | 41 ++++--- app/finders/group_members_finder.rb | 44 +++++-- app/graphql/mutations/snippets/create.rb | 2 +- app/helpers/application_settings_helper.rb | 6 + app/helpers/groups/group_members_helper.rb | 4 + app/helpers/selects_helper.rb | 3 +- app/models/application_setting.rb | 6 +- app/models/group.rb | 6 + app/models/group_group_link.rb | 4 + app/models/project_group_link.rb | 2 + .../admin/application_settings/_gitaly.html.haml | 3 + .../group_members/_new_group_member.html.haml | 22 ---- app/views/groups/group_members/index.html.haml | 42 +++++-- .../projects/project_members/_groups.html.haml | 4 +- .../project_members/_new_project_group.html.haml | 21 ---- .../project_members/_new_project_member.html.haml | 23 ---- app/views/projects/project_members/_team.html.haml | 2 +- app/views/projects/project_members/index.html.haml | 8 +- app/views/projects/registry/settings/_index.haml | 5 +- app/views/shared/members/_group.html.haml | 9 +- app/views/shared/members/_invite_group.html.haml | 26 ++++ app/views/shared/members/_invite_member.html.haml | 28 +++++ .../37359-cablett-support-envelope-to.yml | 5 + .../unreleased/bvl-gitaly-dynamic-deadline.yml | 5 + config/gitlab.yml.example | 3 + config/initializers/1_settings.rb | 1 + config/initializers/lograge.rb | 2 +- config/initializers/request_context.rb | 2 +- config/routes/admin.rb | 2 + doc/administration/raketasks/uploads/migrate.md | 12 +- doc/raketasks/README.md | 4 +- doc/update/README.md | 4 +- lib/gitlab/auth/unique_ips_limiter.rb | 2 +- lib/gitlab/email/receiver.rb | 10 +- lib/gitlab/gitaly_client.rb | 23 +++- lib/gitlab/graphql/authorize/authorize_resource.rb | 4 +- lib/gitlab/middleware/request_context.rb | 27 +++++ lib/gitlab/request_context.rb | 41 ++++--- locale/gitlab.pot | 43 ++++--- qa/qa/page/group/sub_menus/members.rb | 9 +- qa/qa/page/project/settings/members.rb | 12 +- .../groups/group_links_controller_spec.rb | 96 +++++++++++++++ .../groups/group_members_controller_spec.rb | 6 + spec/controllers/health_check_controller_spec.rb | 8 +- spec/controllers/metrics_controller_spec.rb | 8 +- spec/features/admin/admin_groups_spec.rb | 10 +- spec/features/admin/admin_projects_spec.rb | 4 +- spec/features/groups/members/manage_groups_spec.rb | 89 ++++++++++++++ .../features/groups/members/manage_members_spec.rb | 8 +- .../features/groups/members/search_members_spec.rb | 2 +- spec/features/projects/members/list_spec.rb | 4 +- ...master_adds_member_with_expiration_date_spec.rb | 4 +- .../settings/user_manages_project_members_spec.rb | 2 +- spec/finders/group_members_finder_spec.rb | 40 ++++++ spec/fixtures/emails/envelope_to_header.eml | 32 +++++ spec/fixtures/emails/forwarded_new_issue.eml | 16 +-- .../__snapshots__/settings_form_spec.js.snap | 134 ++++++++++++--------- .../settings/components/settings_form_spec.js | 33 +++-- spec/frontend/registry/settings/mock_data.js | 12 ++ .../registry/settings/store/mutations_spec.js | 10 +- spec/frontend/vuex_shared/bindings_spec.js | 80 ++++++++---- spec/helpers/application_settings_helper_spec.rb | 14 +++ spec/lib/gitlab/email/receiver_spec.rb | 19 +-- spec/lib/gitlab/gitaly_client_spec.rb | 53 ++++++++ spec/lib/gitlab/middleware/request_context_spec.rb | 78 ++++++++++++ spec/lib/gitlab/request_context_spec.rb | 67 ++++------- spec/models/application_setting_spec.rb | 5 + spec/models/group_group_link_spec.rb | 8 ++ spec/models/group_spec.rb | 51 ++++++++ spec/requests/self_monitoring_project_spec.rb | 108 ++++++++++++++++- .../shared_examples/email_shared_examples.rb | 9 ++ ...aster_manages_access_requests_shared_example.rb | 2 +- .../unique_ip_check_shared_examples.rb | 7 +- 85 files changed, 1366 insertions(+), 406 deletions(-) create mode 100644 app/assets/javascripts/registry/settings/store/getters.js create mode 100644 app/assets/javascripts/registry/settings/utils.js delete mode 100644 app/views/groups/group_members/_new_group_member.html.haml delete mode 100644 app/views/projects/project_members/_new_project_group.html.haml delete mode 100644 app/views/projects/project_members/_new_project_member.html.haml create mode 100644 app/views/shared/members/_invite_group.html.haml create mode 100644 app/views/shared/members/_invite_member.html.haml create mode 100644 changelogs/unreleased/37359-cablett-support-envelope-to.yml create mode 100644 changelogs/unreleased/bvl-gitaly-dynamic-deadline.yml create mode 100644 lib/gitlab/middleware/request_context.rb create mode 100644 spec/features/groups/members/manage_groups_spec.rb create mode 100644 spec/fixtures/emails/envelope_to_header.eml create mode 100644 spec/frontend/registry/settings/mock_data.js create mode 100644 spec/lib/gitlab/middleware/request_context_spec.rb create mode 100644 spec/support/shared_examples/email_shared_examples.rb diff --git a/app/assets/javascripts/pages/groups/group_members/index/index.js b/app/assets/javascripts/pages/groups/group_members/index/index.js index e77a7cf8e0a..0c732922e81 100644 --- a/app/assets/javascripts/pages/groups/group_members/index/index.js +++ b/app/assets/javascripts/pages/groups/group_members/index/index.js @@ -3,9 +3,12 @@ import Members from 'ee_else_ce/members'; import memberExpirationDate from '~/member_expiration_date'; import UsersSelect from '~/users_select'; +import groupsSelect from '~/groups_select'; document.addEventListener('DOMContentLoaded', () => { memberExpirationDate(); + memberExpirationDate('.js-access-expiration-date-groups'); new Members(); + groupsSelect(); new UsersSelect(); }); diff --git a/app/assets/javascripts/registry/settings/components/settings_form.vue b/app/assets/javascripts/registry/settings/components/settings_form.vue index 402763e2e21..55a6a1ace55 100644 --- a/app/assets/javascripts/registry/settings/components/settings_form.vue +++ b/app/assets/javascripts/registry/settings/components/settings_form.vue @@ -1,6 +1,6 @@ diff --git a/app/assets/javascripts/registry/settings/store/getters.js b/app/assets/javascripts/registry/settings/store/getters.js new file mode 100644 index 00000000000..fc32a9f08e4 --- /dev/null +++ b/app/assets/javascripts/registry/settings/store/getters.js @@ -0,0 +1,8 @@ +import { findDefaultOption } from '../utils'; + +export const getCadence = state => + state.settings.cadence || findDefaultOption(state.formOptions.cadence); +export const getKeepN = state => + state.settings.keep_n || findDefaultOption(state.formOptions.keepN); +export const getOlderThan = state => + state.settings.older_than || findDefaultOption(state.formOptions.olderThan); diff --git a/app/assets/javascripts/registry/settings/store/index.js b/app/assets/javascripts/registry/settings/store/index.js index 91a35aac149..c2500454d8e 100644 --- a/app/assets/javascripts/registry/settings/store/index.js +++ b/app/assets/javascripts/registry/settings/store/index.js @@ -2,6 +2,7 @@ import Vue from 'vue'; import Vuex from 'vuex'; import * as actions from './actions'; import mutations from './mutations'; +import * as getters from './getters'; import state from './state'; Vue.use(Vuex); @@ -11,6 +12,7 @@ export const createStore = () => state, actions, mutations, + getters, }); export default createStore(); diff --git a/app/assets/javascripts/registry/settings/store/mutations.js b/app/assets/javascripts/registry/settings/store/mutations.js index b8384fd4a45..25a67cc6973 100644 --- a/app/assets/javascripts/registry/settings/store/mutations.js +++ b/app/assets/javascripts/registry/settings/store/mutations.js @@ -3,6 +3,11 @@ import * as types from './mutation_types'; export default { [types.SET_INITIAL_STATE](state, initialState) { state.projectId = initialState.projectId; + state.formOptions = { + cadence: JSON.parse(initialState.cadenceOptions), + keepN: JSON.parse(initialState.keepNOptions), + olderThan: JSON.parse(initialState.olderThanOptions), + }; }, [types.UPDATE_SETTINGS](state, settings) { state.settings = { ...state.settings, ...settings }; diff --git a/app/assets/javascripts/registry/settings/store/state.js b/app/assets/javascripts/registry/settings/store/state.js index c3a26083c9f..50c882e1839 100644 --- a/app/assets/javascripts/registry/settings/store/state.js +++ b/app/assets/javascripts/registry/settings/store/state.js @@ -23,4 +23,8 @@ export default () => ({ * Same structure as settings, above but Frozen object and used only in case the user clicks 'cancel' */ original: {}, + /* + * Contains the options used to populate the form selects + */ + formOptions: {}, }); diff --git a/app/assets/javascripts/registry/settings/utils.js b/app/assets/javascripts/registry/settings/utils.js new file mode 100644 index 00000000000..75af401e96d --- /dev/null +++ b/app/assets/javascripts/registry/settings/utils.js @@ -0,0 +1,6 @@ +export const findDefaultOption = options => { + const item = options.find(o => o.default); + return item ? item.key : null; +}; + +export default () => {}; diff --git a/app/assets/javascripts/vuex_shared/bindings.js b/app/assets/javascripts/vuex_shared/bindings.js index 51035ebc8a8..817a90f8149 100644 --- a/app/assets/javascripts/vuex_shared/bindings.js +++ b/app/assets/javascripts/vuex_shared/bindings.js @@ -1,9 +1,29 @@ -export const mapComputed = (root, updateFn, list) => { +/** + * Returns computed properties two way bound to vuex + * + * @param {(string[]|Object[])} list - list of string matching state keys or list objects + * @param {string} list[].key - the key matching the key present in the vuex state + * @param {string} list[].getter - the name of the getter, leave it empty to not use a getter + * @param {string} list[].updateFn - the name of the action, leave it empty to use the default action + * @param {string} defaultUpdateFn - the default function to dispatch + * @param {string} root - the key of the state where to search fo they keys described in list + * @returns {Object} a dictionary with all the computed properties generated + */ +export const mapComputed = (list, defaultUpdateFn, root) => { const result = {}; - list.forEach(key => { + list.forEach(item => { + const [getter, key, updateFn] = + typeof item === 'string' + ? [false, item, defaultUpdateFn] + : [item.getter, item.key, item.updateFn || defaultUpdateFn]; result[key] = { get() { - return this.$store.state[root][key]; + if (getter) { + return this.$store.getters[getter]; + } else if (root) { + return this.$store.state[root][key]; + } + return this.$store.state[key]; }, set(value) { this.$store.dispatch(updateFn, { [key]: value }); diff --git a/app/assets/stylesheets/pages/members.scss b/app/assets/stylesheets/pages/members.scss index ae92a2fbd7b..54bca80194f 100644 --- a/app/assets/stylesheets/pages/members.scss +++ b/app/assets/stylesheets/pages/members.scss @@ -3,7 +3,7 @@ border-bottom: 1px solid $border-color; } -.users-project-form { +.invite-users-form { .btn-success { margin-right: 10px; } diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 58f088c6335..3047ee02680 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -5,6 +5,12 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController before_action :set_application_setting before_action :whitelist_query_limiting, only: [:usage_data] + before_action :validate_self_monitoring_feature_flag_enabled, only: [ + :create_self_monitoring_project, + :status_create_self_monitoring_project, + :delete_self_monitoring_project, + :status_delete_self_monitoring_project + ] before_action do push_frontend_feature_flag(:self_monitoring_project) @@ -74,8 +80,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController end def create_self_monitoring_project - return self_monitoring_project_not_implemented unless Feature.enabled?(:self_monitoring_project) - job_id = SelfMonitoringProjectCreateWorker.perform_async render status: :accepted, json: { @@ -85,8 +89,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController end def status_create_self_monitoring_project - return self_monitoring_project_not_implemented unless Feature.enabled?(:self_monitoring_project) - job_id = params[:job_id].to_s unless job_id.length <= PARAM_JOB_ID_MAX_SIZE @@ -97,23 +99,66 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController end if Gitlab::CurrentSettings.instance_administration_project_id.present? - render status: :ok, json: self_monitoring_data + return render status: :ok, json: self_monitoring_data elsif SelfMonitoringProjectCreateWorker.in_progress?(job_id) ::Gitlab::PollingInterval.set_header(response, interval: 3_000) - render status: :accepted, json: { message: _('Job is in progress') } + return render status: :accepted, json: { + message: _('Job to create self-monitoring project is in progress') + } + end + + render status: :bad_request, json: { + message: _('Self-monitoring project does not exist. Please check logs ' \ + 'for any error messages') + } + end + + def delete_self_monitoring_project + job_id = SelfMonitoringProjectDeleteWorker.perform_async + + render status: :accepted, json: { + job_id: job_id, + monitor_status: status_delete_self_monitoring_project_admin_application_settings_path + } + end + + def status_delete_self_monitoring_project + job_id = params[:job_id].to_s + + unless job_id.length <= PARAM_JOB_ID_MAX_SIZE + return render status: :bad_request, json: { + message: _('Parameter "job_id" cannot exceed length of %{job_id_max_size}' % + { job_id_max_size: PARAM_JOB_ID_MAX_SIZE }) + } + end - else - render status: :bad_request, json: { - message: _('Self-monitoring project does not exist. Please check logs ' \ - 'for any error messages') + if Gitlab::CurrentSettings.instance_administration_project_id.nil? + return render status: :ok, json: { + message: _('Self-monitoring project has been successfully deleted') + } + + elsif SelfMonitoringProjectDeleteWorker.in_progress?(job_id) + ::Gitlab::PollingInterval.set_header(response, interval: 3_000) + + return render status: :accepted, json: { + message: _('Job to delete self-monitoring project is in progress') } end + + render status: :bad_request, json: { + message: _('Self-monitoring project was not deleted. Please check logs ' \ + 'for any error messages') + } end private + def validate_self_monitoring_feature_flag_enabled + self_monitoring_project_not_implemented unless Feature.enabled?(:self_monitoring_project) + end + def self_monitoring_data { project_id: Gitlab::CurrentSettings.instance_administration_project_id, diff --git a/app/controllers/concerns/requires_whitelisted_monitoring_client.rb b/app/controllers/concerns/requires_whitelisted_monitoring_client.rb index 2e9905997db..c92b1cecaaa 100644 --- a/app/controllers/concerns/requires_whitelisted_monitoring_client.rb +++ b/app/controllers/concerns/requires_whitelisted_monitoring_client.rb @@ -18,7 +18,7 @@ module RequiresWhitelistedMonitoringClient # debugging purposes return true if Rails.env.development? && request.local? - ip_whitelist.any? { |e| e.include?(Gitlab::RequestContext.client_ip) } + ip_whitelist.any? { |e| e.include?(Gitlab::RequestContext.instance.client_ip) } end def ip_whitelist diff --git a/app/controllers/groups/group_links_controller.rb b/app/controllers/groups/group_links_controller.rb index 7965311c5f1..6796a862c00 100644 --- a/app/controllers/groups/group_links_controller.rb +++ b/app/controllers/groups/group_links_controller.rb @@ -3,6 +3,7 @@ class Groups::GroupLinksController < Groups::ApplicationController before_action :check_feature_flag! before_action :authorize_admin_group! + before_action :group_link, only: [:update, :destroy] def create shared_with_group = Group.find(params[:shared_with_group_id]) if params[:shared_with_group_id].present? @@ -22,12 +23,35 @@ class Groups::GroupLinksController < Groups::ApplicationController redirect_to group_group_members_path(group) end + def update + @group_link.update(group_link_params) + end + + def destroy + Groups::GroupLinks::DestroyService.new(nil, nil).execute(@group_link) + + respond_to do |format| + format.html do + redirect_to group_group_members_path(group), status: :found + end + format.js { head :ok } + end + end + private + def group_link + @group_link ||= group.shared_with_group_links.find(params[:id]) + end + def group_link_create_params params.permit(:shared_group_access, :expires_at) end + def group_link_params + params.require(:group_link).permit(:group_access, :expires_at) + end + def check_feature_flag! render_404 unless Feature.enabled?(:share_group_with_group) end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index dcdf9aced1a..d1eed85fde6 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -20,28 +20,17 @@ class Groups::GroupMembersController < Groups::ApplicationController :override def index - can_manage_members = can?(current_user, :admin_group_member, @group) - @sort = params[:sort].presence || sort_value_name @project = @group.projects.find(params[:project_id]) if params[:project_id] @members = find_members if can_manage_members - @invited_members = @members.invite - @invited_members = @invited_members.search_invite_email(params[:search_invited]) if params[:search_invited].present? - @invited_members = present_members(@invited_members.page(params[:invited_members_page]).per(MEMBER_PER_PAGE_LIMIT)) + @skip_groups = @group.related_group_ids + @invited_members = present_invited_members(@members) end @members = @members.non_invite - @members = @members.search(params[:search]) if params[:search].present? - @members = @members.sort_by_attribute(@sort) - - if can_manage_members && params[:two_factor].present? - @members = @members.filter_by_2fa(params[:two_factor]) - end - - @members = @members.page(params[:page]).per(MEMBER_PER_PAGE_LIMIT) - @members = present_members(@members) + @members = present_group_members(@members) @requesters = present_members( AccessRequestsFinder.new(@group).execute(current_user)) @@ -54,8 +43,30 @@ class Groups::GroupMembersController < Groups::ApplicationController private + def present_invited_members(members) + invited_members = members.invite + + if params[:search_invited].present? + invited_members = invited_members.search_invite_email(params[:search_invited]) + end + + present_members(invited_members + .page(params[:invited_members_page]) + .per(MEMBER_PER_PAGE_LIMIT)) + end + def find_members - GroupMembersFinder.new(@group).execute(include_relations: requested_relations) + filter_params = params.slice(:two_factor, :search).merge(sort: @sort) + GroupMembersFinder.new(@group, current_user).execute(include_relations: requested_relations, params: filter_params) + end + + def can_manage_members + can?(current_user, :admin_group_member, @group) + end + + def present_group_members(original_members) + members = original_members.page(params[:page]).per(MEMBER_PER_PAGE_LIMIT) + present_members(members) end end diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index d8739c350e4..05151b4f1f3 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -1,38 +1,64 @@ # frozen_string_literal: true class GroupMembersFinder < UnionFinder - def initialize(group) + # Params can be any of the following: + # two_factor: string. 'enabled' or 'disabled' are returning different set of data, other values are not effective. + # sort: string + # search: string + + def initialize(group, user = nil) @group = group + @user = user end # rubocop: disable CodeReuse/ActiveRecord - def execute(include_relations: [:inherited, :direct]) - group_members = @group.members + def execute(include_relations: [:inherited, :direct], params: {}) + group_members = group.members relations = [] return group_members if include_relations == [:direct] relations << group_members if include_relations.include?(:direct) - if include_relations.include?(:inherited) && @group.parent + if include_relations.include?(:inherited) && group.parent parents_members = GroupMember.non_request - .where(source_id: @group.ancestors.select(:id)) - .where.not(user_id: @group.users.select(:id)) + .where(source_id: group.ancestors.select(:id)) + .where.not(user_id: group.users.select(:id)) relations << parents_members end if include_relations.include?(:descendants) descendant_members = GroupMember.non_request - .where(source_id: @group.descendants.select(:id)) - .where.not(user_id: @group.users.select(:id)) + .where(source_id: group.descendants.select(:id)) + .where.not(user_id: group.users.select(:id)) relations << descendant_members end - find_union(relations, GroupMember) + members = find_union(relations, GroupMember) + filter_members(members, params) end # rubocop: enable CodeReuse/ActiveRecord + + private + + attr_reader :user, :group + + def filter_members(members, params) + members = members.search(params[:search]) if params[:search].present? + members = members.sort_by_attribute(params[:sort]) if params[:sort].present? + + if can_manage_members && params[:two_factor].present? + members = members.filter_by_2fa(params[:two_factor]) + end + + members + end + + def can_manage_members + Ability.allowed?(user, :admin_group_member, group) + end end GroupMembersFinder.prepend_if_ee('EE::GroupMembersFinder') diff --git a/app/graphql/mutations/snippets/create.rb b/app/graphql/mutations/snippets/create.rb index fe1f543ea1a..c439a6cfc2f 100644 --- a/app/graphql/mutations/snippets/create.rb +++ b/app/graphql/mutations/snippets/create.rb @@ -42,7 +42,7 @@ module Mutations if project_path.present? project = find_project!(project_path: project_path) elsif !can_create_personal_snippet? - raise_resource_not_avaiable_error! + raise_resource_not_available_error! end snippet = CreateSnippetService.new(project, diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index ffea7d53b14..7115fd834fd 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -344,6 +344,12 @@ module ApplicationSettingsHelper 'status_create_self_monitoring_project_path' => status_create_self_monitoring_project_admin_application_settings_path, + 'delete_self_monitoring_project_path' => + delete_self_monitoring_project_admin_application_settings_path, + + 'status_delete_self_monitoring_project_path' => + status_delete_self_monitoring_project_admin_application_settings_path, + 'self_monitoring_project_exists' => Gitlab::CurrentSettings.instance_administration_project.present?, diff --git a/app/helpers/groups/group_members_helper.rb b/app/helpers/groups/group_members_helper.rb index a8f6c974bbd..1952325c504 100644 --- a/app/helpers/groups/group_members_helper.rb +++ b/app/helpers/groups/group_members_helper.rb @@ -4,6 +4,10 @@ module Groups::GroupMembersHelper def group_member_select_options { multiple: true, class: 'input-clamp qa-member-select-field ', scope: :all, email_user: true } end + + def render_invite_member_for_group(group, default_access_level) + render 'shared/members/invite_member', submit_url: group_group_members_path(group), access_levels: GroupMember.access_level_roles, default_access_level: default_access_level + end end Groups::GroupMembersHelper.prepend_if_ee('EE::Groups::GroupMembersHelper') diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb index 90c54123597..4d0f9e530fb 100644 --- a/app/helpers/selects_helper.rb +++ b/app/helpers/selects_helper.rb @@ -85,7 +85,8 @@ module SelectsHelper first_user: opts[:first_user] && current_user ? current_user.username : false, current_user: opts[:current_user] || false, author_id: opts[:author_id] || '', - skip_users: opts[:skip_users] ? opts[:skip_users].map(&:id) : nil + skip_users: opts[:skip_users] ? opts[:skip_users].map(&:id) : nil, + qa_selector: opts[:qa_selector] || '' } end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 3858adfc71f..a0a600a340e 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -169,7 +169,11 @@ class ApplicationSetting < ApplicationRecord validates :gitaly_timeout_default, presence: true, - numericality: { only_integer: true, greater_than_or_equal_to: 0 } + numericality: { + only_integer: true, + greater_than_or_equal_to: 0, + less_than_or_equal_to: Settings.gitlab.max_request_duration_seconds + } validates :gitaly_timeout_medium, presence: true, diff --git a/app/models/group.rb b/app/models/group.rb index 8289d4f099c..186253619fe 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -420,6 +420,12 @@ class Group < Namespace GroupMember.where(source_id: self_and_ancestors_ids, user_id: user.id).order(:access_level).last end + def related_group_ids + [id, + *ancestors.pluck(:id), + *shared_with_group_links.pluck(:shared_with_group_id)] + end + def hashed_storage?(_feature) false end diff --git a/app/models/group_group_link.rb b/app/models/group_group_link.rb index 4b279b7af5b..5a0d9b08cb0 100644 --- a/app/models/group_group_link.rb +++ b/app/models/group_group_link.rb @@ -20,4 +20,8 @@ class GroupGroupLink < ApplicationRecord def self.default_access Gitlab::Access::DEVELOPER end + + def human_access + Gitlab::Access.human_access(self.group_access) + end end diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb index 0d3a2d4e398..b70c07a8386 100644 --- a/app/models/project_group_link.rb +++ b/app/models/project_group_link.rb @@ -21,6 +21,8 @@ class ProjectGroupLink < ApplicationRecord after_commit :refresh_group_members_authorized_projects + alias_method :shared_with_group, :group + def self.access_options Gitlab::Access.options end diff --git a/app/views/admin/application_settings/_gitaly.html.haml b/app/views/admin/application_settings/_gitaly.html.haml index 1da02de0461..fac2de8811f 100644 --- a/app/views/admin/application_settings/_gitaly.html.haml +++ b/app/views/admin/application_settings/_gitaly.html.haml @@ -8,6 +8,9 @@ .form-text.text-muted Timeout for Gitaly calls from the GitLab application (in seconds). This timeout is not enforced for git fetch/push operations or Sidekiq jobs. + This timeout should be less than the worker timeout. If a Gitaly call timeout would exceed the + worker timeout, the remaining time from the worker timeout would be used to avoid having to terminate + the worker. .form-group = f.label :gitaly_timeout_fast, 'Fast Timeout Period', class: 'label-bold' = f.number_field :gitaly_timeout_fast, class: 'form-control' diff --git a/app/views/groups/group_members/_new_group_member.html.haml b/app/views/groups/group_members/_new_group_member.html.haml deleted file mode 100644 index 93dd8f48a60..00000000000 --- a/app/views/groups/group_members/_new_group_member.html.haml +++ /dev/null @@ -1,22 +0,0 @@ -= form_for @group_member, url: group_group_members_path(@group), html: { class: 'users-project-form users-group-form' } do |f| - .row - .col-md-4.col-lg-6 - = users_select_tag(:user_ids, group_member_select_options) - .form-text.text-muted.append-bottom-10 - Search for members by name, username, or email, or invite new ones using their email address. - - .col-md-3.col-lg-2 - = select_tag :access_level, options_for_select(GroupMember.access_level_roles, @group_member.access_level), class: "form-control project-access-select" - .form-text.text-muted.append-bottom-10 - = link_to "Read more", help_page_path("user/permissions") - about role permissions - - .col-md-3.col-lg-2 - .clearable-input - = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Expiration date' - %i.clear-icon.js-clear-input - .form-text.text-muted.append-bottom-10 - On this date, the member(s) will automatically lose access to this group and all of its projects. - - .col-md-2 - = f.submit 'Add to group', class: "btn btn-success btn-block", data: { qa_selector: 'add_to_group_button' } diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index 882fcc79421..6eb8a8947cc 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -1,17 +1,28 @@ -- page_title _("Members") +- page_title _("Group members") - can_manage_members = can?(current_user, :admin_group_member, @group) - show_invited_members = can_manage_members && @invited_members.exists? - pending_active = params[:search_invited].present? +- total_count = @members.count + @group.shared_with_group_links.count .project-members-page.prepend-top-default %h4 - = _("Members") + = _("Group members") %hr - if can_manage_members - .project-members-new.append-bottom-default - %p.clearfix - = _("Add new member to %{strong_start}%{group_name}%{strong_end}").html_safe % { group_name: @group.name, strong_start: ''.html_safe, strong_end: ''.html_safe } - = render "new_group_member" + - if Feature.enabled?(:share_group_with_group) + %ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' } + %li.nav-tab{ role: 'presentation' } + %a.nav-link.active{ href: '#invite-member-pane', id: 'invite-member-tab', data: { toggle: 'tab' }, role: 'tab' }= _("Invite member") + %li.nav-tab{ role: 'presentation' } + %a.nav-link{ href: '#invite-group-pane', id: 'invite-group-tab', data: { toggle: 'tab', qa_selector: 'invite_group_tab' }, role: 'tab' }= _("Invite group") + .tab-content.gitlab-tab-content + .tab-pane.active{ id: 'invite-member-pane', role: 'tabpanel' } + = render_invite_member_for_group(@group, @group_member.access_level) + - if Feature.enabled?(:share_group_with_group) + .tab-pane{ id: 'invite-group-pane', role: 'tabpanel' } + = render 'shared/members/invite_group', submit_url: group_group_links_path(@group), access_levels: GroupMember.access_level_roles, default_access_level: @group_member.access_level, group_link_field: 'shared_with_group_id', group_access_field: 'shared_group_access' + - else + = render_invite_member_for_group(@group, @group_member.access_level) = render 'shared/members/requests', membership_source: @group, requesters: @requesters @@ -19,10 +30,10 @@ %ul.nav-links.mobile-separator.nav.nav-tabs.clearfix %li.nav-item - = link_to "#existing_members", class: ["nav-link", ("active" unless pending_active)] , 'data-toggle' => 'tab' do + = link_to "#existing_shares", class: ["nav-link", ("active" unless pending_active)] , 'data-toggle' => 'tab' do %span - = _("Existing") - %span.badge.badge-pill= @members.total_count + = _("Existing shares") + %span.badge.badge-pill= total_count - if show_invited_members %li.nav-item = link_to "#invited_members", class: ["nav-link", ("active" if pending_active)], 'data-toggle' => 'tab' do @@ -31,7 +42,16 @@ %span.badge.badge-pill= @invited_members.total_count .tab-content - #existing_members.tab-pane{ :class => ("active" unless pending_active) } + #existing_shares.tab-pane{ :class => ("active" unless pending_active) } + - if @group.shared_with_group_links.any? + .card.card-without-border + .d-flex.flex-column.flex-md-row.row-content-block.second-block + %span.flex-grow-1.align-self-md-center.col-form-label + = _("Groups with access to %{strong_start}%{group_name}%{strong_end}").html_safe % { group_name: @group.name, strong_start: ''.html_safe, strong_end: ''.html_safe } + %ul.content-list.members-list{ data: { qa_selector: "groups_list" } } + - can_admin_member = can?(current_user, :admin_group_member, @group) + - @group.shared_with_group_links.each do |group_link| + = render 'shared/members/group', group_link: group_link, can_admin_member: can_admin_member, group_link_path: group_group_link_path(@group, group_link) .card.card-without-border .d-flex.flex-column.flex-md-row.row-content-block.second-block %span.flex-grow-1.align-self-md-center.col-form-label @@ -46,7 +66,7 @@ = label_tag '2fa', '2FA', class: 'col-form-label label-bold pr-md-2' = render 'shared/members/filter_2fa_dropdown' = render 'shared/members/sort_dropdown' - %ul.content-list.members-list + %ul.content-list.members-list{ data: { qa_selector: "members_list" } } = render partial: 'shared/members/member', collection: @members, as: :member = paginate @members, theme: 'gitlab' diff --git a/app/views/projects/project_members/_groups.html.haml b/app/views/projects/project_members/_groups.html.haml index 00321014f91..353c36d0fed 100644 --- a/app/views/projects/project_members/_groups.html.haml +++ b/app/views/projects/project_members/_groups.html.haml @@ -3,4 +3,6 @@ = _("Groups with access to %{project_name}").html_safe % { project_name: sanitize(@project.name, tags: []) } %span.badge.badge-pill= group_links.size %ul.content-list.members-list - = render partial: 'shared/members/group', collection: group_links, as: :group_link + - can_admin_member = can?(current_user, :admin_project_member, @project) + - @group_links.each do |group_link| + = render 'shared/members/group', group_link: group_link, can_admin_member: can_admin_member, group_link_path: project_group_link_path(@project, group_link) diff --git a/app/views/projects/project_members/_new_project_group.html.haml b/app/views/projects/project_members/_new_project_group.html.haml deleted file mode 100644 index d413048ca10..00000000000 --- a/app/views/projects/project_members/_new_project_group.html.haml +++ /dev/null @@ -1,21 +0,0 @@ -.row - .col-sm-12 - = form_tag project_group_links_path(@project), class: 'js-requires-input', method: :post do - .form-group - = label_tag :link_group_id, _("Select a group to invite"), class: "label-bold" - = groups_select_tag(:link_group_id, data: { skip_groups: @skip_groups }, class: "input-clamp qa-group-select-field", required: true) - .form-group - = label_tag :link_group_access, _("Max access level"), class: "label-bold" - .select-wrapper - = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control select-control" - = icon('chevron-down') - .form-text.text-muted.append-bottom-10 - - permissions_docs_path = help_page_path('user/permissions') - - link_start = %q{}.html_safe % { url: permissions_docs_path } - = _("%{link_start}Read more%{link_end} about role permissions").html_safe % { link_start: link_start, link_end: ''.html_safe } - .form-group - = label_tag :expires_at, _('Access expiration date'), class: 'label-bold' - .clearable-input - = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date-groups', placeholder: _('Expiration date'), id: 'expires_at_groups' - %i.clear-icon.js-clear-input - = submit_tag _("Invite"), class: "btn btn-success", data: { qa_selector: 'invite_group_button' } diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml deleted file mode 100644 index 149b0d6cddd..00000000000 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ /dev/null @@ -1,23 +0,0 @@ -.row - .col-sm-12 - = form_for @project_member, as: :project_member, url: project_project_members_path(@project), html: { class: 'users-project-form' } do |f| - .form-group - = label_tag :user_ids, _("GitLab member or Email address"), class: "label-bold" - = users_select_tag(:user_ids, multiple: true, class: "input-clamp qa-member-select-input", scope: :all, email_user: true, placeholder: "Search for members to update or invite") - .form-group - = label_tag :access_level, _("Choose a role permission"), class: "label-bold" - .select-wrapper - = select_tag :access_level, options_for_select(ProjectMember.access_level_roles, @project_member.access_level), class: "form-control project-access-select select-control" - = icon('chevron-down') - .form-text.text-muted.append-bottom-10 - - permissions_docs_path = help_page_path('user/permissions') - - link_start = %q{}.html_safe % { url: permissions_docs_path } - = _("%{link_start}Read more%{link_end} about role permissions").html_safe % { link_start: link_start, link_end: ''.html_safe } - .form-group - .clearable-input - = label_tag :expires_at, _('Access expiration date'), class: 'label-bold' - = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Expiration date' - %i.clear-icon.js-clear-input - = f.submit _("Add to project"), class: "btn btn-success qa-add-member-button" - - if can_import_members? - = link_to _("Import"), import_project_project_members_path(@project), class: "btn btn-default", title: _("Import members from another project") diff --git a/app/views/projects/project_members/_team.html.haml b/app/views/projects/project_members/_team.html.haml index 5310c1fad01..5d8005b2e2a 100644 --- a/app/views/projects/project_members/_team.html.haml +++ b/app/views/projects/project_members/_team.html.haml @@ -13,5 +13,5 @@ %button.user-search-btn{ type: "submit", "aria-label" => _("Submit search") } = icon("search") = render 'shared/members/sort_dropdown' - %ul.content-list.members-list.qa-members-list + %ul.content-list.members-list{ data: { qa_selector: 'members_list' } } = render partial: 'shared/members/member', collection: members, as: :member diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index 24fe583a9b5..c24a9061146 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -23,13 +23,13 @@ .tab-content.gitlab-tab-content .tab-pane.active{ id: 'invite-member-pane', role: 'tabpanel' } - = render 'projects/project_members/new_project_member', tab_title: _('Invite member') + = render 'shared/members/invite_member', submit_url: project_project_members_path(@project), access_levels: ProjectMember.access_level_roles, default_access_level: @project_member.access_level, can_import_members?: can_import_members?, import_path: import_project_project_members_path(@project) .tab-pane{ id: 'invite-group-pane', role: 'tabpanel', class: ('active' if membership_locked?) } - = render 'projects/project_members/new_project_group', tab_title: _('Invite group') + = render 'shared/members/invite_group', submit_url: project_group_links_path(@project), access_levels: ProjectGroupLink.access_options, default_access_level: ProjectGroupLink.default_access, group_link_field: 'link_group_id', group_access_field: 'link_group_access' - elsif !membership_locked? - .invite-member= render 'projects/project_members/new_project_member', tab_title: _('Invite member') + .invite-member= render 'shared/members/invite_member', submit_url: project_project_members_path(@project), access_levels: ProjectMember.access_level_roles, default_access_level: @project_member.access_level, can_import_members?: can_import_members?, import_path: import_project_project_members_path(@project) - elsif @project.allowed_to_share_with_group? - .invite-group= render 'projects/project_members/new_project_group', tab_title: _('Invite group') + .invite-group= render 'shared/members/invite_group', access_levels: ProjectGroupLink.access_options, default_access_level: ProjectGroupLink.default_access, submit_url: project_group_links_path(@project), group_link_field: 'link_group_id', group_access_field: 'link_group_access' = render 'shared/members/requests', membership_source: @project, requesters: @requesters .clearfix diff --git a/app/views/projects/registry/settings/_index.haml b/app/views/projects/registry/settings/_index.haml index d023a083445..0e0341a9923 100644 --- a/app/views/projects/registry/settings/_index.haml +++ b/app/views/projects/registry/settings/_index.haml @@ -1 +1,4 @@ -#js-registry-settings{ data: { project_id: @project.id, } } +#js-registry-settings{ data: { project_id: @project.id, + cadence_options: cadence_options.to_json, + keep_n_options: keep_n_options.to_json, + older_than_options: older_than_options.to_json} } diff --git a/app/views/shared/members/_group.html.haml b/app/views/shared/members/_group.html.haml index 18368ecc9ff..4aeeac87f3c 100644 --- a/app/views/shared/members/_group.html.haml +++ b/app/views/shared/members/_group.html.haml @@ -1,6 +1,7 @@ - group_link = local_assigns[:group_link] -- group = group_link.group -- can_admin_member = can?(current_user, :admin_project_member, @project) +- group = group_link.shared_with_group +- can_admin_member = local_assigns[:can_admin_member] +- group_link_path = local_assigns[:group_link_path] - dom_id = "group_member_#{group_link.id}" -# Note this is just for groups. For individual members please see shared/members/_member @@ -17,7 +18,7 @@ %span{ class: ('text-warning' if group_link.expires_soon?) } = _("Expires in %{expires_at}").html_safe % { expires_at: distance_of_time_in_words_to_now(group_link.expires_at) } .controls.member-controls.align-items-center - = form_tag project_group_link_path(@project, group_link), method: :put, remote: true, class: 'js-edit-member-form form-group d-sm-flex' do + = form_tag group_link_path, method: :put, remote: true, class: 'js-edit-member-form form-group d-sm-flex' do = hidden_field_tag "group_link[group_access]", group_link.group_access .member-form-control.dropdown.mr-sm-2.d-sm-inline-block %button.dropdown-menu-toggle.js-member-permissions-dropdown{ type: "button", @@ -39,7 +40,7 @@ = text_field_tag 'group_link[expires_at]', group_link.expires_at, class: 'form-control js-access-expiration-date js-member-update-control', placeholder: _('Expiration date'), id: "member_expires_at_#{group.id}", disabled: !can_admin_member %i.clear-icon.js-clear-input - if can_admin_member - = link_to project_group_link_path(@project, group_link), + = link_to group_link_path, method: :delete, data: { confirm: _("Are you sure you want to remove %{group_name}?") % { group_name: group.name }, qa_selector: 'delete_group_access_link' }, class: 'btn btn-remove m-0 ml-sm-2 align-self-center' do diff --git a/app/views/shared/members/_invite_group.html.haml b/app/views/shared/members/_invite_group.html.haml new file mode 100644 index 00000000000..27c930bcbb5 --- /dev/null +++ b/app/views/shared/members/_invite_group.html.haml @@ -0,0 +1,26 @@ +- access_levels = local_assigns[:access_levels] +- default_access_level = local_assigns[:default_access_level] +- submit_url = local_assigns[:submit_url] +- group_link_field = local_assigns[:group_link_field] +- group_access_field = local_assigns[:group_access_field] +.row + .col-sm-12 + = form_tag submit_url, class: 'invite-group-form js-requires-input', method: :post do + .form-group + = label_tag group_link_field, _("Select a group to invite"), class: "label-bold" + = groups_select_tag(group_link_field, data: { skip_groups: @skip_groups }, class: 'input-clamp qa-group-select-field', required: true) + .form-group + = label_tag group_access_field, _("Max access level"), class: "label-bold" + .select-wrapper + = select_tag group_access_field, options_for_select(access_levels, default_access_level), data: { qa_selector: 'group_access_field' }, class: "form-control select-control" + = icon('chevron-down') + .form-text.text-muted.append-bottom-10 + - permissions_docs_path = help_page_path('user/permissions') + - link_start = %q{}.html_safe % { url: permissions_docs_path } + = _("%{link_start}Read more%{link_end} about role permissions").html_safe % { link_start: link_start, link_end: ''.html_safe } + .form-group + = label_tag :expires_at, _('Access expiration date'), class: 'label-bold' + .clearable-input + = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date-groups', placeholder: _('Expiration date'), id: 'expires_at_groups' + %i.clear-icon.js-clear-input + = submit_tag _("Invite"), class: "btn btn-success", data: { qa_selector: 'invite_group_button' } diff --git a/app/views/shared/members/_invite_member.html.haml b/app/views/shared/members/_invite_member.html.haml new file mode 100644 index 00000000000..d3a1c85e285 --- /dev/null +++ b/app/views/shared/members/_invite_member.html.haml @@ -0,0 +1,28 @@ +- access_levels = local_assigns[:access_levels] +- default_access_level = local_assigns[:default_access_level] +- submit_url = local_assigns[:submit_url] +- can_import_members = local_assigns[:can_import_members?] +- import_path = local_assigns[:import_path] +.row + .col-sm-12 + = form_tag submit_url, class: 'invite-users-form', method: :post do + .form-group + = label_tag :user_ids, _("GitLab member or Email address"), class: "label-bold" + = users_select_tag(:user_ids, multiple: true, class: 'input-clamp qa-member-select-field', scope: :all, email_user: true, placeholder: 'Search for members to update or invite') + .form-group + = label_tag :access_level, _("Choose a role permission"), class: "label-bold" + .select-wrapper + = select_tag :access_level, options_for_select(access_levels, default_access_level), class: "form-control project-access-select select-control" + = icon('chevron-down') + .form-text.text-muted.append-bottom-10 + - permissions_docs_path = help_page_path('user/permissions') + - link_start = %q{}.html_safe % { url: permissions_docs_path } + = _("%{link_start}Read more%{link_end} about role permissions").html_safe % { link_start: link_start, link_end: ''.html_safe } + .form-group + .clearable-input + = label_tag :expires_at, _('Access expiration date'), class: 'label-bold' + = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Expiration date' + %i.clear-icon.js-clear-input + = submit_tag _("Invite"), class: "btn btn-success", data: { qa_selector: 'invite_member_button' } + - if can_import_members + = link_to _("Import"), import_path, class: "btn btn-default", title: _("Import members from another project") diff --git a/changelogs/unreleased/37359-cablett-support-envelope-to.yml b/changelogs/unreleased/37359-cablett-support-envelope-to.yml new file mode 100644 index 00000000000..510ebe1c32d --- /dev/null +++ b/changelogs/unreleased/37359-cablett-support-envelope-to.yml @@ -0,0 +1,5 @@ +--- +title: Accept `Envelope-To` as possible location for Service Desk key +merge_request: 22354 +author: Max Winterstein +type: added diff --git a/changelogs/unreleased/bvl-gitaly-dynamic-deadline.yml b/changelogs/unreleased/bvl-gitaly-dynamic-deadline.yml new file mode 100644 index 00000000000..e44127ccf60 --- /dev/null +++ b/changelogs/unreleased/bvl-gitaly-dynamic-deadline.yml @@ -0,0 +1,5 @@ +--- +title: Don't let Gitaly calls exceed a request time of 55 seconds +merge_request: 21492 +author: +type: performance diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 6613afdfed2..dc66b2adb70 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -33,6 +33,9 @@ production: &base host: localhost port: 80 # Set to 443 if using HTTPS, see installation.md#using-https for additional HTTPS configuration details https: false # Set to true if using HTTPS, see installation.md#using-https for additional HTTPS configuration details + # The maximum time unicorn/puma can spend on the request. This needs to be smaller than the worker timeout. + # Default is 95% of the worker timeout + max_request_duration: 57 # Uncomment this line below if your ssh host is different from HTTP/HTTPS one # (you'd obviously need to replace ssh.host_example.com with your own host). diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index ce45dfe06f8..d7d4bd9d3a1 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -209,6 +209,7 @@ Settings.gitlab['content_security_policy'] ||= Gitlab::ContentSecurityPolicy::Co Settings.gitlab['no_todos_messages'] ||= YAML.load_file(Rails.root.join('config', 'no_todos_messages.yml')) Settings.gitlab['impersonation_enabled'] ||= true if Settings.gitlab['impersonation_enabled'].nil? Settings.gitlab['usage_ping_enabled'] = true if Settings.gitlab['usage_ping_enabled'].nil? +Settings.gitlab['max_request_duration_seconds'] ||= 57 Gitlab.ee do Settings.gitlab['mirror_max_delay'] ||= 300 diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb index 0acbe6a9258..dc064a76033 100644 --- a/config/initializers/lograge.rb +++ b/config/initializers/lograge.rb @@ -37,7 +37,7 @@ unless Gitlab::Runtime.sidekiq? payload[:response] = event.payload[:response] if event.payload[:response] payload[Labkit::Correlation::CorrelationId::LOG_KEY] = Labkit::Correlation::CorrelationId.current_id - if cpu_s = Gitlab::Metrics::System.thread_cpu_duration(::Gitlab::RequestContext.start_thread_cpu_time) + if cpu_s = Gitlab::Metrics::System.thread_cpu_duration(::Gitlab::RequestContext.instance.start_thread_cpu_time) payload[:cpu_s] = cpu_s end diff --git a/config/initializers/request_context.rb b/config/initializers/request_context.rb index 0b485fc1adc..f79f1f32d70 100644 --- a/config/initializers/request_context.rb +++ b/config/initializers/request_context.rb @@ -1,3 +1,3 @@ Rails.application.configure do |config| - config.middleware.insert_after RequestStore::Middleware, Gitlab::RequestContext + config.middleware.insert_after RequestStore::Middleware, Gitlab::Middleware::RequestContext end diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c58b74ecfec..45cca1d3191 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -119,6 +119,8 @@ namespace :admin do post :create_self_monitoring_project get :status_create_self_monitoring_project + delete :delete_self_monitoring_project + get :status_delete_self_monitoring_project end resources :labels diff --git a/doc/administration/raketasks/uploads/migrate.md b/doc/administration/raketasks/uploads/migrate.md index 26c811ca54d..aef15e3f388 100644 --- a/doc/administration/raketasks/uploads/migrate.md +++ b/doc/administration/raketasks/uploads/migrate.md @@ -1,4 +1,4 @@ -# Uploads Migrate Rake Task +# Uploads Migrate Rake Tasks ## Migrate to Object Storage @@ -110,7 +110,15 @@ sudo -u git -H bundle exec rake "gitlab:uploads:migrate[FileUploader, MergeReque To migrate all uploads created by legacy uploaders, run: -```shell +**Omnibus Installation** + +```bash +gitlab-rake gitlab:uploads:legacy:migrate +``` + +**Source Installation** + +```bash bundle exec rake gitlab:uploads:legacy:migrate ``` diff --git a/doc/raketasks/README.md b/doc/raketasks/README.md index ad86555fc17..a9ba44f82c6 100644 --- a/doc/raketasks/README.md +++ b/doc/raketasks/README.md @@ -14,5 +14,5 @@ comments: false - [Webhooks](web_hooks.md) - [Import](import.md) of Git repositories in bulk - [Rebuild authorized_keys file](../administration/raketasks/maintenance.md#rebuild-authorized_keys-file) task for administrators -- [Migrate Uploads](../administration/raketasks/uploads/migrate.md) -- [Sanitize Uploads](../administration/raketasks/uploads/sanitize.md) +- [Uploads Migrate](../administration/raketasks/uploads/migrate.md) +- [Uploads Sanitize](../administration/raketasks/uploads/sanitize.md) diff --git a/doc/update/README.md b/doc/update/README.md index e9db0a73ac5..f23716f3df8 100644 --- a/doc/update/README.md +++ b/doc/update/README.md @@ -134,8 +134,8 @@ Please follow the [Upgrade Recommendations](../policy/maintenance.md#upgrade-rec to identify the ideal upgrade path. Before upgrading to a new major version, you should ensure that any background -migration jobs from previous releases have been completed. The number of remaining -migrations jobs can be found by running the following command: +migration jobs from previous releases have been completed. To see the current size +of the `background_migration` queue, [check for background migrations before upgrading](#checking-for-background-migrations-before-upgrading). ## Upgrading between editions diff --git a/lib/gitlab/auth/unique_ips_limiter.rb b/lib/gitlab/auth/unique_ips_limiter.rb index 97e78ecf094..74f7fdfc180 100644 --- a/lib/gitlab/auth/unique_ips_limiter.rb +++ b/lib/gitlab/auth/unique_ips_limiter.rb @@ -8,7 +8,7 @@ module Gitlab class << self def limit_user_id!(user_id) if config.unique_ips_limit_enabled - ip = RequestContext.client_ip + ip = RequestContext.instance.client_ip unique_ips = update_and_return_ips_count(user_id, ip) raise TooManyIps.new(user_id, ip, unique_ips) if unique_ips > config.unique_ips_limit_per_user diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 847260b2e0f..f028102da9b 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -66,7 +66,8 @@ module Gitlab def key_from_additional_headers(mail) find_key_from_references(mail) || - find_key_from_delivered_to_header(mail) + find_key_from_delivered_to_header(mail) || + find_key_from_envelope_to_header(mail) end def ensure_references_array(references) @@ -96,6 +97,13 @@ module Gitlab end end + def find_key_from_envelope_to_header(mail) + Array(mail[:envelope_to]).find do |header| + key = Gitlab::IncomingEmail.key_from_address(header.value) + break key if key + end + end + def ignore_auto_reply!(mail) if auto_submitted?(mail) || auto_replied?(mail) raise AutoGeneratedEmailError diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 9636e75aba1..262a1ef653f 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -160,6 +160,7 @@ module Gitlab def self.execute(storage, service, rpc, request, remote_storage:, timeout:) enforce_gitaly_request_limits(:call) + Gitlab::RequestContext.instance.ensure_deadline_not_exceeded! kwargs = request_kwargs(storage, timeout: timeout.to_f, remote_storage: remote_storage) kwargs = yield(kwargs) if block_given? @@ -234,12 +235,28 @@ module Gitlab metadata['gitaly-session-id'] = session_id metadata.merge!(Feature::Gitaly.server_feature_flags) - result = { metadata: metadata } + deadline_info = request_deadline(timeout) + metadata.merge!(deadline_info.slice(:deadline_type)) - result[:deadline] = real_time + timeout if timeout > 0 - result + { metadata: metadata, deadline: deadline_info[:deadline] } end + def self.request_deadline(timeout) + # timeout being 0 means the request is allowed to run indefinitely. + # We can't allow that inside a request, but this won't count towards Gitaly + # error budgets + regular_deadline = real_time.to_i + timeout if timeout > 0 + + return { deadline: regular_deadline } if Sidekiq.server? + return { deadline: regular_deadline } unless Gitlab::RequestContext.instance.request_deadline + + limited_deadline = [regular_deadline, Gitlab::RequestContext.instance.request_deadline].compact.min + limited = limited_deadline < regular_deadline + + { deadline: limited_deadline, deadline_type: limited ? "limited" : "regular" } + end + private_class_method :request_deadline + def self.session_id Gitlab::SafeRequestStore[:gitaly_session_id] ||= SecureRandom.uuid end diff --git a/lib/gitlab/graphql/authorize/authorize_resource.rb b/lib/gitlab/graphql/authorize/authorize_resource.rb index 26e8c53032f..94871498cf8 100644 --- a/lib/gitlab/graphql/authorize/authorize_resource.rb +++ b/lib/gitlab/graphql/authorize/authorize_resource.rb @@ -40,7 +40,7 @@ module Gitlab def authorize!(object) unless authorized_resource?(object) - raise_resource_not_avaiable_error! + raise_resource_not_available_error! end end @@ -63,7 +63,7 @@ module Gitlab end end - def raise_resource_not_avaiable_error! + def raise_resource_not_available_error! raise Gitlab::Graphql::Errors::ResourceNotAvailable, RESOURCE_ACCESS_ERROR end end diff --git a/lib/gitlab/middleware/request_context.rb b/lib/gitlab/middleware/request_context.rb new file mode 100644 index 00000000000..953423b371c --- /dev/null +++ b/lib/gitlab/middleware/request_context.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Gitlab + module Middleware + class RequestContext + def initialize(app) + @app = app + end + + def call(env) + # We should be using ActionDispatch::Request instead of + # Rack::Request to be consistent with Rails, but due to a Rails + # bug described in + # https://gitlab.com/gitlab-org/gitlab-foss/issues/58573#note_149799010 + # hosts behind a load balancer will only see 127.0.0.1 for the + # load balancer's IP. + req = Rack::Request.new(env) + + Gitlab::RequestContext.instance.client_ip = req.ip + Gitlab::RequestContext.instance.start_thread_cpu_time = Gitlab::Metrics::System.thread_cpu_time + Gitlab::RequestContext.instance.request_start_time = Gitlab::Metrics::System.real_time + + @app.call(env) + end + end + end +end diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb index 13187836e02..49c2c0c982c 100644 --- a/lib/gitlab/request_context.rb +++ b/lib/gitlab/request_context.rb @@ -2,34 +2,37 @@ module Gitlab class RequestContext - class << self - def client_ip - Gitlab::SafeRequestStore[:client_ip] - end + include Singleton + + RequestDeadlineExceeded = Class.new(StandardError) + + attr_accessor :client_ip, :start_thread_cpu_time, :request_start_time - def start_thread_cpu_time - Gitlab::SafeRequestStore[:start_thread_cpu_time] + class << self + def instance + Gitlab::SafeRequestStore[:request_context] ||= new end end - def initialize(app) - @app = app + def request_deadline + return unless request_start_time + return unless Feature.enabled?(:request_deadline) + + @request_deadline ||= request_start_time + max_request_duration_seconds end - def call(env) - # We should be using ActionDispatch::Request instead of - # Rack::Request to be consistent with Rails, but due to a Rails - # bug described in - # https://gitlab.com/gitlab-org/gitlab-foss/issues/58573#note_149799010 - # hosts behind a load balancer will only see 127.0.0.1 for the - # load balancer's IP. - req = Rack::Request.new(env) + def ensure_deadline_not_exceeded! + return unless request_deadline + return if Gitlab::Metrics::System.real_time < request_deadline - Gitlab::SafeRequestStore[:client_ip] = req.ip + raise RequestDeadlineExceeded, + "Request takes longer than #{max_request_duration_seconds}" + end - Gitlab::SafeRequestStore[:start_thread_cpu_time] = Gitlab::Metrics::System.thread_cpu_time + private - @app.call(env) + def max_request_duration_seconds + Settings.gitlab.max_request_duration_seconds end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b8fa6e845cb..5b4db60d020 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1068,9 +1068,6 @@ msgstr "" msgid "Add new directory" msgstr "" -msgid "Add new member to %{strong_start}%{group_name}%{strong_end}" -msgstr "" - msgid "Add or subtract spent time" msgstr "" @@ -1095,9 +1092,6 @@ msgstr "" msgid "Add to merge train when pipeline succeeds" msgstr "" -msgid "Add to project" -msgstr "" - msgid "Add to review" msgstr "" @@ -7458,10 +7452,10 @@ msgstr "" msgid "Excluding merge commits. Limited to 6,000 commits." msgstr "" -msgid "Existing" +msgid "Existing members and groups" msgstr "" -msgid "Existing members and groups" +msgid "Existing shares" msgstr "" msgid "Expand" @@ -9122,6 +9116,9 @@ msgstr "" msgid "Group maintainers can register group runners in the %{link}" msgstr "" +msgid "Group members" +msgstr "" + msgid "Group name" msgstr "" @@ -9392,6 +9389,9 @@ msgstr "" msgid "Groups can also be nested by creating %{subgroup_docs_link_start}subgroups%{subgroup_docs_link_end}." msgstr "" +msgid "Groups with access to %{strong_start}%{group_name}%{strong_end}" +msgstr "" + msgid "Groups with access to %{project_name}" msgstr "" @@ -10314,9 +10314,6 @@ msgstr "" msgid "Job has wrong arguments format." msgstr "" -msgid "Job is in progress" -msgstr "" - msgid "Job is missing the `model_type` argument." msgstr "" @@ -10326,6 +10323,12 @@ msgstr "" msgid "Job logs and artifacts" msgstr "" +msgid "Job to create self-monitoring project is in progress" +msgstr "" + +msgid "Job to delete self-monitoring project is in progress" +msgstr "" + msgid "Job was retried" msgstr "" @@ -12639,12 +12642,6 @@ msgstr "" msgid "OperationsDashboard|The operations dashboard provides a summary of each project's operational health, including pipeline and alert statuses." msgstr "" -msgid "Option 1" -msgstr "" - -msgid "Option 2" -msgstr "" - msgid "Optional" msgstr "" @@ -15784,9 +15781,6 @@ msgstr "" msgid "Save Changes" msgstr "" -msgid "Save Expiration Policy" -msgstr "" - msgid "Save anyway" msgstr "" @@ -15802,6 +15796,9 @@ msgstr "" msgid "Save comment" msgstr "" +msgid "Save expiration policy" +msgstr "" + msgid "Save password" msgstr "" @@ -16373,6 +16370,12 @@ msgstr "" msgid "Self-monitoring project does not exist. Please check logs for any error messages" msgstr "" +msgid "Self-monitoring project has been successfully deleted" +msgstr "" + +msgid "Self-monitoring project was not deleted. Please check logs for any error messages" +msgstr "" + msgid "Send a separate email notification to Developers." msgstr "" diff --git a/qa/qa/page/group/sub_menus/members.rb b/qa/qa/page/group/sub_menus/members.rb index c8b3f5bb422..33c4caaddcb 100644 --- a/qa/qa/page/group/sub_menus/members.rb +++ b/qa/qa/page/group/sub_menus/members.rb @@ -7,12 +7,9 @@ module QA class Members < Page::Base include Page::Component::UsersSelect - view 'app/views/groups/group_members/_new_group_member.html.haml' do - element :add_to_group_button - end - - view 'app/helpers/groups/group_members_helper.rb' do + view 'app/views/shared/members/_invite_member.html.haml' do element :member_select_field + element :invite_member_button end view 'app/views/shared/members/_member.html.haml' do @@ -24,7 +21,7 @@ module QA def add_member(username) select_user :member_select_field, username - click_element :add_to_group_button + click_element :invite_member_button end def update_access_level(username, access_level) diff --git a/qa/qa/page/project/settings/members.rb b/qa/qa/page/project/settings/members.rb index 2ef018fd983..fd3e0add2a6 100644 --- a/qa/qa/page/project/settings/members.rb +++ b/qa/qa/page/project/settings/members.rb @@ -8,9 +8,9 @@ module QA include Page::Component::UsersSelect include QA::Page::Component::Select2 - view 'app/views/projects/project_members/_new_project_member.html.haml' do - element :member_select_input - element :add_member_button + view 'app/views/shared/members/_invite_member.html.haml' do + element :member_select_field + element :invite_member_button end view 'app/views/projects/project_members/_team.html.haml' do @@ -21,7 +21,7 @@ module QA element :invite_group_tab end - view 'app/views/projects/project_members/_new_project_group.html.haml' do + view 'app/views/shared/members/_invite_group.html.haml' do element :group_select_field element :invite_group_button end @@ -43,8 +43,8 @@ module QA end def add_member(username) - select_user :member_select_input, username - click_element :add_member_button + select_user :member_select_field, username + click_element :invite_member_button end def remove_group(group_name) diff --git a/spec/controllers/groups/group_links_controller_spec.rb b/spec/controllers/groups/group_links_controller_spec.rb index 8f04822fee6..04f2e33b26a 100644 --- a/spec/controllers/groups/group_links_controller_spec.rb +++ b/spec/controllers/groups/group_links_controller_spec.rb @@ -111,4 +111,100 @@ describe Groups::GroupLinksController do end end end + + describe '#update' do + let!(:link) do + create(:group_group_link, { shared_group: shared_group, + shared_with_group: shared_with_group }) + end + + let(:expiry_date) { 1.month.from_now.to_date } + + subject do + post(:update, params: { group_id: shared_group, + id: link.id, + group_link: { group_access: Gitlab::Access::GUEST, + expires_at: expiry_date } }) + end + + context 'when user has admin access to the shared group' do + before do + shared_group.add_owner(user) + end + + it 'updates existing link' do + expect(link.group_access).to eq(Gitlab::Access::DEVELOPER) + expect(link.expires_at).to be_nil + + subject + + link.reload + + expect(link.group_access).to eq(Gitlab::Access::GUEST) + expect(link.expires_at).to eq(expiry_date) + end + end + + context 'when user does not have admin access to the shared group' do + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(share_group_with_group: false) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end + + describe '#destroy' do + let!(:link) do + create(:group_group_link, { shared_group: shared_group, + shared_with_group: shared_with_group }) + end + + subject do + post(:destroy, params: { group_id: shared_group, + id: link.id }) + end + + context 'when user has admin access to the shared group' do + before do + shared_group.add_owner(user) + end + + it 'deletes existing link' do + expect { subject }.to change(GroupGroupLink, :count).by(-1) + end + end + + context 'when user does not have admin access to the shared group' do + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(share_group_with_group: false) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end end diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index a144d9e0786..1c8a2bd160d 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -31,6 +31,12 @@ describe Groups::GroupMembersController do expect(assigns(:invited_members).map(&:invite_email)).to match_array(invited.map(&:invite_email)) end + it 'assigns skip groups' do + get :index, params: { group_id: group } + + expect(assigns(:skip_groups)).to match_array(group.related_group_ids) + end + it 'restricts search to one email' do get :index, params: { group_id: group, search_invited: invited.first.invite_email } diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb index b48b7dc86e0..cbcda5d0dc7 100644 --- a/spec/controllers/health_check_controller_spec.rb +++ b/spec/controllers/health_check_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe HealthCheckController do +describe HealthCheckController, :request_store do include StubENV let(:xml_response) { Hash.from_xml(response.body)['hash'] } @@ -18,7 +18,7 @@ describe HealthCheckController do describe 'GET #index' do context 'when services are up but accessed from outside whitelisted ips' do before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip) + allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(not_whitelisted_ip) end it 'returns a not found page' do @@ -48,7 +48,7 @@ describe HealthCheckController do context 'when services are up and accessed from whitelisted ips' do before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip) + allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(whitelisted_ip) end it 'supports successful plaintext response' do @@ -95,7 +95,7 @@ describe HealthCheckController do before do allow(HealthCheck::Utils).to receive(:process_checks).with(['standard']).and_return('The server is on fire') allow(HealthCheck::Utils).to receive(:process_checks).with(['email']).and_return('Email is on fire') - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip) + allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(whitelisted_ip) end it 'supports failure plaintext response' do diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 1d378b9b9dc..331eafba0d3 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MetricsController do +describe MetricsController, :request_store do include StubENV let(:metrics_multiproc_dir) { @metrics_multiproc_dir } @@ -53,7 +53,7 @@ describe MetricsController do context 'accessed from whitelisted ip' do before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip) + allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(whitelisted_ip) end it_behaves_like 'endpoint providing metrics' @@ -61,7 +61,7 @@ describe MetricsController do context 'accessed from ip in whitelisted range' do before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(ip_in_whitelisted_range) + allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(ip_in_whitelisted_range) end it_behaves_like 'endpoint providing metrics' @@ -69,7 +69,7 @@ describe MetricsController do context 'accessed from not whitelisted ip' do before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip) + allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(not_whitelisted_ip) end it 'returns the expected error response' do diff --git a/spec/features/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index 257e5cb8bf0..9a4889a0335 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -167,14 +167,14 @@ describe 'Admin Groups' do it 'adds admin a to a group as developer', :js do visit group_group_members_path(group) - page.within '.users-group-form' do + page.within '.invite-users-form' do select2(current_user.id, from: '#user_ids', multiple: true) select 'Developer', from: 'access_level' end - click_button 'Add to group' + click_button 'Invite' - page.within '.content-list' do + page.within '[data-qa-selector="members_list"]' do expect(page).to have_content(current_user.name) expect(page).to have_content('Developer') end @@ -187,7 +187,7 @@ describe 'Admin Groups' do visit group_group_members_path(group) - page.within '.content-list' do + page.within '[data-qa-selector="members_list"]' do expect(page).to have_content(current_user.name) expect(page).to have_content('Developer') end @@ -196,7 +196,7 @@ describe 'Admin Groups' do visit group_group_members_path(group) - page.within '.content-list' do + page.within '[data-qa-selector="members_list"]' do expect(page).not_to have_content(current_user.name) expect(page).not_to have_content('Developer') end diff --git a/spec/features/admin/admin_projects_spec.rb b/spec/features/admin/admin_projects_spec.rb index 7c40ac5bde3..d1889d3a89a 100644 --- a/spec/features/admin/admin_projects_spec.rb +++ b/spec/features/admin/admin_projects_spec.rb @@ -98,12 +98,12 @@ describe "Admin::Projects" do it 'adds admin a to a project as developer', :js do visit project_project_members_path(project) - page.within '.users-project-form' do + page.within '.invite-users-form' do select2(current_user.id, from: '#user_ids', multiple: true) select 'Developer', from: 'access_level' end - click_button 'Add to project' + click_button 'Invite' page.within '.content-list' do expect(page).to have_content(current_user.name) diff --git a/spec/features/groups/members/manage_groups_spec.rb b/spec/features/groups/members/manage_groups_spec.rb new file mode 100644 index 00000000000..55f9418521f --- /dev/null +++ b/spec/features/groups/members/manage_groups_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Groups > Members > Manage groups', :js do + include Select2Helper + include Spec::Support::Helpers::Features::ListRowsHelpers + + let(:user) { create(:user) } + let(:shared_with_group) { create(:group) } + let(:shared_group) { create(:group) } + + before do + shared_group.add_owner(user) + sign_in(user) + end + + context 'with share groups with groups feature flag' do + before do + stub_feature_flags(shared_with_group: true) + end + + it 'add group to group' do + visit group_group_members_path(shared_group) + + add_group(shared_with_group.id, 'Reporter') + + page.within(first_row) do + expect(page).to have_content(shared_with_group.name) + expect(page).to have_content('Reporter') + end + end + + it 'remove user from group' do + create(:group_group_link, shared_group: shared_group, + shared_with_group: shared_with_group, group_access: ::Gitlab::Access::DEVELOPER) + + visit group_group_members_path(shared_group) + + expect(page).to have_content(shared_with_group.name) + + accept_confirm do + find(:css, '#existing_shares li', text: shared_with_group.name).find(:css, 'a.btn-remove').click + end + + wait_for_requests + + expect(page).not_to have_content(shared_with_group.name) + end + + it 'update group to owner level' do + create(:group_group_link, shared_group: shared_group, + shared_with_group: shared_with_group, group_access: ::Gitlab::Access::DEVELOPER) + + visit group_group_members_path(shared_group) + + page.within(first_row) do + click_button('Developer') + click_link('Maintainer') + + wait_for_requests + + expect(page).to have_button('Maintainer') + end + end + + def add_group(id, role) + page.click_link 'Invite group' + page.within ".invite-group-form" do + select2(id, from: "#shared_with_group_id") + select(role, from: "shared_group_access") + click_button "Invite" + end + end + end + + context 'without share groups with groups feature flag' do + before do + stub_feature_flags(share_group_with_group: false) + end + + it 'does not render invitation form and tabs' do + visit group_group_members_path(shared_group) + + expect(page).not_to have_link('Invite member') + expect(page).not_to have_link('Invite group') + end + end +end diff --git a/spec/features/groups/members/manage_members_spec.rb b/spec/features/groups/members/manage_members_spec.rb index cdd16ae9441..e4ba3022d8b 100644 --- a/spec/features/groups/members/manage_members_spec.rb +++ b/spec/features/groups/members/manage_members_spec.rb @@ -113,7 +113,8 @@ describe 'Groups > Members > Manage members' do visit group_group_members_path(group) - expect(page).not_to have_button 'Add to group' + expect(page).not_to have_selector '.invite-users-form' + expect(page).not_to have_selector '.invite-group-form' page.within(second_row) do # Can not modify user2 role @@ -125,11 +126,10 @@ describe 'Groups > Members > Manage members' do end def add_user(id, role) - page.within ".users-group-form" do + page.within ".invite-users-form" do select2(id, from: "#user_ids", multiple: true) select(role, from: "access_level") + click_button "Invite" end - - click_button "Add to group" end end diff --git a/spec/features/groups/members/search_members_spec.rb b/spec/features/groups/members/search_members_spec.rb index 9c17aac09e8..fda129ce422 100644 --- a/spec/features/groups/members/search_members_spec.rb +++ b/spec/features/groups/members/search_members_spec.rb @@ -24,7 +24,7 @@ describe 'Search group member' do find('.user-search-btn').click end - group_members_list = find(".card .content-list") + group_members_list = find('[data-qa-selector="members_list"]') expect(group_members_list).to have_content(member.name) expect(group_members_list).not_to have_content(user.name) end diff --git a/spec/features/projects/members/list_spec.rb b/spec/features/projects/members/list_spec.rb index 6d92c777033..84000ef73ce 100644 --- a/spec/features/projects/members/list_spec.rb +++ b/spec/features/projects/members/list_spec.rb @@ -87,12 +87,12 @@ describe 'Project members list' do end def add_user(id, role) - page.within ".users-project-form" do + page.within ".invite-users-form" do select2(id, from: "#user_ids", multiple: true) select(role, from: "access_level") end - click_button "Add to project" + click_button "Invite" end def visit_members_page diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index 501dd05300a..cbcd03b33ce 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -20,10 +20,10 @@ describe 'Projects > Members > Maintainer adds member with expiration date', :js date = 4.days.from_now visit project_project_members_path(project) - page.within '.users-project-form' do + page.within '.invite-users-form' do select2(new_member.id, from: '#user_ids', multiple: true) fill_in 'expires_at', with: date.to_s(:medium) + "\n" - click_on 'Add to project' + click_on 'Invite' end page.within "#project_member_#{new_member.project_members.first.id}" do diff --git a/spec/features/projects/settings/user_manages_project_members_spec.rb b/spec/features/projects/settings/user_manages_project_members_spec.rb index 6d94388a6e2..705c60f15ee 100644 --- a/spec/features/projects/settings/user_manages_project_members_spec.rb +++ b/spec/features/projects/settings/user_manages_project_members_spec.rb @@ -37,7 +37,7 @@ describe 'Projects > Settings > User manages project members' do visit(project_project_members_path(project)) - page.within('.users-project-form') do + page.within('.invite-users-form') do click_link('Import') end diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb index f161a1df9c3..5636b5db84f 100644 --- a/spec/finders/group_members_finder_spec.rb +++ b/spec/finders/group_members_finder_spec.rb @@ -10,6 +10,7 @@ describe GroupMembersFinder, '#execute' do let(:user2) { create(:user) } let(:user3) { create(:user) } let(:user4) { create(:user) } + let(:user5) { create(:user, :two_factor_via_otp) } it 'returns members for top-level group' do member1 = group.add_maintainer(user1) @@ -67,4 +68,43 @@ describe GroupMembersFinder, '#execute' do expect(result.to_a).to match_array([member1, member2, member3, member4]) end + + it 'returns searched members if requested' do + group.add_maintainer(user2) + nested_group.add_maintainer(user2) + nested_group.add_maintainer(user3) + nested_group.add_maintainer(user4) + member = group.add_maintainer(user1) + + result = described_class.new(group).execute(include_relations: [:direct, :descendants], params: { search: user1.name }) + + expect(result.to_a).to match_array([member]) + end + + it 'returns members with two-factor auth if requested by owner' do + group.add_owner(user2) + group.add_maintainer(user1) + nested_group.add_maintainer(user2) + nested_group.add_maintainer(user3) + nested_group.add_maintainer(user4) + member = group.add_maintainer(user5) + + result = described_class.new(group, user2).execute(include_relations: [:direct, :descendants], params: { two_factor: 'enabled' }) + + expect(result.to_a).to contain_exactly(member) + end + + it 'returns members without two-factor auth if requested by owner' do + member1 = group.add_owner(user2) + member2 = group.add_maintainer(user1) + nested_group.add_maintainer(user2) + member3 = nested_group.add_maintainer(user3) + member4 = nested_group.add_maintainer(user4) + member_with_2fa = group.add_maintainer(user5) + + result = described_class.new(group, user2).execute(include_relations: [:direct, :descendants], params: { two_factor: 'disabled' }) + + expect(result.to_a).not_to include(member_with_2fa) + expect(result.to_a).to match_array([member1, member2, member3, member4]) + end end diff --git a/spec/fixtures/emails/envelope_to_header.eml b/spec/fixtures/emails/envelope_to_header.eml new file mode 100644 index 00000000000..4b6418d4c06 --- /dev/null +++ b/spec/fixtures/emails/envelope_to_header.eml @@ -0,0 +1,32 @@ +Return-Path: +Received: from myserver.example.com ([unix socket]) by myserver (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail.example.com (mail.example.com [IPv6:2607:f8b0:4001:c03::234]) by myserver.example.com (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by myserver.example.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +From: "jake@example.com" +To: "support@example.com" +Subject: Insert hilarious subject line here +Date: Tue, 26 Nov 2019 14:22:41 +0000 +Message-ID: <7e2296f83dbf4de388cbf5f56f52c11f@EXDAG29-1.EXCHANGE.INT> +Accept-Language: de-DE, en-US +Content-Language: de-DE +X-MS-Has-Attach: +X-MS-TNEF-Correlator: +x-ms-exchange-transport-fromentityheader: Hosted +x-originating-ip: [62.96.54.178] +Content-Type: multipart/alternative; + boundary="_000_7e2296f83dbf4de388cbf5f56f52c11fEXDAG291EXCHANGEINT_" +MIME-Version: 1.0 +Envelope-To: incoming+gitlabhq/gitlabhq+auth_token@appmail.example.com + +--_000_7e2296f83dbf4de388cbf5f56f52c11fEXDAG291EXCHANGEINT_ +Content-Type: text/plain; charset="iso-8859-1" +Content-Transfer-Encoding: quoted-printable + + + +--_000_7e2296f83dbf4de388cbf5f56f52c11fEXDAG291EXCHANGEINT_ +Content-Type: text/html; charset="iso-8859-1" +Content-Transfer-Encoding: quoted-printable + +Look, a message with some alternate headers! We should really support them. diff --git a/spec/fixtures/emails/forwarded_new_issue.eml b/spec/fixtures/emails/forwarded_new_issue.eml index 258106bb897..e3688697651 100644 --- a/spec/fixtures/emails/forwarded_new_issue.eml +++ b/spec/fixtures/emails/forwarded_new_issue.eml @@ -1,13 +1,13 @@ -Delivered-To: incoming+gitlabhq/gitlabhq+auth_token@appmail.adventuretime.ooo -Return-Path: -Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 -Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 13 Jun 2013 17:03:50 -0400 -Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 +Delivered-To: incoming+gitlabhq/gitlabhq+auth_token@appmail.example.com +Return-Path: +Received: from iceking.example.com ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.example.com (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 Date: Thu, 13 Jun 2013 17:03:48 -0400 -From: Jake the Dog -Delivered-To: support@adventuretime.ooo -To: support@adventuretime.ooo +From: Jake the Dog +Delivered-To: support@example.com +To: support@example.com Message-ID: Subject: New Issue by email Mime-Version: 1.0 diff --git a/spec/frontend/registry/settings/components/__snapshots__/settings_form_spec.js.snap b/spec/frontend/registry/settings/components/__snapshots__/settings_form_spec.js.snap index 9724033f3c9..bef4674bd8b 100644 --- a/spec/frontend/registry/settings/components/__snapshots__/settings_form_spec.js.snap +++ b/spec/frontend/registry/settings/components/__snapshots__/settings_form_spec.js.snap @@ -1,10 +1,11 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Settings Form renders 1`] = ` -
-
+ +
+
@@ -12,11 +13,13 @@ exports[`Settings Form renders 1`] = ` Tag expiration policy
-
- + + + -
- + - - - - - + + - - - - - + + - - - - - + + - - - + +
-
- - Cancel - - - + + Cancel + + + + + Save expiration policy - Save Expiration Policy - - + +
- - + + + `; diff --git a/spec/frontend/registry/settings/components/settings_form_spec.js b/spec/frontend/registry/settings/components/settings_form_spec.js index b944e5a2cea..bd733e965a4 100644 --- a/spec/frontend/registry/settings/components/settings_form_spec.js +++ b/spec/frontend/registry/settings/components/settings_form_spec.js @@ -1,8 +1,10 @@ import Vuex from 'vuex'; -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { mount, createLocalVue } from '@vue/test-utils'; +import stubChildren from 'helpers/stub_children'; import component from '~/registry/settings/components/settings_form.vue'; import { createStore } from '~/registry/settings/store/'; import { NAME_REGEX_LENGTH } from '~/registry/settings/constants'; +import { stringifiedFormOptions } from '../mock_data'; const localVue = createLocalVue(); localVue.use(Vuex); @@ -13,7 +15,6 @@ describe('Settings Form', () => { let saveSpy; let resetSpy; - const helpPagePath = 'foo'; const findFormGroup = name => wrapper.find(`#expiration-policy-${name}-group`); const findFormElements = (name, father = wrapper) => father.find(`#expiration-policy-${name}`); const findCancelButton = () => wrapper.find({ ref: 'cancel-button' }); @@ -23,7 +24,11 @@ describe('Settings Form', () => { const mountComponent = (options = {}) => { saveSpy = jest.fn(); resetSpy = jest.fn(); - wrapper = shallowMount(component, { + wrapper = mount(component, { + stubs: { + ...stubChildren(component), + GlCard: false, + }, store, methods: { saveSettings: saveSpy, @@ -35,7 +40,7 @@ describe('Settings Form', () => { beforeEach(() => { store = createStore(); - store.dispatch('setInitialState', { helpPagePath }); + store.dispatch('setInitialState', stringifiedFormOptions); mountComponent(); }); @@ -48,13 +53,13 @@ describe('Settings Form', () => { }); describe.each` - elementName | modelName | value - ${'toggle'} | ${'enabled'} | ${true} - ${'interval'} | ${'older_than'} | ${'foo'} - ${'schedule'} | ${'cadence'} | ${'foo'} - ${'latest'} | ${'keep_n'} | ${'foo'} - ${'name-matching'} | ${'name_regex'} | ${'foo'} - `('%s form element', ({ elementName, modelName, value }) => { + elementName | modelName | value | disabledByToggle + ${'toggle'} | ${'enabled'} | ${true} | ${'not disabled'} + ${'interval'} | ${'older_than'} | ${'foo'} | ${'disabled'} + ${'schedule'} | ${'cadence'} | ${'foo'} | ${'disabled'} + ${'latest'} | ${'keep_n'} | ${'foo'} | ${'disabled'} + ${'name-matching'} | ${'name_regex'} | ${'foo'} | ${'disabled'} + `('$elementName form element', ({ elementName, modelName, value, disabledByToggle }) => { let formGroup; beforeEach(() => { formGroup = findFormGroup(elementName); @@ -89,6 +94,12 @@ describe('Settings Form', () => { expect(wrapper.vm[modelName]).toBe(value); }); }); + + it(`${elementName} is ${disabledByToggle} by enabled set to false`, () => { + store.dispatch('updateSettings', { enabled: false }); + const expectation = disabledByToggle === 'disabled' ? 'true' : undefined; + expect(findFormElements(elementName, formGroup).attributes('disabled')).toBe(expectation); + }); }); describe('form actions', () => { diff --git a/spec/frontend/registry/settings/mock_data.js b/spec/frontend/registry/settings/mock_data.js new file mode 100644 index 00000000000..411363c2c95 --- /dev/null +++ b/spec/frontend/registry/settings/mock_data.js @@ -0,0 +1,12 @@ +export const options = [{ key: 'foo', label: 'Foo' }, { key: 'bar', label: 'Bar', default: true }]; +export const stringifiedOptions = JSON.stringify(options); +export const stringifiedFormOptions = { + cadenceOptions: stringifiedOptions, + keepNOptions: stringifiedOptions, + olderThanOptions: stringifiedOptions, +}; +export const formOptions = { + cadence: options, + keepN: options, + olderThan: options, +}; diff --git a/spec/frontend/registry/settings/store/mutations_spec.js b/spec/frontend/registry/settings/store/mutations_spec.js index a8c7ed3bafa..1a0effbe125 100644 --- a/spec/frontend/registry/settings/store/mutations_spec.js +++ b/spec/frontend/registry/settings/store/mutations_spec.js @@ -1,6 +1,7 @@ import mutations from '~/registry/settings/store/mutations'; import * as types from '~/registry/settings/store/mutation_types'; import createState from '~/registry/settings/store/state'; +import { formOptions, stringifiedFormOptions } from '../mock_data'; describe('Mutations Registry Store', () => { let mockState; @@ -11,11 +12,14 @@ describe('Mutations Registry Store', () => { describe('SET_INITIAL_STATE', () => { it('should set the initial state', () => { - const payload = { helpPagePath: 'foo', projectId: 'bar' }; - const expectedState = { ...mockState, ...payload }; - mutations[types.SET_INITIAL_STATE](mockState, payload); + const expectedState = { ...mockState, projectId: 'foo', formOptions }; + mutations[types.SET_INITIAL_STATE](mockState, { + projectId: 'foo', + ...stringifiedFormOptions, + }); expect(mockState.projectId).toEqual(expectedState.projectId); + expect(mockState.formOptions).toEqual(expectedState.formOptions); }); }); diff --git a/spec/frontend/vuex_shared/bindings_spec.js b/spec/frontend/vuex_shared/bindings_spec.js index 022d9e8e84c..0f91a09018f 100644 --- a/spec/frontend/vuex_shared/bindings_spec.js +++ b/spec/frontend/vuex_shared/bindings_spec.js @@ -3,49 +3,77 @@ import { mapComputed } from '~/vuex_shared/bindings'; describe('Binding utils', () => { describe('mapComputed', () => { - const dummyComponent = { + const defaultArgs = [['baz'], 'bar', 'foo']; + + const createDummy = (mapComputedArgs = defaultArgs) => ({ computed: { - ...mapComputed('foo', 'bar', ['baz']), + ...mapComputed(...mapComputedArgs), }, render() { return null; }, + }); + + const mocks = { + $store: { + state: { + baz: 2, + foo: { + baz: 1, + }, + }, + getters: { + getBaz: 'foo', + }, + dispatch: jest.fn(), + }, }; - it('returns an object with keys equal to the last fn parameter ', () => { + + it('returns an object with keys equal to the first fn parameter ', () => { const keyList = ['foo1', 'foo2']; - const result = mapComputed('foo', 'bar', keyList); + const result = mapComputed(keyList, 'foo', 'bar'); expect(Object.keys(result)).toEqual(keyList); }); + it('returned object has set and get function', () => { - const result = mapComputed('foo', 'bar', ['baz']); + const result = mapComputed(['baz'], 'foo', 'bar'); expect(result.baz.set).toBeDefined(); expect(result.baz.get).toBeDefined(); }); - it('set function invokes $store.dispatch', () => { - const context = shallowMount(dummyComponent, { - mocks: { - $store: { - dispatch: jest.fn(), - }, - }, + describe('set function', () => { + it('invokes $store.dispatch', () => { + const context = shallowMount(createDummy(), { mocks }); + context.vm.baz = 'a'; + expect(context.vm.$store.dispatch).toHaveBeenCalledWith('bar', { baz: 'a' }); + }); + it('uses updateFn in list object mode if updateFn exists', () => { + const context = shallowMount(createDummy([[{ key: 'foo', updateFn: 'baz' }]]), { mocks }); + context.vm.foo = 'b'; + expect(context.vm.$store.dispatch).toHaveBeenCalledWith('baz', { foo: 'b' }); + }); + it('in list object mode defaults to defaultUpdateFn if updateFn do not exists', () => { + const context = shallowMount(createDummy([[{ key: 'foo' }], 'defaultFn']), { mocks }); + context.vm.foo = 'c'; + expect(context.vm.$store.dispatch).toHaveBeenCalledWith('defaultFn', { foo: 'c' }); }); - context.vm.baz = 'a'; - expect(context.vm.$store.dispatch).toHaveBeenCalledWith('bar', { baz: 'a' }); }); - it('get function returns $store.state[root][key]', () => { - const context = shallowMount(dummyComponent, { - mocks: { - $store: { - state: { - foo: { - baz: 1, - }, - }, - }, - }, + + describe('get function', () => { + it('if root is set returns $store.state[root][key]', () => { + const context = shallowMount(createDummy(), { mocks }); + expect(context.vm.baz).toBe(mocks.$store.state.foo.baz); + }); + + it('if root is not set returns $store.state[key]', () => { + const context = shallowMount(createDummy([['baz'], 'bar']), { mocks }); + expect(context.vm.baz).toBe(mocks.$store.state.baz); + }); + + it('when using getters it invoke the appropriate getter', () => { + const context = shallowMount(createDummy([[{ getter: 'getBaz', key: 'baz' }]]), { mocks }); + expect(context.vm.baz).toBe(mocks.$store.getters.getBaz); }); - expect(context.vm.baz).toBe(1); }); }); }); diff --git a/spec/helpers/application_settings_helper_spec.rb b/spec/helpers/application_settings_helper_spec.rb index 629ac34edc1..e5deeb990a1 100644 --- a/spec/helpers/application_settings_helper_spec.rb +++ b/spec/helpers/application_settings_helper_spec.rb @@ -76,6 +76,20 @@ describe ApplicationSettingsHelper do ) end + it 'returns delete_self_monitoring_project_path' do + expect(helper.self_monitoring_project_data).to include( + 'delete_self_monitoring_project_path' => + delete_self_monitoring_project_admin_application_settings_path + ) + end + + it 'returns status_delete_self_monitoring_project_path' do + expect(helper.self_monitoring_project_data).to include( + 'status_delete_self_monitoring_project_path' => + status_delete_self_monitoring_project_admin_application_settings_path + ) + end + it 'returns self_monitoring_project_exists false' do expect(helper.self_monitoring_project_data).to include( 'self_monitoring_project_exists' => false diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index 43c73242f5f..018219e5647 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -5,22 +5,27 @@ require 'spec_helper' describe Gitlab::Email::Receiver do include_context :email_shared_context - context "when the email contains a valid email address in a Delivered-To header" do - let(:email_raw) { fixture_file('emails/forwarded_new_issue.eml') } + context 'when the email contains a valid email address in a header' do let(:handler) { double(:handler) } before do - stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo") - allow(handler).to receive(:execute) allow(handler).to receive(:metrics_params) allow(handler).to receive(:metrics_event) + + stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.example.com") + end + + context 'when in a Delivered-To header' do + let(:email_raw) { fixture_file('emails/forwarded_new_issue.eml') } + + it_behaves_like 'correctly finds the mail key' end - it "finds the mail key" do - expect(Gitlab::Email::Handler).to receive(:for).with(an_instance_of(Mail::Message), 'gitlabhq/gitlabhq+auth_token').and_return(handler) + context 'when in an Envelope-To header' do + let(:email_raw) { fixture_file('emails/envelope_to_header.eml') } - receiver.execute + it_behaves_like 'correctly finds the mail key' end end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 0d9719a5663..ebf56c0ae66 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -229,6 +229,59 @@ describe Gitlab::GitalyClient do end end end + + context 'deadlines', :request_store do + let(:request_deadline) { real_time + 10.0 } + + before do + allow(Gitlab::RequestContext.instance).to receive(:request_deadline).and_return(request_deadline) + end + + it 'includes the deadline information' do + kword_args = described_class.request_kwargs('default', timeout: 2) + + expect(kword_args[:deadline]) + .to be_within(1).of(real_time + 2) + expect(kword_args[:metadata][:deadline_type]).to eq("regular") + end + + it 'limits the deadline do the request deadline if that is closer', :aggregate_failures do + kword_args = described_class.request_kwargs('default', timeout: 15) + + expect(kword_args[:deadline]).to eq(request_deadline) + expect(kword_args[:metadata][:deadline_type]).to eq("limited") + end + + it 'does not limit calls in sidekiq' do + expect(Sidekiq).to receive(:server?).and_return(true) + + kword_args = described_class.request_kwargs('default', timeout: 6.hours.to_i) + + expect(kword_args[:deadline]).to be_within(1).of(real_time + 6.hours.to_i) + expect(kword_args[:metadata][:deadline_type]).to be_nil + end + + it 'does not limit calls in sidekiq when allowed unlimited' do + expect(Sidekiq).to receive(:server?).and_return(true) + + kword_args = described_class.request_kwargs('default', timeout: 0) + + expect(kword_args[:deadline]).to be_nil + expect(kword_args[:metadata][:deadline_type]).to be_nil + end + + it 'includes only the deadline specified by the timeout when there was no deadline' do + allow(Gitlab::RequestContext.instance).to receive(:request_deadline).and_return(nil) + kword_args = described_class.request_kwargs('default', timeout: 6.hours.to_i) + + expect(kword_args[:deadline]).to be_within(1).of(Gitlab::Metrics::System.real_time + 6.hours.to_i) + expect(kword_args[:metadata][:deadline_type]).to be_nil + end + + def real_time + Gitlab::Metrics::System.real_time + end + end end describe 'enforce_gitaly_request_limits?' do diff --git a/spec/lib/gitlab/middleware/request_context_spec.rb b/spec/lib/gitlab/middleware/request_context_spec.rb new file mode 100644 index 00000000000..1ed06a97c1e --- /dev/null +++ b/spec/lib/gitlab/middleware/request_context_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true +require 'fast_spec_helper' +require 'rack' +require 'request_store' +require_relative '../../../support/helpers/next_instance_of' + +describe Gitlab::Middleware::RequestContext do + include NextInstanceOf + + let(:app) { -> (env) {} } + let(:env) { {} } + + around do |example| + RequestStore.begin! + example.run + RequestStore.end! + RequestStore.clear! + end + + describe '#call' do + context 'setting the client ip' do + subject { Gitlab::RequestContext.instance.client_ip } + + context 'with X-Forwarded-For headers' do + let(:load_balancer_ip) { '1.2.3.4' } + let(:headers) do + { + 'HTTP_X_FORWARDED_FOR' => "#{load_balancer_ip}, 127.0.0.1", + 'REMOTE_ADDR' => '127.0.0.1' + } + end + + let(:env) { Rack::MockRequest.env_for("/").merge(headers) } + + it 'returns the load balancer IP' do + endpoint = proc do + [200, {}, ["Hello"]] + end + + described_class.new(endpoint).call(env) + + expect(subject).to eq(load_balancer_ip) + end + end + + context 'request' do + let(:ip) { '192.168.1.11' } + + before do + allow_next_instance_of(Rack::Request) do |instance| + allow(instance).to receive(:ip).and_return(ip) + end + described_class.new(app).call(env) + end + + it { is_expected.to eq(ip) } + end + + context 'before RequestContext middleware run' do + it { is_expected.to be_nil } + end + end + end + + context 'setting the thread cpu time' do + it 'sets the `start_thread_cpu_time`' do + expect { described_class.new(app).call(env) } + .to change { Gitlab::RequestContext.instance.start_thread_cpu_time }.from(nil).to(Float) + end + end + + context 'setting the request start time' do + it 'sets the `request_start_time`' do + expect { described_class.new(app).call(env) } + .to change { Gitlab::RequestContext.instance.request_start_time }.from(nil).to(Float) + end + end +end diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb index 87b8029de2e..1290071549d 100644 --- a/spec/lib/gitlab/request_context_spec.rb +++ b/spec/lib/gitlab/request_context_spec.rb @@ -2,59 +2,44 @@ require 'spec_helper' -describe Gitlab::RequestContext do - describe '#client_ip' do - subject { described_class.client_ip } +describe Gitlab::RequestContext, :request_store do + subject { described_class.instance } - let(:app) { -> (env) {} } - let(:env) { Hash.new } + it { is_expected.to have_attributes(client_ip: nil, start_thread_cpu_time: nil, request_start_time: nil) } - context 'with X-Forwarded-For headers', :request_store do - let(:load_balancer_ip) { '1.2.3.4' } - let(:headers) do - { - 'HTTP_X_FORWARDED_FOR' => "#{load_balancer_ip}, 127.0.0.1", - 'REMOTE_ADDR' => '127.0.0.1' - } - end + describe '#request_deadline' do + let(:request_start_time) { 1575982156.206008 } - let(:env) { Rack::MockRequest.env_for("/").merge(headers) } + it "sets the time to #{Settings.gitlab.max_request_duration_seconds} seconds in the future" do + allow(subject).to receive(:request_start_time).and_return(request_start_time) - it 'returns the load balancer IP' do - client_ip = nil - - endpoint = proc do - client_ip = Gitlab::SafeRequestStore[:client_ip] - [200, {}, ["Hello"]] - end + expect(subject.request_deadline).to eq(1575982156.206008 + Settings.gitlab.max_request_duration_seconds) + expect(subject.request_deadline).to be_a(Float) + end - described_class.new(endpoint).call(env) + it 'returns nil if there is no start time' do + allow(subject).to receive(:request_start_time).and_return(nil) - expect(client_ip).to eq(load_balancer_ip) - end + expect(subject.request_deadline).to be_nil end + end - context 'when RequestStore::Middleware is used' do - around do |example| - RequestStore::Middleware.new(-> (env) { example.run }).call({}) - end + describe '#ensure_request_deadline_not_exceeded!' do + it 'does not raise an error when there was no deadline' do + expect(subject).to receive(:request_deadline).and_return(nil) + expect { subject.ensure_deadline_not_exceeded! }.not_to raise_error + end - context 'request' do - let(:ip) { '192.168.1.11' } + it 'does not raise an error if the deadline is in the future' do + allow(subject).to receive(:request_deadline).and_return(Gitlab::Metrics::System.real_time + 10) - before do - allow_next_instance_of(Rack::Request) do |instance| - allow(instance).to receive(:ip).and_return(ip) - end - described_class.new(app).call(env) - end + expect { subject.ensure_deadline_not_exceeded! }.not_to raise_error + end - it { is_expected.to eq(ip) } - end + it 'raises an error when the deadline is in the past' do + allow(subject).to receive(:request_deadline).and_return(Gitlab::Metrics::System.real_time - 10) - context 'before RequestContext middleware run' do - it { is_expected.to be_nil } - end + expect { subject.ensure_deadline_not_exceeded! }.to raise_error(described_class::RequestDeadlineExceeded) end end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 52e60a69a52..bbd50f1c0ef 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -319,6 +319,11 @@ describe ApplicationSetting do end context 'gitaly timeouts' do + it "validates that the default_timeout is lower than the max_request_duration" do + is_expected.to validate_numericality_of(:gitaly_timeout_default) + .is_less_than_or_equal_to(Settings.gitlab.max_request_duration_seconds) + end + [:gitaly_timeout_default, :gitaly_timeout_medium, :gitaly_timeout_fast].each do |timeout_name| it do is_expected.to validate_presence_of(timeout_name) diff --git a/spec/models/group_group_link_spec.rb b/spec/models/group_group_link_spec.rb index e4ad5703a10..a877cc803dd 100644 --- a/spec/models/group_group_link_spec.rb +++ b/spec/models/group_group_link_spec.rb @@ -33,4 +33,12 @@ describe GroupGroupLink do validate_inclusion_of(:group_access).in_array(Gitlab::Access.values)) end end + + describe '#human_access' do + it 'delegates to Gitlab::Access' do + expect(Gitlab::Access).to receive(:human_access).with(group_group_link.group_access) + + group_group_link.human_access + end + end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 842acf92c2a..3531c695236 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1003,6 +1003,57 @@ describe Group do end end + describe '#related_group_ids' do + let(:nested_group) { create(:group, parent: group) } + let(:shared_with_group) { create(:group, parent: group) } + + before do + create(:group_group_link, shared_group: nested_group, + shared_with_group: shared_with_group) + end + + subject(:related_group_ids) { nested_group.related_group_ids } + + it 'returns id' do + expect(related_group_ids).to include(nested_group.id) + end + + it 'returns ancestor id' do + expect(related_group_ids).to include(group.id) + end + + it 'returns shared with group id' do + expect(related_group_ids).to include(shared_with_group.id) + end + + context 'with more than one ancestor group' do + let(:ancestor_group) { create(:group) } + + before do + group.update(parent: ancestor_group) + end + + it 'returns all ancestor group ids' do + expect(related_group_ids).to( + include(group.id, ancestor_group.id)) + end + end + + context 'with more than one shared with group' do + let(:another_shared_with_group) { create(:group, parent: group) } + + before do + create(:group_group_link, shared_group: nested_group, + shared_with_group: another_shared_with_group) + end + + it 'returns all shared with group ids' do + expect(related_group_ids).to( + include(shared_with_group.id, another_shared_with_group.id)) + end + end + end + context 'with uploads' do it_behaves_like 'model with uploads', true do let(:model_object) { create(:group, :with_avatar) } diff --git a/spec/requests/self_monitoring_project_spec.rb b/spec/requests/self_monitoring_project_spec.rb index f3ffa8e0ea4..d562a34aec4 100644 --- a/spec/requests/self_monitoring_project_spec.rb +++ b/spec/requests/self_monitoring_project_spec.rb @@ -60,7 +60,7 @@ describe 'Self-Monitoring project requests' do end it_behaves_like 'sets polling header and returns accepted' do - let(:in_progress_message) { 'Job is in progress' } + let(:in_progress_message) { 'Job to create self-monitoring project is in progress' } end end @@ -115,4 +115,110 @@ describe 'Self-Monitoring project requests' do end end end + + describe 'DELETE #delete_self_monitoring_project' do + let(:worker_class) { SelfMonitoringProjectDeleteWorker } + + subject { delete delete_self_monitoring_project_admin_application_settings_path } + + it_behaves_like 'not accessible to non-admin users' + + context 'with admin user' do + before do + login_as(admin) + end + + context 'with feature flag disabled' do + it_behaves_like 'not accessible if feature flag is disabled' + end + + context 'with feature flag enabled' do + let(:status_api) { status_delete_self_monitoring_project_admin_application_settings_path } + + it_behaves_like 'triggers async worker, returns sidekiq job_id with response accepted' + end + end + end + + describe 'GET #status_delete_self_monitoring_project' do + let(:worker_class) { SelfMonitoringProjectDeleteWorker } + let(:job_id) { 'job_id' } + + subject do + get status_delete_self_monitoring_project_admin_application_settings_path, + params: { job_id: job_id } + end + + it_behaves_like 'not accessible to non-admin users' + + context 'with admin user' do + before do + login_as(admin) + end + + context 'with feature flag disabled' do + it_behaves_like 'not accessible if feature flag is disabled' + end + + context 'with feature flag enabled' do + it_behaves_like 'handles invalid job_id' + + context 'when job is in progress' do + before do + allow(worker_class).to receive(:in_progress?) + .with(job_id) + .and_return(true) + + stub_application_setting(instance_administration_project_id: 1) + end + + it_behaves_like 'sets polling header and returns accepted' do + let(:in_progress_message) { 'Job to delete self-monitoring project is in progress' } + end + end + + context 'when self-monitoring project exists and job does not exist' do + before do + stub_application_setting(instance_administration_project_id: 1) + end + + it 'returns bad_request' do + subject + + aggregate_failures do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq( + 'message' => 'Self-monitoring project was not deleted. Please check logs ' \ + 'for any error messages' + ) + end + end + end + + context 'when self-monitoring project does not exist' do + it 'does not need job_id' do + get status_delete_self_monitoring_project_admin_application_settings_path + + aggregate_failures do + expect(response).to have_gitlab_http_status(:success) + expect(json_response).to eq( + 'message' => 'Self-monitoring project has been successfully deleted' + ) + end + end + + it 'returns success with job_id' do + subject + + aggregate_failures do + expect(response).to have_gitlab_http_status(:success) + expect(json_response).to eq( + 'message' => 'Self-monitoring project has been successfully deleted' + ) + end + end + end + end + end + end end diff --git a/spec/support/shared_examples/email_shared_examples.rb b/spec/support/shared_examples/email_shared_examples.rb new file mode 100644 index 00000000000..634a2504766 --- /dev/null +++ b/spec/support/shared_examples/email_shared_examples.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +shared_examples_for 'correctly finds the mail key' do + specify do + expect(Gitlab::Email::Handler).to receive(:for).with(an_instance_of(Mail::Message), 'gitlabhq/gitlabhq+auth_token').and_return(handler) + + receiver.execute + end +end diff --git a/spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb b/spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb index 8e1d24c4be2..98010150e65 100644 --- a/spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb +++ b/spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb @@ -25,7 +25,7 @@ RSpec.shared_examples 'Maintainer manages access requests' do expect_no_visible_access_request(entity, user) - page.within('.members-list') do + page.within('[data-qa-selector="members_list"]') do expect(page).to have_content user.name end end diff --git a/spec/support/shared_examples/unique_ip_check_shared_examples.rb b/spec/support/shared_examples/unique_ip_check_shared_examples.rb index 65d86ddee9e..9bdfa762fc8 100644 --- a/spec/support/shared_examples/unique_ip_check_shared_examples.rb +++ b/spec/support/shared_examples/unique_ip_check_shared_examples.rb @@ -2,6 +2,8 @@ shared_context 'unique ips sign in limit' do include StubENV + let(:request_context) { Gitlab::RequestContext.instance } + before do Gitlab::Redis::Cache.with(&:flushall) Gitlab::Redis::Queues.with(&:flushall) @@ -15,10 +17,13 @@ shared_context 'unique ips sign in limit' do unique_ips_limit_enabled: true, unique_ips_limit_time_window: 10000 ) + + # Make sure we're working with the same reqeust context everywhere + allow(Gitlab::RequestContext).to receive(:instance).and_return(request_context) end def change_ip(ip) - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(ip) + allow(request_context).to receive(:client_ip).and_return(ip) end def request_from_ip(ip) -- cgit v1.2.1