From 75f2fd2ba85fea64ee1a42f704c4872282f55eb5 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 13 Jul 2023 21:10:39 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../javascripts/ci/reports/constants.js | 3 + .../javascripts/ci/reports/sast/constants.js | 44 ++++ app/assets/javascripts/ci/reports/utils.js | 20 ++ .../contribution_event_merged.vue | 29 +++ .../components/contribution_events.vue | 5 + .../javascripts/diffs/components/app.vue | 33 +++ .../diffs/components/diff_code_quality.vue | 51 +++-- .../components/diff_code_quality_item.vue | 32 ++- .../diffs/components/diff_content.vue | 13 +- .../diffs/components/diff_file.vue | 49 +++- .../diffs/components/diff_file_header.vue | 6 - .../diffs/components/diff_line.vue | 14 +- .../javascripts/diffs/components/diff_row.vue | 30 ++- .../diffs/components/diff_row_utils.js | 4 + .../diffs/components/diff_view.vue | 11 +- .../diffs/components/pre_renderer.vue | 83 +++++++ .../virtual_scroller_scroll_sync.js | 14 +- app/assets/javascripts/diffs/i18n.js | 1 + app/assets/javascripts/diffs/index.js | 2 + app/assets/javascripts/diffs/store/actions.js | 54 ++++- app/assets/javascripts/diffs/store/getters.js | 5 + .../javascripts/diffs/store/mutations.js | 14 ++ app/assets/javascripts/diffs/store/utils.js | 2 + .../projects/merge_requests_controller.rb | 1 + app/models/packages/npm/metadatum.rb | 7 +- app/models/packages/package.rb | 5 +- .../packages/npm/create_package_service.rb | 4 +- doc/administration/get_started.md | 2 +- .../reporting/ip_addr_restrictions.md | 33 +++ doc/administration/reporting/spamcheck.md | 69 ++++++ .../settings/sign_up_restrictions.md | 206 +++++++++++++++++ doc/administration/settings/terms.md | 49 ++++ .../settings/terraform_limits.md | 28 +++ .../settings/third_party_offers.md | 38 ++++ doc/api/lint.md | 6 +- doc/subscriptions/gitlab_dedicated/index.md | 2 +- doc/subscriptions/self_managed/index.md | 4 +- doc/user/admin_area/moderate_users.md | 4 +- .../reporting/ip_addr_restrictions.md | 36 +-- doc/user/admin_area/reporting/spamcheck.md | 72 +----- .../settings/sign_up_restrictions.md | 209 +----------------- doc/user/admin_area/settings/terms.md | 52 +---- .../admin_area/settings/terraform_limits.md | 31 +-- .../admin_area/settings/third_party_offers.md | 41 +--- .../admin_area/settings/usage_statistics.md | 2 +- .../group/settings/group_access_tokens.md | 2 +- doc/user/profile/user_passwords.md | 2 +- gems/click_house-client/.rubocop.yml | 17 +- gems/config/rubocop.yml | 8 + gems/gitlab-schema-validation/.rubocop.yml | 3 - .../spec/gitlab/utils/strong_memoize_spec.rb | 2 +- gems/ipynbdiff/.rubocop.yml | 7 - .../spec/rspec_flaky/flaky_example_spec.rb | 2 +- lib/api/concerns/packages/npm_endpoints.rb | 6 +- lib/api/helpers/packages/npm.rb | 15 ++ lib/api/internal/kubernetes.rb | 16 +- lib/api/npm_instance_packages.rb | 4 - lib/api/npm_project_packages.rb | 4 +- lib/gitlab/kas/client.rb | 2 + lib/gitlab/regex.rb | 8 + locale/gitlab.pot | 20 +- qa/qa/page/component/dropdown.rb | 4 +- .../terraform_module_registry_spec.rb | 6 +- .../package_registry/conan_repository_spec.rb | 5 +- .../package_registry/helm_registry_spec.rb | 6 +- .../maven/maven_group_level_spec.rb | 6 +- .../maven/maven_project_level_spec.rb | 6 +- .../maven_gradle_repository_spec.rb | 7 +- .../package_registry/pypi_repository_spec.rb | 6 +- scripts/rspec_helpers.sh | 36 +-- .../merge_request/user_sees_diff_spec.rb | 5 +- .../ci/reports/mock_data/mock_data.js | 54 +++++ spec/frontend/ci/reports/utils_spec.js | 30 +++ .../contribution_event_merged_spec.js | 31 +++ .../components/contribution_events_spec.js | 4 + spec/frontend/contribution_events/utils.js | 3 + spec/frontend/diffs/components/app_spec.js | 12 + .../components/diff_code_quality_item_spec.js | 37 ++-- .../components/diff_code_quality_spec.js | 44 +++- .../diffs/components/diff_content_spec.js | 28 +++ .../diffs/components/diff_file_spec.js | 18 ++ .../diffs/components/diff_line_spec.js | 21 ++ .../diffs/mock_data/diff_code_quality.js | 79 ++++++- spec/frontend/diffs/store/actions_spec.js | 54 ++++- spec/frontend/diffs/store/mutations_spec.js | 1 + spec/frontend/diffs/store/utils_spec.js | 6 +- spec/lib/gitlab/kas/client_spec.rb | 3 +- spec/models/packages/npm/metadatum_spec.rb | 4 +- spec/requests/api/internal/kubernetes_spec.rb | 4 +- .../requests/api/npm_project_packages_spec.rb | 24 +- .../npm/create_package_service_spec.rb | 6 +- .../api/npm_packages_shared_examples.rb | 29 +-- .../api/npm_packages_tags_shared_examples.rb | 80 ++++--- ...ponse_status_with_error_shared_examples.rb | 11 + 94 files changed, 1534 insertions(+), 674 deletions(-) create mode 100644 app/assets/javascripts/ci/reports/sast/constants.js create mode 100644 app/assets/javascripts/ci/reports/utils.js create mode 100644 app/assets/javascripts/contribution_events/components/contribution_event/contribution_event_merged.vue create mode 100644 app/assets/javascripts/diffs/components/pre_renderer.vue create mode 100644 doc/administration/reporting/ip_addr_restrictions.md create mode 100644 doc/administration/reporting/spamcheck.md create mode 100644 doc/administration/settings/sign_up_restrictions.md create mode 100644 doc/administration/settings/terms.md create mode 100644 doc/administration/settings/terraform_limits.md create mode 100644 doc/administration/settings/third_party_offers.md create mode 100644 spec/frontend/ci/reports/utils_spec.js create mode 100644 spec/frontend/contribution_events/components/contribution_event/contribution_event_merged_spec.js create mode 100644 spec/support/shared_examples/requests/response_status_with_error_shared_examples.rb diff --git a/app/assets/javascripts/ci/reports/constants.js b/app/assets/javascripts/ci/reports/constants.js index b24a299a7ed..3968f8db752 100644 --- a/app/assets/javascripts/ci/reports/constants.js +++ b/app/assets/javascripts/ci/reports/constants.js @@ -20,3 +20,6 @@ export const status = { export const SLOT_SUCCESS = 'success'; export const SLOT_LOADING = 'loading'; export const SLOT_ERROR = 'error'; + +export const CODE_QUALITY_SCALE_KEY = 'codeQuality'; +export const SAST_SCALE_KEY = 'sast'; diff --git a/app/assets/javascripts/ci/reports/sast/constants.js b/app/assets/javascripts/ci/reports/sast/constants.js new file mode 100644 index 00000000000..3800065917b --- /dev/null +++ b/app/assets/javascripts/ci/reports/sast/constants.js @@ -0,0 +1,44 @@ +export const SEVERITY_CLASSES = { + info: 'gl-text-blue-400', + low: 'gl-text-orange-300', + medium: 'gl-text-orange-400', + high: 'gl-text-red-600', + critical: 'gl-text-red-800', + unknown: 'gl-text-gray-400', +}; + +export const SEVERITY_ICONS = { + info: 'severity-info', + low: 'severity-low', + medium: 'severity-medium', + high: 'severity-high', + critical: 'severity-critical', + unknown: 'severity-unknown', +}; + +export const SEVERITIES = { + info: { + class: SEVERITY_CLASSES.info, + name: SEVERITY_ICONS.info, + }, + low: { + class: SEVERITY_CLASSES.low, + name: SEVERITY_ICONS.low, + }, + medium: { + class: SEVERITY_CLASSES.medium, + name: SEVERITY_ICONS.medium, + }, + high: { + class: SEVERITY_CLASSES.high, + name: SEVERITY_ICONS.high, + }, + critical: { + class: SEVERITY_CLASSES.critical, + name: SEVERITY_ICONS.critical, + }, + unknown: { + class: SEVERITY_CLASSES.unknown, + name: SEVERITY_ICONS.unknown, + }, +}; diff --git a/app/assets/javascripts/ci/reports/utils.js b/app/assets/javascripts/ci/reports/utils.js new file mode 100644 index 00000000000..bb6eddf2cce --- /dev/null +++ b/app/assets/javascripts/ci/reports/utils.js @@ -0,0 +1,20 @@ +import { SEVERITIES as SEVERITIES_CODE_QUALITY } from '~/ci/reports/codequality_report/constants'; +import { SEVERITIES as SEVERITIES_SAST } from '~/ci/reports/sast/constants'; +import { SAST_SCALE_KEY } from './constants'; + +function mapSeverity(findings) { + const severityInfo = + findings.scale === SAST_SCALE_KEY ? SEVERITIES_SAST : SEVERITIES_CODE_QUALITY; + return { + ...findings, + class: severityInfo[findings.severity].class, + name: severityInfo[findings.severity].name, + }; +} + +export function getSeverity(findings) { + if (Array.isArray(findings)) { + return findings.map((finding) => mapSeverity(finding)); + } + return mapSeverity(findings); +} diff --git a/app/assets/javascripts/contribution_events/components/contribution_event/contribution_event_merged.vue b/app/assets/javascripts/contribution_events/components/contribution_event/contribution_event_merged.vue new file mode 100644 index 00000000000..d2566160b18 --- /dev/null +++ b/app/assets/javascripts/contribution_events/components/contribution_event/contribution_event_merged.vue @@ -0,0 +1,29 @@ + + + diff --git a/app/assets/javascripts/contribution_events/components/contribution_events.vue b/app/assets/javascripts/contribution_events/components/contribution_events.vue index eb28d058e4c..62c803b9217 100644 --- a/app/assets/javascripts/contribution_events/components/contribution_events.vue +++ b/app/assets/javascripts/contribution_events/components/contribution_events.vue @@ -7,6 +7,7 @@ import { EVENT_TYPE_LEFT, EVENT_TYPE_PUSHED, EVENT_TYPE_PRIVATE, + EVENT_TYPE_MERGED, } from '../constants'; import ContributionEventApproved from './contribution_event/contribution_event_approved.vue'; import ContributionEventExpired from './contribution_event/contribution_event_expired.vue'; @@ -14,6 +15,7 @@ import ContributionEventJoined from './contribution_event/contribution_event_joi import ContributionEventLeft from './contribution_event/contribution_event_left.vue'; import ContributionEventPushed from './contribution_event/contribution_event_pushed.vue'; import ContributionEventPrivate from './contribution_event/contribution_event_private.vue'; +import ContributionEventMerged from './contribution_event/contribution_event_merged.vue'; export default { props: { @@ -126,6 +128,9 @@ export default { case EVENT_TYPE_PRIVATE: return ContributionEventPrivate; + case EVENT_TYPE_MERGED: + return ContributionEventMerged; + default: return EmptyComponent; } diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index b44688aa467..b2438efabfc 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -58,6 +58,7 @@ import HiddenFilesWarning from './hidden_files_warning.vue'; import NoChanges from './no_changes.vue'; import TreeList from './tree_list.vue'; import VirtualScrollerScrollSync from './virtual_scroller_scroll_sync'; +import PreRenderer from './pre_renderer.vue'; export default { name: 'DiffsApp', @@ -65,6 +66,7 @@ export default { FindingsDrawer, DynamicScroller, DynamicScrollerItem, + PreRenderer, VirtualScrollerScrollSync, CompareVersions, DiffFile, @@ -93,6 +95,11 @@ export default { required: false, default: '', }, + endpointSast: { + type: String, + required: false, + default: '', + }, endpointCodequality: { type: String, required: false, @@ -275,6 +282,10 @@ export default { this.setCodequalityEndpoint(this.endpointCodequality); } + if (this.endpointSast) { + this.setSastEndpoint(this.endpointSast); + } + if (this.shouldShow) { this.fetchData(); } @@ -356,11 +367,13 @@ export default { 'moveToNeighboringCommit', 'setBaseConfig', 'setCodequalityEndpoint', + 'setSastEndpoint', 'fetchDiffFilesMeta', 'fetchDiffFilesBatch', 'fetchFileByFile', 'fetchCoverageFiles', 'fetchCodequality', + 'fetchSast', 'rereadNoteHash', 'startRenderDiffsQueue', 'assignDiscussionsToDiff', @@ -458,6 +471,10 @@ export default { this.fetchCodequality(); } + if (this.endpointSast) { + this.fetchSast(); + } + if (!this.isNotesFetched) { notesEventHub.$emit('fetchNotesData'); } @@ -663,6 +680,22 @@ export default { diff --git a/app/assets/javascripts/diffs/components/diff_code_quality.vue b/app/assets/javascripts/diffs/components/diff_code_quality.vue index f3f05e3d9d9..68661f3f727 100644 --- a/app/assets/javascripts/diffs/components/diff_code_quality.vue +++ b/app/assets/javascripts/diffs/components/diff_code_quality.vue @@ -1,11 +1,12 @@ @@ -22,19 +27,35 @@ export default { data-testid="diff-codequality" class="gl-relative codequality-findings-list gl-border-top-1 gl-border-bottom-1 gl-bg-gray-10 gl-text-black-normal gl-pl-5 gl-pt-4 gl-pb-4" > -

- {{ $options.i18n.newFindings }} -

- + + + + import { GlLink, GlIcon } from '@gitlab/ui'; import { mapActions } from 'vuex'; -import { SEVERITY_CLASSES, SEVERITY_ICONS } from '~/ci/reports/codequality_report/constants'; +import { getSeverity } from '~/ci/reports/utils'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; export default { @@ -12,14 +12,21 @@ export default { type: Object, required: true, }, + link: { + type: Boolean, + required: false, + default: true, + }, + }, + computed: { + enhancedFinding() { + return getSeverity(this.finding); + }, + listText() { + return `${this.finding.severity} - ${this.finding.description}`; + }, }, methods: { - severityClass(severity) { - return SEVERITY_CLASSES[severity] || SEVERITY_CLASSES.unknown; - }, - severityIcon(severity) { - return SEVERITY_ICONS[severity] || SEVERITY_ICONS.unknown; - }, toggleDrawer() { this.setDrawer(this.finding); }, @@ -33,8 +40,8 @@ export default { @@ -43,12 +50,13 @@ export default { data-testid="description-button-section" class="gl-display-flex" > - - {{ finding.severity }} - {{ finding.description }} + {{ listText }} + {{ listText }} - {{ finding.severity }} - {{ finding.description }} + {{ listText }} diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index c176d6294b4..64cabaf4367 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -3,7 +3,7 @@ import { GlLoadingIcon, GlButton } from '@gitlab/ui'; import { mapActions, mapGetters, mapState } from 'vuex'; import { sprintf } from '~/locale'; import { createAlert } from '~/alert'; -import { mapParallel } from 'ee_else_ce/diffs/components/diff_row_utils'; +import { mapParallel, mapParallelNoSast } from 'ee_else_ce/diffs/components/diff_row_utils'; import DiffFileDrafts from '~/batch_comments/components/diff_file_drafts.vue'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; import { diffViewerModes } from '~/ide/constants'; @@ -14,6 +14,7 @@ import NotDiffableViewer from '~/vue_shared/components/diff_viewer/viewers/not_d import NoteForm from '~/notes/components/note_form.vue'; import eventHub from '~/notes/event_hub'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { IMAGE_DIFF_POSITION_TYPE } from '../constants'; import { SAVING_THE_COMMENT_FAILED, SOMETHING_WENT_WRONG } from '../i18n'; import { getDiffMode } from '../store/utils'; @@ -35,7 +36,7 @@ export default { UserAvatarLink, DiffFileDrafts, }, - mixins: [diffLineNoteFormMixin, draftCommentsMixin], + mixins: [diffLineNoteFormMixin, draftCommentsMixin, glFeatureFlagsMixin()], props: { diffFile: { type: Object, @@ -54,6 +55,7 @@ export default { 'getCommentFormForDiffFile', 'diffLines', 'fileLineCodequality', + 'fileLineSast', ]), ...mapGetters(['getNoteableData', 'noteableType', 'getUserData']), diffMode() { @@ -90,8 +92,11 @@ export default { return this.getUserData; }, mappedLines() { - // TODO: Do this data generation when we receive a response to save a computed property being created - return this.diffLines(this.diffFile).map(mapParallel(this)) || []; + if (this.glFeatures.sastReportsInInlineDiff) { + return this.diffLines(this.diffFile).map(mapParallel(this)) || []; + } + + return this.diffLines(this.diffFile).map(mapParallelNoSast(this)) || []; }, imageDiscussions() { return this.diffFile.discussions.filter( diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 4dfa9186c6d..f8643dfea5a 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -88,6 +88,11 @@ export default { required: false, default: true, }, + preRender: { + type: Boolean, + required: false, + default: false, + }, }, idState() { return { @@ -117,7 +122,7 @@ export default { return getShortShaFromFile(this.file); }, showLoadingIcon() { - return this.idState.isLoadingCollapsedDiff; + return this.idState.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); }, hasDiff() { return hasDiff(this.file); @@ -172,9 +177,6 @@ export default { showLocalFileReviews() { return Boolean(gon.current_user_id); }, - codequalityDiffForFile() { - return this.codequalityDiff?.files?.[this.file.file_path] || []; - }, isCollapsed() { if (collapsedType(this.file) !== DIFF_FILE_MANUAL_COLLAPSE) { return this.viewDiffsFileByFile ? false : this.file.viewer?.automaticallyCollapsed; @@ -200,6 +202,8 @@ export default { watch: { 'file.id': { handler: function fileIdHandler() { + if (this.preRender) return; + this.manageViewedEffects(); }, }, @@ -212,6 +216,7 @@ export default { newHash && oldHash && !this.hasDiff && + !this.preRender && !this.idState.hasLoadedCollapsedDiff ) { this.requestDiff(); @@ -220,10 +225,14 @@ export default { }, }, created() { + if (this.preRender) return; + notesEventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiff); eventHub.$on(EVT_EXPAND_ALL_FILES, this.expandAllListener); }, mounted() { + if (this.preRender) return; + if (this.hasDiff) { this.postRender(); } @@ -231,12 +240,15 @@ export default { this.manageViewedEffects(); }, beforeDestroy() { + if (this.preRender) return; + eventHub.$off(EVT_EXPAND_ALL_FILES, this.expandAllListener); }, methods: { ...mapActions('diffs', [ 'loadCollapsedDiff', 'assignDiscussionsToDiff', + 'setRenderIt', 'setFileCollapsedByUser', 'saveDiffDiscussion', 'toggleFileCommentForm', @@ -300,6 +312,10 @@ export default { .then(() => { idState.isLoadingCollapsedDiff = false; idState.hasLoadedCollapsedDiff = true; + + if (this.file.file_hash === file.file_hash) { + this.setRenderIt(this.file); + } }) .then(() => { if (this.file.file_hash !== file.file_hash) return; @@ -359,14 +375,15 @@ export default { + + diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index a0679bafddc..84ea74ca784 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -101,11 +101,6 @@ export default { required: false, default: false, }, - codequalityDiff: { - type: Array, - required: false, - default: () => [], - }, }, idState() { return { @@ -428,7 +423,6 @@ export default { toggle-class="btn-icon js-diff-more-actions" class="gl-pt-0!" data-qa-selector="dropdown_button" - lazy @show="setMoreActionsShown(true)" @hidden="setMoreActionsShown(false)" > diff --git a/app/assets/javascripts/diffs/components/diff_line.vue b/app/assets/javascripts/diffs/components/diff_line.vue index 448272549d3..40e53438bc8 100644 --- a/app/assets/javascripts/diffs/components/diff_line.vue +++ b/app/assets/javascripts/diffs/components/diff_line.vue @@ -15,13 +15,22 @@ export default { parsedCodeQuality() { return (this.line.left ?? this.line.right)?.codequality; }, + parsedSast() { + return (this.line.left ?? this.line.right)?.sast; + }, codeQualityLineNumber() { - return this.parsedCodeQuality[0].line; + return this.parsedCodeQuality[0]?.line; + }, + sastLineNumber() { + return this.parsedSast[0]?.line; }, }, methods: { hideCodeQualityFindings() { - this.$emit('hideCodeQualityFindings', this.codeQualityLineNumber); + this.$emit( + 'hideCodeQualityFindings', + this.codeQualityLineNumber ? this.codeQualityLineNumber : this.sastLineNumber, + ); }, }, }; @@ -30,6 +39,7 @@ export default { diff --git a/app/assets/javascripts/diffs/components/diff_row.vue b/app/assets/javascripts/diffs/components/diff_row.vue index 47bec4e42ed..3c9770864fa 100644 --- a/app/assets/javascripts/diffs/components/diff_row.vue +++ b/app/assets/javascripts/diffs/components/diff_row.vue @@ -141,6 +141,18 @@ export default { }, (props) => [props.inline, props.line.right?.codequality?.length].join(':'), ), + showSecurityLeft: memoize( + (props) => { + return props.inline && props.line.left?.sast?.length > 0; + }, + (props) => [props.inline, props.line.left?.sast?.length].join(':'), + ), + showSecurityRight: memoize( + (props) => { + return !props.inline && props.line.right?.sast?.length > 0; + }, + (props) => [props.inline, props.line.right?.sast?.length].join(':'), + ), classNameMapCellLeft: memoize( (props) => { return utils.classNameMapCell({ @@ -325,12 +337,17 @@ export default { > @@ -461,12 +478,17 @@ export default { > diff --git a/app/assets/javascripts/diffs/components/diff_row_utils.js b/app/assets/javascripts/diffs/components/diff_row_utils.js index a489c96b0c9..28834dab3b3 100644 --- a/app/assets/javascripts/diffs/components/diff_row_utils.js +++ b/app/assets/javascripts/diffs/components/diff_row_utils.js @@ -189,3 +189,7 @@ export const mapParallel = (content) => (line) => { commentRowClasses: hasDiscussions(left) || hasDiscussions(right) ? '' : 'js-temp-notes-holder', }; }; + +export const mapParallelNoSast = (content) => { + return mapParallel(content); +}; diff --git a/app/assets/javascripts/diffs/components/diff_view.vue b/app/assets/javascripts/diffs/components/diff_view.vue index 7c87ea1cbf2..6bacc6839d8 100644 --- a/app/assets/javascripts/diffs/components/diff_view.vue +++ b/app/assets/javascripts/diffs/components/diff_view.vue @@ -57,6 +57,7 @@ export default { ...mapGetters('diffs', ['commitId', 'fileLineCoverage']), ...mapState('diffs', [ 'codequalityDiff', + 'sastDiff', 'highlightedRow', 'coverageLoaded', 'selectedCommentPosition', @@ -75,7 +76,10 @@ export default { ); }, hasCodequalityChanges() { - return this.codequalityDiff?.files?.[this.diffFile.file_path]?.length > 0; + return ( + this.codequalityDiff?.files?.[this.diffFile.file_path]?.length > 0 || + this.sastDiff?.added?.length > 0 + ); }, }, created() { @@ -183,7 +187,10 @@ export default { ); }, getCodeQualityLine(line) { - return (line.left ?? line.right)?.codequality?.[0]?.line; + return ( + (line.left ?? line.right)?.codequality?.[0]?.line || + (line.left ?? line.right)?.sast?.[0]?.line + ); }, lineDrafts(line, side) { return (line[side]?.lineDrafts || []).filter((entry) => entry.isDraft); diff --git a/app/assets/javascripts/diffs/components/pre_renderer.vue b/app/assets/javascripts/diffs/components/pre_renderer.vue new file mode 100644 index 00000000000..e4320c40d2c --- /dev/null +++ b/app/assets/javascripts/diffs/components/pre_renderer.vue @@ -0,0 +1,83 @@ + + + + + diff --git a/app/assets/javascripts/diffs/components/virtual_scroller_scroll_sync.js b/app/assets/javascripts/diffs/components/virtual_scroller_scroll_sync.js index cd9faffd99d..d44dffecc38 100644 --- a/app/assets/javascripts/diffs/components/virtual_scroller_scroll_sync.js +++ b/app/assets/javascripts/diffs/components/virtual_scroller_scroll_sync.js @@ -18,18 +18,17 @@ export default { if (index < 0) return; - this.scrollToIndex(index); - - if (!this.vscrollParent.itemsWithSize[index].size) { + if (this.vscrollParent.itemsWithSize[index].size) { + this.scrollToIndex(index); + } else { this.$_itemsWithSizeWatcher = this.$watch('vscrollParent.itemsWithSize', async () => { await this.$nextTick(); if (this.vscrollParent.itemsWithSize[index].size) { this.$_itemsWithSizeWatcher(); + this.scrollToIndex(index); await this.$nextTick(); - - this.scrollToIndex(index); } }); } @@ -41,15 +40,12 @@ export default { if (this.$_itemsWithSizeWatcher) this.$_itemsWithSizeWatcher(); }, methods: { - async scrollToIndex(index) { + scrollToIndex(index) { this.vscrollParent.scrollToItem(index); this.$emit('update', -1); - await this.$nextTick(); - setTimeout(() => { handleLocationHash(); - window.scrollBy(0, 150); }); }, }, diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index 8d0e7d6d7e6..15e3893d22a 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -57,6 +57,7 @@ export const CONFLICT_TEXT = { export const HIDE_COMMENTS = __('Hide comments'); export const NEW_CODE_QUALITY_FINDINGS = __('New code quality findings'); +export const NEW_SAST_FINDINGS = __('New Security findings'); export const BUILDING_YOUR_MR = __( 'Building your merge request… This page will update when the build is complete.', diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 29cf90dcbe2..b9cf26827f2 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -32,6 +32,7 @@ export default function initDiffsApp(store = notesStore) { return { endpointCoverage: dataset.endpointCoverage || '', endpointCodequality: dataset.endpointCodequality || '', + endpointSast: dataset.endpointSast || '', helpPagePath: dataset.helpPagePath, currentUser: JSON.parse(dataset.currentUserData) || {}, changesEmptyStateIllustration: dataset.changesEmptyStateIllustration, @@ -79,6 +80,7 @@ export default function initDiffsApp(store = notesStore) { props: { endpointCoverage: this.endpointCoverage, endpointCodequality: this.endpointCodequality, + endpointSast: this.endpointSast, currentUser: this.currentUser, helpPagePath: this.helpPagePath, shouldShow: this.activeTab === 'diffs', diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 2a557017953..2f401e78f9c 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -6,6 +6,7 @@ import { scrollToElement, } from '~/lib/utils/common_utils'; import { createAlert, VARIANT_WARNING } from '~/alert'; +import { diffViewerModes } from '~/ide/constants'; import axios from '~/lib/utils/axios_utils'; import { HTTP_STATUS_NOT_FOUND, HTTP_STATUS_OK } from '~/lib/utils/http_status'; @@ -59,6 +60,7 @@ import { ERROR_DISMISSING_SUGESTION_POPOVER, } from '../i18n'; import eventHub from '../event_hub'; +import { isCollapsed } from '../utils/diff_file'; import { markFileReview, setReviewsForMergeRequest } from '../utils/file_reviews'; import { getDerivedMergeRequestInformation } from '../utils/merge_request'; import { queueRedisHllEvents } from '../utils/queue_events'; @@ -177,7 +179,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { }; const hash = window.location.hash.replace('#', '').split('diff-content-').pop(); let totalLoaded = 0; - let scrolledVirtualScroller = hash === ''; + let scrolledVirtualScroller = false; commit(types.SET_BATCH_LOADING_STATE, 'loading'); commit(types.SET_RETRIEVING_BATCHES, true); @@ -249,6 +251,8 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { return nextPage; }) .then((nextPage) => { + dispatch('startRenderDiffsQueue'); + if (nextPage) { return getBatch(nextPage); } @@ -381,6 +385,10 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi const file = state.diffFiles.find((f) => f.file_hash === discussion.diff_file.file_hash); if (file) { + if (!file.renderIt) { + commit(types.RENDER_FILE, file); + } + if (file.viewer.automaticallyCollapsed) { notesEventHub.$emit(`loadCollapsedDiff/${file.file_hash}`); scrollToElement(document.getElementById(file.file_hash)); @@ -398,6 +406,46 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi } }; +export const startRenderDiffsQueue = ({ state, commit }) => { + const diffFilesToRender = state.diffFiles.filter( + (file) => + !file.renderIt && + file.viewer && + (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text), + ); + let currentDiffFileIndex = 0; + + const checkItem = () => { + const nextFile = diffFilesToRender[currentDiffFileIndex]; + + if (nextFile) { + let retryCount = 0; + currentDiffFileIndex += 1; + commit(types.RENDER_FILE, nextFile); + + const requestIdle = () => + requestIdleCallback((idleDeadline) => { + // Wait for at least 5ms before trying to render + // or for 5 tries and then force render the file + if (idleDeadline.timeRemaining() >= 5 || retryCount > 4) { + checkItem(); + } else { + requestIdle(); + retryCount += 1; + } + }); + + requestIdle(); + } + }; + + if (diffFilesToRender.length) { + checkItem(); + } +}; + +export const setRenderIt = ({ commit }, file) => commit(types.RENDER_FILE, file); + export const setInlineDiffViewType = ({ commit }) => { commit(types.SET_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE); @@ -798,7 +846,7 @@ export const toggleFullDiff = ({ dispatch, commit, getters, state }, filePath) = } }; -export function switchToFullDiffFromRenamedFile({ commit }, { diffFile }) { +export function switchToFullDiffFromRenamedFile({ commit, dispatch }, { diffFile }) { return axios .get(diffFile.context_lines_path, { params: { @@ -825,6 +873,8 @@ export function switchToFullDiffFromRenamedFile({ commit }, { diffFile }) { }, }); commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: diffFile.file_path, lines }); + + dispatch('startRenderDiffsQueue'); }); } diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index a8a831fb269..d82959daa9d 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -153,6 +153,11 @@ export const fileLineCodequality = () => () => { return null; }; +// This function is overwritten for the inline SAST feature in EE +export const fileLineSast = () => () => { + return null; +}; + /** * Returns index of a currently selected diff in diffFiles * @returns {number} diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 4855ca87e91..1ea207d0043 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -23,6 +23,12 @@ function updateDiffFilesInState(state, files) { return Object.assign(state, { diffFiles: files }); } +function renderFile(file) { + Object.assign(file, { + renderIt: true, + }); +} + export default { [types.SET_BASE_CONFIG](state, options) { const { @@ -96,6 +102,10 @@ export default { Object.assign(state, { coverageFiles, coverageLoaded: true }); }, + [types.RENDER_FILE](state, file) { + renderFile(file); + }, + [types.SET_MERGE_REQUEST_DIFFS](state, mergeRequestDiffs) { Object.assign(state, { mergeRequestDiffs, @@ -345,6 +355,10 @@ export default { file.viewer.manuallyCollapsed = null; } } + + if (file && !collapsed) { + renderFile(file); + } }, [types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) { const file = state.diffFiles.find((f) => f.file_path === filePath); diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 97dfd351e67..68536d36ac0 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -307,6 +307,7 @@ function mergeTwoFiles(target, source) { ...target, [INLINE_DIFF_LINES_KEY]: missingInline ? source[INLINE_DIFF_LINES_KEY] : originalInline, parallel_diff_lines: null, + renderIt: source.renderIt, collapsed: source.collapsed, }; } @@ -387,6 +388,7 @@ function prepareDiffFileLines(file) { function finalizeDiffFile(file) { Object.assign(file, { + renderIt: true, isShowingFullFile: false, isLoadingFullFile: false, discussions: [], diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 1a57549a79f..911e484393b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -42,6 +42,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:core_security_mr_widget_counts, project) push_frontend_feature_flag(:issue_assignees_widget, @project) push_frontend_feature_flag(:moved_mr_sidebar, project) + push_frontend_feature_flag(:sast_reports_in_inline_diff, project) push_frontend_feature_flag(:mr_experience_survey, project) push_frontend_feature_flag(:saved_replies, current_user) push_frontend_feature_flag(:code_quality_inline_drawer, project) diff --git a/app/models/packages/npm/metadatum.rb b/app/models/packages/npm/metadatum.rb index ccbf056ec7b..2fc1c05cd48 100644 --- a/app/models/packages/npm/metadatum.rb +++ b/app/models/packages/npm/metadatum.rb @@ -26,6 +26,11 @@ class Packages::Npm::Metadatum < ApplicationRecord def ensure_package_json_size return if package_json.to_s.size < MAX_PACKAGE_JSON_SIZE - errors.add(:package_json, _('structure is too large')) + errors.add(:package_json, :too_large, + message: format( + _('structure is too large. Maximum size is %{max_size} characters'), + max_size: MAX_PACKAGE_JSON_SIZE + ) + ) end end diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index e15f157b36a..b618c7c20c4 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -83,7 +83,7 @@ class Packages::Package < ApplicationRecord validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan? validates :name, format: { with: Gitlab::Regex.generic_package_name_regex }, if: :generic? validates :name, format: { with: Gitlab::Regex.helm_package_regex }, if: :helm? - validates :name, format: { with: Gitlab::Regex.npm_package_name_regex }, if: :npm? + validates :name, format: { with: Gitlab::Regex.npm_package_name_regex, message: Gitlab::Regex.npm_package_name_regex_message }, if: :npm? validates :name, format: { with: Gitlab::Regex.nuget_package_name_regex }, if: :nuget? validates :name, format: { with: Gitlab::Regex.terraform_module_package_name_regex }, if: :terraform_module? validates :name, format: { with: Gitlab::Regex.debian_package_name_regex }, if: :debian_package? @@ -94,7 +94,8 @@ class Packages::Package < ApplicationRecord validates :version, format: { with: Gitlab::Regex.pypi_version_regex }, if: :pypi? validates :version, format: { with: Gitlab::Regex.prefixed_semver_regex }, if: :golang? validates :version, format: { with: Gitlab::Regex.helm_version_regex }, if: :helm? - validates :version, format: { with: Gitlab::Regex.semver_regex }, if: -> { composer_tag_version? || npm? || terraform_module? } + validates :version, format: { with: Gitlab::Regex.semver_regex, message: Gitlab::Regex.semver_regex_message }, + if: -> { composer_tag_version? || npm? || terraform_module? } validates :version, presence: true, diff --git a/app/services/packages/npm/create_package_service.rb b/app/services/packages/npm/create_package_service.rb index 2c578760cc5..f6f2dbb8415 100644 --- a/app/services/packages/npm/create_package_service.rb +++ b/app/services/packages/npm/create_package_service.rb @@ -18,7 +18,7 @@ module Packages ApplicationRecord.transaction { create_npm_package! } end - return error('Could not obtain package lease.', 400) unless package + return error('Could not obtain package lease. Please try again.', 400) unless package package end @@ -40,7 +40,7 @@ module Packages def create_npm_metadatum!(package) package.create_npm_metadatum!(package_json: package_json) rescue ActiveRecord::RecordInvalid => e - if package.npm_metadatum && package.npm_metadatum.errors.added?(:package_json, 'structure is too large') + if package.npm_metadatum && package.npm_metadatum.errors.where(:package_json, :too_large).any? # rubocop: disable CodeReuse/ActiveRecord Gitlab::ErrorTracking.track_exception(e, field_sizes: field_sizes_for_error_tracking) end diff --git a/doc/administration/get_started.md b/doc/administration/get_started.md index b4c7a7be18c..3fde3f476d1 100644 --- a/doc/administration/get_started.md +++ b/doc/administration/get_started.md @@ -76,7 +76,7 @@ While this isn't an exhaustive list, following these steps gives you a solid sta - Use a long root password, stored in a vault. - Install trusted SSL certificate and establish a process for renewal and revocation. - [Configure SSH key restrictions](../security/ssh_keys_restrictions.md#restrict-allowed-ssh-key-technologies-and-minimum-length) per your organization's guidelines. -- [Disable new sign-ups](../user/admin_area/settings/sign_up_restrictions.md#disable-new-sign-ups). +- [Disable new sign-ups](settings/sign_up_restrictions.md#disable-new-sign-ups). - Require email confirmation. - Set password length limit, configure SSO or SAML user management. - Limit email domains if allowing sign-up. diff --git a/doc/administration/reporting/ip_addr_restrictions.md b/doc/administration/reporting/ip_addr_restrictions.md new file mode 100644 index 00000000000..5b749c62c30 --- /dev/null +++ b/doc/administration/reporting/ip_addr_restrictions.md @@ -0,0 +1,33 @@ +--- +stage: Anti-Abuse +group: Anti-Abuse +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +--- + +# IP address restrictions **(FREE SELF)** + +IP address restrictions help prevent malicious users hiding their activities behind multiple IP addresses. + +GitLab maintains a list of the unique IP addresses used by a user to make requests over a specified period. When the +specified limit is reached, any requests made by the user from a new IP address are rejected with a `403 Forbidden` error. + +IP addresses are cleared from the list when no further requests have been made by the user from the IP address in the specified time period. + +NOTE: +When a runner runs a CI/CD job as a particular user, the runner IP address is also stored against the user's list of +unique IP addresses. Therefore, the IP addresses per user limit should take into account the number of configured active runners. + +## Configure IP address restrictions + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > Reporting**. +1. Expand **Spam and Anti-bot Protection**. +1. Update the IP address restrictions settings: + 1. Select the **Limit sign in from multiple IP addresses** checkbox to enable IP address restrictions. + 1. Enter a number in the **IP addresses per user** field, greater than or equal to `1`. This number specifies the + maximum number of unique IP addresses a user can access GitLab from in the specified time period before requests + from a new IP address are rejected. + 1. Enter a number in the **IP address expiration time** field, greater than or equal to `0`. This number specifies the + time in seconds an IP address counts towards the limit for a user, taken from the time the last request was made. +1. Select **Save changes**. diff --git a/doc/administration/reporting/spamcheck.md b/doc/administration/reporting/spamcheck.md new file mode 100644 index 00000000000..8e478729299 --- /dev/null +++ b/doc/administration/reporting/spamcheck.md @@ -0,0 +1,69 @@ +--- +stage: Systems +group: Distribution +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +--- + +# Spamcheck anti-spam service **(FREE SELF)** + +> [Introduced](https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6259) in GitLab 14.8. + +WARNING: +Spamcheck is available to all tiers, but only on instances using GitLab Enterprise Edition (EE). For [licensing reasons](https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6259#note_726605397), it is not included in the GitLab Community Edition (CE) package. You can [migrate from CE to EE](../../update/package/convert_to_ee.md). + +[Spamcheck](https://gitlab.com/gitlab-org/spamcheck) is an anti-spam engine +developed by GitLab originally to combat rising amount of spam in GitLab.com, +and later made public to be used in self-managed GitLab instances. + +## Enable Spamcheck + +Spamcheck is only available for package-based installations: + +1. Edit `/etc/gitlab/gitlab.rb` and enable Spamcheck: + + ```ruby + spamcheck['enable'] = true + ``` + +1. Reconfigure GitLab: + + ```shell + sudo gitlab-ctl reconfigure + ``` + +1. Verify that the new services `spamcheck` and `spam-classifier` are + up and running: + + ```shell + sudo gitlab-ctl status + ``` + +## Configure GitLab to use Spamcheck + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > Reporting**. +1. Expand **Spam and Anti-bot Protection**. +1. Update the Spam Check settings: + 1. Check the "Enable Spam Check via external API endpoint" checkbox. + 1. For **URL of the external Spam Check endpoint** use `grpc://localhost:8001`. + 1. Leave **Spam Check API key** blank. +1. Select **Save changes**. + +NOTE: +In single-node instances, Spamcheck runs over `localhost`, and hence is running +in an unauthenticated mode. If on multi-node instances where GitLab runs on one +server and Spamcheck runs on another server listening over a public endpoint, it +is recommended to enforce some sort of authentication using a reverse proxy in +front of the Spamcheck service that can be used along with an API key. One +example would be to use `JWT` authentication for this and specifying a bearer +token as the API key. +[Native authentication for Spamcheck is in the works](https://gitlab.com/gitlab-com/gl-security/engineering-and-research/automation-team/spam/spamcheck/-/issues/171). + +## Running Spamcheck over TLS + +Spamcheck service on its own cannot communicate directly over TLS with GitLab. +However, Spamcheck can be deployed behind a reverse proxy which performs TLS +termination. In such a scenario, GitLab can be made to communicate with +Spamcheck over TLS by specifying `tls://` scheme for the external Spamcheck URL +instead of `grpc://` in the Admin Area settings. diff --git a/doc/administration/settings/sign_up_restrictions.md b/doc/administration/settings/sign_up_restrictions.md new file mode 100644 index 00000000000..c9daeb109da --- /dev/null +++ b/doc/administration/settings/sign_up_restrictions.md @@ -0,0 +1,206 @@ +--- +stage: none +group: unassigned +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +type: reference +--- + +# Sign-up restrictions **(FREE SELF)** + +You can enforce the following restrictions on sign ups: + +- Disable new sign ups. +- Require administrator approval for new sign ups. +- Require user email confirmation. +- Allow or deny sign ups using specific email domains. + +## Disable new sign ups + +By default, any user visiting your GitLab domain can sign up for an account. For customers running +public-facing GitLab instances, we **highly** recommend that you consider disabling new sign ups if +you do not expect public users to sign up for an account. + +To disable sign ups: + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand **Sign-up restrictions**. +1. Clear the **Sign-up enabled** checkbox, then select **Save changes**. + +## Require administrator approval for new sign ups + +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4491) in GitLab 13.5. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/267568) in GitLab 13.6. + +When this setting is enabled, any user visiting your GitLab domain and signing up for a new account using the registration form +must be explicitly [approved](../../user/admin_area/moderate_users.md#approve-or-reject-a-user-sign-up) by an +administrator before they can start using their account. In GitLab 13.6 and later, this setting is +enabled by default for new GitLab instances. It is only applicable if sign ups are enabled. + +To require administrator approval for new sign ups: + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand **Sign-up restrictions**. +1. Select the **Require admin approval for new sign-ups** checkbox, then select **Save changes**. + +In [GitLab 13.7 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/273258), if an administrator disables this setting, the users in pending approval state are +automatically approved in a background job. + +NOTE: +This setting doesn't apply to LDAP or OmniAuth users. To enforce approvals for new users +signing up using OmniAuth or LDAP, set `block_auto_created_users` to `true` in the +[OmniAuth configuration](../../integration/omniauth.md#configure-common-settings) or +[LDAP configuration](../auth/ldap/index.md#basic-configuration-settings). +A [user cap](#user-cap) can also be used to enforce approvals for new users. + +## Confirm user email + +> - Soft email confirmation [introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/47003) in GitLab 12.2 [with a flag](../../operations/feature_flags.md) named `soft_email_confirmation`. +> - Soft email confirmation [changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107302/diffs) from a feature flag to an application setting in GitLab 15.9. + +You can send confirmation emails during sign up and require that users confirm +their email address before they are allowed to sign in. + +For example, to enforce confirmation of the email address used for new sign ups: + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand **Sign-up restrictions**. +1. Under **Email confirmation settings**, select **Hard**. + +The following settings are available: + +- **Hard** - Send a confirmation email during sign up. New users must confirm their email address before they can log in. +- **Soft** - Send a confirmation email during sign up. New users can log in immediately, but must confirm their email in three days. Unconfirmed accounts are deleted. +- **Off** - New users can sign up without confirming their email address. + +## User cap + +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4315) in GitLab 13.7. +> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/292600) in GitLab 13.9. + +When the number of billable users reaches the user cap, any user who is added or requests access must be +[approved](../../user/admin_area/moderate_users.md#approve-or-reject-a-user-sign-up) by an administrator before they can start using +their account. + +If an administrator [increases](#set-the-user-cap-number) or [removes](#remove-the-user-cap) the +user cap, the users in pending approval state are automatically approved in a background job. + +NOTE: +The amount of billable users [is updated once a day](../../subscriptions/self_managed/index.md#billable-users). +This means the user cap might apply only retrospectively after the cap has already been exceeded. +To ensure the cap is enabled immediately, set it to a low value below the current number of +billable users, for example: `1`. + +On instances that use LDAP or OmniAuth, enabling and disabling +[administrator approval for new sign ups](#require-administrator-approval-for-new-sign-ups) +involves changing the Rails configuration, and may require downtime. +User cap can be used instead. As noted above, set the cap to value that ensures it is enforced immediately. + +### Set the user cap number + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand **Sign-up restrictions**. +1. Enter a number in **User cap**. +1. Select **Save changes**. + +New user sign ups are subject to the user cap restriction. + +## Remove the user cap + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand **Sign-up restrictions**. +1. Remove the number from **User cap**. +1. Select **Save changes**. + +New users sign ups are not subject to the user cap restriction. Users in pending approval state are +automatically approved in a background job. + +## Minimum password length limit + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20661) in GitLab 12.6 + +You can [change](../../security/password_length_limits.md#modify-minimum-password-length) +the minimum number of characters a user must have in their password using the GitLab UI. + +### Password complexity requirements **(PREMIUM SELF)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/354965) in GitLab 15.2. + +By default, the only requirement for user passwords is [minimum password length](#minimum-password-length-limit). +You can add additional complexity requirements. Changes to password complexity requirements apply to new passwords: + +- For new users that sign up. +- For existing users that reset their password. + +Existing passwords are unaffected. To change password complexity requirements: + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand **Sign-up restrictions**. +1. Under **Minimum password length (number of characters)**, select additional password complexity requirements. You can require numbers, uppercase letters, lowercase letters, + and symbols. +1. Select **Save changes**. + +## Allow or deny sign ups using specific email domains + +You can specify an inclusive or exclusive list of email domains which can be used for user sign up. + +These restrictions are only applied during sign up from an external user. An administrator can add a +user through the administrator panel with a disallowed domain. The users can also change their +email addresses to disallowed domains after sign up. + +### Allowlist email domains + +You can restrict users only to sign up using email addresses matching the given +domains list. + +### Denylist email domains + +You can block users from signing up when using an email addresses of specific domains. This can +reduce the risk of malicious users creating spam accounts with disposable email addresses. + +### Create email domain allowlist or denylist + +To create an email domain allowlist or denylist: + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand **Sign-up restrictions**. +1. For the allowlist, you must enter the list manually. For the denylist, you can enter the list + manually or upload a `.txt` file that contains list entries. + + Both the allowlist and denylist accept wildcards. For example, you can use +`*.company.com` to accept every `company.com` subdomain, or `*.io` to block all +domains ending in `.io`. Domains must be separated by a whitespace, +semicolon, comma, or a new line. + + ![Domain Denylist](../../user/admin_area/settings/img/domain_denylist_v14_1.png) + +## Set up LDAP user filter + +You can limit GitLab access to a subset of the LDAP users on your LDAP server. + +See the [documentation on setting up an LDAP user filter](../auth/ldap/index.md#set-up-ldap-user-filter) for more information. + + diff --git a/doc/administration/settings/terms.md b/doc/administration/settings/terms.md new file mode 100644 index 00000000000..c9b7a40a29c --- /dev/null +++ b/doc/administration/settings/terms.md @@ -0,0 +1,49 @@ +--- +stage: none +group: unassigned +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +type: reference +--- + +# Terms of Service and Privacy Policy **(FREE SELF)** + +An administrator can enforce acceptance of a terms of service and privacy policy. +When this option is enabled, new and existing users must accept the terms. + +When enabled, you can view the Terms of Service at the `-/users/terms` page on the instance, +for example `https://gitlab.example.com/-/users/terms`. + +## Enforce a Terms of Service and Privacy Policy + +To enforce acceptance of a Terms of Service and Privacy Policy: + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand the **Terms of Service and Privacy Policy** section. +1. Check the **All users must accept the Terms of Service and Privacy Policy to access GitLab** checkbox. +1. Input the text of the **Terms of Service and Privacy Policy**. You can use [Markdown](../../user/markdown.md) + in this text box. +1. Select **Save changes**. + +For each update to the terms, a new version is stored. When a user accepts or declines the terms, +GitLab records which version they accepted or declined. + +Existing users must accept the terms on their next GitLab interaction. +If an authenticated user declines the terms, they are signed out. + +When enabled, it adds a mandatory checkbox to the sign up page for new users: + +![Sign up form](../../user/admin_area/settings/img/sign_up_terms.png) + + diff --git a/doc/administration/settings/terraform_limits.md b/doc/administration/settings/terraform_limits.md new file mode 100644 index 00000000000..90ab1c25522 --- /dev/null +++ b/doc/administration/settings/terraform_limits.md @@ -0,0 +1,28 @@ +--- +stage: Deploy +group: Environments +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +type: reference +--- + +# Terraform limits **(FREE SELF)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/352951) in GitLab 15.7. + +You can limit the total storage of [Terraform state files](../terraform_state.md). +The limit applies to each individual +state file version, and is checked whenever a new version is created. + +To add a storage limit: + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > Preferences**. +1. Expand **Terraform limits**. +1. Adjust the size limit. + +## Available settings + +| Setting | Default | Description | +|------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------------------------------| +| Terraform state size limit (bytes) | 0 | Terraform state files that exceed this size are not saved, and associated Terraform operations are rejected. Set to 0 to allow files of unlimited size. | diff --git a/doc/administration/settings/third_party_offers.md b/doc/administration/settings/third_party_offers.md new file mode 100644 index 00000000000..39e2275f411 --- /dev/null +++ b/doc/administration/settings/third_party_offers.md @@ -0,0 +1,38 @@ +--- +stage: Data Stores +group: Tenant Scale +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +type: reference +--- + +# Customer experience improvement and third-party offers **(FREE SELF)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/20379) in GitLab 11.1. + +Within GitLab, we inform users of available third-party offers they might find valuable in order +to enhance the development of their projects. An example is the Google Cloud Platform free credit +for using [Google Kubernetes Engine](https://cloud.google.com/kubernetes-engine/). + +Furthermore, we use content to improve customer experience. An example are the personalization +questions when creating a group. + +To toggle the display of customer experience improvement content and third-party offers: + +1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). +1. Select **Admin Area**. +1. Select **Settings > General**. +1. Expand **Customer experience improvement and third-party offers**. +1. Select **Do not display content for customer experience improvement and offers from third parties**. +1. Select **Save changes**. + + diff --git a/doc/api/lint.md b/doc/api/lint.md index 21db4810b6d..af7c2f68a14 100644 --- a/doc/api/lint.md +++ b/doc/api/lint.md @@ -114,11 +114,11 @@ Checks if CI/CD YAML configuration is valid. This endpoint validates basic CI/CD configuration syntax. It doesn't have any namespace-specific context. Access to this endpoint does not require authentication when the instance -[allows new sign ups](../user/admin_area/settings/sign_up_restrictions.md#disable-new-sign-ups) +[allows new sign ups](../administration/settings/sign_up_restrictions.md#disable-new-sign-ups) and: -- Does not have an [allowlist or denylist](../user/admin_area/settings/sign_up_restrictions.md#allow-or-deny-sign-ups-using-specific-email-domains). -- Does not [require administrator approval for new sign ups](../user/admin_area/settings/sign_up_restrictions.md#require-administrator-approval-for-new-sign-ups). +- Does not have an [allowlist or denylist](../administration/settings/sign_up_restrictions.md#allow-or-deny-sign-ups-using-specific-email-domains). +- Does not [require administrator approval for new sign ups](../administration/settings/sign_up_restrictions.md#require-administrator-approval-for-new-sign-ups). - Does not have additional [sign up restrictions](../user/admin_area/settings/sign_up_restrictions.md). Otherwise, authentication is required. diff --git a/doc/subscriptions/gitlab_dedicated/index.md b/doc/subscriptions/gitlab_dedicated/index.md index 9bdf089e4ee..75fcdb70161 100644 --- a/doc/subscriptions/gitlab_dedicated/index.md +++ b/doc/subscriptions/gitlab_dedicated/index.md @@ -92,7 +92,7 @@ GitLab may conduct unscheduled maintenance to address high-severity issues affec ### Application -GitLab Dedicated comes with the self-managed [Ultimate feature set](https://about.gitlab.com/pricing/feature-comparison/) with the exception of the unsupported features [listed below](#features-that-are-not-available). +GitLab Dedicated comes with the self-managed [Ultimate feature set](https://about.gitlab.com/pricing/feature-comparison/) with the exception of the [unsupported features](#features-that-are-not-available) listed below. #### GitLab Runners diff --git a/doc/subscriptions/self_managed/index.md b/doc/subscriptions/self_managed/index.md index 86230646c50..05194db8781 100644 --- a/doc/subscriptions/self_managed/index.md +++ b/doc/subscriptions/self_managed/index.md @@ -114,10 +114,10 @@ Managing the number of users against the number of subscription seats can be a c GitLab has several features which can help you manage the number of users: -- Enable the [**Require administrator approval for new sign ups**](../../user/admin_area/settings/sign_up_restrictions.md#require-administrator-approval-for-new-sign-ups) +- Enable the [**Require administrator approval for new sign ups**](../../administration/settings/sign_up_restrictions.md#require-administrator-approval-for-new-sign-ups) option. - Enable `block_auto_created_users` for new sign-ups via [LDAP](../../administration/auth/ldap/index.md#basic-configuration-settings) or [OmniAuth](../../integration/omniauth.md#configure-common-settings). -- Enable the [User cap](../../user/admin_area/settings/sign_up_restrictions.md#user-cap) +- Enable the [User cap](../../administration/settings/sign_up_restrictions.md#user-cap) option. **Available in GitLab 13.7 and later**. - [Disable new sign-ups](../../user/admin_area/settings/sign_up_restrictions.md), and instead manage new users manually. diff --git a/doc/user/admin_area/moderate_users.md b/doc/user/admin_area/moderate_users.md index 9869fdfc5e6..06915622f32 100644 --- a/doc/user/admin_area/moderate_users.md +++ b/doc/user/admin_area/moderate_users.md @@ -17,8 +17,8 @@ users. A user in _pending approval_ state requires action by an administrator. A user sign up can be in a pending approval state because an administrator has enabled any of the following options: -- [Require administrator approval for new sign-ups](settings/sign_up_restrictions.md#require-administrator-approval-for-new-sign-ups) setting. -- [User cap](settings/sign_up_restrictions.md#user-cap). +- [Require administrator approval for new sign-ups](../../administration/settings/sign_up_restrictions.md#require-administrator-approval-for-new-sign-ups) setting. +- [User cap](../../administration/settings/sign_up_restrictions.md#user-cap). - [Block auto-created users (OmniAuth)](../../integration/omniauth.md#configure-common-settings) - [Block auto-created users (LDAP)](../../administration/auth/ldap/index.md#basic-configuration-settings) diff --git a/doc/user/admin_area/reporting/ip_addr_restrictions.md b/doc/user/admin_area/reporting/ip_addr_restrictions.md index 5b749c62c30..85783640b28 100644 --- a/doc/user/admin_area/reporting/ip_addr_restrictions.md +++ b/doc/user/admin_area/reporting/ip_addr_restrictions.md @@ -1,33 +1,11 @@ --- -stage: Anti-Abuse -group: Anti-Abuse -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +redirect_to: '../../../administration/reporting/ip_addr_restrictions.md' +remove_date: '2023-10-13' --- -# IP address restrictions **(FREE SELF)** +This document was moved to [another location](../../../administration/reporting/ip_addr_restrictions.md). -IP address restrictions help prevent malicious users hiding their activities behind multiple IP addresses. - -GitLab maintains a list of the unique IP addresses used by a user to make requests over a specified period. When the -specified limit is reached, any requests made by the user from a new IP address are rejected with a `403 Forbidden` error. - -IP addresses are cleared from the list when no further requests have been made by the user from the IP address in the specified time period. - -NOTE: -When a runner runs a CI/CD job as a particular user, the runner IP address is also stored against the user's list of -unique IP addresses. Therefore, the IP addresses per user limit should take into account the number of configured active runners. - -## Configure IP address restrictions - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > Reporting**. -1. Expand **Spam and Anti-bot Protection**. -1. Update the IP address restrictions settings: - 1. Select the **Limit sign in from multiple IP addresses** checkbox to enable IP address restrictions. - 1. Enter a number in the **IP addresses per user** field, greater than or equal to `1`. This number specifies the - maximum number of unique IP addresses a user can access GitLab from in the specified time period before requests - from a new IP address are rejected. - 1. Enter a number in the **IP address expiration time** field, greater than or equal to `0`. This number specifies the - time in seconds an IP address counts towards the limit for a user, taken from the time the last request was made. -1. Select **Save changes**. + + + + diff --git a/doc/user/admin_area/reporting/spamcheck.md b/doc/user/admin_area/reporting/spamcheck.md index e2508476c80..043481b6255 100644 --- a/doc/user/admin_area/reporting/spamcheck.md +++ b/doc/user/admin_area/reporting/spamcheck.md @@ -1,69 +1,11 @@ --- -stage: Systems -group: Distribution -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +redirect_to: '../../../administration/reporting/spamcheck.md' +remove_date: '2023-10-13' --- -# Spamcheck anti-spam service **(FREE SELF)** +This document was moved to [another location](../../../administration/reporting/spamcheck.md). -> [Introduced](https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6259) in GitLab 14.8. - -WARNING: -Spamcheck is available to all tiers, but only on instances using GitLab Enterprise Edition (EE). For [licensing reasons](https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6259#note_726605397), it is not included in the GitLab Community Edition (CE) package. You can [migrate from CE to EE](../../../update/package/convert_to_ee.md). - -[Spamcheck](https://gitlab.com/gitlab-org/spamcheck) is an anti-spam engine -developed by GitLab originally to combat rising amount of spam in GitLab.com, -and later made public to be used in self-managed GitLab instances. - -## Enable Spamcheck - -Spamcheck is only available for package-based installations: - -1. Edit `/etc/gitlab/gitlab.rb` and enable Spamcheck: - - ```ruby - spamcheck['enable'] = true - ``` - -1. Reconfigure GitLab: - - ```shell - sudo gitlab-ctl reconfigure - ``` - -1. Verify that the new services `spamcheck` and `spam-classifier` are - up and running: - - ```shell - sudo gitlab-ctl status - ``` - -## Configure GitLab to use Spamcheck - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > Reporting**. -1. Expand **Spam and Anti-bot Protection**. -1. Update the Spam Check settings: - 1. Check the "Enable Spam Check via external API endpoint" checkbox. - 1. For **URL of the external Spam Check endpoint** use `grpc://localhost:8001`. - 1. Leave **Spam Check API key** blank. -1. Select **Save changes**. - -NOTE: -In single-node instances, Spamcheck runs over `localhost`, and hence is running -in an unauthenticated mode. If on multi-node instances where GitLab runs on one -server and Spamcheck runs on another server listening over a public endpoint, it -is recommended to enforce some sort of authentication using a reverse proxy in -front of the Spamcheck service that can be used along with an API key. One -example would be to use `JWT` authentication for this and specifying a bearer -token as the API key. -[Native authentication for Spamcheck is in the works](https://gitlab.com/gitlab-com/gl-security/engineering-and-research/automation-team/spam/spamcheck/-/issues/171). - -## Running Spamcheck over TLS - -Spamcheck service on its own cannot communicate directly over TLS with GitLab. -However, Spamcheck can be deployed behind a reverse proxy which performs TLS -termination. In such a scenario, GitLab can be made to communicate with -Spamcheck over TLS by specifying `tls://` scheme for the external Spamcheck URL -instead of `grpc://` in the Admin Area settings. + + + + diff --git a/doc/user/admin_area/settings/sign_up_restrictions.md b/doc/user/admin_area/settings/sign_up_restrictions.md index 0dcca5e7518..553caa9ff0d 100644 --- a/doc/user/admin_area/settings/sign_up_restrictions.md +++ b/doc/user/admin_area/settings/sign_up_restrictions.md @@ -1,206 +1,11 @@ --- -stage: none -group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments -type: reference +redirect_to: '../../../administration/settings/sign_up_restrictions.md' +remove_date: '2023-10-13' --- -# Sign-up restrictions **(FREE SELF)** +This document was moved to [another location](../../../administration/settings/sign_up_restrictions.md). -You can enforce the following restrictions on sign ups: - -- Disable new sign ups. -- Require administrator approval for new sign ups. -- Require user email confirmation. -- Allow or deny sign ups using specific email domains. - -## Disable new sign ups - -By default, any user visiting your GitLab domain can sign up for an account. For customers running -public-facing GitLab instances, we **highly** recommend that you consider disabling new sign ups if -you do not expect public users to sign up for an account. - -To disable sign ups: - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand **Sign-up restrictions**. -1. Clear the **Sign-up enabled** checkbox, then select **Save changes**. - -## Require administrator approval for new sign ups - -> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4491) in GitLab 13.5. -> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/267568) in GitLab 13.6. - -When this setting is enabled, any user visiting your GitLab domain and signing up for a new account using the registration form -must be explicitly [approved](../moderate_users.md#approve-or-reject-a-user-sign-up) by an -administrator before they can start using their account. In GitLab 13.6 and later, this setting is -enabled by default for new GitLab instances. It is only applicable if sign ups are enabled. - -To require administrator approval for new sign ups: - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand **Sign-up restrictions**. -1. Select the **Require admin approval for new sign-ups** checkbox, then select **Save changes**. - -In [GitLab 13.7 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/273258), if an administrator disables this setting, the users in pending approval state are -automatically approved in a background job. - -NOTE: -This setting doesn't apply to LDAP or OmniAuth users. To enforce approvals for new users -signing up using OmniAuth or LDAP, set `block_auto_created_users` to `true` in the -[OmniAuth configuration](../../../integration/omniauth.md#configure-common-settings) or -[LDAP configuration](../../../administration/auth/ldap/index.md#basic-configuration-settings). -A [user cap](#user-cap) can also be used to enforce approvals for new users. - -## Confirm user email - -> - Soft email confirmation [introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/47003) in GitLab 12.2 [with a flag](../../../operations/feature_flags.md) named `soft_email_confirmation`. -> - Soft email confirmation [changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107302/diffs) from a feature flag to an application setting in GitLab 15.9. - -You can send confirmation emails during sign up and require that users confirm -their email address before they are allowed to sign in. - -For example, to enforce confirmation of the email address used for new sign ups: - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand **Sign-up restrictions**. -1. Under **Email confirmation settings**, select **Hard**. - -The following settings are available: - -- **Hard** - Send a confirmation email during sign up. New users must confirm their email address before they can log in. -- **Soft** - Send a confirmation email during sign up. New users can log in immediately, but must confirm their email in three days. Unconfirmed accounts are deleted. -- **Off** - New users can sign up without confirming their email address. - -## User cap - -> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4315) in GitLab 13.7. -> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/292600) in GitLab 13.9. - -When the number of billable users reaches the user cap, any user who is added or requests access must be -[approved](../moderate_users.md#approve-or-reject-a-user-sign-up) by an administrator before they can start using -their account. - -If an administrator [increases](#set-the-user-cap-number) or [removes](#remove-the-user-cap) the -user cap, the users in pending approval state are automatically approved in a background job. - -NOTE: -The amount of billable users [is updated once a day](../../../subscriptions/self_managed/index.md#billable-users). -This means the user cap might apply only retrospectively after the cap has already been exceeded. -To ensure the cap is enabled immediately, set it to a low value below the current number of -billable users, for example: `1`. - -On instances that use LDAP or OmniAuth, enabling and disabling -[administrator approval for new sign ups](#require-administrator-approval-for-new-sign-ups) -involves changing the Rails configuration, and may require downtime. -User cap can be used instead. As noted above, set the cap to value that ensures it is enforced immediately. - -### Set the user cap number - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand **Sign-up restrictions**. -1. Enter a number in **User cap**. -1. Select **Save changes**. - -New user sign ups are subject to the user cap restriction. - -## Remove the user cap - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand **Sign-up restrictions**. -1. Remove the number from **User cap**. -1. Select **Save changes**. - -New users sign ups are not subject to the user cap restriction. Users in pending approval state are -automatically approved in a background job. - -## Minimum password length limit - -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20661) in GitLab 12.6 - -You can [change](../../../security/password_length_limits.md#modify-minimum-password-length) -the minimum number of characters a user must have in their password using the GitLab UI. - -### Password complexity requirements **(PREMIUM SELF)** - -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/354965) in GitLab 15.2. - -By default, the only requirement for user passwords is [minimum password length](#minimum-password-length-limit). -You can add additional complexity requirements. Changes to password complexity requirements apply to new passwords: - -- For new users that sign up. -- For existing users that reset their password. - -Existing passwords are unaffected. To change password complexity requirements: - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand **Sign-up restrictions**. -1. Under **Minimum password length (number of characters)**, select additional password complexity requirements. You can require numbers, uppercase letters, lowercase letters, - and symbols. -1. Select **Save changes**. - -## Allow or deny sign ups using specific email domains - -You can specify an inclusive or exclusive list of email domains which can be used for user sign up. - -These restrictions are only applied during sign up from an external user. An administrator can add a -user through the administrator panel with a disallowed domain. The users can also change their -email addresses to disallowed domains after sign up. - -### Allowlist email domains - -You can restrict users only to sign up using email addresses matching the given -domains list. - -### Denylist email domains - -You can block users from signing up when using an email addresses of specific domains. This can -reduce the risk of malicious users creating spam accounts with disposable email addresses. - -### Create email domain allowlist or denylist - -To create an email domain allowlist or denylist: - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand **Sign-up restrictions**. -1. For the allowlist, you must enter the list manually. For the denylist, you can enter the list - manually or upload a `.txt` file that contains list entries. - - Both the allowlist and denylist accept wildcards. For example, you can use -`*.company.com` to accept every `company.com` subdomain, or `*.io` to block all -domains ending in `.io`. Domains must be separated by a whitespace, -semicolon, comma, or a new line. - - ![Domain Denylist](img/domain_denylist_v14_1.png) - -## Set up LDAP user filter - -You can limit GitLab access to a subset of the LDAP users on your LDAP server. - -See the [documentation on setting up an LDAP user filter](../../../administration/auth/ldap/index.md#set-up-ldap-user-filter) for more information. - - + + + + diff --git a/doc/user/admin_area/settings/terms.md b/doc/user/admin_area/settings/terms.md index 8563f778292..444eeeb15ea 100644 --- a/doc/user/admin_area/settings/terms.md +++ b/doc/user/admin_area/settings/terms.md @@ -1,49 +1,11 @@ --- -stage: none -group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments -type: reference +redirect_to: '../../../administration/settings/terms.md' +remove_date: '2023-10-13' --- -# Terms of Service and Privacy Policy **(FREE SELF)** +This document was moved to [another location](../../../administration/settings/terms.md). -An administrator can enforce acceptance of a terms of service and privacy policy. -When this option is enabled, new and existing users must accept the terms. - -When enabled, you can view the Terms of Service at the `-/users/terms` page on the instance, -for example `https://gitlab.example.com/-/users/terms`. - -## Enforce a Terms of Service and Privacy Policy - -To enforce acceptance of a Terms of Service and Privacy Policy: - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand the **Terms of Service and Privacy Policy** section. -1. Check the **All users must accept the Terms of Service and Privacy Policy to access GitLab** checkbox. -1. Input the text of the **Terms of Service and Privacy Policy**. You can use [Markdown](../../markdown.md) - in this text box. -1. Select **Save changes**. - -For each update to the terms, a new version is stored. When a user accepts or declines the terms, -GitLab records which version they accepted or declined. - -Existing users must accept the terms on their next GitLab interaction. -If an authenticated user declines the terms, they are signed out. - -When enabled, it adds a mandatory checkbox to the sign up page for new users: - -![Sign up form](img/sign_up_terms.png) - - + + + + diff --git a/doc/user/admin_area/settings/terraform_limits.md b/doc/user/admin_area/settings/terraform_limits.md index 0e620bb84ce..8fed7589bb7 100644 --- a/doc/user/admin_area/settings/terraform_limits.md +++ b/doc/user/admin_area/settings/terraform_limits.md @@ -1,28 +1,11 @@ --- -stage: Deploy -group: Environments -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments -type: reference +redirect_to: '../../../administration/settings/terraform_limits.md' +remove_date: '2023-10-13' --- -# Terraform limits **(FREE SELF)** +This document was moved to [another location](../../../administration/settings/terraform_limits.md). -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/352951) in GitLab 15.7. - -You can limit the total storage of [Terraform state files](../../../administration/terraform_state.md). -The limit applies to each individual -state file version, and is checked whenever a new version is created. - -To add a storage limit: - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > Preferences**. -1. Expand **Terraform limits**. -1. Adjust the size limit. - -## Available settings - -| Setting | Default | Description | -|------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------------------------------| -| Terraform state size limit (bytes) | 0 | Terraform state files that exceed this size are not saved, and associated Terraform operations are rejected. Set to 0 to allow files of unlimited size. | + + + + diff --git a/doc/user/admin_area/settings/third_party_offers.md b/doc/user/admin_area/settings/third_party_offers.md index 39e2275f411..54c5b36bbc0 100644 --- a/doc/user/admin_area/settings/third_party_offers.md +++ b/doc/user/admin_area/settings/third_party_offers.md @@ -1,38 +1,11 @@ --- -stage: Data Stores -group: Tenant Scale -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments -type: reference +redirect_to: '../../../administration/settings/third_party_offers.md' +remove_date: '2023-10-13' --- -# Customer experience improvement and third-party offers **(FREE SELF)** +This document was moved to [another location](../../../administration/settings/third_party_offers.md). -> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/20379) in GitLab 11.1. - -Within GitLab, we inform users of available third-party offers they might find valuable in order -to enhance the development of their projects. An example is the Google Cloud Platform free credit -for using [Google Kubernetes Engine](https://cloud.google.com/kubernetes-engine/). - -Furthermore, we use content to improve customer experience. An example are the personalization -questions when creating a group. - -To toggle the display of customer experience improvement content and third-party offers: - -1. On the left sidebar, expand the top-most chevron (**{chevron-down}**). -1. Select **Admin Area**. -1. Select **Settings > General**. -1. Expand **Customer experience improvement and third-party offers**. -1. Select **Do not display content for customer experience improvement and offers from third parties**. -1. Select **Save changes**. - - + + + + diff --git a/doc/user/admin_area/settings/usage_statistics.md b/doc/user/admin_area/settings/usage_statistics.md index 99cea767b5f..13ce142b14e 100644 --- a/doc/user/admin_area/settings/usage_statistics.md +++ b/doc/user/admin_area/settings/usage_statistics.md @@ -56,7 +56,7 @@ tier. Users can continue to access the features in a paid tier without sharing u - [Maintenance mode](../../../administration/maintenance_mode/index.md). - [Configurable issue boards](../../project/issue_board.md#configurable-issue-boards). - [Coverage-guided fuzz testing](../../application_security/coverage_fuzzing/index.md). -- [Password complexity requirements](../../../user/admin_area/settings/sign_up_restrictions.md#password-complexity-requirements). +- [Password complexity requirements](../../../administration/settings/sign_up_restrictions.md#password-complexity-requirements). NOTE: Registration is not yet required for participation, but may be added in a future milestone. diff --git a/doc/user/group/settings/group_access_tokens.md b/doc/user/group/settings/group_access_tokens.md index 805c274afa6..448c7514c93 100644 --- a/doc/user/group/settings/group_access_tokens.md +++ b/doc/user/group/settings/group_access_tokens.md @@ -35,7 +35,7 @@ You can use group access tokens: - On GitLab SaaS: If you have the Premium or Ultimate license tier. Group access tokens are not available with a [trial license](https://about.gitlab.com/free-trial/). - On self-managed instances: With any license tier. If you have the Free tier: - Review your security and compliance policies around - [user self-enrollment](../../admin_area/settings/sign_up_restrictions.md#disable-new-sign-ups). + [user self-enrollment](../../../administration/settings/sign_up_restrictions.md#disable-new-sign-ups). - Consider [disabling group access tokens](#enable-or-disable-group-access-token-creation) to lower potential abuse. diff --git a/doc/user/profile/user_passwords.md b/doc/user/profile/user_passwords.md index 50624a43893..c57a81c00bf 100644 --- a/doc/user/profile/user_passwords.md +++ b/doc/user/profile/user_passwords.md @@ -67,7 +67,7 @@ By default GitLab enforces the following password requirements: Self-managed installations can configure the following additional password requirements: - [Password minimum and maximum length limits](../../security/password_length_limits.md). -- [Password complexity requirements](../admin_area/settings/sign_up_restrictions.md#password-complexity-requirements). +- [Password complexity requirements](../../administration/settings/sign_up_restrictions.md#password-complexity-requirements). ## Block weak passwords diff --git a/gems/click_house-client/.rubocop.yml b/gems/click_house-client/.rubocop.yml index 24d88acff81..8c670b439d3 100644 --- a/gems/click_house-client/.rubocop.yml +++ b/gems/click_house-client/.rubocop.yml @@ -1,17 +1,2 @@ inherit_from: - - ../../.rubocop.yml - -CodeReuse/ActiveRecord: - Enabled: false - -Rails/ApplicationRecord: - Enabled: false - -AllCops: - TargetRubyVersion: 3.0 - -Naming/FileName: - Exclude: - - spec/**/*.rb - - lib/gitlab/rspec.rb - - lib/gitlab/rspec/all.rb + - ../config/rubocop.yml diff --git a/gems/config/rubocop.yml b/gems/config/rubocop.yml index 3458998e114..a97d759b471 100644 --- a/gems/config/rubocop.yml +++ b/gems/config/rubocop.yml @@ -16,6 +16,7 @@ AllCops: # Target the current Ruby version. For example, "3.0" or "3.1". TargetRubyVersion: <%= RUBY_VERSION[/^\d+\.\d+/, 0] %> SuggestExtensions: false + NewCops: disable # This cop doesn't make sense in the context of gems CodeReuse/ActiveRecord: @@ -60,6 +61,10 @@ Naming/FileName: Exclude: - spec/**/*.rb +# This cop doesn't make sense in the context of gems +RSpec/AvoidConditionalStatements: + Enabled: false + RSpec/ContextWording: Prefixes: - 'when' @@ -80,6 +85,9 @@ RSpec/MissingFeatureCategory: Style/HashSyntax: Enabled: false +Style/Lambda: + EnforcedStyle: literal + Style/RegexpLiteralMixedPreserve: Enabled: true SupportedStyles: diff --git a/gems/gitlab-schema-validation/.rubocop.yml b/gems/gitlab-schema-validation/.rubocop.yml index 7a531adfcf8..1dc800520ca 100644 --- a/gems/gitlab-schema-validation/.rubocop.yml +++ b/gems/gitlab-schema-validation/.rubocop.yml @@ -4,8 +4,5 @@ inherit_from: AllCops: NewCops: enable -Gitlab/RSpec/AvoidSetup: - Enabled: false - RSpec/MultipleMemoizedHelpers: Max: 25 diff --git a/gems/gitlab-utils/spec/gitlab/utils/strong_memoize_spec.rb b/gems/gitlab-utils/spec/gitlab/utils/strong_memoize_spec.rb index bb0641d9a27..f23a12ca6a2 100644 --- a/gems/gitlab-utils/spec/gitlab/utils/strong_memoize_spec.rb +++ b/gems/gitlab-utils/spec/gitlab/utils/strong_memoize_spec.rb @@ -353,7 +353,7 @@ RSpec.describe Gitlab::Utils::StrongMemoize, feature_category: :shared do it { is_expected.to eq(output) } - if params[:valid] # rubocop:disable RSpec/AvoidConditionalStatements + if params[:valid] it 'is a valid ivar name' do expect { instance_variable_defined?(ivar) }.not_to raise_error end diff --git a/gems/ipynbdiff/.rubocop.yml b/gems/ipynbdiff/.rubocop.yml index e30c6c44434..00a3ed337f1 100644 --- a/gems/ipynbdiff/.rubocop.yml +++ b/gems/ipynbdiff/.rubocop.yml @@ -7,10 +7,6 @@ CodeReuse/ActiveRecord: Gitlab/Json: Enabled: false -# FIXME -Gitlab/RSpec/AvoidSetup: - Enabled: false - Naming/FileName: Exclude: - spec/**/*.rb @@ -20,9 +16,6 @@ Naming/FileName: Rails/Pluck: Enabled: false -RSpec/AvoidConditionalStatements: - Enabled: false - RSpec/MultipleMemoizedHelpers: Max: 6 AllowSubject: true diff --git a/gems/rspec_flaky/spec/rspec_flaky/flaky_example_spec.rb b/gems/rspec_flaky/spec/rspec_flaky/flaky_example_spec.rb index 9a26d11d30c..244ca275f14 100644 --- a/gems/rspec_flaky/spec/rspec_flaky/flaky_example_spec.rb +++ b/gems/rspec_flaky/spec/rspec_flaky/flaky_example_spec.rb @@ -84,7 +84,7 @@ RSpec.describe RspecFlaky::FlakyExample, :aggregate_failures do end it 'updates the flaky_reports' do - expected_flaky_reports = flaky_example.to_h[:first_flaky_at] ? flaky_example.to_h[:flaky_reports] + 1 : 1 # rubocop:disable RSpec/AvoidConditionalStatements + expected_flaky_reports = flaky_example.to_h[:first_flaky_at] ? flaky_example.to_h[:flaky_reports] + 1 : 1 expect { flaky_example.update!(example_attrs) }.to change { flaky_example.to_h[:flaky_reports] }.by(1) expect(flaky_example.to_h[:flaky_reports]).to eq(expected_flaky_reports) diff --git a/lib/api/concerns/packages/npm_endpoints.rb b/lib/api/concerns/packages/npm_endpoints.rb index 8f92889dedc..ec20440f013 100644 --- a/lib/api/concerns/packages/npm_endpoints.rb +++ b/lib/api/concerns/packages/npm_endpoints.rb @@ -21,6 +21,10 @@ module API included do helpers ::API::Helpers::Packages::DependencyProxyHelpers + rescue_from ActiveRecord::RecordInvalid do |e| + render_structured_api_error!({ message: e.message, error: e.message }, 400) + end + before do require_packages_enabled! authenticate_non_get! @@ -88,7 +92,7 @@ module API packages = ::Packages::Npm::PackageFinder.new(package_name, project: project) .execute - not_found! if packages.empty? + not_found!('Package') if packages.empty? track_package_event(:list_tags, :npm, project: project, namespace: project.namespace) diff --git a/lib/api/helpers/packages/npm.rb b/lib/api/helpers/packages/npm.rb index d253be0b8cf..a80122c5309 100644 --- a/lib/api/helpers/packages/npm.rb +++ b/lib/api/helpers/packages/npm.rb @@ -6,6 +6,7 @@ module API module Npm include Gitlab::Utils::StrongMemoize include ::API::Helpers::PackagesHelpers + extend ::Gitlab::Utils::Override NPM_ENDPOINT_REQUIREMENTS = { package_name: API::NO_SLASH_URL_PART_REGEX @@ -105,6 +106,20 @@ module API group end strong_memoize_attr :group + + override :not_found! + def not_found!(resource = nil) + reason = "#{resource} not found" + message = "404 #{reason}".titleize + render_structured_api_error!({ message: message, error: reason }, 404) + end + + override :bad_request_missing_attribute! + def bad_request_missing_attribute!(attribute) + reason = "\"#{attribute}\" not given" + message = "400 Bad request - #{reason}" + render_structured_api_error!({ message: message, error: reason }, 400) + end end end end diff --git a/lib/api/internal/kubernetes.rb b/lib/api/internal/kubernetes.rb index ecd602cd8e0..35d26b08a90 100644 --- a/lib/api/internal/kubernetes.rb +++ b/lib/api/internal/kubernetes.rb @@ -29,21 +29,13 @@ module API end def gitaly_info(project) - shard = repo_type.repository_for(project).shard - { - address: Gitlab::GitalyClient.address(shard), - token: Gitlab::GitalyClient.token(shard), - features: Feature::Gitaly.server_feature_flags - } + gitaly_features = Feature::Gitaly.server_feature_flags + + Gitlab::GitalyClient.connection_data(project.repository_storage).merge(features: gitaly_features) end def gitaly_repository(project) - { - storage_name: project.repository_storage, - relative_path: project.disk_path + '.git', - gl_repository: repo_type.identifier_for_container(project), - gl_project_path: repo_type.repository_for(project).full_path - } + project.repository.gitaly_repository.to_h end def check_feature_enabled diff --git a/lib/api/npm_instance_packages.rb b/lib/api/npm_instance_packages.rb index 8215296b617..ea92818e76c 100644 --- a/lib/api/npm_instance_packages.rb +++ b/lib/api/npm_instance_packages.rb @@ -6,10 +6,6 @@ module API feature_category :package_registry urgency :low - rescue_from ActiveRecord::RecordInvalid do |e| - render_api_error!(e.message, 400) - end - helpers do def endpoint_scope :instance diff --git a/lib/api/npm_project_packages.rb b/lib/api/npm_project_packages.rb index a7463f4a5cd..e1d0455b1e2 100644 --- a/lib/api/npm_project_packages.rb +++ b/lib/api/npm_project_packages.rb @@ -7,7 +7,7 @@ module API urgency :low rescue_from ActiveRecord::RecordInvalid do |e| - render_api_error!(e.message, 400) + render_structured_api_error!({ message: e.message, error: e.message }, 400) end helpers do @@ -78,7 +78,7 @@ module API .new(project, current_user, params.merge(build: current_authenticated_job)).execute if created_package[:status] == :error - render_api_error!(created_package[:message], created_package[:http_status]) + render_structured_api_error!({ message: created_package[:message], error: created_package[:message] }, created_package[:http_status]) else enqueue_sync_metadata_cache_worker(project, created_package.name) track_package_event('push_package', :npm, category: 'API::NpmPackages', project: project, namespace: project.namespace) diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index 1751e8d5829..fe244bd88a0 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -68,7 +68,9 @@ module Gitlab end def gitaly_info(project) + gitaly_features = Feature::Gitaly.server_feature_flags connection_data = Gitlab::GitalyClient.connection_data(project.repository_storage) + .merge(features: gitaly_features) Gitlab::Agent::Entity::GitalyInfo.new(connection_data) end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 26ca9d2547c..4e666dbaf77 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -78,6 +78,10 @@ module Gitlab @npm_package_name_regex ||= %r{\A(?:@(#{Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX})/)?[-+\.\_a-zA-Z0-9]+\z}o end + def npm_package_name_regex_message + 'should be a valid NPM package name: https://github.com/npm/validate-npm-package-name#naming-rules.' + end + def nuget_package_name_regex @nuget_package_name_regex ||= %r{\A[-+\.\_a-zA-Z0-9]+\z}.freeze end @@ -177,6 +181,10 @@ module Gitlab @semver_regex ||= Regexp.new("\\A#{::Gitlab::Regex.unbounded_semver_regex.source}\\z", ::Gitlab::Regex.unbounded_semver_regex.options).freeze end + def semver_regex_message + 'should follow SemVer: https://semver.org' + end + # These partial semver regexes are intended for use in composing other # regexes rather than being used alone. def _semver_major_minor_patch_regex diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4be697cd8af..d7ab526bf1b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1536,8 +1536,8 @@ msgstr "" msgid "0 B" msgstr "" -msgid "1 Code quality finding" -msgid_plural "%d Code quality findings" +msgid "1 Code Quality finding" +msgid_plural "%d Code Quality findings" msgstr[0] "" msgstr[1] "" @@ -1551,6 +1551,11 @@ msgid_plural "%d Issues" msgstr[0] "" msgstr[1] "" +msgid "1 Security finding" +msgid_plural "%d Security findings" +msgstr[0] "" +msgstr[1] "" + msgid "1 closed issue" msgid_plural "%{issues} closed issues" msgstr[0] "" @@ -5189,6 +5194,9 @@ msgstr "" msgid "An unexpected error occurred while communicating with the Web Terminal." msgstr "" +msgid "An unexpected error occurred while loading the Sast diff." +msgstr "" + msgid "An unexpected error occurred while loading the code quality diff." msgstr "" @@ -12639,6 +12647,9 @@ msgstr "" msgid "ContributionAnalytics|Total Contributions" msgstr "" +msgid "ContributionEvent|Accepted merge request %{targetLink} in %{resourceParentLink}." +msgstr "" + msgid "ContributionEvent|Approved merge request %{targetLink} in %{resourceParentLink}." msgstr "" @@ -30228,6 +30239,9 @@ msgstr "" msgid "New Requirement" msgstr "" +msgid "New Security findings" +msgstr "" + msgid "New Snippet" msgstr "" @@ -55513,7 +55527,7 @@ msgstr "" msgid "starts on %{timebox_start_date}" msgstr "" -msgid "structure is too large" +msgid "structure is too large. Maximum size is %{max_size} characters" msgstr "" msgid "stuck" diff --git a/qa/qa/page/component/dropdown.rb b/qa/qa/page/component/dropdown.rb index 767cd40daa2..f0cab83686d 100644 --- a/qa/qa/page/component/dropdown.rb +++ b/qa/qa/page/component/dropdown.rb @@ -36,9 +36,9 @@ module QA if has_css?('button[data-testid="listbox-reset-button"]') find('button[data-testid="listbox-reset-button"]').click - elsif dropdown_open? - expand_select_list end + + expand_select_list if dropdown_open? end def search_item(item_text) diff --git a/qa/qa/specs/features/browser_ui/5_package/infrastructure_registry/terraform_module_registry_spec.rb b/qa/qa/specs/features/browser_ui/5_package/infrastructure_registry/terraform_module_registry_spec.rb index 3834efcca98..8dc9fb6db36 100644 --- a/qa/qa/specs/features/browser_ui/5_package/infrastructure_registry/terraform_module_registry_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/infrastructure_registry/terraform_module_registry_spec.rb @@ -4,9 +4,9 @@ module QA RSpec.describe 'Package', :requires_admin, product_group: :package_registry do describe 'Terraform Module Registry', quarantine: { - only: { pipeline: :nightly }, - type: :investigating, - issue: 'https://gitlab.com/gitlab-org/quality/quality-engineering/team-tasks/-/issues/1883' + only: { job: 'airgapped' }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/417407', + type: :investigating } do include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/conan_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/conan_repository_spec.rb index 0ad86153c3a..83662e04fab 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/conan_repository_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/conan_repository_spec.rb @@ -2,8 +2,9 @@ module QA RSpec.describe 'Package', :object_storage, product_group: :package_registry, quarantine: { - issue: 'https://gitlab.com/gitlab-org/quality/quality-engineering/team-tasks/-/issues/1883', - type: :investigating + only: { job: %w[object_storage relative_url airgapped], condition: -> { QA::Support::FIPS.enabled? } }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/417584', + type: :bug } do describe 'Conan Repository' do include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/helm_registry_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/helm_registry_spec.rb index 0cc6f41dc08..7bfda7f5956 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/helm_registry_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/helm_registry_spec.rb @@ -4,9 +4,9 @@ module QA RSpec.describe 'Package', :object_storage, product_group: :package_registry do describe 'Helm Registry', quarantine: { - only: { pipeline: :nightly }, - type: :investigating, - issue: 'https://gitlab.com/gitlab-org/quality/quality-engineering/team-tasks/-/issues/1883' + only: { job: %w[relative_url airgapped] }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/417590', + type: :investigating } do using RSpec::Parameterized::TableSyntax include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb index 6e5d39b1f1e..ce6c54b6ed8 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb @@ -4,9 +4,9 @@ module QA RSpec.describe 'Package', :object_storage, product_group: :package_registry do describe 'Maven group level endpoint', quarantine: { - only: { pipeline: :nightly }, - type: :investigating, - issue: 'https://gitlab.com/gitlab-org/quality/quality-engineering/team-tasks/-/issues/1883' + only: { job: %w[relative_url airgapped] }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/417600', + type: :investigating } do include Runtime::Fixtures include Support::Helpers::MaskToken diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_project_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_project_level_spec.rb index d29fcf695b8..35f80f8d447 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_project_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_project_level_spec.rb @@ -3,9 +3,9 @@ module QA RSpec.describe 'Package', :object_storage, quarantine: { - only: { pipeline: :nightly }, - type: :investigating, - issue: 'https://gitlab.com/gitlab-org/quality/quality-engineering/team-tasks/-/issues/1883' + only: { job: %w[relative_url airgapped] }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/417600', + type: :investigating } do describe 'Maven project level endpoint', product_group: :package_registry do include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven_gradle_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven_gradle_repository_spec.rb index 82588bb63cb..f24466ed003 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven_gradle_repository_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven_gradle_repository_spec.rb @@ -1,7 +1,12 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :object_storage, except: { job: 'relative-url' }, product_group: :package_registry do + RSpec.describe 'Package', :object_storage, + quarantine: { + only: { job: 'relative_url', condition: -> { QA::Support::FIPS.enabled? } }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/417600', + type: :investigating + }, product_group: :package_registry do describe 'Maven Repository with Gradle' do using RSpec::Parameterized::TableSyntax include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/pypi_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/pypi_repository_spec.rb index 2d30ce82c17..7e2885d3724 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/pypi_repository_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/pypi_repository_spec.rb @@ -4,9 +4,9 @@ module QA RSpec.describe 'Package', :object_storage, product_group: :package_registry do describe 'PyPI Repository', quarantine: { - only: { pipeline: :nightly }, - type: :investigating, - issue: 'https://gitlab.com/gitlab-org/quality/quality-engineering/team-tasks/-/issues/1883' + only: { job: %w[relative_url airgapped] }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/417592', + type: :investigating } do include Runtime::Fixtures include Support::Helpers::MaskToken diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh index b559a8de52a..bb81972b6b8 100644 --- a/scripts/rspec_helpers.sh +++ b/scripts/rspec_helpers.sh @@ -20,19 +20,19 @@ function retrieve_tests_metadata() { } function update_tests_metadata() { - local rspec_flaky_folder_path="$(dirname "${FLAKY_RSPEC_SUITE_REPORT_PATH}")/" - local knapsack_folder_path="$(dirname "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}")/" + local rspec_flaky_folder_path="$(dirname "${FLAKY_RSPEC_SUITE_REPORT_PATH:-unknown_folder}")/" + local knapsack_folder_path="$(dirname "${KNAPSACK_RSPEC_SUITE_REPORT_PATH:-unknown_folder}")/" - echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" + echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH:-unknown_file}" - scripts/merge-reports "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" ${knapsack_folder_path}rspec*.json + scripts/merge-reports "${KNAPSACK_RSPEC_SUITE_REPORT_PATH:-unknown_file}" ${knapsack_folder_path:-unknown_folder}rspec*.json export FLAKY_RSPEC_GENERATE_REPORT="true" - scripts/merge-reports "${FLAKY_RSPEC_SUITE_REPORT_PATH}" ${rspec_flaky_folder_path}all_*.json + scripts/merge-reports "${FLAKY_RSPEC_SUITE_REPORT_PATH:-unknown_file}" ${rspec_flaky_folder_path:-unknown_folder}all_*.json # Prune flaky tests that weren't flaky in the last 7 days, *after* updating the flaky tests detected # in this pipeline, so that first_flaky_at for tests that are still flaky is maintained. - scripts/flaky_examples/prune-old-flaky-examples "${FLAKY_RSPEC_SUITE_REPORT_PATH}" + scripts/flaky_examples/prune-old-flaky-examples "${FLAKY_RSPEC_SUITE_REPORT_PATH:-unknown_file}" if [[ "$CI_PIPELINE_SOURCE" == "schedule" ]]; then if [[ -n "$RSPEC_PROFILING_PGSSLKEY" ]]; then @@ -70,10 +70,10 @@ function update_tests_mapping() { return 0 fi - scripts/generate-test-mapping "${RSPEC_TESTS_MAPPING_PATH}" crystalball/rspec*.yml - scripts/pack-test-mapping "${RSPEC_TESTS_MAPPING_PATH}" "${RSPEC_PACKED_TESTS_MAPPING_PATH}" - gzip "${RSPEC_PACKED_TESTS_MAPPING_PATH}" - rm -f crystalball/rspec*.yml "${RSPEC_PACKED_TESTS_MAPPING_PATH}" + scripts/generate-test-mapping "${RSPEC_TESTS_MAPPING_PATH:-unknown_file}" crystalball/rspec*.yml + scripts/pack-test-mapping "${RSPEC_TESTS_MAPPING_PATH:-unknown_file}" "${RSPEC_PACKED_TESTS_MAPPING_PATH:-unknown_file}" + gzip "${RSPEC_PACKED_TESTS_MAPPING_PATH:-unknown_file}" + rm -f crystalball/rspec*.yml "${RSPEC_PACKED_TESTS_MAPPING_PATH:-unknown_file}" } function crystalball_rspec_data_exists() { @@ -451,14 +451,14 @@ function cleanup_individual_job_reports() { local rspec_flaky_folder_path="$(dirname "${FLAKY_RSPEC_SUITE_REPORT_PATH}")/" local knapsack_folder_path="$(dirname "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}")/" - rm -rf ${knapsack_folder_path}rspec*.json \ - ${rspec_flaky_folder_path}all_*.json \ - ${rspec_flaky_folder_path}new_*.json \ - ${rspec_flaky_folder_path}skipped_flaky_tests_*_report.txt \ - ${rspec_flaky_folder_path}retried_tests_*_report.txt \ - ${RSPEC_LAST_RUN_RESULTS_FILE} \ - ${RSPEC_PROFILING_FOLDER_PATH}/**/* - rmdir ${RSPEC_PROFILING_FOLDER_PATH} || true + rm -rf ${knapsack_folder_path:-unknown_folder}rspec*.json \ + ${rspec_flaky_folder_path:-unknown_folder}all_*.json \ + ${rspec_flaky_folder_path:-unknown_folder}new_*.json \ + ${rspec_flaky_folder_path:-unknown_folder}skipped_flaky_tests_*_report.txt \ + ${rspec_flaky_folder_path:-unknown_folder}retried_tests_*_report.txt \ + ${RSPEC_LAST_RUN_RESULTS_FILE:-unknown_folder} \ + ${RSPEC_PROFILING_FOLDER_PATH:-unknown_folder}/**/* + rmdir ${RSPEC_PROFILING_FOLDER_PAT:-unknown_folder} || true } function generate_flaky_tests_reports() { diff --git a/spec/features/merge_request/user_sees_diff_spec.rb b/spec/features/merge_request/user_sees_diff_spec.rb index 57f378a86b6..3fb3ef12fcc 100644 --- a/spec/features/merge_request/user_sees_diff_spec.rb +++ b/spec/features/merge_request/user_sees_diff_spec.rb @@ -77,9 +77,8 @@ RSpec.describe 'Merge request > User sees diff', :js, feature_category: :code_re sign_in(author_user) visit diffs_project_merge_request_path(project, merge_request) - first(".js-diff-more-actions").click - - expect(page).to have_selector(".js-edit-blob") + # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax + expect(page).to have_selector(".js-edit-blob", visible: false) end end diff --git a/spec/frontend/ci/reports/mock_data/mock_data.js b/spec/frontend/ci/reports/mock_data/mock_data.js index 2599b0ac365..2983a9f1125 100644 --- a/spec/frontend/ci/reports/mock_data/mock_data.js +++ b/spec/frontend/ci/reports/mock_data/mock_data.js @@ -1,3 +1,6 @@ +import { SEVERITIES as SEVERITIES_CODE_QUALITY } from '~/ci/reports/codequality_report/constants'; +import { SEVERITIES as SEVERITIES_SAST } from '~/ci/reports/sast/constants'; + export const failedIssue = { result: 'failure', name: 'Test#sum when a is 1 and b is 2 returns summary', @@ -36,3 +39,54 @@ export const failedReport = { }, ], }; + +export const findingSastInfo = { + scale: 'sast', + severity: 'info', +}; + +export const findingSastInfoEnhanced = { + scale: 'sast', + severity: 'info', + class: SEVERITIES_SAST.info.class, + name: SEVERITIES_SAST.info.name, +}; + +export const findingsCodeQualityBlocker = { + scale: 'codeQuality', + severity: 'blocker', +}; + +export const findingCodeQualityBlockerEnhanced = { + scale: 'codeQuality', + severity: 'blocker', + class: SEVERITIES_CODE_QUALITY.blocker.class, + name: SEVERITIES_CODE_QUALITY.blocker.name, +}; + +export const findingCodeQualityInfo = { + scale: 'codeQuality', + severity: 'info', +}; + +export const findingCodeQualityInfoEnhanced = { + scale: 'codeQuality', + severity: 'info', + class: SEVERITIES_CODE_QUALITY.info.class, + name: SEVERITIES_CODE_QUALITY.info.name, +}; + +export const findingUnknownInfo = { + scale: 'codeQuality', + severity: 'info', +}; + +export const findingUnknownInfoEnhanced = { + scale: 'codeQuality', + severity: 'info', + class: SEVERITIES_CODE_QUALITY.info.class, + name: SEVERITIES_CODE_QUALITY.info.name, +}; + +export const findingsArray = [findingSastInfo, findingsCodeQualityBlocker]; +export const findingsArrayEnhanced = [findingSastInfoEnhanced, findingCodeQualityBlockerEnhanced]; diff --git a/spec/frontend/ci/reports/utils_spec.js b/spec/frontend/ci/reports/utils_spec.js new file mode 100644 index 00000000000..e01aa903a97 --- /dev/null +++ b/spec/frontend/ci/reports/utils_spec.js @@ -0,0 +1,30 @@ +import { getSeverity } from '~/ci/reports/utils'; + +import { + findingSastInfo, + findingSastInfoEnhanced, + findingCodeQualityInfo, + findingCodeQualityInfoEnhanced, + findingUnknownInfo, + findingUnknownInfoEnhanced, + findingsArray, + findingsArrayEnhanced, +} from './mock_data/mock_data'; + +describe('getSeverity utility function', () => { + it('should enhance finding with sast scale', () => { + expect(getSeverity(findingSastInfo)).toEqual(findingSastInfoEnhanced); + }); + + it('should enhance finding with codequality scale', () => { + expect(getSeverity(findingCodeQualityInfo)).toEqual(findingCodeQualityInfoEnhanced); + }); + + it('should use codeQuality scale when scale is unknown', () => { + expect(getSeverity(findingUnknownInfo)).toEqual(findingUnknownInfoEnhanced); + }); + + it('should correctly enhance an array of findings', () => { + expect(getSeverity(findingsArray)).toEqual(findingsArrayEnhanced); + }); +}); diff --git a/spec/frontend/contribution_events/components/contribution_event/contribution_event_merged_spec.js b/spec/frontend/contribution_events/components/contribution_event/contribution_event_merged_spec.js new file mode 100644 index 00000000000..88494c24ddf --- /dev/null +++ b/spec/frontend/contribution_events/components/contribution_event/contribution_event_merged_spec.js @@ -0,0 +1,31 @@ +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import ContributionEventMerged from '~/contribution_events/components/contribution_event/contribution_event_merged.vue'; +import ContributionEventBase from '~/contribution_events/components/contribution_event/contribution_event_base.vue'; +import { eventMerged } from '../../utils'; + +const defaultPropsData = { + event: eventMerged(), +}; + +describe('ContributionEventMerged', () => { + let wrapper; + + const createComponent = () => { + wrapper = shallowMountExtended(ContributionEventMerged, { + propsData: defaultPropsData, + }); + }; + + beforeEach(() => { + createComponent(); + }); + + it('renders `ContributionEventBase`', () => { + expect(wrapper.findComponent(ContributionEventBase).props()).toEqual({ + event: defaultPropsData.event, + iconName: 'git-merge', + iconClass: 'gl-text-blue-600', + message: ContributionEventMerged.i18n.message, + }); + }); +}); diff --git a/spec/frontend/contribution_events/components/contribution_events_spec.js b/spec/frontend/contribution_events/components/contribution_events_spec.js index 4f32d2f6360..31e1bc3e569 100644 --- a/spec/frontend/contribution_events/components/contribution_events_spec.js +++ b/spec/frontend/contribution_events/components/contribution_events_spec.js @@ -6,6 +6,7 @@ import ContributionEventJoined from '~/contribution_events/components/contributi import ContributionEventLeft from '~/contribution_events/components/contribution_event/contribution_event_left.vue'; import ContributionEventPushed from '~/contribution_events/components/contribution_event/contribution_event_pushed.vue'; import ContributionEventPrivate from '~/contribution_events/components/contribution_event/contribution_event_private.vue'; +import ContributionEventMerged from '~/contribution_events/components/contribution_event/contribution_event_merged.vue'; import { eventApproved, eventExpired, @@ -13,6 +14,7 @@ import { eventLeft, eventPushedBranch, eventPrivate, + eventMerged, } from '../utils'; describe('ContributionEvents', () => { @@ -28,6 +30,7 @@ describe('ContributionEvents', () => { eventLeft(), eventPushedBranch(), eventPrivate(), + eventMerged(), ], }, }); @@ -41,6 +44,7 @@ describe('ContributionEvents', () => { ${ContributionEventLeft} | ${eventLeft()} ${ContributionEventPushed} | ${eventPushedBranch()} ${ContributionEventPrivate} | ${eventPrivate()} + ${ContributionEventMerged} | ${eventMerged()} `( 'renders `$expectedComponent.name` component and passes expected event', ({ expectedComponent, expectedEvent }) => { diff --git a/spec/frontend/contribution_events/utils.js b/spec/frontend/contribution_events/utils.js index 7cadfd71635..6e97455582d 100644 --- a/spec/frontend/contribution_events/utils.js +++ b/spec/frontend/contribution_events/utils.js @@ -6,6 +6,7 @@ import { EVENT_TYPE_LEFT, EVENT_TYPE_PUSHED, EVENT_TYPE_PRIVATE, + EVENT_TYPE_MERGED, PUSH_EVENT_REF_TYPE_BRANCH, PUSH_EVENT_REF_TYPE_TAG, } from '~/contribution_events/constants'; @@ -20,6 +21,8 @@ export const eventJoined = () => findEventByAction(EVENT_TYPE_JOINED); export const eventLeft = () => findEventByAction(EVENT_TYPE_LEFT); +export const eventMerged = () => findEventByAction(EVENT_TYPE_MERGED); + const findPushEvent = ({ isNew = false, isRemoved = false, diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index b69452069c0..fb5cf4dfd0a 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -73,6 +73,8 @@ describe('diffs/components/app', () => { propsData: { endpointCoverage: `${TEST_HOST}/diff/endpointCoverage`, endpointCodequality: '', + endpointSast: '', + projectPath: 'namespace/project', currentUser: {}, changesEmptyStateIllustration: '', ...props, @@ -184,6 +186,16 @@ describe('diffs/components/app', () => { }); }); + describe('SAST diff', () => { + it('does not fetch Sast data on FOSS', () => { + createComponent(); + jest.spyOn(wrapper.vm, 'fetchSast'); + wrapper.vm.fetchData(false); + + expect(wrapper.vm.fetchSast).not.toHaveBeenCalled(); + }); + }); + it('displays loading icon on loading', () => { createComponent({}, ({ state }) => { state.diffs.isLoading = true; diff --git a/spec/frontend/diffs/components/diff_code_quality_item_spec.js b/spec/frontend/diffs/components/diff_code_quality_item_spec.js index be9fb61a77d..085eb096239 100644 --- a/spec/frontend/diffs/components/diff_code_quality_item_spec.js +++ b/spec/frontend/diffs/components/diff_code_quality_item_spec.js @@ -2,20 +2,22 @@ import { GlIcon, GlLink } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import DiffCodeQualityItem from '~/diffs/components/diff_code_quality_item.vue'; import { SEVERITY_CLASSES, SEVERITY_ICONS } from '~/ci/reports/codequality_report/constants'; -import { multipleFindingsArr } from '../mock_data/diff_code_quality'; +import { multipleFindingsArrCodeQualityScale } from '../mock_data/diff_code_quality'; let wrapper; +const [codeQualityFinding] = multipleFindingsArrCodeQualityScale; const findIcon = () => wrapper.findComponent(GlIcon); const findButton = () => wrapper.findComponent(GlLink); const findDescriptionPlainText = () => wrapper.findByTestId('description-plain-text'); const findDescriptionLinkSection = () => wrapper.findByTestId('description-button-section'); describe('DiffCodeQuality', () => { - const createWrapper = ({ glFeatures = {} } = {}) => { + const createWrapper = ({ glFeatures = {}, link = true } = {}) => { return shallowMountExtended(DiffCodeQualityItem, { propsData: { - finding: multipleFindingsArr[0], + finding: codeQualityFinding, + link, }, provide: { glFeatures, @@ -28,8 +30,8 @@ describe('DiffCodeQuality', () => { expect(findIcon().exists()).toBe(true); expect(findIcon().attributes()).toMatchObject({ - class: `codequality-severity-icon ${SEVERITY_CLASSES[multipleFindingsArr[0].severity]}`, - name: SEVERITY_ICONS[multipleFindingsArr[0].severity], + class: `codequality-severity-icon ${SEVERITY_CLASSES[codeQualityFinding.severity]}`, + name: SEVERITY_ICONS[codeQualityFinding.severity], size: '12', }); }); @@ -41,26 +43,35 @@ describe('DiffCodeQuality', () => { codeQualityInlineDrawer: false, }, }); - expect(findDescriptionPlainText().text()).toContain(multipleFindingsArr[0].severity); - expect(findDescriptionPlainText().text()).toContain(multipleFindingsArr[0].description); + expect(findDescriptionPlainText().text()).toContain(codeQualityFinding.severity); + expect(findDescriptionPlainText().text()).toContain(codeQualityFinding.description); }); }); describe('with codeQualityInlineDrawer flag true', () => { - beforeEach(() => { + const [{ description, severity }] = multipleFindingsArrCodeQualityScale; + const renderedText = `${severity} - ${description}`; + it('when link prop is true, should render gl-link', () => { wrapper = createWrapper({ glFeatures: { codeQualityInlineDrawer: true, }, }); + + expect(findButton().exists()).toBe(true); + expect(findButton().text()).toBe(renderedText); }); - it('should render severity as plain text', () => { - expect(findDescriptionLinkSection().text()).toContain(multipleFindingsArr[0].severity); - }); + it('when link prop is false, should not render gl-link', () => { + wrapper = createWrapper({ + glFeatures: { + codeQualityInlineDrawer: true, + }, + link: false, + }); - it('should render button with description text', () => { - expect(findButton().text()).toContain(multipleFindingsArr[0].description); + expect(findButton().exists()).toBe(false); + expect(findDescriptionLinkSection().text()).toBe(renderedText); }); }); }); diff --git a/spec/frontend/diffs/components/diff_code_quality_spec.js b/spec/frontend/diffs/components/diff_code_quality_spec.js index 9ecfb62e1c5..dda71f362b0 100644 --- a/spec/frontend/diffs/components/diff_code_quality_spec.js +++ b/spec/frontend/diffs/components/diff_code_quality_spec.js @@ -1,26 +1,32 @@ import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; import DiffCodeQuality from '~/diffs/components/diff_code_quality.vue'; import DiffCodeQualityItem from '~/diffs/components/diff_code_quality_item.vue'; -import { NEW_CODE_QUALITY_FINDINGS } from '~/diffs/i18n'; -import { multipleFindingsArr } from '../mock_data/diff_code_quality'; +import { NEW_CODE_QUALITY_FINDINGS, NEW_SAST_FINDINGS } from '~/diffs/i18n'; +import { + multipleCodeQualityNoSast, + multipleSastNoCodeQuality, + multipleFindingsArrSastScale, +} from '../mock_data/diff_code_quality'; let wrapper; const diffItems = () => wrapper.findAllComponents(DiffCodeQualityItem); -const findHeading = () => wrapper.findByTestId(`diff-codequality-findings-heading`); +const findCodeQualityHeading = () => wrapper.findByTestId(`diff-codequality-findings-heading`); +const findSastHeading = () => wrapper.findByTestId(`diff-sast-findings-heading`); describe('DiffCodeQuality', () => { - const createWrapper = (codeQuality, mountFunction = mountExtended) => { + const createWrapper = (findings, mountFunction = mountExtended) => { return mountFunction(DiffCodeQuality, { propsData: { expandedLines: [], - codeQuality, + codeQuality: findings.codeQuality, + sast: findings.sast, }, }); }; it('hides details and throws hideCodeQualityFindings event on close click', async () => { - wrapper = createWrapper(multipleFindingsArr); + wrapper = createWrapper(multipleCodeQualityNoSast); expect(wrapper.findByTestId('diff-codequality').exists()).toBe(true); await wrapper.findByTestId('diff-codequality-close').trigger('click'); @@ -28,11 +34,29 @@ describe('DiffCodeQuality', () => { }); it('renders heading and correct amount of list items for codequality array and their description', () => { - wrapper = createWrapper(multipleFindingsArr, shallowMountExtended); + wrapper = createWrapper(multipleCodeQualityNoSast, shallowMountExtended); - expect(findHeading().text()).toEqual(NEW_CODE_QUALITY_FINDINGS); + expect(findCodeQualityHeading().text()).toEqual(NEW_CODE_QUALITY_FINDINGS); - expect(diffItems()).toHaveLength(multipleFindingsArr.length); - expect(diffItems().at(0).props().finding).toEqual(multipleFindingsArr[0]); + expect(diffItems()).toHaveLength(multipleCodeQualityNoSast.codeQuality.length); + expect(diffItems().at(0).props().finding).toEqual(multipleCodeQualityNoSast.codeQuality[0]); + }); + + it('does not render codeQuality section when codeQuality array is empty', () => { + wrapper = createWrapper(multipleSastNoCodeQuality, shallowMountExtended); + expect(findCodeQualityHeading().exists()).toBe(false); + }); + + it('renders heading and correct amount of list items for sast array and their description', () => { + wrapper = createWrapper(multipleSastNoCodeQuality, shallowMountExtended); + + expect(findSastHeading().text()).toEqual(NEW_SAST_FINDINGS); + expect(diffItems()).toHaveLength(multipleSastNoCodeQuality.sast.length); + expect(diffItems().at(0).props().finding).toEqual(multipleFindingsArrSastScale[0]); + }); + + it('does not render sast section when sast array is empty', () => { + wrapper = createWrapper(multipleCodeQualityNoSast, shallowMountExtended); + expect(findSastHeading().exists()).toBe(false); }); }); diff --git a/spec/frontend/diffs/components/diff_content_spec.js b/spec/frontend/diffs/components/diff_content_spec.js index 69e378771c5..3b37edbcb1d 100644 --- a/spec/frontend/diffs/components/diff_content_spec.js +++ b/spec/frontend/diffs/components/diff_content_spec.js @@ -5,6 +5,7 @@ import Vuex from 'vuex'; import waitForPromises from 'helpers/wait_for_promises'; import { sprintf } from '~/locale'; import { createAlert } from '~/alert'; +import * as diffRowUtils from 'ee_else_ce/diffs/components/diff_row_utils'; import DiffContentComponent from '~/diffs/components/diff_content.vue'; import DiffDiscussions from '~/diffs/components/diff_discussions.vue'; import DiffView from '~/diffs/components/diff_view.vue'; @@ -77,6 +78,7 @@ describe('DiffContent', () => { getCommentFormForDiffFile: getCommentFormForDiffFileGetterMock, diffLines: () => () => [...getDiffFileMock().parallel_diff_lines], fileLineCodequality: () => () => [], + fileLineSast: () => () => [], }, actions: { saveDiffDiscussion: saveDiffDiscussionMock, @@ -118,6 +120,32 @@ describe('DiffContent', () => { expect(wrapper.findComponent(GlLoadingIcon).exists()).toBe(true); }); + + it('should include Sast findings when sastReportsInInlineDiff flag is true', () => { + const mapParallelSpy = jest.spyOn(diffRowUtils, 'mapParallel'); + const mapParallelNoSastSpy = jest.spyOn(diffRowUtils, 'mapParallelNoSast'); + createComponent({ + provide: { + glFeatures: { + sastReportsInInlineDiff: true, + }, + }, + props: { diffFile: { ...textDiffFile, renderingLines: true } }, + }); + + expect(mapParallelSpy).toHaveBeenCalled(); + expect(mapParallelNoSastSpy).not.toHaveBeenCalled(); + }); + + it('should not include Sast findings when sastReportsInInlineDiff flag is false', () => { + const mapParallelSpy = jest.spyOn(diffRowUtils, 'mapParallel'); + const mapParallelNoSastSpy = jest.spyOn(diffRowUtils, 'mapParallelNoSast'); + + createComponent({ props: { diffFile: { ...textDiffFile, renderingLines: true } } }); + + expect(mapParallelNoSastSpy).toHaveBeenCalled(); + expect(mapParallelSpy).not.toHaveBeenCalled(); + }); }); describe('with whitespace only change', () => { diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js index db6cde883f3..9b9a1a84b1d 100644 --- a/spec/frontend/diffs/components/diff_file_spec.js +++ b/spec/frontend/diffs/components/diff_file_spec.js @@ -546,6 +546,24 @@ describe('DiffFile', () => { }); }); + it('loads collapsed file on mounted when single file mode is enabled', async () => { + const file = { + ...getReadableFile(), + load_collapsed_diff_url: '/diff_for_path', + highlighted_diff_lines: [], + parallel_diff_lines: [], + viewer: { name: 'collapsed', automaticallyCollapsed: true }, + }; + + axiosMock.onGet(file.load_collapsed_diff_url).reply(HTTP_STATUS_OK, getReadableFile()); + + ({ wrapper, store } = createComponent({ file, props: { viewDiffsFileByFile: true } })); + + await nextTick(); + + expect(findLoader(wrapper).exists()).toBe(true); + }); + describe('merge conflicts', () => { it('does not render conflict alert', () => { const file = { diff --git a/spec/frontend/diffs/components/diff_line_spec.js b/spec/frontend/diffs/components/diff_line_spec.js index 37368eb1461..a552a9d3e7f 100644 --- a/spec/frontend/diffs/components/diff_line_spec.js +++ b/spec/frontend/diffs/components/diff_line_spec.js @@ -16,6 +16,13 @@ const left = { severity: EXAMPLE_SEVERITY, }, ], + sast: [ + { + line: EXAMPLE_LINE_NUMBER, + description: EXAMPLE_DESCRIPTION, + severity: EXAMPLE_SEVERITY, + }, + ], }, }, }; @@ -30,6 +37,13 @@ const right = { severity: EXAMPLE_SEVERITY, }, ], + sast: [ + { + line: EXAMPLE_LINE_NUMBER, + description: EXAMPLE_DESCRIPTION, + severity: EXAMPLE_SEVERITY, + }, + ], }, }, }; @@ -60,6 +74,13 @@ describe('DiffLine', () => { severity: EXAMPLE_SEVERITY, }, ]); + expect(wrapper.findComponent(DiffCodeQuality).props('sast')).toEqual([ + { + line: EXAMPLE_LINE_NUMBER, + description: EXAMPLE_DESCRIPTION, + severity: EXAMPLE_SEVERITY, + }, + ]); }); }); }); diff --git a/spec/frontend/diffs/mock_data/diff_code_quality.js b/spec/frontend/diffs/mock_data/diff_code_quality.js index 29f16da8d89..83fc5707e5b 100644 --- a/spec/frontend/diffs/mock_data/diff_code_quality.js +++ b/spec/frontend/diffs/mock_data/diff_code_quality.js @@ -1,4 +1,4 @@ -export const multipleFindingsArr = [ +export const multipleFindingsArrCodeQualityScale = [ { severity: 'minor', description: 'mocked minor Issue', @@ -31,19 +31,78 @@ export const multipleFindingsArr = [ }, ]; -export const fiveFindings = { - filePath: 'index.js', - codequality: multipleFindingsArr.slice(0, 5), +export const multipleFindingsArrSastScale = [ + { + severity: 'low', + description: 'mocked low Issue', + line: 2, + }, + { + severity: 'medium', + description: 'mocked medium Issue', + line: 3, + }, + { + severity: 'info', + description: 'mocked info Issue', + line: 3, + }, + { + severity: 'high', + description: 'mocked high Issue', + line: 3, + }, + { + severity: 'critical', + description: 'mocked critical Issue', + line: 3, + }, + { + severity: 'unknown', + description: 'mocked unknown Issue', + line: 3, + }, +]; + +export const multipleCodeQualityNoSast = { + codeQuality: multipleFindingsArrCodeQualityScale, + sast: [], }; -export const threeFindings = { - filePath: 'index.js', - codequality: multipleFindingsArr.slice(0, 3), +export const multipleSastNoCodeQuality = { + codeQuality: [], + sast: multipleFindingsArrSastScale, }; -export const singularFinding = { +export const fiveCodeQualityFindings = { filePath: 'index.js', - codequality: [multipleFindingsArr[0]], + codequality: multipleFindingsArrCodeQualityScale.slice(0, 5), +}; + +export const threeCodeQualityFindings = { + filePath: 'index.js', + codequality: multipleFindingsArrCodeQualityScale.slice(0, 3), +}; + +export const singularCodeQualityFinding = { + filePath: 'index.js', + codequality: [multipleFindingsArrCodeQualityScale[0]], +}; + +export const singularFindingSast = { + filePath: 'index.js', + sast: [multipleFindingsArrSastScale[0]], +}; + +export const threeSastFindings = { + filePath: 'index.js', + sast: multipleFindingsArrSastScale.slice(0, 3), +}; + +export const oneCodeQualityTwoSastFindings = { + filePath: 'index.js', + sast: multipleFindingsArrSastScale.slice(0, 2), + codequality: [multipleFindingsArrCodeQualityScale[0]], }; export const diffCodeQuality = { @@ -73,7 +132,7 @@ export const diffCodeQuality = { old_line: null, new_line: 2, - codequality: [multipleFindingsArr[0]], + codequality: [multipleFindingsArrCodeQualityScale[0]], lineDrafts: [], }, }, diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index bbe748b8e1f..963346a34b6 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -365,7 +365,7 @@ describe('DiffsStoreActions', () => { { type: types.SET_RETRIEVING_BATCHES, payload: false }, { type: types.SET_BATCH_LOADING_STATE, payload: 'error' }, ], - [], + [{ type: 'startRenderDiffsQueue' }, { type: 'startRenderDiffsQueue' }], ); }); }); @@ -664,6 +664,41 @@ describe('DiffsStoreActions', () => { }); }); + describe('startRenderDiffsQueue', () => { + it('should set all files to RENDER_FILE', () => { + const state = { + diffFiles: [ + { + id: 1, + renderIt: false, + viewer: { + automaticallyCollapsed: false, + }, + }, + { + id: 2, + renderIt: false, + viewer: { + automaticallyCollapsed: false, + }, + }, + ], + }; + + const pseudoCommit = (commitType, file) => { + expect(commitType).toBe(types.RENDER_FILE); + Object.assign(file, { + renderIt: true, + }); + }; + + diffActions.startRenderDiffsQueue({ state, commit: pseudoCommit }); + + expect(state.diffFiles[0].renderIt).toBe(true); + expect(state.diffFiles[1].renderIt).toBe(true); + }); + }); + describe('setInlineDiffViewType', () => { it('should set diff view type to inline and also set the cookie properly', async () => { await testAction( @@ -1251,11 +1286,12 @@ describe('DiffsStoreActions', () => { $emit = jest.spyOn(eventHub, '$emit'); }); - it('expands the file for the given discussion id', () => { + it('renders and expands file for the given discussion id', () => { const localState = state({ collapsed: true, renderIt: false }); diffActions.renderFileForDiscussionId({ rootState, state: localState, commit }, '123'); + expect(commit).toHaveBeenCalledWith('RENDER_FILE', localState.diffFiles[0]); expect($emit).toHaveBeenCalledTimes(1); expect(commonUtils.scrollToElement).toHaveBeenCalledTimes(1); }); @@ -1342,6 +1378,18 @@ describe('DiffsStoreActions', () => { }); }); + describe('setRenderIt', () => { + it('commits RENDER_FILE', () => { + return testAction( + diffActions.setRenderIt, + 'file', + {}, + [{ type: types.RENDER_FILE, payload: 'file' }], + [], + ); + }); + }); + describe('receiveFullDiffError', () => { it('updates state with the file that did not load', () => { return testAction( @@ -1466,7 +1514,7 @@ describe('DiffsStoreActions', () => { payload: { filePath: testFilePath, lines: [preparedLine, preparedLine] }, }, ], - [], + [{ type: 'startRenderDiffsQueue' }], ); }, ); diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index 274cb40dac8..4f9534ed5db 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -105,6 +105,7 @@ describe('DiffsStoreMutations', () => { mutations[types.SET_DIFF_DATA_BATCH](state, diffMock); + expect(state.diffFiles[0].renderIt).toEqual(true); expect(state.diffFiles[0].collapsed).toEqual(false); expect(state.treeEntries[mockFile.file_path].diffLoaded).toBe(true); }); diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js index 117ed56e347..888df06d6b9 100644 --- a/spec/frontend/diffs/store/utils_spec.js +++ b/spec/frontend/diffs/store/utils_spec.js @@ -437,7 +437,7 @@ describe('DiffsStoreUtils', () => { }); }); - it('sets the collapsed attribute on files', () => { + it('sets the renderIt and collapsed attribute on files', () => { const checkLine = preparedDiff.diff_files[0][INLINE_DIFF_LINES_KEY][0]; expect(checkLine.discussions.length).toBe(0); @@ -448,6 +448,7 @@ describe('DiffsStoreUtils', () => { expect(firstChar).not.toBe('+'); expect(firstChar).not.toBe('-'); + expect(preparedDiff.diff_files[0].renderIt).toBe(true); expect(preparedDiff.diff_files[0].collapsed).toBe(false); }); @@ -528,7 +529,8 @@ describe('DiffsStoreUtils', () => { preparedDiffFiles = utils.prepareDiffData({ diff: mock, meta: true }); }); - it('sets the collapsed attribute on files', () => { + it('sets the renderIt and collapsed attribute on files', () => { + expect(preparedDiffFiles[0].renderIt).toBe(true); expect(preparedDiffFiles[0].collapsed).toBeUndefined(); }); diff --git a/spec/lib/gitlab/kas/client_spec.rb b/spec/lib/gitlab/kas/client_spec.rb index 811f07a6bec..e8884ce352f 100644 --- a/spec/lib/gitlab/kas/client_spec.rb +++ b/spec/lib/gitlab/kas/client_spec.rb @@ -79,6 +79,7 @@ RSpec.describe Gitlab::Kas::Client do let(:repository) { instance_double(Gitlab::Agent::Entity::GitalyRepository) } let(:gitaly_info) { instance_double(Gitlab::Agent::Entity::GitalyInfo) } + let(:gitaly_features) { Feature::Gitaly.server_feature_flags } let(:agent_configurations) { [double] } @@ -94,7 +95,7 @@ RSpec.describe Gitlab::Kas::Client do .and_return(repository) expect(Gitlab::Agent::Entity::GitalyInfo).to receive(:new) - .with(Gitlab::GitalyClient.connection_data(project.repository_storage)) + .with(Gitlab::GitalyClient.connection_data(project.repository_storage).merge(features: gitaly_features)) .and_return(gitaly_info) expect(Gitlab::Agent::ConfigurationProject::Rpc::ListAgentConfigFilesRequest).to receive(:new) diff --git a/spec/models/packages/npm/metadatum_spec.rb b/spec/models/packages/npm/metadatum_spec.rb index 418194bffdd..e5586dca15c 100644 --- a/spec/models/packages/npm/metadatum_spec.rb +++ b/spec/models/packages/npm/metadatum_spec.rb @@ -38,7 +38,9 @@ RSpec.describe Packages::Npm::Metadatum, type: :model, feature_category: :packag it { is_expected.not_to allow_value({}).for(:package_json) } - it { is_expected.not_to allow_value(test: 'test' * 10000).for(:package_json) } + it { + is_expected.not_to allow_value(test: 'test' * 10000).for(:package_json).with_message(/structure is too large/) + } def with_dist valid_json.tap do |h| diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index e1ef503c8fe..43a92a7b941 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -247,7 +247,7 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme 'gitaly_info' => a_hash_including( 'address' => match(/\.socket$/), 'token' => 'secret', - 'features' => {} + 'features' => Feature::Gitaly.server_feature_flags ), 'gitaly_repository' => a_hash_including( 'storage_name' => project.repository_storage, @@ -290,7 +290,7 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme 'gitaly_info' => a_hash_including( 'address' => match(/\.socket$/), 'token' => 'secret', - 'features' => {} + 'features' => Feature::Gitaly.server_feature_flags ), 'gitaly_repository' => a_hash_including( 'storage_name' => project.repository_storage, diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 955013845da..5069a00eea6 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -240,12 +240,13 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do subject(:upload_package_with_token) { upload_with_token(package_name, params) } - shared_examples 'handling invalid record with 400 error' do + shared_examples 'handling invalid record with 400 error' do |error_message| it 'handles an ActiveRecord::RecordInvalid exception with 400 error' do expect { upload_package_with_token } .not_to change { project.packages.count } expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq(error_message) end end @@ -255,7 +256,7 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do let(:package_name) { "@#{group.path}/my_inv@@lid_package_name" } let(:params) { upload_params(package_name: package_name) } - it_behaves_like 'handling invalid record with 400 error' + it_behaves_like 'handling invalid record with 400 error', "Validation failed: Name is invalid, Name #{Gitlab::Regex.npm_package_name_regex_message}" it_behaves_like 'not a package tracking event' end @@ -277,7 +278,7 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do with_them do let(:params) { upload_params(package_name: package_name, package_version: version) } - it_behaves_like 'handling invalid record with 400 error' + it_behaves_like 'handling invalid record with 400 error', "Validation failed: Version #{Gitlab::Regex.semver_regex_message}" it_behaves_like 'not a package tracking event' end end @@ -286,7 +287,7 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do let(:package_name) { "@#{group.path}/my_package_name" } let(:params) { upload_params(package_name: package_name, file: 'npm/payload_with_empty_attachment.json') } - it_behaves_like 'handling invalid record with 400 error' + it_behaves_like 'handling invalid record with 400 error', 'Attachment data is empty.' it_behaves_like 'not a package tracking event' end end @@ -373,7 +374,7 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do let(:package_name) { "@#{group.path}/test" } - it_behaves_like 'handling invalid record with 400 error' + it_behaves_like 'handling invalid record with 400 error', 'Validation failed: Package already exists' it_behaves_like 'not a package tracking event' context 'with a new version' do @@ -408,6 +409,7 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do .not_to change { project.packages.count } expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['error']).to eq('Package already exists.') end it_behaves_like 'does not enqueue a worker to sync a metadata cache' do @@ -461,7 +463,8 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do .not_to change { project.packages.count } expect(response).to have_gitlab_http_status(:bad_request) - expect(response.body).to include('Could not obtain package lease.') + expect(response.body).to include('Could not obtain package lease. Please try again.') + expect(json_response['error']).to eq('Could not obtain package lease. Please try again.') end end @@ -487,15 +490,8 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do end end + it_behaves_like 'handling invalid record with 400 error', 'Validation failed: Package json structure is too large. Maximum size is 20000 characters' it_behaves_like 'not a package tracking event' - - it 'returns an error' do - expect { upload_package_with_token } - .not_to change { project.packages.count } - - expect(response).to have_gitlab_http_status(:bad_request) - expect(response.body).to include('Validation failed: Package json structure is too large') - end end end diff --git a/spec/services/packages/npm/create_package_service_spec.rb b/spec/services/packages/npm/create_package_service_spec.rb index d0fca2a5f08..8b94bce6650 100644 --- a/spec/services/packages/npm/create_package_service_spec.rb +++ b/spec/services/packages/npm/create_package_service_spec.rb @@ -69,7 +69,7 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r field_sizes: expected_field_sizes ) - expect { subject }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Package json structure is too large') + expect { subject }.to raise_error(ActiveRecord::RecordInvalid, /structure is too large/) .and not_change { Packages::Package.count } .and not_change { Packages::Package.npm.count } .and not_change { Packages::Tag.count } @@ -296,7 +296,7 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r end with_them do - it { expect { subject }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Version is invalid') } + it { expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Version #{Gitlab::Regex.semver_regex_message}") } end end @@ -319,7 +319,7 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r end it { expect(subject[:http_status]).to eq 400 } - it { expect(subject[:message]).to eq 'Could not obtain package lease.' } + it { expect(subject[:message]).to eq 'Could not obtain package lease. Please try again.' } end context 'when many of the same packages are created at the same time', :delete do diff --git a/spec/support/shared_examples/requests/api/npm_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/npm_packages_shared_examples.rb index 5284ed2de21..6b6bf375827 100644 --- a/spec/support/shared_examples/requests/api/npm_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/npm_packages_shared_examples.rb @@ -44,11 +44,7 @@ RSpec.shared_examples 'handling get metadata requests' do |scope: :project| end shared_examples 'reject metadata request' do |status:| - it 'rejects the metadata request' do - subject - - expect(response).to have_gitlab_http_status(status) - end + it_behaves_like 'returning response status', status end shared_examples 'redirect metadata request' do |status:| @@ -280,13 +276,15 @@ RSpec.shared_examples 'handling get metadata requests' do |scope: :project| example_name = 'redirect metadata request' status = :redirected else - example_name = 'reject metadata request' status = :not_found end end status = :not_found if scope == :group && params[:package_name_type] == :non_existing && !params[:request_forward] + # Check the error message for :not_found + example_name = 'returning response status with error' if status == :not_found + it_behaves_like example_name, status: status end end @@ -361,11 +359,11 @@ RSpec.shared_examples 'handling audit request' do |path:, scope: :project| end shared_examples 'reject audit request' do |status:| - it 'rejects the audit request' do - subject + it_behaves_like 'returning response status', status + end - expect(response).to have_gitlab_http_status(status) - end + shared_examples 'reject audit request with error' do |status:| + it_behaves_like 'returning response status with error', status: status, error: 'Project not found' end shared_examples 'redirect audit request' do |status:| @@ -464,7 +462,7 @@ RSpec.shared_examples 'handling audit request' do |path:, scope: :project| example_name = 'redirect audit request' status = :temporary_redirect else - example_name = 'reject audit request' + example_name = 'reject audit request with error' status = :not_found end end @@ -633,12 +631,12 @@ RSpec.shared_examples 'handling get dist tags requests' do |scope: :project| example_name = "#{params[:expected_result]} package tags request" status = params[:expected_status] - if scope == :instance && params[:package_name_type] != :scoped_naming_convention - example_name = 'reject package tags request' + if (scope == :instance && params[:package_name_type] != :scoped_naming_convention) || (scope == :group && params[:package_name_type] == :non_existing) status = :not_found end - status = :not_found if scope == :group && params[:package_name_type] == :non_existing + # Check the error message for :not_found + example_name = 'returning response status with error' if status == :not_found it_behaves_like example_name, status: status end @@ -858,6 +856,9 @@ RSpec.shared_examples 'handling different package names, visibilities and user r status = :not_found if scope == :group && params[:package_name_type] == :non_existing && params[:auth].present? + # Check the error message for :not_found + example_name = 'returning response status with error' if status == :not_found + it_behaves_like example_name, status: status end end diff --git a/spec/support/shared_examples/requests/api/npm_packages_tags_shared_examples.rb b/spec/support/shared_examples/requests/api/npm_packages_tags_shared_examples.rb index 7c20ea661b5..403344efe03 100644 --- a/spec/support/shared_examples/requests/api/npm_packages_tags_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/npm_packages_tags_shared_examples.rb @@ -36,18 +36,18 @@ RSpec.shared_examples 'accept package tags request' do |status:| end context 'with invalid package name' do - where(:package_name, :status) do - '%20' | :bad_request - nil | :not_found + where(:package_name, :status, :error) do + '%20' | :bad_request | '"Package Name" not given' + nil | :not_found | %r{\A(Packages|Project) not found\z} end with_them do - it_behaves_like 'returning response status', params[:status] + it_behaves_like 'returning response status with error', status: params[:status], error: params[:error] end end end -RSpec.shared_examples 'accept create package tag request' do |user_type| +RSpec.shared_examples 'accept create package tag request' do |status:| using RSpec::Parameterized::TableSyntax context 'with valid package name' do @@ -92,45 +92,55 @@ RSpec.shared_examples 'accept create package tag request' do |user_type| expect(response.body).to be_empty end end + + context 'with ActiveRecord::RecordInvalid error' do + before do + allow_next_instance_of(Packages::Tag) do |tag| + allow(tag).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) + end + end + + it_behaves_like 'returning response status with error', status: :bad_request, error: 'Record invalid' + end end context 'with invalid package name' do - where(:package_name, :status) do - 'unknown' | :not_found - '' | :not_found - '%20' | :bad_request + where(:package_name, :status, :error) do + 'unknown' | :not_found | %r{\A(Package|Project) not found\z} + '' | :not_found | '404 Not Found' + '%20' | :bad_request | '"Package Name" not given' end with_them do - it_behaves_like 'returning response status', params[:status] + it_behaves_like 'returning response status with error', status: params[:status], error: params[:error] end end context 'with invalid tag name' do - where(:tag_name, :status) do - '' | :not_found - '%20' | :bad_request + where(:tag_name, :status, :error) do + '' | :not_found | '404 Not Found' + '%20' | :bad_request | '"Tag" not given' end with_them do - it_behaves_like 'returning response status', params[:status] + it_behaves_like 'returning response status with error', status: params[:status], error: params[:error] end end context 'with invalid version' do - where(:version, :status) do - ' ' | :bad_request - '' | :bad_request - nil | :bad_request + where(:version, :status, :error) do + ' ' | :bad_request | '"Version" not given' + '' | :bad_request | '"Version" not given' + nil | :bad_request | '"Version" not given' end with_them do - it_behaves_like 'returning response status', params[:status] + it_behaves_like 'returning response status with error', status: params[:status], error: params[:error] end end end -RSpec.shared_examples 'accept delete package tag request' do |user_type| +RSpec.shared_examples 'accept delete package tag request' do |status:| using RSpec::Parameterized::TableSyntax context 'with valid package name' do @@ -159,29 +169,39 @@ RSpec.shared_examples 'accept delete package tag request' do |user_type| it_behaves_like 'returning response status', :not_found end + + context 'with ActiveRecord::RecordInvalid error' do + before do + allow_next_instance_of(::Packages::RemoveTagService) do |service| + allow(service).to receive(:execute).and_raise(ActiveRecord::RecordInvalid) + end + end + + it_behaves_like 'returning response status with error', status: :bad_request, error: 'Record invalid' + end end context 'with invalid package name' do - where(:package_name, :status) do - 'unknown' | :not_found - '' | :not_found - '%20' | :bad_request + where(:package_name, :status, :error) do + 'unknown' | :not_found | %r{\A(Package tag|Project) not found\z} + '' | :not_found | '404 Not Found' + '%20' | :bad_request | '"Package Name" not given' end with_them do - it_behaves_like 'returning response status', params[:status] + it_behaves_like 'returning response status with error', status: params[:status], error: params[:error] end end context 'with invalid tag name' do - where(:tag_name, :status) do - 'unknown' | :not_found - '' | :not_found - '%20' | :bad_request + where(:tag_name, :status, :error) do + 'unknown' | :not_found | %r{\A(Package tag|Project) not found\z} + '' | :not_found | '404 Not Found' + '%20' | :bad_request | '"Tag" not given' end with_them do - it_behaves_like 'returning response status', params[:status] + it_behaves_like 'returning response status with error', status: params[:status], error: params[:error] end end end diff --git a/spec/support/shared_examples/requests/response_status_with_error_shared_examples.rb b/spec/support/shared_examples/requests/response_status_with_error_shared_examples.rb new file mode 100644 index 00000000000..de012aebaad --- /dev/null +++ b/spec/support/shared_examples/requests/response_status_with_error_shared_examples.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'returning response status with error' do |status:, error: nil| + it "returns #{status} and error message" do + subject + + expect(response).to have_gitlab_http_status(status) + expect(json_response['error']).to be_present + expect(json_response['error']).to match(error) if error + end +end