diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-06-16 09:09:15 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-06-16 09:09:15 +0000 |
| commit | b7b44de429911864686599ef1643baf525bf75ec (patch) | |
| tree | 5c066cff07a9900bbc4c1907b0563857b4361b40 | |
| parent | 06bcbc77e472a70b8332150a941539c55953ef2b (diff) | |
| download | gitlab-ce-b7b44de429911864686599ef1643baf525bf75ec.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
102 files changed, 2112 insertions, 830 deletions
diff --git a/.eslintrc.yml b/.eslintrc.yml index 97c24d99871..da771a4a6f6 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -164,8 +164,8 @@ overrides: #'@graphql-eslint/unique-fragment-name': error # TODO: Uncomment these rules when then `schema` is available #'@graphql-eslint/fragments-on-composite-type': error - #'@graphql-eslint/known-argument-names': error - #'@graphql-eslint/known-type-names': error + '@graphql-eslint/known-argument-names': error + '@graphql-eslint/known-type-names': error '@graphql-eslint/no-anonymous-operations': error '@graphql-eslint/unique-operation-name': error '@graphql-eslint/require-id-when-available': error diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 6d6d9f40723..4c4e13b0436 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -589,7 +589,6 @@ Layout/LineLength: - 'app/services/compare_service.rb' - 'app/services/concerns/base_service_utility.rb' - 'app/services/concerns/exclusive_lease_guard.rb' - - 'app/services/concerns/members/bulk_create_users.rb' - 'app/services/concerns/merge_requests/assigns_merge_params.rb' - 'app/services/concerns/rate_limited_service.rb' - 'app/services/concerns/schedule_bulk_repository_shard_moves_methods.rb' diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_settings.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_settings.vue index 12bc5ad3549..4cc00eb01d9 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_variable_settings.vue +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_settings.vue @@ -1,32 +1,9 @@ <script> -import { mapState, mapActions } from 'vuex'; -import CiVariableModal from './ci_variable_modal.vue'; -import CiVariableTable from './ci_variable_table.vue'; - -export default { - components: { - CiVariableModal, - CiVariableTable, - }, - computed: { - ...mapState(['isGroup']), - }, - mounted() { - if (!this.isGroup) { - this.fetchEnvironments(); - } - }, - methods: { - ...mapActions(['fetchEnvironments']), - }, -}; +export default {}; </script> <template> <div class="row"> - <div class="col-lg-12"> - <ci-variable-table /> - <ci-variable-modal /> - </div> + <div class="col-lg-12"></div> </div> </template> diff --git a/app/assets/javascripts/ci_variable_list/components/legacy_ci_environments_dropdown.vue b/app/assets/javascripts/ci_variable_list/components/legacy_ci_environments_dropdown.vue new file mode 100644 index 00000000000..ecb39f214ec --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/legacy_ci_environments_dropdown.vue @@ -0,0 +1,81 @@ +<script> +import { GlDropdown, GlDropdownItem, GlDropdownDivider, GlSearchBoxByType } from '@gitlab/ui'; +import { mapGetters } from 'vuex'; +import { __, sprintf } from '~/locale'; + +export default { + name: 'CiEnvironmentsDropdown', + components: { + GlDropdown, + GlDropdownItem, + GlDropdownDivider, + GlSearchBoxByType, + }, + props: { + value: { + type: String, + required: false, + default: '', + }, + }, + data() { + return { + searchTerm: '', + }; + }, + computed: { + ...mapGetters(['joinedEnvironments']), + composedCreateButtonLabel() { + return sprintf(__('Create wildcard: %{searchTerm}'), { searchTerm: this.searchTerm }); + }, + shouldRenderCreateButton() { + return this.searchTerm && !this.joinedEnvironments.includes(this.searchTerm); + }, + filteredResults() { + const lowerCasedSearchTerm = this.searchTerm.toLowerCase(); + return this.joinedEnvironments.filter((resultString) => + resultString.toLowerCase().includes(lowerCasedSearchTerm), + ); + }, + }, + methods: { + selectEnvironment(selected) { + this.$emit('selectEnvironment', selected); + this.searchTerm = ''; + }, + createClicked() { + this.$emit('createClicked', this.searchTerm); + this.searchTerm = ''; + }, + isSelected(env) { + return this.value === env; + }, + clearSearch() { + this.searchTerm = ''; + }, + }, +}; +</script> +<template> + <gl-dropdown :text="value" @show="clearSearch"> + <gl-search-box-by-type v-model.trim="searchTerm" data-testid="ci-environment-search" /> + <gl-dropdown-item + v-for="environment in filteredResults" + :key="environment" + :is-checked="isSelected(environment)" + is-check-item + @click="selectEnvironment(environment)" + > + {{ environment }} + </gl-dropdown-item> + <gl-dropdown-item v-if="!filteredResults.length" ref="noMatchingResults">{{ + __('No matching results') + }}</gl-dropdown-item> + <template v-if="shouldRenderCreateButton"> + <gl-dropdown-divider /> + <gl-dropdown-item data-testid="create-wildcard-button" @click="createClicked"> + {{ composedCreateButtonLabel }} + </gl-dropdown-item> + </template> + </gl-dropdown> +</template> diff --git a/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_modal.vue b/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_modal.vue new file mode 100644 index 00000000000..7dcc5ce42d7 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_modal.vue @@ -0,0 +1,426 @@ +<script> +import { + GlAlert, + GlButton, + GlCollapse, + GlFormCheckbox, + GlFormCombobox, + GlFormGroup, + GlFormSelect, + GlFormInput, + GlFormTextarea, + GlIcon, + GlLink, + GlModal, + GlSprintf, +} from '@gitlab/ui'; +import { mapActions, mapState } from 'vuex'; +import { getCookie, setCookie } from '~/lib/utils/common_utils'; +import { __ } from '~/locale'; +import Tracking from '~/tracking'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import { mapComputed } from '~/vuex_shared/bindings'; +import { + AWS_TOKEN_CONSTANTS, + ADD_CI_VARIABLE_MODAL_ID, + AWS_TIP_DISMISSED_COOKIE_NAME, + AWS_TIP_MESSAGE, + CONTAINS_VARIABLE_REFERENCE_MESSAGE, + ENVIRONMENT_SCOPE_LINK_TITLE, + EVENT_LABEL, + EVENT_ACTION, +} from '../constants'; +import CiEnvironmentsDropdown from './ci_environments_dropdown.vue'; +import { awsTokens, awsTokenList } from './ci_variable_autocomplete_tokens'; + +const trackingMixin = Tracking.mixin({ label: EVENT_LABEL }); + +export default { + modalId: ADD_CI_VARIABLE_MODAL_ID, + tokens: awsTokens, + tokenList: awsTokenList, + awsTipMessage: AWS_TIP_MESSAGE, + containsVariableReferenceMessage: CONTAINS_VARIABLE_REFERENCE_MESSAGE, + environmentScopeLinkTitle: ENVIRONMENT_SCOPE_LINK_TITLE, + components: { + CiEnvironmentsDropdown, + GlAlert, + GlButton, + GlCollapse, + GlFormCheckbox, + GlFormCombobox, + GlFormGroup, + GlFormSelect, + GlFormInput, + GlFormTextarea, + GlIcon, + GlLink, + GlModal, + GlSprintf, + }, + mixins: [glFeatureFlagsMixin(), trackingMixin], + data() { + return { + isTipDismissed: getCookie(AWS_TIP_DISMISSED_COOKIE_NAME) === 'true', + validationErrorEventProperty: '', + }; + }, + computed: { + ...mapState([ + 'projectId', + 'environments', + 'typeOptions', + 'variable', + 'variableBeingEdited', + 'isGroup', + 'maskableRegex', + 'selectedEnvironment', + 'isProtectedByDefault', + 'awsLogoSvgPath', + 'awsTipDeployLink', + 'awsTipCommandsLink', + 'awsTipLearnLink', + 'containsVariableReferenceLink', + 'protectedEnvironmentVariablesLink', + 'maskedEnvironmentVariablesLink', + 'environmentScopeLink', + ]), + ...mapComputed( + [ + { key: 'key', updateFn: 'updateVariableKey' }, + { key: 'secret_value', updateFn: 'updateVariableValue' }, + { key: 'variable_type', updateFn: 'updateVariableType' }, + { key: 'environment_scope', updateFn: 'setEnvironmentScope' }, + { key: 'protected_variable', updateFn: 'updateVariableProtected' }, + { key: 'masked', updateFn: 'updateVariableMasked' }, + ], + false, + 'variable', + ), + isTipVisible() { + return !this.isTipDismissed && AWS_TOKEN_CONSTANTS.includes(this.variable.key); + }, + canSubmit() { + return ( + this.variableValidationState && + this.variable.key !== '' && + this.variable.secret_value !== '' + ); + }, + canMask() { + const regex = RegExp(this.maskableRegex); + return regex.test(this.variable.secret_value); + }, + containsVariableReference() { + const regex = /\$/; + return regex.test(this.variable.secret_value); + }, + displayMaskedError() { + return !this.canMask && this.variable.masked; + }, + maskedState() { + if (this.displayMaskedError) { + return false; + } + return true; + }, + modalActionText() { + return this.variableBeingEdited ? __('Update variable') : __('Add variable'); + }, + maskedFeedback() { + return this.displayMaskedError ? __('This variable can not be masked.') : ''; + }, + tokenValidationFeedback() { + const tokenSpecificFeedback = this.$options.tokens?.[this.variable.key]?.invalidMessage; + if (!this.tokenValidationState && tokenSpecificFeedback) { + return tokenSpecificFeedback; + } + return ''; + }, + tokenValidationState() { + const validator = this.$options.tokens?.[this.variable.key]?.validation; + + if (validator) { + return validator(this.variable.secret_value); + } + + return true; + }, + scopedVariablesAvailable() { + return !this.isGroup || this.glFeatures.groupScopedCiVariables; + }, + variableValidationFeedback() { + return `${this.tokenValidationFeedback} ${this.maskedFeedback}`; + }, + variableValidationState() { + return this.variable.secret_value === '' || (this.tokenValidationState && this.maskedState); + }, + }, + watch: { + variable: { + handler() { + this.trackVariableValidationErrors(); + }, + deep: true, + }, + }, + methods: { + ...mapActions([ + 'addVariable', + 'updateVariable', + 'resetEditing', + 'displayInputValue', + 'clearModal', + 'deleteVariable', + 'setEnvironmentScope', + 'addWildCardScope', + 'resetSelectedEnvironment', + 'setSelectedEnvironment', + 'setVariableProtected', + ]), + dismissTip() { + setCookie(AWS_TIP_DISMISSED_COOKIE_NAME, 'true', { expires: 90 }); + this.isTipDismissed = true; + }, + deleteVarAndClose() { + this.deleteVariable(); + this.hideModal(); + }, + hideModal() { + this.$refs.modal.hide(); + }, + resetModalHandler() { + if (this.variableBeingEdited) { + this.resetEditing(); + } + + this.clearModal(); + this.resetSelectedEnvironment(); + this.resetValidationErrorEvents(); + }, + updateOrAddVariable() { + if (this.variableBeingEdited) { + this.updateVariable(); + } else { + this.addVariable(); + } + this.hideModal(); + }, + setVariableProtectedByDefault() { + if (this.isProtectedByDefault && !this.variableBeingEdited) { + this.setVariableProtected(); + } + }, + trackVariableValidationErrors() { + const property = this.getTrackingErrorProperty(); + if (!this.validationErrorEventProperty && property) { + this.track(EVENT_ACTION, { property }); + this.validationErrorEventProperty = property; + } + }, + getTrackingErrorProperty() { + let property; + if (this.variable.secret_value?.length && !property) { + if (this.displayMaskedError && this.maskableRegex?.length) { + const supportedChars = this.maskableRegex.replace('^', '').replace(/{(\d,)}\$/, ''); + const regex = new RegExp(supportedChars, 'g'); + property = this.variable.secret_value.replace(regex, ''); + } + if (this.containsVariableReference) { + property = '$'; + } + } + + return property; + }, + resetValidationErrorEvents() { + this.validationErrorEventProperty = ''; + }, + }, +}; +</script> + +<template> + <gl-modal + ref="modal" + :modal-id="$options.modalId" + :title="modalActionText" + static + lazy + @hidden="resetModalHandler" + @shown="setVariableProtectedByDefault" + > + <form> + <gl-form-combobox + v-model="key" + :token-list="$options.tokenList" + :label-text="__('Key')" + data-qa-selector="ci_variable_key_field" + /> + + <gl-form-group + :label="__('Value')" + label-for="ci-variable-value" + :state="variableValidationState" + :invalid-feedback="variableValidationFeedback" + > + <gl-form-textarea + id="ci-variable-value" + ref="valueField" + v-model="secret_value" + :state="variableValidationState" + rows="3" + max-rows="6" + data-qa-selector="ci_variable_value_field" + class="gl-font-monospace!" + /> + </gl-form-group> + + <div class="d-flex"> + <gl-form-group :label="__('Type')" label-for="ci-variable-type" class="w-50 gl-mr-5"> + <gl-form-select id="ci-variable-type" v-model="variable_type" :options="typeOptions" /> + </gl-form-group> + + <gl-form-group label-for="ci-variable-env" class="w-50" data-testid="environment-scope"> + <template #label> + {{ __('Environment scope') }} + <gl-link + :title="$options.environmentScopeLinkTitle" + :href="environmentScopeLink" + target="_blank" + data-testid="environment-scope-link" + > + <gl-icon name="question" :size="12" /> + </gl-link> + </template> + <ci-environments-dropdown + v-if="scopedVariablesAvailable" + class="w-100" + :value="environment_scope" + @selectEnvironment="setEnvironmentScope" + @createClicked="addWildCardScope" + /> + + <gl-form-input v-else v-model="environment_scope" class="w-100" readonly /> + </gl-form-group> + </div> + + <gl-form-group :label="__('Flags')" label-for="ci-variable-flags"> + <gl-form-checkbox + v-model="protected_variable" + class="mb-0" + data-testid="ci-variable-protected-checkbox" + > + {{ __('Protect variable') }} + <gl-link target="_blank" :href="protectedEnvironmentVariablesLink"> + <gl-icon name="question" :size="12" /> + </gl-link> + <p class="gl-mt-2 text-secondary"> + {{ __('Export variable to pipelines running on protected branches and tags only.') }} + </p> + </gl-form-checkbox> + + <gl-form-checkbox + ref="masked-ci-variable" + v-model="masked" + data-testid="ci-variable-masked-checkbox" + > + {{ __('Mask variable') }} + <gl-link target="_blank" :href="maskedEnvironmentVariablesLink"> + <gl-icon name="question" :size="12" /> + </gl-link> + <p class="gl-mt-2 gl-mb-0 text-secondary"> + {{ __('Variable will be masked in job logs.') }} + <span + :class="{ + 'bold text-plain': displayMaskedError, + }" + > + {{ __('Requires values to meet regular expression requirements.') }}</span + > + <gl-link target="_blank" :href="maskedEnvironmentVariablesLink">{{ + __('More information') + }}</gl-link> + </p> + </gl-form-checkbox> + </gl-form-group> + </form> + <gl-collapse :visible="isTipVisible"> + <gl-alert + :title="__('Deploying to AWS is easy with GitLab')" + variant="tip" + data-testid="aws-guidance-tip" + @dismiss="dismissTip" + > + <div class="gl-display-flex gl-flex-direction-row"> + <div> + <p> + <gl-sprintf :message="$options.awsTipMessage"> + <template #deployLink="{ content }"> + <gl-link :href="awsTipDeployLink" target="_blank">{{ content }}</gl-link> + </template> + <template #commandsLink="{ content }"> + <gl-link :href="awsTipCommandsLink" target="_blank">{{ content }}</gl-link> + </template> + </gl-sprintf> + </p> + <p> + <gl-button + :href="awsTipLearnLink" + target="_blank" + category="secondary" + variant="info" + class="gl-overflow-wrap-break" + >{{ __('Learn more about deploying to AWS') }}</gl-button + > + </p> + </div> + <img + class="gl-mt-3" + :alt="__('Amazon Web Services Logo')" + :src="awsLogoSvgPath" + height="32" + /> + </div> + </gl-alert> + </gl-collapse> + <gl-alert + v-if="containsVariableReference" + :title="__('Value might contain a variable reference')" + :dismissible="false" + variant="warning" + data-testid="contains-variable-reference" + > + <gl-sprintf :message="$options.containsVariableReferenceMessage"> + <template #code="{ content }"> + <code>{{ content }}</code> + </template> + <template #docsLink="{ content }"> + <gl-link :href="containsVariableReferenceLink" target="_blank">{{ content }}</gl-link> + </template> + </gl-sprintf> + </gl-alert> + <template #modal-footer> + <gl-button @click="hideModal">{{ __('Cancel') }}</gl-button> + <gl-button + v-if="variableBeingEdited" + ref="deleteCiVariable" + variant="danger" + category="secondary" + data-qa-selector="ci_variable_delete_button" + @click="deleteVarAndClose" + >{{ __('Delete variable') }}</gl-button + > + <gl-button + ref="updateOrAddVariable" + :disabled="!canSubmit" + variant="confirm" + category="primary" + data-testid="ciUpdateOrAddVariableBtn" + data-qa-selector="ci_variable_save_button" + @click="updateOrAddVariable" + >{{ modalActionText }} + </gl-button> + </template> + </gl-modal> +</template> diff --git a/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_settings.vue b/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_settings.vue new file mode 100644 index 00000000000..9acc9fbffb6 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_settings.vue @@ -0,0 +1,32 @@ +<script> +import { mapState, mapActions } from 'vuex'; +import LegacyCiVariableModal from './legacy_ci_variable_modal.vue'; +import LegacyCiVariableTable from './legacy_ci_variable_table.vue'; + +export default { + components: { + LegacyCiVariableModal, + LegacyCiVariableTable, + }, + computed: { + ...mapState(['isGroup']), + }, + mounted() { + if (!this.isGroup) { + this.fetchEnvironments(); + } + }, + methods: { + ...mapActions(['fetchEnvironments']), + }, +}; +</script> + +<template> + <div class="row"> + <div class="col-lg-12"> + <legacy-ci-variable-table /> + <legacy-ci-variable-modal /> + </div> + </div> +</template> diff --git a/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_table.vue b/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_table.vue new file mode 100644 index 00000000000..f078234829a --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_table.vue @@ -0,0 +1,199 @@ +<script> +import { GlTable, GlButton, GlModalDirective, GlIcon, GlTooltipDirective } from '@gitlab/ui'; +import { mapState, mapActions } from 'vuex'; +import { s__, __ } from '~/locale'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue'; +import { ADD_CI_VARIABLE_MODAL_ID } from '../constants'; +import CiVariablePopover from './ci_variable_popover.vue'; + +export default { + modalId: ADD_CI_VARIABLE_MODAL_ID, + trueIcon: 'mobile-issue-close', + falseIcon: 'close', + iconSize: 16, + fields: [ + { + key: 'variable_type', + label: s__('CiVariables|Type'), + customStyle: { width: '70px' }, + }, + { + key: 'key', + label: s__('CiVariables|Key'), + tdClass: 'text-plain', + sortable: true, + customStyle: { width: '40%' }, + }, + { + key: 'value', + label: s__('CiVariables|Value'), + customStyle: { width: '40%' }, + }, + { + key: 'protected', + label: s__('CiVariables|Protected'), + customStyle: { width: '100px' }, + }, + { + key: 'masked', + label: s__('CiVariables|Masked'), + customStyle: { width: '100px' }, + }, + { + key: 'environment_scope', + label: s__('CiVariables|Environments'), + customStyle: { width: '20%' }, + }, + { + key: 'actions', + label: '', + tdClass: 'text-right', + customStyle: { width: '35px' }, + }, + ], + components: { + CiVariablePopover, + GlButton, + GlIcon, + GlTable, + TooltipOnTruncate, + }, + directives: { + GlModalDirective, + GlTooltip: GlTooltipDirective, + }, + mixins: [glFeatureFlagsMixin()], + computed: { + ...mapState(['variables', 'valuesHidden', 'isLoading', 'isDeleting']), + valuesButtonText() { + return this.valuesHidden ? __('Reveal values') : __('Hide values'); + }, + isTableEmpty() { + return !this.variables || this.variables.length === 0; + }, + fields() { + return this.$options.fields; + }, + }, + mounted() { + this.fetchVariables(); + }, + methods: { + ...mapActions(['fetchVariables', 'toggleValues', 'editVariable']), + }, +}; +</script> + +<template> + <div class="ci-variable-table" data-testid="ci-variable-table"> + <gl-table + :fields="fields" + :items="variables" + tbody-tr-class="js-ci-variable-row" + data-qa-selector="ci_variable_table_content" + sort-by="key" + sort-direction="asc" + stacked="lg" + table-class="text-secondary" + fixed + show-empty + sort-icon-left + no-sort-reset + > + <template #table-colgroup="scope"> + <col v-for="field in scope.fields" :key="field.key" :style="field.customStyle" /> + </template> + <template #cell(key)="{ item }"> + <div class="gl-display-flex gl-align-items-center"> + <tooltip-on-truncate :title="item.key" truncate-target="child"> + <span + :id="`ci-variable-key-${item.id}`" + class="gl-display-inline-block gl-max-w-full gl-text-truncate" + >{{ item.key }}</span + > + </tooltip-on-truncate> + <gl-button + v-gl-tooltip + category="tertiary" + icon="copy-to-clipboard" + :title="__('Copy key')" + :data-clipboard-text="item.key" + :aria-label="__('Copy to clipboard')" + /> + </div> + </template> + <template #cell(value)="{ item }"> + <div class="gl-display-flex gl-align-items-center"> + <span v-if="valuesHidden">*********************</span> + <span + v-else + :id="`ci-variable-value-${item.id}`" + class="gl-display-inline-block gl-max-w-full gl-text-truncate" + >{{ item.value }}</span + > + <gl-button + v-gl-tooltip + category="tertiary" + icon="copy-to-clipboard" + :title="__('Copy value')" + :data-clipboard-text="item.value" + :aria-label="__('Copy to clipboard')" + /> + </div> + </template> + <template #cell(protected)="{ item }"> + <gl-icon v-if="item.protected" :size="$options.iconSize" :name="$options.trueIcon" /> + <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" /> + </template> + <template #cell(masked)="{ item }"> + <gl-icon v-if="item.masked" :size="$options.iconSize" :name="$options.trueIcon" /> + <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" /> + </template> + <template #cell(environment_scope)="{ item }"> + <div class="gl-display-flex"> + <span + :id="`ci-variable-env-${item.id}`" + class="gl-display-inline-block gl-max-w-full gl-text-truncate" + >{{ item.environment_scope }}</span + > + <ci-variable-popover + :target="`ci-variable-env-${item.id}`" + :value="item.environment_scope" + :tooltip-text="__('Copy environment')" + /> + </div> + </template> + <template #cell(actions)="{ item }"> + <gl-button + v-gl-modal-directive="$options.modalId" + icon="pencil" + :aria-label="__('Edit')" + data-qa-selector="edit_ci_variable_button" + @click="editVariable(item)" + /> + </template> + <template #empty> + <p class="gl-text-center gl-py-6 gl-text-black-normal gl-mb-0"> + {{ __('There are no variables yet.') }} + </p> + </template> + </gl-table> + <div class="ci-variable-actions gl-display-flex gl-mt-5"> + <gl-button + v-gl-modal-directive="$options.modalId" + class="gl-mr-3" + data-qa-selector="add_ci_variable_button" + variant="confirm" + category="primary" + >{{ __('Add variable') }}</gl-button + > + <gl-button + v-if="!isTableEmpty" + data-qa-selector="reveal_ci_variable_value_button" + @click="toggleValues(!valuesHidden)" + >{{ valuesButtonText }}</gl-button + > + </div> + </div> +</template> diff --git a/app/assets/javascripts/ci_variable_list/index.js b/app/assets/javascripts/ci_variable_list/index.js index f771751194c..2b54af6a2a4 100644 --- a/app/assets/javascripts/ci_variable_list/index.js +++ b/app/assets/javascripts/ci_variable_list/index.js @@ -1,10 +1,63 @@ import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; import { parseBoolean } from '~/lib/utils/common_utils'; import CiVariableSettings from './components/ci_variable_settings.vue'; +import LegacyCiVariableSettings from './components/legacy_ci_variable_settings.vue'; import createStore from './store'; const mountCiVariableListApp = (containerEl) => { const { + awsLogoSvgPath, + awsTipCommandsLink, + awsTipDeployLink, + awsTipLearnLink, + containsVariableReferenceLink, + environmentScopeLink, + group, + maskedEnvironmentVariablesLink, + maskableRegex, + projectFullPath, + projectId, + protectedByDefault, + protectedEnvironmentVariablesLink, + } = containerEl.dataset; + + const isGroup = parseBoolean(group); + const isProtectedByDefault = parseBoolean(protectedByDefault); + + Vue.use(VueApollo); + + const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), + }); + + return new Vue({ + el: containerEl, + apolloProvider, + provide: { + awsLogoSvgPath, + awsTipCommandsLink, + awsTipDeployLink, + awsTipLearnLink, + containsVariableReferenceLink, + environmentScopeLink, + isGroup, + isProtectedByDefault, + maskedEnvironmentVariablesLink, + maskableRegex, + projectFullPath, + projectId, + protectedEnvironmentVariablesLink, + }, + render(createElement) { + return createElement(CiVariableSettings); + }, + }); +}; + +const mountLegacyCiVariableListApp = (containerEl) => { + const { endpoint, projectId, group, @@ -42,7 +95,7 @@ const mountCiVariableListApp = (containerEl) => { el: containerEl, store, render(createElement) { - return createElement(CiVariableSettings); + return createElement(LegacyCiVariableSettings); }, }); }; @@ -50,5 +103,11 @@ const mountCiVariableListApp = (containerEl) => { export default (containerId = 'js-ci-project-variables') => { const el = document.getElementById(containerId); - return !el ? {} : mountCiVariableListApp(el); + if (el) { + if (gon.features?.ciVariableSettingsGraphql) { + mountCiVariableListApp(el); + } else { + mountLegacyCiVariableListApp(el); + } + } }; diff --git a/app/assets/javascripts/editor/graphql/add_items.mutation.graphql b/app/assets/javascripts/editor/graphql/add_items.mutation.graphql index 13afcc04a48..340a1ec09dc 100644 --- a/app/assets/javascripts/editor/graphql/add_items.mutation.graphql +++ b/app/assets/javascripts/editor/graphql/add_items.mutation.graphql @@ -1,3 +1,3 @@ -mutation addItems($items: [Item]) { +mutation addItems($items: [ItemInput]) { addToolbarItems(items: $items) @client } diff --git a/app/assets/javascripts/editor/graphql/typedefs.graphql b/app/assets/javascripts/editor/graphql/typedefs.graphql index 2433ebf6c66..2733db26282 100644 --- a/app/assets/javascripts/editor/graphql/typedefs.graphql +++ b/app/assets/javascripts/editor/graphql/typedefs.graphql @@ -8,6 +8,16 @@ type Item { selectedLabel: String } +input ItemInput { + id: ID! + label: String! + icon: String + selected: Boolean + group: Int! + category: String + selectedLabel: String +} + type Items { nodes: [Item]! } @@ -17,7 +27,7 @@ extend type Query { } extend type Mutation { - updateToolbarItem(id: ID!, propsToUpdate: Item!): LocalErrors + updateToolbarItem(id: ID!, propsToUpdate: ItemInput!): LocalErrors removeToolbarItems(ids: [ID!]): LocalErrors - addToolbarItems(items: [Item]): LocalErrors + addToolbarItems(items: [ItemInput]): LocalErrors } diff --git a/app/assets/javascripts/editor/graphql/update_item.mutation.graphql b/app/assets/javascripts/editor/graphql/update_item.mutation.graphql index 05c18988c87..f9158570abb 100644 --- a/app/assets/javascripts/editor/graphql/update_item.mutation.graphql +++ b/app/assets/javascripts/editor/graphql/update_item.mutation.graphql @@ -1,3 +1,3 @@ -mutation updateItem($id: ID!, $propsToUpdate: Item!) { +mutation updateItem($id: ID!, $propsToUpdate: ItemInput!) { updateToolbarItem(id: $id, propsToUpdate: $propsToUpdate) @client } diff --git a/app/assets/javascripts/environments/graphql/mutations/action.mutation.graphql b/app/assets/javascripts/environments/graphql/mutations/action.mutation.graphql index bc2c9b33367..33da47aabfb 100644 --- a/app/assets/javascripts/environments/graphql/mutations/action.mutation.graphql +++ b/app/assets/javascripts/environments/graphql/mutations/action.mutation.graphql @@ -1,4 +1,4 @@ -mutation action($action: LocalAction) { +mutation action($action: LocalActionInput) { action(action: $action) @client { errors } diff --git a/app/assets/javascripts/environments/graphql/typedefs.graphql b/app/assets/javascripts/environments/graphql/typedefs.graphql index b4d1f7326f6..4d4cdac0d1d 100644 --- a/app/assets/javascripts/environments/graphql/typedefs.graphql +++ b/app/assets/javascripts/environments/graphql/typedefs.graphql @@ -9,6 +9,11 @@ type LocalEnvironment { autoStopPath: String } +input LocalActionInput { + name: String! + playPath: String +} + input LocalEnvironmentInput { id: Int! globalId: ID! @@ -64,7 +69,7 @@ type LocalPageInfo { extend type Query { environmentApp(page: Int, scope: String): LocalEnvironmentApp - folder(environment: NestedLocalEnvironmentInput): LocalEnvironmentFolder + folder(environment: NestedLocalEnvironmentInput, scope: String): LocalEnvironmentFolder environmentToDelete: LocalEnvironment pageInfo: LocalPageInfo environmentToRollback: LocalEnvironment @@ -82,5 +87,5 @@ extend type Mutation { setEnvironmentToRollback(environment: LocalEnvironmentInput): LocalErrors setEnvironmentToStop(environment: LocalEnvironmentInput): LocalErrors setEnvironmentToChangeCanary(environment: LocalEnvironmentInput, weight: Int): LocalErrors - action(environment: LocalEnvironmentInput): LocalErrors + action(action: LocalActionInput): LocalErrors } diff --git a/app/assets/javascripts/import_entities/import_groups/graphql/mutations/import_groups.mutation.graphql b/app/assets/javascripts/import_entities/import_groups/graphql/mutations/import_groups.mutation.graphql index 39289887b75..78f9af2b71f 100644 --- a/app/assets/javascripts/import_entities/import_groups/graphql/mutations/import_groups.mutation.graphql +++ b/app/assets/javascripts/import_entities/import_groups/graphql/mutations/import_groups.mutation.graphql @@ -1,4 +1,4 @@ -mutation importGroups($importRequests: [ImportGroupInput!]!) { +mutation importGroups($importRequests: [ImportRequestInput!]!) { importGroups(importRequests: $importRequests) @client { id lastImportTarget { diff --git a/app/assets/javascripts/repository/queries/commit.query.graphql b/app/assets/javascripts/repository/queries/commit.query.graphql index 7ae4a3b984a..387d8416105 100644 --- a/app/assets/javascripts/repository/queries/commit.query.graphql +++ b/app/assets/javascripts/repository/queries/commit.query.graphql @@ -1,6 +1,6 @@ #import "ee_else_ce/repository/queries/commit.fragment.graphql" -query getCommit($fileName: String!, $type: String!, $path: String!, $maxOffset: Number!) { +query getCommit($fileName: String!, $type: String!, $path: String!, $maxOffset: Int!) { commit(path: $path, fileName: $fileName, type: $type, maxOffset: $maxOffset) @client { ...TreeEntryCommit } diff --git a/app/assets/javascripts/repository/queries/typedefs.graphql b/app/assets/javascripts/repository/queries/typedefs.graphql new file mode 100644 index 00000000000..3b1c1c3fdcd --- /dev/null +++ b/app/assets/javascripts/repository/queries/typedefs.graphql @@ -0,0 +1,10 @@ +type LogTreeCommit { + sha: String + message: String + titleHtml: String + committedDate: Time + commitPath: String + fileName: String + filePath: String + type: String +} diff --git a/app/assets/javascripts/terraform/graphql/mutations/add_data_to_state.mutation.graphql b/app/assets/javascripts/terraform/graphql/mutations/add_data_to_state.mutation.graphql index 645b9766e2b..cbefaec48bb 100644 --- a/app/assets/javascripts/terraform/graphql/mutations/add_data_to_state.mutation.graphql +++ b/app/assets/javascripts/terraform/graphql/mutations/add_data_to_state.mutation.graphql @@ -1,3 +1,3 @@ -mutation addDataToTerraformState($terraformState: State!) { +mutation addDataToTerraformState($terraformState: LocalTerraformStateInput!) { addDataToTerraformState(terraformState: $terraformState) @client } diff --git a/app/assets/javascripts/terraform/graphql/typedefs.graphql b/app/assets/javascripts/terraform/graphql/typedefs.graphql new file mode 100644 index 00000000000..9ae3fb27d0e --- /dev/null +++ b/app/assets/javascripts/terraform/graphql/typedefs.graphql @@ -0,0 +1,22 @@ +extend type TerraformState { + _showDetails: Boolean + errorMessages: [String] + loadingLock: Boolean + loadingRemove: Boolean +} + +input LocalTerraformStateInput { + _showDetails: Boolean + errorMessages: [String] + loadingLock: Boolean + loadingRemove: Boolean + id: ID! + name: String! + lockedAt: Time + updatedAt: Time! + deletedAt: Time +} + +extend type Mutation { + addDataToTerraformState(terraformState: LocalTerraformStateInput!): Boolean +} diff --git a/app/assets/javascripts/work_items/graphql/typedefs.graphql b/app/assets/javascripts/work_items/graphql/typedefs.graphql index bfe2f0fe0ce..4cbd1dafff0 100644 --- a/app/assets/javascripts/work_items/graphql/typedefs.graphql +++ b/app/assets/javascripts/work_items/graphql/typedefs.graphql @@ -21,7 +21,7 @@ extend type WorkItem { mockWidgets: [LocalWorkItemWidget] } -type LocalWorkItemAssigneesInput { +input LocalWorkItemAssigneesInput { id: WorkItemID! assigneeIds: [ID!] } diff --git a/app/assets/stylesheets/_page_specific_files.scss b/app/assets/stylesheets/_page_specific_files.scss index 24549a170bd..092cf643e0f 100644 --- a/app/assets/stylesheets/_page_specific_files.scss +++ b/app/assets/stylesheets/_page_specific_files.scss @@ -17,7 +17,6 @@ @import './pages/note_form'; @import './pages/notes'; @import './pages/notifications'; -@import './pages/pages'; @import './pages/pipelines'; @import './pages/profile'; @import './pages/profiles/preferences'; diff --git a/app/assets/stylesheets/errors.scss b/app/assets/stylesheets/errors.scss index 00cc3409fa7..3ef6452b706 100644 --- a/app/assets/stylesheets/errors.scss +++ b/app/assets/stylesheets/errors.scss @@ -9,6 +9,10 @@ @import 'bootstrap/scss/buttons'; @import 'bootstrap/scss/forms'; +@import '@gitlab/ui/src/scss/variables'; +@import '@gitlab/ui/src/scss/utility-mixins/index'; +@import '@gitlab/ui/src/components/base/button/button'; + $body-color: #666; $header-color: #456; diff --git a/app/assets/stylesheets/pages/pages.scss b/app/assets/stylesheets/pages/pages.scss deleted file mode 100644 index 2de33f20595..00000000000 --- a/app/assets/stylesheets/pages/pages.scss +++ /dev/null @@ -1,55 +0,0 @@ -.pages-domain-list { - &-item { - align-items: center; - - .domain-status { - display: inline-flex; - left: $gl-padding; - position: absolute; - } - - .domain-name { - flex-grow: 1; - } - - } - - &.has-verification-status > li { - padding-left: 3 * $gl-padding; - } - -} - -.status-badge { - - display: inline-flex; - margin-bottom: $gl-padding-8; - - // Most of the following settings "stolen" from btn-sm - // Border radius is overwritten for both - .label, - .btn { - padding: $gl-padding-4 $gl-padding-8; - font-size: $gl-font-size; - line-height: $gl-btn-line-height; - border-radius: 0; - display: flex; - align-items: center; - } - - .btn svg { - top: auto; - } - - :first-child { - line-height: $gl-line-height; - } - - :not(:first-child) { - border-left: 0; - } - - :last-child { - border-radius: $border-radius-default; - } -} diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 491074f39f9..cda6c8abea7 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -13,6 +13,7 @@ module Projects before_action :define_variables before_action do push_frontend_feature_flag(:ajax_new_deploy_token, @project) + push_frontend_feature_flag(:ci_variable_settings_graphql, @project) end helper_method :highlight_badge diff --git a/app/helpers/invite_members_helper.rb b/app/helpers/invite_members_helper.rb index e46270ab819..5d537767eaf 100644 --- a/app/helpers/invite_members_helper.rb +++ b/app/helpers/invite_members_helper.rb @@ -29,7 +29,7 @@ module InviteMembersHelper invalid_groups: source.related_group_ids, help_link: help_page_url('user/permissions'), is_project: is_project, - access_levels: member_class.access_level_roles.to_json + access_levels: member_class.permissible_access_level_roles(current_user, source).to_json }.merge(group_select_data(source)) end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index d669727d2fd..6112d05f37d 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -52,12 +52,6 @@ module ProjectsHelper content_tag(:span, username, name_tag_options) end - def permissible_access_level_roles(current_user, project) - # Access level roles that the current user is able to grant others. - # This method is a stopgap in preparation for https://gitlab.com/gitlab-org/gitlab/-/issues/364087 - current_user.can?(:manage_owners, project) ? Gitlab::Access.options_with_owner : Gitlab::Access.options - end - def link_to_member(project, author, opts = {}, &block) default_opts = { avatar: true, name: true, title: ":name" } opts = default_opts.merge(opts) diff --git a/app/models/clusters/agent.rb b/app/models/clusters/agent.rb index e7e88a41bab..fb12ce7d292 100644 --- a/app/models/clusters/agent.rb +++ b/app/models/clusters/agent.rb @@ -22,6 +22,7 @@ module Clusters scope :ordered_by_name, -> { order(:name) } scope :with_name, -> (name) { where(name: name) } + scope :has_vulnerabilities, -> (value = true) { where(has_vulnerabilities: value) } validates :name, presence: true, diff --git a/app/models/group.rb b/app/models/group.rb index aa810b62d7f..f5aad6e74ff 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -362,7 +362,7 @@ class Group < Namespace end def add_users(users, access_level, current_user: nil, expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil) - Members::Groups::BulkCreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass + Members::Groups::CreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass self, users, access_level, @@ -374,7 +374,7 @@ class Group < Namespace end def add_user(user, access_level, current_user: nil, expires_at: nil, ldap: false, blocking_refresh: true) - Members::Groups::CreatorService.new( # rubocop:disable CodeReuse/ServiceClass + Members::Groups::CreatorService.add_user( # rubocop:disable CodeReuse/ServiceClass self, user, access_level, @@ -382,7 +382,7 @@ class Group < Namespace expires_at: expires_at, ldap: ldap, blocking_refresh: blocking_refresh - ).execute + ) end def add_guest(user, current_user = nil) diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index 9f45160d3a8..b7ace34141e 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -31,11 +31,6 @@ class ProjectHook < WebHook _('Webhooks') end - override :rate_limit - def rate_limit - project.actual_limits.limit_for(:web_hook_calls) - end - override :application_context def application_context super.merge(project: project) diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index f239c26773e..37fd612e652 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -127,19 +127,12 @@ class WebHook < ApplicationRecord # @return [Boolean] Whether or not the WebHook is currently throttled. def rate_limited? - return false unless rate_limit - - Gitlab::ApplicationRateLimiter.peek( - :web_hook_calls, - scope: [self], - threshold: rate_limit - ) + rate_limiter.rate_limited? end - # Threshold for the rate-limit. - # Overridden in ProjectHook and GroupHook, other WebHooks are not rate-limited. + # @return [Integer] The rate limit for the WebHook. `0` for no limit. def rate_limit - nil + rate_limiter.limit end # Returns the associated Project or Group for the WebHook if one exists. @@ -180,4 +173,8 @@ class WebHook < ApplicationRecord def initialize_url_variables self.url_variables = {} if encrypted_url_variables.nil? end + + def rate_limiter + @rate_limiter ||= Gitlab::WebHooks::RateLimiter.new(self) + end end diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index 112fcc389e3..87af6a9a7f7 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -29,6 +29,12 @@ class GroupMember < Member attr_accessor :last_owner, :last_blocked_owner + # For those who get to see a modal with a role dropdown, here are the options presented + def self.permissible_access_level_roles(_, _) + # This method is a stopgap in preparation for https://gitlab.com/gitlab-org/gitlab/-/issues/364087 + access_level_roles + end + def self.access_level_roles Gitlab::Access.options_with_owner end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index fe69d9f9382..791cb6f0dff 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -44,7 +44,7 @@ class ProjectMember < Member project_ids.each do |project_id| project = Project.find(project_id) - Members::Projects::BulkCreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass + Members::Projects::CreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass project, users, access_level, @@ -73,6 +73,16 @@ class ProjectMember < Member truncate_teams [project.id] end + # For those who get to see a modal with a role dropdown, here are the options presented + def permissible_access_level_roles(current_user, project) + # This method is a stopgap in preparation for https://gitlab.com/gitlab-org/gitlab/-/issues/364087 + if Ability.allowed?(current_user, :manage_owners, project) + Gitlab::Access.options_with_owner + else + ProjectMember.access_level_roles + end + end + def access_level_roles Gitlab::Access.options end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index bb5363598df..97ab5aa2619 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -44,7 +44,7 @@ class ProjectTeam end def add_users(users, access_level, current_user: nil, expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil) - Members::Projects::BulkCreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass + Members::Projects::CreatorService.add_users( # rubocop:disable CodeReuse/ServiceClass project, users, access_level, @@ -56,12 +56,12 @@ class ProjectTeam end def add_user(user, access_level, current_user: nil, expires_at: nil) - Members::Projects::CreatorService.new(project, # rubocop:disable CodeReuse/ServiceClass - user, - access_level, - current_user: current_user, - expires_at: expires_at) - .execute + Members::Projects::CreatorService.add_user( # rubocop:disable CodeReuse/ServiceClass + project, + user, + access_level, + current_user: current_user, + expires_at: expires_at) end # Remove all users from project team diff --git a/app/services/concerns/members/bulk_create_users.rb b/app/services/concerns/members/bulk_create_users.rb deleted file mode 100644 index 5b2cd0a2e43..00000000000 --- a/app/services/concerns/members/bulk_create_users.rb +++ /dev/null @@ -1,93 +0,0 @@ -# frozen_string_literal: true - -module Members - module BulkCreateUsers - extend ActiveSupport::Concern - - included do - class << self - def add_users(source, users, access_level, current_user: nil, expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil) - return [] unless users.present? - - # If this user is attempting to manage Owner members and doesn't have permission, do not allow - return [] if managing_owners?(current_user, access_level) && cannot_manage_owners?(source, current_user) - - emails, users, existing_members = parse_users_list(source, users) - - Member.transaction do - (emails + users).map! do |user| - new(source, - user, - access_level, - existing_members: existing_members, - current_user: current_user, - expires_at: expires_at, - tasks_to_be_done: tasks_to_be_done, - tasks_project_id: tasks_project_id) - .execute - end - end - end - - private - - def managing_owners?(current_user, access_level) - current_user && Gitlab::Access.sym_options_with_owner[access_level] == Gitlab::Access::OWNER - end - - def parse_users_list(source, list) - emails = [] - user_ids = [] - users = [] - existing_members = {} - - list.each do |item| - case item - when User - users << item - when Integer - user_ids << item - when /\A\d+\Z/ - user_ids << item.to_i - when Devise.email_regexp - emails << item - end - end - - # the below will automatically discard invalid user_ids - users.concat(User.id_in(user_ids)) if user_ids.present? - users.uniq! # de-duplicate just in case as there is no controlling if user records and ids are sent multiple times - - users_by_emails = source.users_by_emails(emails) # preloads our request store for all emails - # in case emails belong to a user that is being invited by user or user_id, remove them from - # emails and let users/user_ids handle it. - parsed_emails = emails.select do |email| - user = users_by_emails[email] - !user || (users.exclude?(user) && user_ids.exclude?(user.id)) - end - - if users.present? - # helps not have to perform another query per user id to see if the member exists later on when fetching - existing_members = source.members_and_requesters.with_user(users).index_by(&:user_id) - end - - [parsed_emails, users, existing_members] - end - end - end - - def initialize(source, user, access_level, **args) - super - - @existing_members = args[:existing_members] || (raise ArgumentError, "existing_members must be included in the args hash") - end - - private - - attr_reader :existing_members - - def find_or_initialize_member_by_user - existing_members[user.id] || source.members.build(user_id: user.id) - end - end -end diff --git a/app/services/jira_connect_subscriptions/create_service.rb b/app/services/jira_connect_subscriptions/create_service.rb index 2f31a3c8d4e..d5ab3800dcf 100644 --- a/app/services/jira_connect_subscriptions/create_service.rb +++ b/app/services/jira_connect_subscriptions/create_service.rb @@ -5,13 +5,18 @@ module JiraConnectSubscriptions include Gitlab::Utils::StrongMemoize MERGE_REQUEST_SYNC_BATCH_SIZE = 20 MERGE_REQUEST_SYNC_BATCH_DELAY = 1.minute.freeze - NOT_SITE_ADMIN = 'The Jira user is not a site administrator.' def execute - return error(NOT_SITE_ADMIN, 403) unless can_administer_jira? + if !params[:jira_user] + return error(s_('JiraConnect|Could not fetch user information from Jira. ' \ + 'Check the permissions in Jira and try again.'), 403) + elsif !can_administer_jira? + return error(s_('JiraConnect|The Jira user is not a site administrator. ' \ + 'Check the permissions in Jira and try again.'), 403) + end unless namespace && can?(current_user, :create_jira_connect_subscription, namespace) - return error('Invalid namespace. Please make sure you have sufficient permissions', 401) + return error(s_('JiraConnect|Cannot find namespace. Make sure you have sufficient permissions.'), 401) end create_subscription @@ -20,7 +25,7 @@ module JiraConnectSubscriptions private def can_administer_jira? - @params[:jira_user]&.site_admin? + params[:jira_user]&.site_admin? end def create_subscription diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb index 81986a2883f..276093a00a9 100644 --- a/app/services/members/creator_service.rb +++ b/app/services/members/creator_service.rb @@ -12,6 +12,105 @@ module Members def access_levels Gitlab::Access.sym_options_with_owner end + + def add_users( # rubocop:disable Metrics/ParameterLists + source, + users, + access_level, + current_user: nil, + expires_at: nil, + tasks_to_be_done: [], + tasks_project_id: nil, + ldap: nil, + blocking_refresh: nil + ) + return [] unless users.present? + + # If this user is attempting to manage Owner members and doesn't have permission, do not allow + return [] if managing_owners?(current_user, access_level) && cannot_manage_owners?(source, current_user) + + emails, users, existing_members = parse_users_list(source, users) + + Member.transaction do + (emails + users).map! do |user| + new(source, + user, + access_level, + existing_members: existing_members, + current_user: current_user, + expires_at: expires_at, + tasks_to_be_done: tasks_to_be_done, + tasks_project_id: tasks_project_id, + ldap: ldap, + blocking_refresh: blocking_refresh) + .execute + end + end + end + + def add_user( # rubocop:disable Metrics/ParameterLists + source, + user, + access_level, + current_user: nil, + expires_at: nil, + ldap: nil, + blocking_refresh: nil + ) + add_users(source, + [user], + access_level, + current_user: current_user, + expires_at: expires_at, + ldap: ldap, + blocking_refresh: blocking_refresh).first + end + + private + + def managing_owners?(current_user, access_level) + current_user && Gitlab::Access.sym_options_with_owner[access_level] == Gitlab::Access::OWNER + end + + def parse_users_list(source, list) + emails = [] + user_ids = [] + users = [] + existing_members = {} + + list.each do |item| + case item + when User + users << item + when Integer + user_ids << item + when /\A\d+\Z/ + user_ids << item.to_i + when Devise.email_regexp + emails << item + end + end + + # the below will automatically discard invalid user_ids + users.concat(User.id_in(user_ids)) if user_ids.present? + # de-duplicate just in case as there is no controlling if user records and ids are sent multiple times + users.uniq! + + users_by_emails = source.users_by_emails(emails) # preloads our request store for all emails + # in case emails belong to a user that is being invited by user or user_id, remove them from + # emails and let users/user_ids handle it. + parsed_emails = emails.select do |email| + user = users_by_emails[email] + !user || (users.exclude?(user) && user_ids.exclude?(user.id)) + end + + if users.present? || users_by_emails.present? + # helps not have to perform another query per user id to see if the member exists later on when fetching + existing_members = source.members_and_requesters.with_user(users + users_by_emails.values).index_by(&:user_id) + end + + [parsed_emails, users, existing_members] + end end def initialize(source, user, access_level, **args) @@ -21,10 +120,12 @@ module Members @args = args end + private_class_method :new + def execute find_or_build_member commit_member - create_member_task + after_commit_tasks member end @@ -92,6 +193,10 @@ module Members end end + def after_commit_tasks + create_member_task + end + def create_member_task return unless member.persisted? return if member_task_attributes.value?(nil) @@ -163,15 +268,19 @@ module Members end def find_or_initialize_member_by_user - # have to use members and requesters here since project/group limits on requested_at being nil for members and - # wouldn't be found in `source.members` if it already existed - # this of course will not treat active invites the same since we aren't searching on email - source.members_and_requesters.find_or_initialize_by(user_id: user.id) # rubocop:disable CodeReuse/ActiveRecord + # We have to use `members_and_requesters` here since the given `members` is modified in the models + # to act more like a scope(removing the requested_at members) and therefore ActiveRecord has issues with that + # on build and refreshing that relation. + existing_members[user.id] || source.members_and_requesters.build(user_id: user.id) # rubocop:disable CodeReuse/ActiveRecord end def ldap args[:ldap] || false end + + def existing_members + args[:existing_members] || {} + end end end diff --git a/app/services/members/groups/bulk_creator_service.rb b/app/services/members/groups/bulk_creator_service.rb deleted file mode 100644 index e93ef0e021c..00000000000 --- a/app/services/members/groups/bulk_creator_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module Members - module Groups - class BulkCreatorService < Members::Groups::CreatorService - include Members::BulkCreateUsers - - class << self - def cannot_manage_owners?(source, current_user) - source.max_member_access_for_user(current_user) < Gitlab::Access::OWNER - end - end - end - end -end diff --git a/app/services/members/groups/creator_service.rb b/app/services/members/groups/creator_service.rb index a6f0daa99aa..dd3d44e4d96 100644 --- a/app/services/members/groups/creator_service.rb +++ b/app/services/members/groups/creator_service.rb @@ -3,6 +3,12 @@ module Members module Groups class CreatorService < Members::CreatorService + class << self + def cannot_manage_owners?(source, current_user) + source.max_member_access_for_user(current_user) < Gitlab::Access::OWNER + end + end + private def can_create_new_member? diff --git a/app/services/members/projects/bulk_creator_service.rb b/app/services/members/projects/bulk_creator_service.rb deleted file mode 100644 index 17488aa3b38..00000000000 --- a/app/services/members/projects/bulk_creator_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module Members - module Projects - class BulkCreatorService < Members::Projects::CreatorService - include Members::BulkCreateUsers - - class << self - def cannot_manage_owners?(source, current_user) - !Ability.allowed?(current_user, :manage_owners, source) - end - end - end - end -end diff --git a/app/services/members/projects/creator_service.rb b/app/services/members/projects/creator_service.rb index 56d005e24ab..cde1d0462e8 100644 --- a/app/services/members/projects/creator_service.rb +++ b/app/services/members/projects/creator_service.rb @@ -3,6 +3,12 @@ module Members module Projects class CreatorService < Members::CreatorService + class << self + def cannot_manage_owners?(source, current_user) + !Ability.allowed?(current_user, :manage_owners, source) + end + end + private def can_create_new_member? diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 7caccd13a99..f2f94563e56 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -104,7 +104,7 @@ class WebHookService def async_execute Gitlab::ApplicationContext.with_context(hook.application_context) do - break log_rate_limited if rate_limited? + break log_rate_limited if rate_limit! break log_recursion_blocked if recursion_blocked? params = { @@ -215,24 +215,16 @@ class WebHookService string_size_limit(response_body, RESPONSE_BODY_SIZE_LIMIT) end - def rate_limited? - return false if rate_limit.nil? - - Gitlab::ApplicationRateLimiter.throttled?( - :web_hook_calls, - scope: [hook], - threshold: rate_limit - ) + # Increments rate-limit counter. + # Returns true if hook should be rate-limited. + def rate_limit! + Gitlab::WebHooks::RateLimiter.new(hook).rate_limit! end def recursion_blocked? Gitlab::WebHooks::RecursionDetection.block?(hook) end - def rate_limit - @rate_limit ||= hook.rate_limit - end - def log_rate_limited log_auth_error('Webhook rate limit exceeded') end diff --git a/app/views/errors/access_denied.html.haml b/app/views/errors/access_denied.html.haml index e368ee1a75a..56c26f93402 100644 --- a/app/views/errors/access_denied.html.haml +++ b/app/views/errors/access_denied.html.haml @@ -11,6 +11,6 @@ %p = s_('403|Please contact your GitLab administrator to get permission.') .action-container.js-go-back{ hidden: true } - %button{ type: 'button', class: 'gl-button btn btn-success' } + = render Pajamas::ButtonComponent.new(variant: :confirm) do = _('Go Back') = render "errors/footer" diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml index 51e9e344d26..3b117022d1e 100644 --- a/app/views/groups/settings/ci_cd/show.html.haml +++ b/app/views/groups/settings/ci_cd/show.html.haml @@ -52,5 +52,4 @@ .settings-content = render 'groups/settings/ci_cd/auto_devops_form', group: @group -- if ::Feature.enabled?(:group_level_protected_environment, @group) - = render_if_exists 'groups/settings/ci_cd/protected_environments', expanded: expanded += render_if_exists 'groups/settings/ci_cd/protected_environments', expanded: expanded diff --git a/app/views/projects/_invite_members_modal.html.haml b/app/views/projects/_invite_members_modal.html.haml index 6375a56bf5d..16288f4357a 100644 --- a/app/views/projects/_invite_members_modal.html.haml +++ b/app/views/projects/_invite_members_modal.html.haml @@ -1,5 +1,5 @@ - return unless can_admin_project_member?(project) .js-invite-members-modal{ data: { is_project: 'true', - access_levels: ProjectMember.access_level_roles.to_json, + access_levels: ProjectMember.permissible_access_level_roles(current_user, project).to_json, help_link: help_page_url('user/permissions') }.merge(common_invite_modal_dataset(project)).merge(users_filter_data(project.group)) } diff --git a/app/views/projects/pages/_list.html.haml b/app/views/projects/pages/_list.html.haml index 04178804de4..0ddf105ef60 100644 --- a/app/views/projects/pages/_list.html.haml +++ b/app/views/projects/pages/_list.html.haml @@ -4,20 +4,21 @@ .card .card-header Domains (#{@domains.size}) - %ul.list-group.list-group-flush.pages-domain-list{ class: ("has-verification-status" if verification_enabled) } + %ul.list-group.list-group-flush - @domains.each do |domain| - %li.pages-domain-list-item.list-group-item.d-flex.justify-content-between - - if verification_enabled - - tooltip, status = domain.unverified? ? [s_('GitLabPages|Unverified'), 'failed'] : [s_('GitLabPages|Verified'), 'success'] - .domain-status.ci-status-icon.has-tooltip{ class: "ci-status-icon-#{status}", title: tooltip } - = sprite_icon("status_#{status}" ) - .domain-name - = external_link(domain.url, domain.url) - - if domain.certificate - %div - = gl_badge_tag(s_('GitLabPages|Certificate: %{subject}') % { subject: domain.pages_domain.subject }) - - if domain.expired? - = gl_badge_tag s_('GitLabPages|Expired'), variant: :danger + %li.list-group-item.gl-display-flex.gl-justify-content-space-between.gl-align-items-center + .gl-display-flex.gl-align-items-center + - if verification_enabled + - tooltip, status = domain.unverified? ? [s_('GitLabPages|Unverified'), 'failed'] : [s_('GitLabPages|Verified'), 'success'] + .domain-status.ci-status-icon.has-tooltip{ class: "gl-mr-5 ci-status-icon-#{status}", title: tooltip } + = sprite_icon("status_#{status}" ) + .domain-name + = external_link(domain.url, domain.url) + - if domain.certificate + %div + = gl_badge_tag(s_('GitLabPages|Certificate: %{subject}') % { subject: domain.pages_domain.subject }) + - if domain.expired? + = gl_badge_tag s_('GitLabPages|Expired'), variant: :danger %div = link_to s_('GitLabPages|Edit'), project_pages_domain_path(@project, domain), class: "btn gl-button btn-sm btn-grouped btn-confirm btn-inverted" = link_to s_('GitLabPages|Remove'), project_pages_domain_path(@project, domain), data: { confirm: s_('GitLabPages|Are you sure?'), 'confirm-btn-variant': 'danger'}, "aria-label": s_("GitLabPages|Remove domain"), method: :delete, class: "btn gl-button btn-danger btn-sm btn-grouped" diff --git a/app/views/projects/pages_domains/_dns.html.haml b/app/views/projects/pages_domains/_dns.html.haml index 6943469aaac..2c6b808eb1c 100644 --- a/app/views/projects/pages_domains/_dns.html.haml +++ b/app/views/projects/pages_domains/_dns.html.haml @@ -19,10 +19,10 @@ .col-sm-2 = _("Verification status") .col-sm-10 - .status-badge + .gl-mb-3 - text, status = domain_presenter.unverified? ? [_('Unverified'), :danger] : [_('Verified'), :success] = gl_badge_tag text, variant: status - = link_to sprite_icon("redo"), verify_project_pages_domain_path(@project, domain_presenter), method: :post, class: "gl-ml-2 gl-button btn btn-default has-tooltip", title: _("Retry verification") + = link_to sprite_icon("redo"), verify_project_pages_domain_path(@project, domain_presenter), method: :post, class: "gl-ml-2 gl-button btn btn-sm btn-default has-tooltip", title: _("Retry verification") .input-group = text_field_tag :domain_verification, verification_record, class: "monospace js-select-on-focus form-control", readonly: true .input-group-append diff --git a/app/views/projects/settings/access_tokens/index.html.haml b/app/views/projects/settings/access_tokens/index.html.haml index bd9c9a05e5e..359e34d8918 100644 --- a/app/views/projects/settings/access_tokens/index.html.haml +++ b/app/views/projects/settings/access_tokens/index.html.haml @@ -36,7 +36,7 @@ resource: @project, token: @resource_access_token, scopes: @scopes, - access_levels: permissible_access_level_roles(current_user, @project), + access_levels: ProjectMember.permissible_access_level_roles(current_user, @project), default_access_level: Gitlab::Access::MAINTAINER, prefix: :resource_access_token, help_path: help_page_path('user/project/settings/project_access_tokens', anchor: 'scopes-for-a-project-access-token') diff --git a/config/feature_flags/development/group_level_protected_environment.yml b/config/feature_flags/development/ci_variable_settings_graphql.yml index f52838f4a46..0af109968ab 100644 --- a/config/feature_flags/development/group_level_protected_environment.yml +++ b/config/feature_flags/development/ci_variable_settings_graphql.yml @@ -1,8 +1,8 @@ --- -name: group_level_protected_environment -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/88506 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/363450 +name: ci_variable_settings_graphql +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89332 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/364423 milestone: '15.1' type: development -group: group::release +group: group::pipeline authoring default_enabled: false diff --git a/db/migrate/20220523030804_add_web_hook_calls_med_and_max_to_plan_limits.rb b/db/migrate/20220523030804_add_web_hook_calls_med_and_max_to_plan_limits.rb new file mode 100644 index 00000000000..c1ed306551f --- /dev/null +++ b/db/migrate/20220523030804_add_web_hook_calls_med_and_max_to_plan_limits.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddWebHookCallsMedAndMaxToPlanLimits < Gitlab::Database::Migration[2.0] + def change + add_column :plan_limits, :web_hook_calls_mid, :integer, null: false, default: 0 + add_column :plan_limits, :web_hook_calls_low, :integer, null: false, default: 0 + end +end diff --git a/db/migrate/20220523030805_add_web_hook_calls_to_plan_limits_paid_tiers.rb b/db/migrate/20220523030805_add_web_hook_calls_to_plan_limits_paid_tiers.rb new file mode 100644 index 00000000000..842bb297803 --- /dev/null +++ b/db/migrate/20220523030805_add_web_hook_calls_to_plan_limits_paid_tiers.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +class AddWebHookCallsToPlanLimitsPaidTiers < Gitlab::Database::Migration[2.0] + restrict_gitlab_migration gitlab_schema: :gitlab_main + + MAX_RATE_LIMIT_NAME = 'web_hook_calls' + MID_RATE_LIMIT_NAME = 'web_hook_calls_mid' + MIN_RATE_LIMIT_NAME = 'web_hook_calls_low' + + UP_FREE_LIMITS = { + MAX_RATE_LIMIT_NAME => 500, + MID_RATE_LIMIT_NAME => 500, + MIN_RATE_LIMIT_NAME => 500 + }.freeze + + UP_PREMIUM_LIMITS = { + MAX_RATE_LIMIT_NAME => 4_000, + MID_RATE_LIMIT_NAME => 2_800, + MIN_RATE_LIMIT_NAME => 1_600 + }.freeze + + UP_ULTIMATE_LIMITS = { + MAX_RATE_LIMIT_NAME => 13_000, + MID_RATE_LIMIT_NAME => 9_000, + MIN_RATE_LIMIT_NAME => 6_000 + }.freeze + + DOWN_FREE_LIMITS = { + # 120 is the value for 'free' migrated in `db/migrate/20210601131742_update_web_hook_calls_limit.rb` + MAX_RATE_LIMIT_NAME => 120, + MID_RATE_LIMIT_NAME => 0, + MIN_RATE_LIMIT_NAME => 0 + }.freeze + + DOWN_PAID_LIMITS = { + MAX_RATE_LIMIT_NAME => 0, + MID_RATE_LIMIT_NAME => 0, + MIN_RATE_LIMIT_NAME => 0 + }.freeze + + def up + return unless Gitlab.com? + + apply_limits('free', UP_FREE_LIMITS) + + # Apply Premium limits + apply_limits('bronze', UP_PREMIUM_LIMITS) + apply_limits('silver', UP_PREMIUM_LIMITS) + apply_limits('premium', UP_PREMIUM_LIMITS) + apply_limits('premium_trial', UP_PREMIUM_LIMITS) + + # Apply Ultimate limits + apply_limits('gold', UP_ULTIMATE_LIMITS) + apply_limits('ultimate', UP_ULTIMATE_LIMITS) + apply_limits('ultimate_trial', UP_ULTIMATE_LIMITS) + apply_limits('opensource', UP_ULTIMATE_LIMITS) + end + + def down + return unless Gitlab.com? + + apply_limits('free', DOWN_FREE_LIMITS) + + apply_limits('bronze', DOWN_PAID_LIMITS) + apply_limits('silver', DOWN_PAID_LIMITS) + apply_limits('premium', DOWN_PAID_LIMITS) + apply_limits('premium_trial', DOWN_PAID_LIMITS) + apply_limits('gold', DOWN_PAID_LIMITS) + apply_limits('ultimate', DOWN_PAID_LIMITS) + apply_limits('ultimate_trial', DOWN_PAID_LIMITS) + apply_limits('opensource', DOWN_PAID_LIMITS) + end + + private + + def apply_limits(plan_name, limits) + limits.each_pair do |limit_name, limit| + create_or_update_plan_limit(limit_name, plan_name, limit) + end + end +end diff --git a/db/migrate/20220530104431_add_timestamps_to_compliance_frameworks.rb b/db/migrate/20220530104431_add_timestamps_to_compliance_frameworks.rb new file mode 100644 index 00000000000..88013fddc81 --- /dev/null +++ b/db/migrate/20220530104431_add_timestamps_to_compliance_frameworks.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddTimestampsToComplianceFrameworks < Gitlab::Database::Migration[2.0] + def up + add_column :compliance_management_frameworks, :created_at, :datetime_with_timezone, null: true + add_column :compliance_management_frameworks, :updated_at, :datetime_with_timezone, null: true + end + + def down + remove_column :compliance_management_frameworks, :created_at + remove_column :compliance_management_frameworks, :updated_at + end +end diff --git a/db/migrate/20220614095912_add_has_vulnerabilities_to_cluster_agents.rb b/db/migrate/20220614095912_add_has_vulnerabilities_to_cluster_agents.rb new file mode 100644 index 00000000000..e4e4e3ab7ae --- /dev/null +++ b/db/migrate/20220614095912_add_has_vulnerabilities_to_cluster_agents.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddHasVulnerabilitiesToClusterAgents < Gitlab::Database::Migration[2.0] + enable_lock_retries! + + def change + add_column :cluster_agents, :has_vulnerabilities, :boolean, default: false, null: false + end +end diff --git a/db/migrate/20220615091059_add_created_at_index_to_compliance_management_frameworks.rb b/db/migrate/20220615091059_add_created_at_index_to_compliance_management_frameworks.rb new file mode 100644 index 00000000000..a930dde9a83 --- /dev/null +++ b/db/migrate/20220615091059_add_created_at_index_to_compliance_management_frameworks.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddCreatedAtIndexToComplianceManagementFrameworks < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + INDEX_NAME = "i_compliance_frameworks_on_id_and_created_at" + + def up + add_concurrent_index :compliance_management_frameworks, + [:id, :created_at, :pipeline_configuration_full_path], + name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :compliance_management_frameworks, INDEX_NAME + end +end diff --git a/db/migrate/20220615105811_add_index_on_clusters_agent_project_id_and_has_vulnerabilities_columns.rb b/db/migrate/20220615105811_add_index_on_clusters_agent_project_id_and_has_vulnerabilities_columns.rb new file mode 100644 index 00000000000..007f36c26ed --- /dev/null +++ b/db/migrate/20220615105811_add_index_on_clusters_agent_project_id_and_has_vulnerabilities_columns.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexOnClustersAgentProjectIdAndHasVulnerabilitiesColumns < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_cluster_agents_on_project_id_and_has_vulnerabilities' + + def up + add_concurrent_index :cluster_agents, + [:project_id, :has_vulnerabilities], + name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :cluster_agents, INDEX_NAME + end +end diff --git a/db/schema_migrations/20220523030804 b/db/schema_migrations/20220523030804 new file mode 100644 index 00000000000..6a9bdd4f66d --- /dev/null +++ b/db/schema_migrations/20220523030804 @@ -0,0 +1 @@ +80535374849c10d41663d339b95b9ffddbec9b40a8af4585c18602cbe92c14d1
\ No newline at end of file diff --git a/db/schema_migrations/20220523030805 b/db/schema_migrations/20220523030805 new file mode 100644 index 00000000000..3714e71a3f3 --- /dev/null +++ b/db/schema_migrations/20220523030805 @@ -0,0 +1 @@ +92a7ed079521ccb8ab04e59826947778c37bccd30d47f1b0e29727f769e3ff32
\ No newline at end of file diff --git a/db/schema_migrations/20220530104431 b/db/schema_migrations/20220530104431 new file mode 100644 index 00000000000..4e809f44d25 --- /dev/null +++ b/db/schema_migrations/20220530104431 @@ -0,0 +1 @@ +f49e691c46ddaaf1b18d95726e7c2473fab946ea79885727ba09bb92591e4a01
\ No newline at end of file diff --git a/db/schema_migrations/20220614095912 b/db/schema_migrations/20220614095912 new file mode 100644 index 00000000000..e84b4a2fb3d --- /dev/null +++ b/db/schema_migrations/20220614095912 @@ -0,0 +1 @@ +96d899efc1fa39cf3433987ee4d8062456f7a6af6248b97eda2ddc5491dcf7f5
\ No newline at end of file diff --git a/db/schema_migrations/20220615091059 b/db/schema_migrations/20220615091059 new file mode 100644 index 00000000000..1d1b35fc8f6 --- /dev/null +++ b/db/schema_migrations/20220615091059 @@ -0,0 +1 @@ +bbfcaf59734b67142b237b7ea479c5eaa3c2152cdd84c87ad541e5a0e75466ef
\ No newline at end of file diff --git a/db/schema_migrations/20220615105811 b/db/schema_migrations/20220615105811 new file mode 100644 index 00000000000..e2ada7879b8 --- /dev/null +++ b/db/schema_migrations/20220615105811 @@ -0,0 +1 @@ +33456ce3af299e010011b1346b4097ffa1ee642ffb90d342ea22171c3f079d7a
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index fb9bbd72aa8..a3f5ece22b9 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13346,6 +13346,7 @@ CREATE TABLE cluster_agents ( project_id bigint NOT NULL, name text NOT NULL, created_by_user_id bigint, + has_vulnerabilities boolean DEFAULT false NOT NULL, CONSTRAINT check_3498369510 CHECK ((char_length(name) <= 255)) ); @@ -13795,6 +13796,8 @@ CREATE TABLE compliance_management_frameworks ( color text NOT NULL, namespace_id integer NOT NULL, pipeline_configuration_full_path text, + created_at timestamp with time zone, + updated_at timestamp with time zone, CONSTRAINT check_08cd34b2c2 CHECK ((char_length(color) <= 10)), CONSTRAINT check_1617e0b87e CHECK ((char_length(description) <= 255)), CONSTRAINT check_ab00bc2193 CHECK ((char_length(name) <= 255)), @@ -18780,7 +18783,9 @@ CREATE TABLE plan_limits ( pipeline_triggers integer DEFAULT 25000 NOT NULL, project_ci_secure_files integer DEFAULT 100 NOT NULL, repository_size bigint DEFAULT 0 NOT NULL, - security_policy_scan_execution_schedules integer DEFAULT 0 NOT NULL + security_policy_scan_execution_schedules integer DEFAULT 0 NOT NULL, + web_hook_calls_mid integer DEFAULT 0 NOT NULL, + web_hook_calls_low integer DEFAULT 0 NOT NULL ); CREATE SEQUENCE plan_limits_id_seq @@ -26767,6 +26772,8 @@ CREATE INDEX i_batched_background_migration_job_transition_logs_on_job_id ON ONL CREATE UNIQUE INDEX i_ci_job_token_project_scope_links_on_source_and_target_project ON ci_job_token_project_scope_links USING btree (source_project_id, target_project_id); +CREATE INDEX i_compliance_frameworks_on_id_and_created_at ON compliance_management_frameworks USING btree (id, created_at, pipeline_configuration_full_path); + CREATE INDEX idx_analytics_devops_adoption_segments_on_namespace_id ON analytics_devops_adoption_segments USING btree (namespace_id); CREATE INDEX idx_analytics_devops_adoption_snapshots_finalized ON analytics_devops_adoption_snapshots USING btree (namespace_id, end_time) WHERE (recorded_at >= end_time); @@ -27543,6 +27550,8 @@ CREATE UNIQUE INDEX index_cluster_agent_tokens_on_token_encrypted ON cluster_age CREATE INDEX index_cluster_agents_on_created_by_user_id ON cluster_agents USING btree (created_by_user_id); +CREATE INDEX index_cluster_agents_on_project_id_and_has_vulnerabilities ON cluster_agents USING btree (project_id, has_vulnerabilities); + CREATE UNIQUE INDEX index_cluster_agents_on_project_id_and_name ON cluster_agents USING btree (project_id, name); CREATE UNIQUE INDEX index_cluster_enabled_grants_on_namespace_id ON cluster_enabled_grants USING btree (namespace_id); diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index 7ff22549a84..d86414ae285 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -133,8 +133,9 @@ Limit the maximum daily member invitations allowed per group hierarchy. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61151) in GitLab 13.12. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/330133) in GitLab 14.1. +> - [Limit changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89591) from per-hook to per-top-level namespace in GitLab 15.1. -Limit the number of times any given webhook can be called per minute. +Limit the number of times a webhook can be called per minute, per top-level namespace. This only applies to project and group webhooks. Calls over the rate limit are logged into `auth.log`. diff --git a/doc/api/invitations.md b/doc/api/invitations.md index eb7351c2e09..5a39a86d039 100644 --- a/doc/api/invitations.md +++ b/doc/api/invitations.md @@ -22,9 +22,9 @@ levels are defined in the `Gitlab::Access` module. Currently, these levels are v - Maintainer (`40`) - Owner (`50`) - Only valid to set for groups -WARNING: -Due to [an issue](https://gitlab.com/gitlab-org/gitlab/-/issues/219299), -projects in personal namespaces don't show owner (`50`) permission. +NOTE: +From [GitLab 14.9](https://gitlab.com/gitlab-org/gitlab/-/issues/351211) and later, projects have a maximum role of Owner. +Because of a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/219299) in GitLab 14.8 and earlier, projects have a maximum role of Maintainer. ## Add a member to a group or project diff --git a/doc/ci/environments/protected_environments.md b/doc/ci/environments/protected_environments.md index fed8ec62f3e..35ee4f9dd33 100644 --- a/doc/ci/environments/protected_environments.md +++ b/doc/ci/environments/protected_environments.md @@ -233,7 +233,7 @@ To protect a group-level environment, make sure your environments have the corre #### Using the UI -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325249) in GitLab 15.1 with a flag named `group_level_protected_environment`. Disabled by default. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325249) in GitLab 15.1. 1. On the top bar, select **Menu > Groups** and find your group. 1. On the left sidebar, select **Settings > CI/CD**. diff --git a/doc/integration/saml.md b/doc/integration/saml.md index e9ad443bd23..9f707ba9bc6 100644 --- a/doc/integration/saml.md +++ b/doc/integration/saml.md @@ -407,6 +407,10 @@ The requirements are the same as the previous settings: } } ``` +## Group Sync + +For information on automatically managing GitLab group membership, see [SAML Group Sync](../user/group/saml_sso/group_sync.md). + ## Bypass two factor authentication If you want some SAML authentication methods to count as 2FA on a per session diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index 31b6dba0ed5..d515f9f4558 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -234,7 +234,7 @@ The following limits apply for [webhooks](../project/integrations/webhooks.md): | Setting | Default for GitLab.com | |----------------------|-------------------------| -| Webhook rate limit | `120` calls per minute for GitLab Free, unlimited for GitLab Premium and GitLab Ultimate | +| Webhook rate limit | `500` calls per minute for GitLab Free, unlimited for GitLab Premium and GitLab Ultimate. Webhook rate limits are applied per top-level namespace. | | Number of webhooks | `100` per project, `50` per group | | Maximum payload size | 25 MB | diff --git a/doc/user/group/saml_sso/group_sync.md b/doc/user/group/saml_sso/group_sync.md new file mode 100644 index 00000000000..2239562b831 --- /dev/null +++ b/doc/user/group/saml_sso/group_sync.md @@ -0,0 +1,161 @@ +--- +type: reference, howto +stage: Manage +group: Authentication and Authorization +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# SAML Group Sync **(PREMIUM)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/363084) for self-managed instances in GitLab 15.1. + +WARNING: +Changing Group Sync configuration can remove users from the mapped GitLab group. +Removal happens if there is any mismatch between the group names and the list of `groups` in the SAML response. +If changes must be made, ensure either the SAML response includes the `groups` attribute +and the `AttributeValue` value matches the **SAML Group Name** in GitLab, +or that all groups are removed from GitLab to disable Group Sync. + +<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> +For a demo of Group Sync using Azure, see [Demo: SAML Group Sync](https://youtu.be/Iqvo2tJfXjg). + +## Configure SAML Group Sync + +To configure SAML Group Sync: + +1. Configure SAML authentication: + - For GitLab self-managed, see [SAML OmniAuth Provider](../../../integration/saml.md). + - For GitLab.com, see [SAML SSO for GitLab.com groups](index.md). +1. Ensure your SAML identity provider sends an attribute statement named `Groups` or `groups`. + +NOTE: +The value for `Groups` or `groups` in the SAML response can be either the group name or the group ID. + +```xml +<saml:AttributeStatement> + <saml:Attribute Name="Groups"> + <saml:AttributeValue xsi:type="xs:string">Developers</saml:AttributeValue> + <saml:AttributeValue xsi:type="xs:string">Product Managers</saml:AttributeValue> + </saml:Attribute> +</saml:AttributeStatement> +``` + +Other attribute names such as `http://schemas.microsoft.com/ws/2008/06/identity/claims/groups` +are not accepted as a source of groups. +See the [SAML troubleshooting page](../../../administration/troubleshooting/group_saml_scim.md) +for examples on configuring the required attribute name in the SAML identity provider's settings. + +## Configure SAML Group Links + +When SAML is enabled, users with the Maintainer or Owner role +see a new menu item in group **Settings > SAML Group Links**. You can configure one or more **SAML Group Links** to map +a SAML identity provider group name to a GitLab role. This can be done for a top-level group or any subgroup. + +To link the SAML groups: + +1. In **SAML Group Name**, enter the value of the relevant `saml:AttributeValue`. +1. Choose the role in **Access Level**. +1. Select **Save**. +1. Repeat to add additional group links if required. + + + +If a user is a member of multiple SAML groups mapped to the same GitLab group, +the user gets the highest role from the groups. For example, if one group +is linked as Guest and another Maintainer, a user in both groups gets the Maintainer +role. + +Users granted: + +- A higher role with Group Sync are displayed as having + [direct membership](../../project/members/#display-direct-members) of the group. +- A lower or the same role with Group Sync are displayed as having + [inherited membership](../../project/members/#display-inherited-members) of the group. + +### Automatic member removal + +After a group sync, for GitLab subgroups, users who are not members of a mapped SAML +group are removed from the group. + +FLAG: +In [GitLab 15.1 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/364144), on GitLab.com, users in the top-level +group are assigned the [default membership role](index.md#role) rather than removed. This setting is enabled with the +`saml_group_sync_retain_default_membership` feature flag and can be configured by GitLab.com administrators only. + +For example, in the following diagram: + +- Alex Garcia signs into GitLab and is removed from GitLab Group C because they don't belong + to SAML Group C. +- Sidney Jones belongs to SAML Group C, but is not added to GitLab Group C because they have + not yet signed in. + +```mermaid +graph TB + subgraph SAML users + SAMLUserA[Sidney Jones] + SAMLUserB[Zhang Wei] + SAMLUserC[Alex Garcia] + SAMLUserD[Charlie Smith] + end + + subgraph SAML groups + SAMLGroupA["Group A"] --> SAMLGroupB["Group B"] + SAMLGroupA --> SAMLGroupC["Group C"] + SAMLGroupA --> SAMLGroupD["Group D"] + end + + SAMLGroupB --> |Member|SAMLUserA + SAMLGroupB --> |Member|SAMLUserB + + SAMLGroupC --> |Member|SAMLUserA + SAMLGroupC --> |Member|SAMLUserB + + SAMLGroupD --> |Member|SAMLUserD + SAMLGroupD --> |Member|SAMLUserC +``` + +```mermaid +graph TB + subgraph GitLab users + GitLabUserA[Sidney Jones] + GitLabUserB[Zhang Wei] + GitLabUserC[Alex Garcia] + GitLabUserD[Charlie Smith] + end + + subgraph GitLab groups + GitLabGroupA["Group A (SAML configured)"] --> GitLabGroupB["Group B (SAML Group Link not configured)"] + GitLabGroupA --> GitLabGroupC["Group C (SAML Group Link configured)"] + GitLabGroupA --> GitLabGroupD["Group D (SAML Group Link configured)"] + end + + GitLabGroupB --> |Member|GitLabUserA + + GitLabGroupC --> |Member|GitLabUserB + GitLabGroupC --> |Member|GitLabUserC + + GitLabGroupD --> |Member|GitLabUserC + GitLabGroupD --> |Member|GitLabUserD +``` + +```mermaid +graph TB + subgraph GitLab users + GitLabUserA[Sidney Jones] + GitLabUserB[Zhang Wei] + GitLabUserC[Alex Garcia] + GitLabUserD[Charlie Smith] + end + + subgraph GitLab groups after Alex Garcia signs in + GitLabGroupA[Group A] + GitLabGroupA["Group A (SAML configured)"] --> GitLabGroupB["Group B (SAML Group Link not configured)"] + GitLabGroupA --> GitLabGroupC["Group C (SAML Group Link configured)"] + GitLabGroupA --> GitLabGroupD["Group D (SAML Group Link configured)"] + end + + GitLabGroupB --> |Member|GitLabUserA + GitLabGroupC --> |Member|GitLabUserB + GitLabGroupD --> |Member|GitLabUserC + GitLabGroupD --> |Member|GitLabUserD +``` diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index d7da479e405..c05e847e2c9 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -372,7 +372,7 @@ To rescind a user's access to the group when only SAML SSO is configured, either - Remove (in order) the user from: 1. The user data store on the identity provider or the list of users on the specific app. 1. The GitLab.com group. -- Use Group Sync at the top-level of your group to [automatically remove the user](#automatic-member-removal). +- Use Group Sync at the top-level of your group to [automatically remove the user](group_sync.md#automatic-member-removal). To rescind a user's access to the group when also using SCIM, refer to [Blocking access](scim_setup.md#blocking-access). @@ -402,151 +402,7 @@ For example, to unlink the `MyOrg` account: ## Group Sync -WARNING: -Changing Group Sync configuration can remove users from the relevant GitLab group. -Removal happens if there is any mismatch between the group names and the list of `groups` in the SAML response. -If changes must be made, ensure either the SAML response includes the `groups` attribute -and the `AttributeValue` value matches the **SAML Group Name** in GitLab, -or that all groups are removed from GitLab to disable Group Sync. - -<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> -For a demo of Group Sync using Azure, see [Demo: SAML Group Sync](https://youtu.be/Iqvo2tJfXjg). - -When the SAML response includes a user and their group memberships from the SAML identity provider, -GitLab uses that information to automatically manage that user's GitLab group memberships. - -Ensure your SAML identity provider sends an attribute statement named `Groups` or `groups` like the following: - -```xml -<saml:AttributeStatement> - <saml:Attribute Name="Groups"> - <saml:AttributeValue xsi:type="xs:string">Developers</saml:AttributeValue> - <saml:AttributeValue xsi:type="xs:string">Product Managers</saml:AttributeValue> - </saml:Attribute> -</saml:AttributeStatement> -``` - -Other attribute names such as `http://schemas.microsoft.com/ws/2008/06/identity/claims/groups` -are not accepted as a source of groups. -See the [SAML troubleshooting page](../../../administration/troubleshooting/group_saml_scim.md) -for examples on configuring the required attribute name in the SAML identity provider's settings. - -NOTE: -The value for `Groups` or `groups` in the SAML response can be either the group name or the group ID. -To inspect the SAML response, you can use one of these [SAML debugging tools](#saml-debugging-tools). - -When SAML SSO is enabled for the top-level group, `Maintainer` and `Owner` level users -see a new menu item in group **Settings > SAML Group Links**. You can configure one or more **SAML Group Links** to map -a SAML identity provider group name to a GitLab Access Level. This can be done for the parent group or the subgroups. - -To link the SAML groups from the `saml:AttributeStatement` example above: - -1. In the **SAML Group Name** box, enter the value of `saml:AttributeValue`. -1. Choose the desired **Access Level**. -1. **Save** the group link. -1. Repeat to add additional group links if desired. - - - -If a user is a member of multiple SAML groups mapped to the same GitLab group, -the user gets the highest access level from the groups. For example, if one group -is linked as `Guest` and another `Maintainer`, a user in both groups gets `Maintainer` -access. - -Users granted: - -- A higher role with Group Sync are displayed as having - [direct membership](../../project/members/#display-direct-members) of the group. -- A lower or the same role with Group Sync are displayed as having - [inherited membership](../../project/members/#display-inherited-members) of the group. - -### Automatic member removal - -After a group sync, for GitLab subgroups, users who are not members of a mapped SAML -group are removed from the group. - -FLAG: -In [GitLab 15.1 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/364144), on GitLab.com, users in the top-level -group are assigned the [default membership role](#role) rather than removed. This setting is enabled with the -`saml_group_sync_retain_default_membership` feature flag and can be configured by GitLab.com administrators only. - -For example, in the following diagram: - -- Alex Garcia signs into GitLab and is removed from GitLab Group C because they don't belong - to SAML Group C. -- Sidney Jones belongs to SAML Group C, but is not added to GitLab Group C because they have - not yet signed in. - -```mermaid -graph TB - subgraph SAML users - SAMLUserA[Sidney Jones] - SAMLUserB[Zhang Wei] - SAMLUserC[Alex Garcia] - SAMLUserD[Charlie Smith] - end - - subgraph SAML groups - SAMLGroupA["Group A"] --> SAMLGroupB["Group B"] - SAMLGroupA --> SAMLGroupC["Group C"] - SAMLGroupA --> SAMLGroupD["Group D"] - end - - SAMLGroupB --> |Member|SAMLUserA - SAMLGroupB --> |Member|SAMLUserB - - SAMLGroupC --> |Member|SAMLUserA - SAMLGroupC --> |Member|SAMLUserB - - SAMLGroupD --> |Member|SAMLUserD - SAMLGroupD --> |Member|SAMLUserC -``` - -```mermaid -graph TB - subgraph GitLab users - GitLabUserA[Sidney Jones] - GitLabUserB[Zhang Wei] - GitLabUserC[Alex Garcia] - GitLabUserD[Charlie Smith] - end - - subgraph GitLab groups - GitLabGroupA["Group A (SAML configured)"] --> GitLabGroupB["Group B (SAML Group Link not configured)"] - GitLabGroupA --> GitLabGroupC["Group C (SAML Group Link configured)"] - GitLabGroupA --> GitLabGroupD["Group D (SAML Group Link configured)"] - end - - GitLabGroupB --> |Member|GitLabUserA - - GitLabGroupC --> |Member|GitLabUserB - GitLabGroupC --> |Member|GitLabUserC - - GitLabGroupD --> |Member|GitLabUserC - GitLabGroupD --> |Member|GitLabUserD -``` - -```mermaid -graph TB - subgraph GitLab users - GitLabUserA[Sidney Jones] - GitLabUserB[Zhang Wei] - GitLabUserC[Alex Garcia] - GitLabUserD[Charlie Smith] - end - - subgraph GitLab groups after Alex Garcia signs in - GitLabGroupA[Group A] - GitLabGroupA["Group A (SAML configured)"] --> GitLabGroupB["Group B (SAML Group Link not configured)"] - GitLabGroupA --> GitLabGroupC["Group C (SAML Group Link configured)"] - GitLabGroupA --> GitLabGroupD["Group D (SAML Group Link configured)"] - end - - GitLabGroupB --> |Member|GitLabUserA - GitLabGroupC --> |Member|GitLabUserB - GitLabGroupD --> |Member|GitLabUserC - GitLabGroupD --> |Member|GitLabUserD -``` +For information on automatically managing GitLab group membership, see [SAML Group Sync](group_sync.md). ## Passwords for users created via SAML SSO for Groups diff --git a/lib/atlassian/jira_connect/client.rb b/lib/atlassian/jira_connect/client.rb index b8aa2cc8ea0..3c999920d39 100644 --- a/lib/atlassian/jira_connect/client.rb +++ b/lib/atlassian/jira_connect/client.rb @@ -30,6 +30,8 @@ module Atlassian responses.compact end + # Fetch user information for the given account. + # https://developer.atlassian.com/cloud/jira/platform/rest/v3/api-group-users/#api-rest-api-3-user-get def user_info(account_id) r = get('/rest/api/3/user', { accountId: account_id, expand: 'groups' }) diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index b9aa2e8c987..722ee061eba 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -32,6 +32,8 @@ module Gitlab group_testing_hook: { threshold: 5, interval: 1.minute }, profile_add_new_email: { threshold: 5, interval: 1.minute }, web_hook_calls: { interval: 1.minute }, + web_hook_calls_mid: { interval: 1.minute }, + web_hook_calls_low: { interval: 1.minute }, users_get_by_id: { threshold: -> { application_settings.users_get_by_id_limit }, interval: 10.minutes }, username_exists: { threshold: 20, interval: 1.minute }, user_sign_up: { threshold: 20, interval: 1.minute }, diff --git a/lib/gitlab/web_hooks/rate_limiter.rb b/lib/gitlab/web_hooks/rate_limiter.rb new file mode 100644 index 00000000000..73d59f6f786 --- /dev/null +++ b/lib/gitlab/web_hooks/rate_limiter.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Gitlab + module WebHooks + class RateLimiter + include Gitlab::Utils::StrongMemoize + + LIMIT_NAME = :web_hook_calls + NO_LIMIT = 0 + # SystemHooks (instance admin hooks) and ServiceHooks (integration hooks) + # are not rate-limited. + EXCLUDED_HOOK_TYPES = %w(SystemHook ServiceHook).freeze + + def initialize(hook) + @hook = hook + @parent = hook.parent + end + + # Increments the rate-limit counter. + # Returns true if the hook should be rate-limited. + def rate_limit! + return false if no_limit? + + ::Gitlab::ApplicationRateLimiter.throttled?( + limit_name, + scope: [root_namespace], + threshold: limit + ) + end + + # Returns true if the hook is currently over its rate-limit. + # It does not increment the rate-limit counter. + def rate_limited? + return false if no_limit? + + Gitlab::ApplicationRateLimiter.peek( + limit_name, + scope: [root_namespace], + threshold: limit + ) + end + + def limit + strong_memoize(:limit) do + next NO_LIMIT if hook.class.name.in?(EXCLUDED_HOOK_TYPES) + + root_namespace.actual_limits.limit_for(limit_name) || NO_LIMIT + end + end + + private + + attr_reader :hook, :parent + + def no_limit? + limit == NO_LIMIT + end + + def root_namespace + @root_namespace ||= parent.root_ancestor + end + + def limit_name + LIMIT_NAME + end + end + end +end + +Gitlab::WebHooks::RateLimiter.prepend_mod diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 994dfe7ded3..c15522c123f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -21742,9 +21742,15 @@ msgstr "" msgid "Jira-GitLab user mapping template" msgstr "" +msgid "JiraConnect|Cannot find namespace. Make sure you have sufficient permissions." +msgstr "" + msgid "JiraConnect|Configure your Jira Connect Application ID." msgstr "" +msgid "JiraConnect|Could not fetch user information from Jira. Check the permissions in Jira and try again." +msgstr "" + msgid "JiraConnect|Create branch for Jira issue %{jiraIssue}" msgstr "" @@ -21763,6 +21769,9 @@ msgstr "" msgid "JiraConnect|New branch was successfully created." msgstr "" +msgid "JiraConnect|The Jira user is not a site administrator. Check the permissions in Jira and try again." +msgstr "" + msgid "JiraConnect|You can now close this window and return to Jira." msgstr "" diff --git a/qa/qa/page/project/settings/ci_variables.rb b/qa/qa/page/project/settings/ci_variables.rb index e9224fcb159..7ee015ceb98 100644 --- a/qa/qa/page/project/settings/ci_variables.rb +++ b/qa/qa/page/project/settings/ci_variables.rb @@ -14,7 +14,7 @@ module QA element :ci_variable_delete_button end - view 'app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue' do + view 'app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_table.vue' do element :ci_variable_table_content element :add_ci_variable_button element :edit_ci_variable_button diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb index c7d37205b71..9af9baeb5bb 100644 --- a/spec/features/group_variables_spec.rb +++ b/spec/features/group_variables_spec.rb @@ -12,8 +12,18 @@ RSpec.describe 'Group variables', :js do group.add_owner(user) gitlab_sign_in(user) wait_for_requests - visit page_path end - it_behaves_like 'variable list' + context 'with disabled ff `ci_variable_settings_graphql' do + before do + stub_feature_flags(ci_variable_settings_graphql: false) + visit page_path + end + + it_behaves_like 'variable list' + end + + # TODO: Uncomment when the new graphQL app for variable settings + # is enabled. + # it_behaves_like 'variable list' end diff --git a/spec/features/project_variables_spec.rb b/spec/features/project_variables_spec.rb index cc59fea173b..89dbd1afc6b 100644 --- a/spec/features/project_variables_spec.rb +++ b/spec/features/project_variables_spec.rb @@ -12,28 +12,36 @@ RSpec.describe 'Project variables', :js do sign_in(user) project.add_maintainer(user) project.variables << variable - visit page_path end - it_behaves_like 'variable list' - - it 'adds a new variable with an environment scope' do - click_button('Add variable') + # TODO: Add same tests but with FF enabled context when + # the new graphQL app for variable settings is enabled. + context 'with disabled ff `ci_variable_settings_graphql' do + before do + stub_feature_flags(ci_variable_settings_graphql: false) + visit page_path + end - page.within('#add-ci-variable') do - fill_in 'Key', with: 'akey' - find('#ci-variable-value').set('akey_value') - find('[data-testid="environment-scope"]').click - find('[data-testid="ci-environment-search"]').set('review/*') - find('[data-testid="create-wildcard-button"]').click + it_behaves_like 'variable list' + it 'adds a new variable with an environment scope' do click_button('Add variable') - end - wait_for_requests + page.within('#add-ci-variable') do + fill_in 'Key', with: 'akey' + find('#ci-variable-value').set('akey_value') + find('[data-testid="environment-scope"]').click + find('[data-testid="ci-environment-search"]').set('review/*') + find('[data-testid="create-wildcard-button"]').click + + click_button('Add variable') + end + + wait_for_requests - page.within('[data-testid="ci-variable-table"]') do - expect(find('.js-ci-variable-row:first-child [data-label="Environments"]').text).to eq('review/*') + page.within('[data-testid="ci-variable-table"]') do + expect(find('.js-ci-variable-row:first-child [data-label="Environments"]').text).to eq('review/*') + end end end end diff --git a/spec/features/projects/members/manage_members_spec.rb b/spec/features/projects/members/manage_members_spec.rb index 0f4120e88e0..8d229530ef5 100644 --- a/spec/features/projects/members/manage_members_spec.rb +++ b/spec/features/projects/members/manage_members_spec.rb @@ -48,20 +48,48 @@ RSpec.describe 'Projects > Members > Manage members', :js do end end - it 'uses ProjectMember access_level_roles for the invite members modal access option', :aggregate_failures do - visit_members_page + context 'when owner' do + it 'uses ProjectMember access_level_roles for the invite members modal access option', :aggregate_failures do + visit_members_page - click_on 'Invite members' + click_on 'Invite members' - click_on 'Guest' - wait_for_requests + click_on 'Guest' + wait_for_requests - page.within '.dropdown-menu' do - expect(page).to have_button('Guest') - expect(page).to have_button('Reporter') - expect(page).to have_button('Developer') - expect(page).to have_button('Maintainer') - expect(page).not_to have_button('Owner') + page.within '.dropdown-menu' do + expect(page).to have_button('Guest') + expect(page).to have_button('Reporter') + expect(page).to have_button('Developer') + expect(page).to have_button('Maintainer') + expect(page).to have_button('Owner') + end + end + end + + context 'when maintainer' do + let(:maintainer) { create(:user) } + + before do + project.add_maintainer(maintainer) + sign_in(maintainer) + end + + it 'uses ProjectMember access_level_roles for the invite members modal access option', :aggregate_failures do + visit_members_page + + click_on 'Invite members' + + click_on 'Guest' + wait_for_requests + + page.within '.dropdown-menu' do + expect(page).to have_button('Guest') + expect(page).to have_button('Reporter') + expect(page).to have_button('Developer') + expect(page).to have_button('Maintainer') + expect(page).not_to have_button('Owner') + end end end diff --git a/spec/frontend/ci_variable_list/components/ci_environments_dropdown_spec.js b/spec/frontend/ci_variable_list/components/legacy_ci_environments_dropdown_spec.js index e7e4897abfa..b3e23ba4201 100644 --- a/spec/frontend/ci_variable_list/components/ci_environments_dropdown_spec.js +++ b/spec/frontend/ci_variable_list/components/legacy_ci_environments_dropdown_spec.js @@ -2,7 +2,7 @@ import { GlDropdown, GlDropdownItem, GlIcon } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import Vuex from 'vuex'; -import CiEnvironmentsDropdown from '~/ci_variable_list/components/ci_environments_dropdown.vue'; +import LegacyCiEnvironmentsDropdown from '~/ci_variable_list/components/legacy_ci_environments_dropdown.vue'; Vue.use(Vuex); @@ -20,7 +20,7 @@ describe('Ci environments dropdown', () => { }, }); - wrapper = mount(CiEnvironmentsDropdown, { + wrapper = mount(LegacyCiEnvironmentsDropdown, { store, propsData: { value: term, diff --git a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js b/spec/frontend/ci_variable_list/components/legacy_ci_variable_modal_spec.js index 7ce294ad11b..42c6501dcce 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci_variable_list/components/legacy_ci_variable_modal_spec.js @@ -4,7 +4,7 @@ import Vue from 'vue'; import Vuex from 'vuex'; import { mockTracking } from 'helpers/tracking_helper'; import CiEnvironmentsDropdown from '~/ci_variable_list/components/ci_environments_dropdown.vue'; -import CiVariableModal from '~/ci_variable_list/components/ci_variable_modal.vue'; +import LegacyCiVariableModal from '~/ci_variable_list/components/legacy_ci_variable_modal.vue'; import { AWS_ACCESS_KEY_ID, EVENT_LABEL, @@ -30,7 +30,7 @@ describe('Ci variable modal', () => { isGroup: options.isGroup, environmentScopeLink: '/help/environments', }); - wrapper = method(CiVariableModal, { + wrapper = method(LegacyCiVariableModal, { attachTo: document.body, stubs: { GlModal: ModalStub, diff --git a/spec/frontend/ci_variable_list/components/ci_variable_settings_spec.js b/spec/frontend/ci_variable_list/components/legacy_ci_variable_settings_spec.js index 13e417940a8..9c941f99982 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_settings_spec.js +++ b/spec/frontend/ci_variable_list/components/legacy_ci_variable_settings_spec.js @@ -1,7 +1,7 @@ import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; import Vuex from 'vuex'; -import CiVariableSettings from '~/ci_variable_list/components/ci_variable_settings.vue'; +import LegacyCiVariableSettings from '~/ci_variable_list/components/legacy_ci_variable_settings.vue'; import createStore from '~/ci_variable_list/store'; Vue.use(Vuex); @@ -15,7 +15,7 @@ describe('Ci variable table', () => { store = createStore(); store.state.isGroup = groupState; jest.spyOn(store, 'dispatch').mockImplementation(); - wrapper = shallowMount(CiVariableSettings, { + wrapper = shallowMount(LegacyCiVariableSettings, { store, }); }; diff --git a/spec/frontend/ci_variable_list/components/ci_variable_table_spec.js b/spec/frontend/ci_variable_list/components/legacy_ci_variable_table_spec.js index 62f9ae4eb4e..310afc8003a 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_table_spec.js +++ b/spec/frontend/ci_variable_list/components/legacy_ci_variable_table_spec.js @@ -1,7 +1,7 @@ import Vue from 'vue'; import Vuex from 'vuex'; import { mountExtended } from 'helpers/vue_test_utils_helper'; -import CiVariableTable from '~/ci_variable_list/components/ci_variable_table.vue'; +import LegacyCiVariableTable from '~/ci_variable_list/components/legacy_ci_variable_table.vue'; import createStore from '~/ci_variable_list/store'; import mockData from '../services/mock_data'; @@ -14,7 +14,7 @@ describe('Ci variable table', () => { const createComponent = () => { store = createStore(); jest.spyOn(store, 'dispatch').mockImplementation(); - wrapper = mountExtended(CiVariableTable, { + wrapper = mountExtended(LegacyCiVariableTable, { attachTo: document.body, store, }); diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index d19c0c79eca..e0c98bbc161 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -355,30 +355,6 @@ RSpec.describe ProjectsHelper do end end - describe '#permissible_access_level_roles' do - let_it_be(:owner) { create(:user) } - let_it_be(:maintainer) { create(:user) } - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - - before do - project.add_owner(owner) - project.add_maintainer(maintainer) - end - - context 'when member can manage owners' do - it 'returns Gitlab::Access.options_with_owner' do - expect(helper.permissible_access_level_roles(owner, project)).to eq(Gitlab::Access.options_with_owner) - end - end - - context 'when member cannot manage owners' do - it 'returns Gitlab::Access.options' do - expect(helper.permissible_access_level_roles(maintainer, project)).to eq(Gitlab::Access.options) - end - end - end - describe 'default_clone_protocol' do context 'when user is not logged in and gitlab protocol is HTTP' do it 'returns HTTP' do diff --git a/spec/lib/atlassian/jira_connect/client_spec.rb b/spec/lib/atlassian/jira_connect/client_spec.rb index dd3130c78bf..0ae0f02c46e 100644 --- a/spec/lib/atlassian/jira_connect/client_spec.rb +++ b/spec/lib/atlassian/jira_connect/client_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Atlassian::JiraConnect::Client do include StubRequests - subject { described_class.new('https://gitlab-test.atlassian.net', 'sample_secret') } + subject(:client) { described_class.new('https://gitlab-test.atlassian.net', 'sample_secret') } let_it_be(:project) { create_default(:project, :repository) } let_it_be(:mrs_by_title) { create_list(:merge_request, 4, :unique_branches, :jira_title) } @@ -413,4 +413,41 @@ RSpec.describe Atlassian::JiraConnect::Client do expect { subject.send(:store_dev_info, project: project, merge_requests: merge_requests) }.not_to exceed_query_limit(control_count) end end + + describe '#user_info' do + let(:account_id) { '12345' } + let(:response_body) do + { + groups: { + items: [ + { name: 'site-admins' } + ] + } + }.to_json + end + + before do + stub_full_request("https://gitlab-test.atlassian.net/rest/api/3/user?accountId=#{account_id}&expand=groups") + .to_return(status: response_status, body: response_body, headers: { 'Content-Type': 'application/json' }) + end + + context 'with a successful response' do + let(:response_status) { 200 } + + it 'returns a JiraUser instance' do + jira_user = client.user_info(account_id) + + expect(jira_user).to be_a(Atlassian::JiraConnect::JiraUser) + expect(jira_user).to be_site_admin + end + end + + context 'with a failed response' do + let(:response_status) { 401 } + + it 'returns nil' do + expect(client.user_info(account_id)).to be_nil + end + end + end end diff --git a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb index 2740664d200..e676e5fe034 100644 --- a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb +++ b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb @@ -77,6 +77,7 @@ RSpec.describe Gitlab::DatabaseImporters::InstanceAdministrators::CreateGroup do create(:user) expect(result[:status]).to eq(:success) + group.reset expect(group.members.collect(&:user)).to contain_exactly(user, admin1, admin2) expect(group.members.collect(&:access_level)).to contain_exactly( Gitlab::Access::OWNER, diff --git a/spec/lib/gitlab/web_hooks/rate_limiter_spec.rb b/spec/lib/gitlab/web_hooks/rate_limiter_spec.rb new file mode 100644 index 00000000000..b25ce4ea9da --- /dev/null +++ b/spec/lib/gitlab/web_hooks/rate_limiter_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::WebHooks::RateLimiter, :clean_gitlab_redis_rate_limiting do + let_it_be(:plan) { create(:default_plan) } + let_it_be_with_reload(:project_hook) { create(:project_hook) } + let_it_be_with_reload(:system_hook) { create(:system_hook) } + let_it_be_with_reload(:integration_hook) { create(:jenkins_integration).service_hook } + let_it_be(:limit) { 1 } + + using RSpec::Parameterized::TableSyntax + + describe '#rate_limit!' do + def rate_limit!(hook) + described_class.new(hook).rate_limit! + end + + shared_examples 'a hook that is never rate limited' do + specify do + expect(Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + + expect(rate_limit!(hook)).to eq(false) + end + end + + context 'when there is no plan limit' do + where(:hook) { [ref(:project_hook), ref(:system_hook), ref(:integration_hook)] } + + with_them { it_behaves_like 'a hook that is never rate limited' } + end + + context 'when there is a plan limit' do + before_all do + create(:plan_limits, plan: plan, web_hook_calls: limit) + end + + where(:hook, :limitless_hook_type) do + ref(:project_hook) | false + ref(:system_hook) | true + ref(:integration_hook) | true + end + + with_them do + if params[:limitless_hook_type] + it_behaves_like 'a hook that is never rate limited' + else + it 'rate limits the hook, returning true when rate limited' do + expect(Gitlab::ApplicationRateLimiter).to receive(:throttled?) + .exactly(3).times + .and_call_original + + freeze_time do + limit.times { expect(rate_limit!(hook)).to eq(false) } + expect(rate_limit!(hook)).to eq(true) + end + + travel_to(1.day.from_now) do + expect(rate_limit!(hook)).to eq(false) + end + end + end + end + end + + describe 'rate limit scope' do + it 'rate limits all hooks from the same namespace', :freeze_time do + create(:plan_limits, plan: plan, web_hook_calls: limit) + project_hook_in_different_namespace = create(:project_hook) + project_hook_in_same_namespace = create(:project_hook, + project: create(:project, namespace: project_hook.project.namespace) + ) + + limit.times { expect(rate_limit!(project_hook)).to eq(false) } + expect(rate_limit!(project_hook)).to eq(true) + expect(rate_limit!(project_hook_in_same_namespace)).to eq(true) + expect(rate_limit!(project_hook_in_different_namespace)).to eq(false) + end + end + end + + describe '#rate_limited?' do + subject { described_class.new(hook).rate_limited? } + + context 'when no plan limit has been defined' do + where(:hook) { [ref(:project_hook), ref(:system_hook), ref(:integration_hook)] } + + with_them do + it { is_expected.to eq(false) } + end + end + + context 'when there is a plan limit' do + before_all do + create(:plan_limits, plan: plan, web_hook_calls: limit) + end + + context 'when hook is not rate-limited' do + where(:hook) { [ref(:project_hook), ref(:system_hook), ref(:integration_hook)] } + + with_them do + it { is_expected.to eq(false) } + end + end + + context 'when hook is rate-limited' do + before do + allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) + end + + where(:hook, :limitless_hook_type) do + ref(:project_hook) | false + ref(:system_hook) | true + ref(:integration_hook) | true + end + + with_them do + it { is_expected.to eq(!limitless_hook_type) } + end + end + end + end +end diff --git a/spec/migrations/add_web_hook_calls_to_plan_limits_paid_tiers_spec.rb b/spec/migrations/add_web_hook_calls_to_plan_limits_paid_tiers_spec.rb new file mode 100644 index 00000000000..63ad9367503 --- /dev/null +++ b/spec/migrations/add_web_hook_calls_to_plan_limits_paid_tiers_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe AddWebHookCallsToPlanLimitsPaidTiers do + let_it_be(:plans) { table(:plans) } + let_it_be(:plan_limits) { table(:plan_limits) } + + context 'when on Gitlab.com' do + let(:free_plan) { plans.create!(name: 'free') } + let(:bronze_plan) { plans.create!(name: 'bronze') } + let(:silver_plan) { plans.create!(name: 'silver') } + let(:gold_plan) { plans.create!(name: 'gold') } + let(:premium_plan) { plans.create!(name: 'premium') } + let(:premium_trial_plan) { plans.create!(name: 'premium_trial') } + let(:ultimate_plan) { plans.create!(name: 'ultimate') } + let(:ultimate_trial_plan) { plans.create!(name: 'ultimate_trial') } + let(:opensource_plan) { plans.create!(name: 'opensource') } + + before do + allow(Gitlab).to receive(:com?).and_return(true) + # 120 is the value for 'free' migrated in `db/migrate/20210601131742_update_web_hook_calls_limit.rb` + plan_limits.create!(plan_id: free_plan.id, web_hook_calls: 120) + plan_limits.create!(plan_id: bronze_plan.id) + plan_limits.create!(plan_id: silver_plan.id) + plan_limits.create!(plan_id: gold_plan.id) + plan_limits.create!(plan_id: premium_plan.id) + plan_limits.create!(plan_id: premium_trial_plan.id) + plan_limits.create!(plan_id: ultimate_plan.id) + plan_limits.create!(plan_id: ultimate_trial_plan.id) + plan_limits.create!(plan_id: opensource_plan.id) + end + + it 'correctly migrates up and down' do + reversible_migration do |migration| + migration.before -> { + expect( + plan_limits.pluck(:plan_id, :web_hook_calls, :web_hook_calls_mid, :web_hook_calls_low) + ).to contain_exactly( + [free_plan.id, 120, 0, 0], + [bronze_plan.id, 0, 0, 0], + [silver_plan.id, 0, 0, 0], + [gold_plan.id, 0, 0, 0], + [premium_plan.id, 0, 0, 0], + [premium_trial_plan.id, 0, 0, 0], + [ultimate_plan.id, 0, 0, 0], + [ultimate_trial_plan.id, 0, 0, 0], + [opensource_plan.id, 0, 0, 0] + ) + } + + migration.after -> { + expect( + plan_limits.pluck(:plan_id, :web_hook_calls, :web_hook_calls_mid, :web_hook_calls_low) + ).to contain_exactly( + [free_plan.id, 500, 500, 500], + [bronze_plan.id, 4_000, 2_800, 1_600], + [silver_plan.id, 4_000, 2_800, 1_600], + [gold_plan.id, 13_000, 9_000, 6_000], + [premium_plan.id, 4_000, 2_800, 1_600], + [premium_trial_plan.id, 4_000, 2_800, 1_600], + [ultimate_plan.id, 13_000, 9_000, 6_000], + [ultimate_trial_plan.id, 13_000, 9_000, 6_000], + [opensource_plan.id, 13_000, 9_000, 6_000] + ) + } + end + end + end + + context 'when on self hosted' do + let(:default_plan) { plans.create!(name: 'default') } + + before do + allow(Gitlab).to receive(:com?).and_return(false) + + plan_limits.create!(plan_id: default_plan.id) + end + + it 'does nothing' do + reversible_migration do |migration| + migration.before -> { + expect( + plan_limits.pluck(:plan_id, :web_hook_calls, :web_hook_calls_mid, :web_hook_calls_low) + ).to contain_exactly( + [default_plan.id, 0, 0, 0] + ) + } + + migration.after -> { + expect( + plan_limits.pluck(:plan_id, :web_hook_calls, :web_hook_calls_mid, :web_hook_calls_low) + ).to contain_exactly( + [default_plan.id, 0, 0, 0] + ) + } + end + end + end +end diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index f97e89912c6..de67bdb32aa 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -40,6 +40,39 @@ RSpec.describe Clusters::Agent do it { is_expected.to contain_exactly(matching_name) } end + + describe '.has_vulnerabilities' do + let_it_be(:without_vulnerabilities) { create(:cluster_agent, has_vulnerabilities: false) } + let_it_be(:with_vulnerabilities) { create(:cluster_agent, has_vulnerabilities: true) } + + context 'when value is not provided' do + subject { described_class.has_vulnerabilities } + + it 'returns agents which have vulnerabilities' do + is_expected.to contain_exactly(with_vulnerabilities) + end + end + + context 'when value is provided' do + subject { described_class.has_vulnerabilities(value) } + + context 'as true' do + let(:value) { true } + + it 'returns agents which have vulnerabilities' do + is_expected.to contain_exactly(with_vulnerabilities) + end + end + + context 'as false' do + let(:value) { false } + + it 'returns agents which do not have vulnerabilities' do + is_expected.to contain_exactly(without_vulnerabilities) + end + end + end + end end describe 'validation' do diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index ec2eca96755..4253686b843 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -31,15 +31,6 @@ RSpec.describe ProjectHook do end end - describe '#rate_limit' do - let_it_be(:plan_limits) { create(:plan_limits, :default_plan, web_hook_calls: 100) } - let_it_be(:hook) { create(:project_hook) } - - it 'returns the default limit' do - expect(hook.rate_limit).to be(100) - end - end - describe '#parent' do it 'returns the associated project' do project = build(:project) diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 0d65fe302e1..68c284a913c 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -23,14 +23,6 @@ RSpec.describe ServiceHook do end end - describe '#rate_limit' do - let(:hook) { build(:service_hook) } - - it 'returns nil' do - expect(hook.rate_limit).to be_nil - end - end - describe '#parent' do let(:hook) { build(:service_hook, integration: integration) } diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index bf69c7219a8..9f5f81dd6c0 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -185,14 +185,6 @@ RSpec.describe SystemHook do end end - describe '#rate_limit' do - let(:hook) { build(:system_hook) } - - it 'returns nil' do - expect(hook.rate_limit).to be_nil - end - end - describe '#application_context' do let(:hook) { build(:system_hook) } diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index ab40f962af3..fb4d1cee606 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -493,31 +493,30 @@ RSpec.describe WebHook do end describe '#rate_limited?' do - context 'when there are rate limits' do - before do - allow(hook).to receive(:rate_limit).and_return(3) + it 'is false when hook has not been rate limited' do + expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:rate_limited?).and_return(false) end - it 'is false when hook has not been rate limited' do - expect(Gitlab::ApplicationRateLimiter).to receive(:peek).and_return(false) - expect(hook).not_to be_rate_limited - end + expect(hook).not_to be_rate_limited + end - it 'is true when hook has been rate limited' do - expect(Gitlab::ApplicationRateLimiter).to receive(:peek).and_return(true) - expect(hook).to be_rate_limited + it 'is true when hook has been rate limited' do + expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:rate_limited?).and_return(true) end + + expect(hook).to be_rate_limited end + end - context 'when there are no rate limits' do - before do - allow(hook).to receive(:rate_limit).and_return(nil) + describe '#rate_limit' do + it 'returns the hook rate limit' do + expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:limit).and_return(10) end - it 'does not call Gitlab::ApplicationRateLimiter, and is false' do - expect(Gitlab::ApplicationRateLimiter).not_to receive(:peek) - expect(hook).not_to be_rate_limited - end + expect(hook.rate_limit).to eq(10) end end diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 189951715c3..f93c2d36966 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -47,6 +47,16 @@ RSpec.describe GroupMember do end end + describe '#permissible_access_level_roles' do + let_it_be(:group) { create(:group) } + + it 'returns Gitlab::Access.options_with_owner' do + result = described_class.permissible_access_level_roles(group.first_owner, group) + + expect(result).to eq(Gitlab::Access.options_with_owner) + end + end + it_behaves_like 'members notifications', :group describe '#namespace_id' do diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 3923f4161cc..8c989f5aaca 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -23,6 +23,30 @@ RSpec.describe ProjectMember do end end + describe '#permissible_access_level_roles' do + let_it_be(:owner) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + before do + project.add_owner(owner) + project.add_maintainer(maintainer) + end + + context 'when member can manage owners' do + it 'returns Gitlab::Access.options_with_owner' do + expect(described_class.permissible_access_level_roles(owner, project)).to eq(Gitlab::Access.options_with_owner) + end + end + + context 'when member cannot manage owners' do + it 'returns Gitlab::Access.options' do + expect(described_class.permissible_access_level_roles(maintainer, project)).to eq(Gitlab::Access.options) + end + end + end + describe '#real_source_type' do subject { create(:project_member).real_source_type } diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index 78521e4bdf2..f9c458b2c80 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -213,6 +213,8 @@ RSpec.describe PlanLimits do storage_size_limit daily_invites web_hook_calls + web_hook_calls_mid + web_hook_calls_low ci_daily_pipeline_schedule_triggers repository_size security_policy_scan_execution_schedules diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index d093894720e..64ad5733c1b 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -59,13 +59,13 @@ RSpec.describe API::Invitations do context 'when authenticated as a maintainer/owner' do context 'and new member is already a requester' do - it 'does not transform the requester into a proper member' do + it 'transforms the requester into a proper member' do expect do post invitations_url(source, maintainer), params: { email: access_requester.email, access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:created) - end.not_to change { source.members.count } + end.to change { source.members.count }.by(1) end end @@ -258,12 +258,13 @@ RSpec.describe API::Invitations do end end - it "returns a message if member already exists" do + it "updates an already existing active member" do post invitations_url(source, maintainer), params: { email: developer.email, access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:created) - expect(json_response['message'][developer.email]).to eq("User already exists in source") + expect(json_response['status']).to eq("success") + expect(source.members.find_by(user: developer).access_level).to eq Member::MAINTAINER end it 'returns 400 when the invite params of email and user_id are not sent' do @@ -328,7 +329,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 44 # old 48 with 12 per new email, currently there are 11 queries added per email + unresolved_n_plus_ones = 40 # currently there are 10 queries added per email expect do post invitations_url(project, maintainer), params: { email: emails, access_level: Member::DEVELOPER } @@ -351,7 +352,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 67 # currently there are 11 queries added per email + unresolved_n_plus_ones = 59 # currently there are 10 queries added per email expect do post invitations_url(project, maintainer), params: { email: emails, access_level: Member::DEVELOPER } @@ -373,7 +374,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 36 # old 40 with 10 per new email, currently there are 9 queries added per email + unresolved_n_plus_ones = 32 # currently there are 8 queries added per email expect do post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } @@ -396,7 +397,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 62 # currently there are 9 queries added per email + unresolved_n_plus_ones = 56 # currently there are 8 queries added per email expect do post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } diff --git a/spec/services/jira_connect_subscriptions/create_service_spec.rb b/spec/services/jira_connect_subscriptions/create_service_spec.rb index cde4753cde7..85208a30c30 100644 --- a/spec/services/jira_connect_subscriptions/create_service_spec.rb +++ b/spec/services/jira_connect_subscriptions/create_service_spec.rb @@ -3,9 +3,10 @@ require 'spec_helper' RSpec.describe JiraConnectSubscriptions::CreateService do - let(:installation) { create(:jira_connect_installation) } - let(:current_user) { create(:user) } - let(:group) { create(:group) } + let_it_be(:installation) { create(:jira_connect_installation) } + let_it_be(:current_user) { create(:user) } + let_it_be(:group) { create(:group) } + let(:path) { group.full_path } let(:params) { { namespace_path: path, jira_user: jira_user } } let(:jira_user) { double(:JiraUser, site_admin?: true) } @@ -16,38 +17,31 @@ RSpec.describe JiraConnectSubscriptions::CreateService do group.add_maintainer(current_user) end - shared_examples 'a failed execution' do + shared_examples 'a failed execution' do |**status_attributes| it 'does not create a subscription' do expect { subject }.not_to change { installation.subscriptions.count } end it 'returns an error status' do expect(subject[:status]).to eq(:error) + expect(subject).to include(status_attributes) end end context 'remote user does not have access' do let(:jira_user) { double(site_admin?: false) } - it 'does not create a subscription' do - expect { subject }.not_to change { installation.subscriptions.count } - end - - it 'returns error' do - expect(subject[:status]).to eq(:error) - end + it_behaves_like 'a failed execution', + http_status: 403, + message: 'The Jira user is not a site administrator. Check the permissions in Jira and try again.' end context 'remote user cannot be retrieved' do let(:jira_user) { nil } - it 'does not create a subscription' do - expect { subject }.not_to change { installation.subscriptions.count } - end - - it 'returns error' do - expect(subject[:status]).to eq(:error) - end + it_behaves_like 'a failed execution', + http_status: 403, + message: 'Could not fetch user information from Jira. Check the permissions in Jira and try again.' end context 'when user does have access' do @@ -60,8 +54,8 @@ RSpec.describe JiraConnectSubscriptions::CreateService do end context 'namespace has projects' do - let!(:project_1) { create(:project, group: group) } - let!(:project_2) { create(:project, group: group) } + let_it_be(:project_1) { create(:project, group: group) } + let_it_be(:project_2) { create(:project, group: group) } before do stub_const("#{described_class}::MERGE_REQUEST_SYNC_BATCH_SIZE", 1) @@ -81,12 +75,18 @@ RSpec.describe JiraConnectSubscriptions::CreateService do context 'when path is invalid' do let(:path) { 'some_invalid_namespace_path' } - it_behaves_like 'a failed execution' + it_behaves_like 'a failed execution', + http_status: 401, + message: 'Cannot find namespace. Make sure you have sufficient permissions.' end context 'when user does not have access' do - subject { described_class.new(installation, create(:user), namespace_path: path).execute } + let_it_be(:other_group) { create(:group) } + + let(:path) { other_group.full_path } - it_behaves_like 'a failed execution' + it_behaves_like 'a failed execution', + http_status: 401, + message: 'Cannot find namespace. Make sure you have sufficient permissions.' end end diff --git a/spec/services/members/creator_service_spec.rb b/spec/services/members/creator_service_spec.rb index ff5bf705b6c..8b1df2ab86d 100644 --- a/spec/services/members/creator_service_spec.rb +++ b/spec/services/members/creator_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Members::CreatorService do describe '#execute' do it 'raises error for new member on authorization check implementation' do expect do - described_class.new(source, user, :maintainer, current_user: current_user).execute + described_class.add_user(source, user, :maintainer, current_user: current_user) end.to raise_error(NotImplementedError) end @@ -19,7 +19,7 @@ RSpec.describe Members::CreatorService do source.add_developer(user) expect do - described_class.new(source, user, :maintainer, current_user: current_user).execute + described_class.add_user(source, user, :maintainer, current_user: current_user) end.to raise_error(NotImplementedError) end end diff --git a/spec/services/members/groups/bulk_creator_service_spec.rb b/spec/services/members/groups/bulk_creator_service_spec.rb deleted file mode 100644 index 3922c37487c..00000000000 --- a/spec/services/members/groups/bulk_creator_service_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Members::Groups::BulkCreatorService do - let_it_be(:source, reload: true) { create(:group, :public) } - let_it_be(:current_user) { create(:user) } - - it_behaves_like 'bulk member creation' do - let_it_be(:member_type) { GroupMember } - end - - it_behaves_like 'owner management' -end diff --git a/spec/services/members/groups/creator_service_spec.rb b/spec/services/members/groups/creator_service_spec.rb index c3ba7c0374d..b80b7998eac 100644 --- a/spec/services/members/groups/creator_service_spec.rb +++ b/spec/services/members/groups/creator_service_spec.rb @@ -3,16 +3,24 @@ require 'spec_helper' RSpec.describe Members::Groups::CreatorService do + let_it_be(:source, reload: true) { create(:group, :public) } + let_it_be(:user) { create(:user) } + describe '.access_levels' do it 'returns Gitlab::Access.options_with_owner' do expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) end end - describe '#execute' do - let_it_be(:source, reload: true) { create(:group, :public) } - let_it_be(:user) { create(:user) } + it_behaves_like 'owner management' + + describe '.add_users' do + it_behaves_like 'bulk member creation' do + let_it_be(:member_type) { GroupMember } + end + end + describe '.add_user' do it_behaves_like 'member creation' do let_it_be(:member_type) { GroupMember } end @@ -22,7 +30,7 @@ RSpec.describe Members::Groups::CreatorService do expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait).once 1.upto(3) do - described_class.new(source, user, :maintainer).execute + described_class.add_user(source, user, :maintainer) end end end diff --git a/spec/services/members/invite_service_spec.rb b/spec/services/members/invite_service_spec.rb index 8213e8baae0..a948041479b 100644 --- a/spec/services/members/invite_service_spec.rb +++ b/spec/services/members/invite_service_spec.rb @@ -367,20 +367,21 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ context 'when email is already a member with a user on the project' do let!(:existing_member) { create(:project_member, :guest, project: project) } - let(:params) { { email: "#{existing_member.user.email}" } } + let(:params) { { email: "#{existing_member.user.email}", access_level: ProjectMember::MAINTAINER } } - it 'returns an error for the already invited email' do - expect_not_to_create_members - expect(result[:message][existing_member.user.email]).to eq("User already exists in source") + it 'allows re-invite of an already invited email and updates the access_level' do + expect { result }.not_to change(ProjectMember, :count) + expect(result[:status]).to eq(:success) + expect(existing_member.reset.access_level).to eq ProjectMember::MAINTAINER end context 'when email belongs to an existing user as a secondary email' do let(:secondary_email) { create(:email, email: 'secondary@example.com', user: existing_member.user) } let(:params) { { email: "#{secondary_email.email}" } } - it 'returns an error for the already invited email' do - expect_not_to_create_members - expect(result[:message][secondary_email.email]).to eq("User already exists in source") + it 'allows re-invite to an already invited email' do + expect_to_create_members(count: 0) + expect(result[:status]).to eq(:success) end end end diff --git a/spec/services/members/projects/bulk_creator_service_spec.rb b/spec/services/members/projects/bulk_creator_service_spec.rb deleted file mode 100644 index dd998b47eb3..00000000000 --- a/spec/services/members/projects/bulk_creator_service_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Members::Projects::BulkCreatorService do - let_it_be(:source, reload: true) { create(:project, :public) } - let_it_be(:current_user) { create(:user) } - - it_behaves_like 'bulk member creation' do - let_it_be(:member_type) { ProjectMember } - end - - it_behaves_like 'owner management' -end diff --git a/spec/services/members/projects/creator_service_spec.rb b/spec/services/members/projects/creator_service_spec.rb index 7605238c3c5..38955122ab0 100644 --- a/spec/services/members/projects/creator_service_spec.rb +++ b/spec/services/members/projects/creator_service_spec.rb @@ -3,16 +3,24 @@ require 'spec_helper' RSpec.describe Members::Projects::CreatorService do + let_it_be(:source, reload: true) { create(:project, :public) } + let_it_be(:user) { create(:user) } + describe '.access_levels' do it 'returns Gitlab::Access.sym_options_with_owner' do expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) end end - describe '#execute' do - let_it_be(:source, reload: true) { create(:project, :public) } - let_it_be(:user) { create(:user) } + it_behaves_like 'owner management' + + describe '.add_users' do + it_behaves_like 'bulk member creation' do + let_it_be(:member_type) { ProjectMember } + end + end + describe '.add_user' do it_behaves_like 'member creation' do let_it_be(:member_type) { ProjectMember } end @@ -22,7 +30,7 @@ RSpec.describe Members::Projects::CreatorService do expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to receive(:bulk_perform_in).once 1.upto(3) do - described_class.new(source, user, :maintainer).execute + described_class.add_user(source, user, :maintainer) end end end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 9f3093d64f3..068550ec234 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -521,7 +521,7 @@ RSpec.describe WebHookService, :request_store, :clean_gitlab_redis_shared_state def expect_to_rate_limit(hook, threshold:, throttled: false) expect(Gitlab::ApplicationRateLimiter).to receive(:throttled?) - .with(:web_hook_calls, scope: [hook], threshold: threshold) + .with(:web_hook_calls, scope: [hook.parent.root_namespace], threshold: threshold) .and_return(throttled) end @@ -570,13 +570,8 @@ RSpec.describe WebHookService, :request_store, :clean_gitlab_redis_shared_state end end - context 'when the hook is throttled (via Redis)', :clean_gitlab_redis_rate_limiting do + context 'when the hook is throttled (via Redis)', :clean_gitlab_redis_rate_limiting, :freeze_time do before do - # Set a high interval to avoid intermittent failures in CI - allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits).and_return( - web_hook_calls: { interval: 1.day } - ) - expect_to_perform_worker(project_hook).exactly(threshold).times threshold.times { service_instance.async_execute } diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb index 0537c3b7327..75fff11cecd 100644 --- a/spec/support/shared_examples/models/member_shared_examples.rb +++ b/spec/support/shared_examples/models/member_shared_examples.rb @@ -80,7 +80,7 @@ RSpec.shared_examples_for "member creation" do let_it_be(:admin) { create(:admin) } it 'returns a Member object', :aggregate_failures do - member = described_class.new(source, user, :maintainer).execute + member = described_class.add_user(source, user, :maintainer) expect(member).to be_a member_type expect(member).to be_persisted @@ -99,7 +99,7 @@ RSpec.shared_examples_for "member creation" do end it 'does not update the member' do - member = described_class.new(source, project_bot, :maintainer, current_user: user).execute + member = described_class.add_user(source, project_bot, :maintainer, current_user: user) expect(source.users.reload).to include(project_bot) expect(member).to be_persisted @@ -110,7 +110,7 @@ RSpec.shared_examples_for "member creation" do context 'when project_bot is not already a member' do it 'adds the member' do - member = described_class.new(source, project_bot, :maintainer, current_user: user).execute + member = described_class.add_user(source, project_bot, :maintainer, current_user: user) expect(source.users.reload).to include(project_bot) expect(member).to be_persisted @@ -120,7 +120,7 @@ RSpec.shared_examples_for "member creation" do context 'when admin mode is enabled', :enable_admin_mode, :aggregate_failures do it 'sets members.created_by to the given admin current_user' do - member = described_class.new(source, user, :maintainer, current_user: admin).execute + member = described_class.add_user(source, user, :maintainer, current_user: admin) expect(member).to be_persisted expect(source.users.reload).to include(user) @@ -130,7 +130,7 @@ RSpec.shared_examples_for "member creation" do context 'when admin mode is disabled' do it 'rejects setting members.created_by to the given admin current_user', :aggregate_failures do - member = described_class.new(source, user, :maintainer, current_user: admin).execute + member = described_class.add_user(source, user, :maintainer, current_user: admin) expect(member).not_to be_persisted expect(source.users.reload).not_to include(user) @@ -139,7 +139,7 @@ RSpec.shared_examples_for "member creation" do end it 'sets members.expires_at to the given expires_at' do - member = described_class.new(source, user, :maintainer, expires_at: Date.new(2016, 9, 22)).execute + member = described_class.add_user(source, user, :maintainer, expires_at: Date.new(2016, 9, 22)) expect(member.expires_at).to eq(Date.new(2016, 9, 22)) end @@ -148,7 +148,7 @@ RSpec.shared_examples_for "member creation" do it "accepts the :#{sym_key} symbol as access level", :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.new(source, user.id, sym_key).execute + member = described_class.add_user(source, user.id, sym_key) expect(member.access_level).to eq(int_access_level) expect(source.users.reload).to include(user) @@ -157,7 +157,7 @@ RSpec.shared_examples_for "member creation" do it "accepts the #{int_access_level} integer as access level", :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.new(source, user.id, int_access_level).execute + member = described_class.add_user(source, user.id, int_access_level) expect(member.access_level).to eq(int_access_level) expect(source.users.reload).to include(user) @@ -169,7 +169,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.new(source, user.id, :maintainer).execute + described_class.add_user(source, user.id, :maintainer) expect(source.users.reload).to include(user) end @@ -179,7 +179,7 @@ RSpec.shared_examples_for "member creation" do it 'does not add the user as a member' do expect(source.users).not_to include(user) - described_class.new(source, non_existing_record_id, :maintainer).execute + described_class.add_user(source, non_existing_record_id, :maintainer) expect(source.users.reload).not_to include(user) end @@ -189,7 +189,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.new(source, user, :maintainer).execute + described_class.add_user(source, user, :maintainer) expect(source.users.reload).to include(user) end @@ -200,12 +200,12 @@ RSpec.shared_examples_for "member creation" do source.request_access(user) end - it 'adds the requester as a member', :aggregate_failures do + it 'does not add the requester as a regular member', :aggregate_failures do expect(source.users).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy expect do - described_class.new(source, user, :maintainer).execute + described_class.add_user(source, user, :maintainer) end.to raise_error(Gitlab::Access::AccessDeniedError) expect(source.users.reload).not_to include(user) @@ -217,7 +217,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.new(source, user.email, :maintainer).execute + described_class.add_user(source, user.email, :maintainer) expect(source.users.reload).to include(user) end @@ -227,7 +227,7 @@ RSpec.shared_examples_for "member creation" do it 'creates an invited member' do expect(source.users).not_to include(user) - described_class.new(source, 'user@example.com', :maintainer).execute + described_class.add_user(source, 'user@example.com', :maintainer) expect(source.members.invite.pluck(:invite_email)).to include('user@example.com') end @@ -237,7 +237,7 @@ RSpec.shared_examples_for "member creation" do it 'creates an invited member', :aggregate_failures do email_starting_with_number = "#{user.id}_email@example.com" - described_class.new(source, email_starting_with_number, :maintainer).execute + described_class.add_user(source, email_starting_with_number, :maintainer) expect(source.members.invite.pluck(:invite_email)).to include(email_starting_with_number) expect(source.users.reload).not_to include(user) @@ -249,7 +249,7 @@ RSpec.shared_examples_for "member creation" do it 'creates the member' do expect(source.users).not_to include(user) - described_class.new(source, user, :maintainer, current_user: admin).execute + described_class.add_user(source, user, :maintainer, current_user: admin) expect(source.users.reload).to include(user) end @@ -263,7 +263,7 @@ RSpec.shared_examples_for "member creation" do expect(source.users).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy - described_class.new(source, user, :maintainer, current_user: admin).execute + described_class.add_user(source, user, :maintainer, current_user: admin) expect(source.users.reload).to include(user) expect(source.requesters.reload.exists?(user_id: user)).to be_falsy @@ -275,7 +275,7 @@ RSpec.shared_examples_for "member creation" do it 'does not create the member', :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.new(source, user, :maintainer, current_user: user).execute + member = described_class.add_user(source, user, :maintainer, current_user: user) expect(source.users.reload).not_to include(user) expect(member).not_to be_persisted @@ -290,7 +290,7 @@ RSpec.shared_examples_for "member creation" do expect(source.users).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy - described_class.new(source, user, :maintainer, current_user: user).execute + described_class.add_user(source, user, :maintainer, current_user: user) expect(source.users.reload).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy @@ -307,7 +307,7 @@ RSpec.shared_examples_for "member creation" do it 'updates the member' do expect(source.users).to include(user) - described_class.new(source, user, :maintainer).execute + described_class.add_user(source, user, :maintainer) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) end @@ -317,7 +317,7 @@ RSpec.shared_examples_for "member creation" do it 'updates the member' do expect(source.users).to include(user) - described_class.new(source, user, :maintainer, current_user: admin).execute + described_class.add_user(source, user, :maintainer, current_user: admin) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) end @@ -327,242 +327,180 @@ RSpec.shared_examples_for "member creation" do it 'does not update the member' do expect(source.users).to include(user) - described_class.new(source, user, :maintainer, current_user: user).execute + described_class.add_user(source, user, :maintainer, current_user: user) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::DEVELOPER) end end end +end - context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do - let(:task_project) { source.is_a?(Group) ? create(:project, group: source) : source } - - it 'creates a member_task with the correct attributes', :aggregate_failures do - described_class.new(source, user, :developer, tasks_to_be_done: %w(ci code), tasks_project_id: task_project.id).execute +RSpec.shared_examples_for "bulk member creation" do + let_it_be(:admin) { create(:admin) } + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } - member = source.members.last + context 'when current user does not have permission' do + it 'does not succeed' do + # maintainers cannot add owners + source.add_maintainer(user) - expect(member.tasks_to_be_done).to match_array([:ci, :code]) - expect(member.member_task.project).to eq(task_project) + expect(described_class.add_users(source, [user1, user2], :owner, current_user: user)).to be_empty end + end - context 'with an already existing member' do - before do - source.add_user(user, :developer) - end - - it 'does not update tasks to be done if tasks already exist', :aggregate_failures do - member = source.members.find_by(user_id: user.id) - create(:member_task, member: member, project: task_project, tasks_to_be_done: %w(code ci)) - - expect do - described_class.new(source, - user, - :developer, - tasks_to_be_done: %w(issues), - tasks_project_id: task_project.id).execute - end.not_to change(MemberTask, :count) - - member.reset - expect(member.tasks_to_be_done).to match_array([:code, :ci]) - expect(member.member_task.project).to eq(task_project) - end - - it 'adds tasks to be done if they do not exist', :aggregate_failures do - expect do - described_class.new(source, - user, - :developer, - tasks_to_be_done: %w(issues), - tasks_project_id: task_project.id).execute - end.to change(MemberTask, :count).by(1) + it 'returns Member objects' do + members = described_class.add_users(source, [user1, user2], :maintainer) - member = source.members.find_by(user_id: user.id) - expect(member.tasks_to_be_done).to match_array([:issues]) - expect(member.member_task.project).to eq(task_project) - end - end + expect(members.map(&:user)).to contain_exactly(user1, user2) + expect(members).to all(be_a(member_type)) + expect(members).to all(be_persisted) end -end -RSpec.shared_examples_for "bulk member creation" do - let_it_be(:user) { create(:user) } - let_it_be(:admin) { create(:admin) } + it 'returns an empty array' do + members = described_class.add_users(source, [], :maintainer) - describe '#execute' do - it 'raises an error when exiting_members is not passed in the args hash' do - expect do - described_class.new(source, user, :maintainer, current_user: user).execute - end.to raise_error(ArgumentError, 'existing_members must be included in the args hash') - end + expect(members).to be_a Array + expect(members).to be_empty end - describe '.add_users', :aggregate_failures do - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } + it 'supports different formats' do + list = ['joe@local.test', admin, user1.id, user2.id.to_s] - context 'when current user does not have permission' do - it 'does not succeed' do - # maintainers cannot add owners - source.add_maintainer(user) + members = described_class.add_users(source, list, :maintainer) - expect(described_class.add_users(source, [user1, user2], :owner, current_user: user)).to be_empty - end - end + expect(members.size).to eq(4) + expect(members.first).to be_invite + end - it 'returns a Member objects' do - members = described_class.add_users(source, [user1, user2], :maintainer) + context 'with de-duplication' do + it 'has the same user by id and user' do + members = described_class.add_users(source, [user1.id, user1, user1.id, user2, user2.id, user2], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) end - it 'returns an empty array' do - members = described_class.add_users(source, [], :maintainer) + it 'has the same user sent more than once' do + members = described_class.add_users(source, [user1, user1], :maintainer) - expect(members).to be_a Array - expect(members).to be_empty + expect(members.map(&:user)).to contain_exactly(user1) + expect(members).to all(be_a(member_type)) + expect(members).to all(be_persisted) end + end - it 'supports different formats' do - list = ['joe@local.test', admin, user1.id, user2.id.to_s] + it 'with the same user sent more than once by user and by email' do + members = described_class.add_users(source, [user1, user1.email], :maintainer) - members = described_class.add_users(source, list, :maintainer) + expect(members.map(&:user)).to contain_exactly(user1) + expect(members).to all(be_a(member_type)) + expect(members).to all(be_persisted) + end - expect(members.size).to eq(4) - expect(members.first).to be_invite - end + it 'with the same user sent more than once by user id and by email' do + members = described_class.add_users(source, [user1.id, user1.email], :maintainer) - context 'with de-duplication' do - it 'has the same user by id and user' do - members = described_class.add_users(source, [user1.id, user1, user1.id, user2, user2.id, user2], :maintainer) + expect(members.map(&:user)).to contain_exactly(user1) + expect(members).to all(be_a(member_type)) + expect(members).to all(be_persisted) + end + + context 'when a member already exists' do + before do + source.add_user(user1, :developer) + end + it 'has the same user sent more than once with the member already existing' do + expect do + members = described_class.add_users(source, [user1, user1, user2], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) - end + end.to change { Member.count }.by(1) + end - it 'has the same user sent more than once' do - members = described_class.add_users(source, [user1, user1], :maintainer) + it 'supports existing users as expected with user_ids passed' do + user3 = create(:user) - expect(members.map(&:user)).to contain_exactly(user1) + expect do + members = described_class.add_users(source, [user1.id, user2, user3.id], :maintainer) + expect(members.map(&:user)).to contain_exactly(user1, user2, user3) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) - end + end.to change { Member.count }.by(2) end - it 'with the same user sent more than once by user and by email' do - members = described_class.add_users(source, [user1, user1.email], :maintainer) + it 'supports existing users as expected without user ids passed' do + user3 = create(:user) - expect(members.map(&:user)).to contain_exactly(user1) - expect(members).to all(be_a(member_type)) - expect(members).to all(be_persisted) + expect do + members = described_class.add_users(source, [user1, user2, user3], :maintainer) + expect(members.map(&:user)).to contain_exactly(user1, user2, user3) + expect(members).to all(be_a(member_type)) + expect(members).to all(be_persisted) + end.to change { Member.count }.by(2) end + end + + context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do + let(:task_project) { source.is_a?(Group) ? create(:project, group: source) : source } - it 'with the same user sent more than once by user id and by email' do - members = described_class.add_users(source, [user1.id, user1.email], :maintainer) + it 'creates a member_task with the correct attributes', :aggregate_failures do + members = described_class.add_users(source, [user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: task_project.id) + member = members.last - expect(members.map(&:user)).to contain_exactly(user1) - expect(members).to all(be_a(member_type)) - expect(members).to all(be_persisted) + expect(member.tasks_to_be_done).to match_array([:ci, :code]) + expect(member.member_task.project).to eq(task_project) end - context 'when a member already exists' do + context 'with an already existing member' do before do source.add_user(user1, :developer) end - it 'has the same user sent more than once with the member already existing' do - expect do - members = described_class.add_users(source, [user1, user1, user2], :maintainer) - expect(members.map(&:user)).to contain_exactly(user1, user2) - expect(members).to all(be_a(member_type)) - expect(members).to all(be_persisted) - end.to change { Member.count }.by(1) - end - - it 'supports existing users as expected with user_ids passed' do - user3 = create(:user) + it 'does not update tasks to be done if tasks already exist', :aggregate_failures do + member = source.members.find_by(user_id: user1.id) + create(:member_task, member: member, project: task_project, tasks_to_be_done: %w(code ci)) expect do - members = described_class.add_users(source, [user1.id, user2, user3.id], :maintainer) - expect(members.map(&:user)).to contain_exactly(user1, user2, user3) - expect(members).to all(be_a(member_type)) - expect(members).to all(be_persisted) - end.to change { Member.count }.by(2) - end - - it 'supports existing users as expected without user ids passed' do - user3 = create(:user) + described_class.add_users(source, + [user1.id], + :developer, + tasks_to_be_done: %w(issues), + tasks_project_id: task_project.id) + end.not_to change(MemberTask, :count) - expect do - members = described_class.add_users(source, [user1, user2, user3], :maintainer) - expect(members.map(&:user)).to contain_exactly(user1, user2, user3) - expect(members).to all(be_a(member_type)) - expect(members).to all(be_persisted) - end.to change { Member.count }.by(2) + member.reset + expect(member.tasks_to_be_done).to match_array([:code, :ci]) + expect(member.member_task.project).to eq(task_project) end - end - context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do - let(:task_project) { source.is_a?(Group) ? create(:project, group: source) : source } - - it 'creates a member_task with the correct attributes', :aggregate_failures do - members = described_class.add_users(source, [user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: task_project.id) - member = members.last + it 'adds tasks to be done if they do not exist', :aggregate_failures do + expect do + described_class.add_users(source, + [user1.id], + :developer, + tasks_to_be_done: %w(issues), + tasks_project_id: task_project.id) + end.to change(MemberTask, :count).by(1) - expect(member.tasks_to_be_done).to match_array([:ci, :code]) + member = source.members.find_by(user_id: user1.id) + expect(member.tasks_to_be_done).to match_array([:issues]) expect(member.member_task.project).to eq(task_project) end - - context 'with an already existing member' do - before do - source.add_user(user1, :developer) - end - - it 'does not update tasks to be done if tasks already exist', :aggregate_failures do - member = source.members.find_by(user_id: user1.id) - create(:member_task, member: member, project: task_project, tasks_to_be_done: %w(code ci)) - - expect do - described_class.add_users(source, - [user1.id], - :developer, - tasks_to_be_done: %w(issues), - tasks_project_id: task_project.id) - end.not_to change(MemberTask, :count) - - member.reset - expect(member.tasks_to_be_done).to match_array([:code, :ci]) - expect(member.member_task.project).to eq(task_project) - end - - it 'adds tasks to be done if they do not exist', :aggregate_failures do - expect do - described_class.add_users(source, - [user1.id], - :developer, - tasks_to_be_done: %w(issues), - tasks_project_id: task_project.id) - end.to change(MemberTask, :count).by(1) - - member = source.members.find_by(user_id: user1.id) - expect(member.tasks_to_be_done).to match_array([:issues]) - expect(member.member_task.project).to eq(task_project) - end - end end end end RSpec.shared_examples 'owner management' do describe '.cannot_manage_owners?' do - subject { described_class.cannot_manage_owners?(source, current_user) } + subject { described_class.cannot_manage_owners?(source, user) } context 'when maintainer' do before do - source.add_maintainer(current_user) + source.add_maintainer(user) end it 'cannot manage owners' do @@ -572,7 +510,7 @@ RSpec.shared_examples 'owner management' do context 'when owner' do before do - source.add_owner(current_user) + source.add_owner(user) end it 'can manage owners' do |