diff options
117 files changed, 1901 insertions, 313 deletions
diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index a696c9b2279..cf90d98340b 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -32,7 +32,6 @@ Rails/SaveBang: - 'ee/spec/models/approval_project_rule_spec.rb' - 'ee/spec/models/burndown_spec.rb' - 'ee/spec/models/elasticsearch_indexed_namespace_spec.rb' - - 'ee/spec/models/epic_spec.rb' - 'ee/spec/models/gitlab_subscription_spec.rb' - 'ee/spec/models/issue_spec.rb' - 'ee/spec/models/label_note_spec.rb' diff --git a/CHANGELOG.md b/CHANGELOG.md index b356fed4432..a15a5323e65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.4.1 (2021-10-28) + +### Security (13 changes) + +- [Highlight usage of unicode bidi characters](gitlab-org/security/gitlab@cef762a270783780112c7bf318e353a39de1aa1e) ([merge request](gitlab-org/security/gitlab!1937)) +- [Fix dompurify.js to prevent path traversal attacks](gitlab-org/security/gitlab@9a891cbe465a302f260f0f81fc490cacb9e8c70e) ([merge request](gitlab-org/security/gitlab!1929)) +- [Refresh authorizations on transfer of groups having project shares](gitlab-org/security/gitlab@bdf8b6e90d0a1f719c0f389f29ea5dc41c22f119) ([merge request](gitlab-org/security/gitlab!1916)) +- [Adding a '[redacted]' to mask private email addresses](gitlab-org/security/gitlab@324fe6286b266c3990676bc93b3f6ab03eea5f6b) ([merge request](gitlab-org/security/gitlab!1927)) +- [Do not allow Applications API to create apps with blank scopes](gitlab-org/security/gitlab@4e2c4d2a88acf7167e1078e8a27679545ab90c9c) ([merge request](gitlab-org/security/gitlab!1922)) +- [Don't allow author to resolve discussions when MR is locked via GraphQL](gitlab-org/security/gitlab@34ffcb55a70ad6db38292f79fe73c05fb2655738) ([merge request](gitlab-org/security/gitlab!1919)) +- [Workhorse: Allow uploading only a single file](gitlab-org/security/gitlab@0aee710db4bbab84c78b9e38f459bfca606aaf80) ([merge request](gitlab-org/security/gitlab!1913)) +- [Set PipelineSchedules to inactive](gitlab-org/security/gitlab@de405edc9de4519656675ed6825534aac6b738da) ([merge request](gitlab-org/security/gitlab!1911)) +- [Do not display the root password by default](gitlab-org/security/gitlab@138a62f89ce6616d63e3cf18eeda291a380b9ebc) ([merge request](gitlab-org/security/gitlab!1909)) +- [Group owners should see SCIM token only once](gitlab-org/security/gitlab@43d19f580543d0203b1d841f921536474ca4be38) ([merge request](gitlab-org/security/gitlab!1906)) **GitLab Enterprise Edition** +- [Respect visibility level settings when updating project via API](gitlab-org/security/gitlab@f96258f3622cf72b46158f22c4660ff60a2c25ae) ([merge request](gitlab-org/security/gitlab!1903)) +- [Avoid decoding the whole tiff image on isTIFF check](gitlab-org/security/gitlab@b93683df51ce85f909d5072ec2a0e7756d64038e) ([merge request](gitlab-org/security/gitlab!1899)) +- [Remove external_webhook_token from exported project](gitlab-org/security/gitlab@874aa74a23fc3c44f390500bc8379c30ebc51452) ([merge request](gitlab-org/security/gitlab!1872)) + ## 14.4.0 (2021-10-21) ### Added (79 changes) @@ -391,6 +409,24 @@ entry. - [Cleanup bigint conversion for ci_builds](gitlab-org/gitlab@176992aa2b2e76b22637a07d5bafbd6541324a7d) ([merge request](gitlab-org/gitlab!70351)) - [Drop support for data-track-event](gitlab-org/gitlab@ac6027fbef6adf41643412a84945fda6f15c9666) ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70234)) +## 14.3.4 (2021-10-28) + +### Security (13 changes) + +- [Highlight usage of unicode bidi characters](gitlab-org/security/gitlab@0b9bcafa73bc12ad873f75584b993f7b94f1f2e7) ([merge request](gitlab-org/security/gitlab!1938)) +- [Fix dompurify.js to prevent path traversal attacks](gitlab-org/security/gitlab@6599afd4d7357ab356fcb773af19f8388978b3ed) ([merge request](gitlab-org/security/gitlab!1930)) +- [Refresh authorizations on transfer of groups having project shares](gitlab-org/security/gitlab@faad71f44a1b1048b73897d450c923a18ec18c0b) ([merge request](gitlab-org/security/gitlab!1917)) +- [Do not allow Applications API to create apps with blank scopes](gitlab-org/security/gitlab@293931500c84ef7ea9a2117d3ddf094f8ac15dcf) ([merge request](gitlab-org/security/gitlab!1923)) +- [Don't allow author to resolve discussions when MR is locked via GraphQL](gitlab-org/security/gitlab@5027cb2b0303645a921b95d324d3d55dcf7632e4) ([merge request](gitlab-org/security/gitlab!1920)) +- [Workhorse: Allow uploading only a single file](gitlab-org/security/gitlab@c18c2ddfa34a4c3e476136ab3eba9be7f265ad59) ([merge request](gitlab-org/security/gitlab!1914)) +- [Group owners should see SCIM token only once](gitlab-org/security/gitlab@3d6664461da720fb256d8e139961b383e33a3b90) ([merge request](gitlab-org/security/gitlab!1907)) **GitLab Enterprise Edition** +- [Respect visibility level settings when updating project via API](gitlab-org/security/gitlab@124ca62c02bfa8ef6f7de7b328f80756fd01c052) ([merge request](gitlab-org/security/gitlab!1904)) +- [Avoid decoding the whole tiff image on isTIFF check](gitlab-org/security/gitlab@8e6ffd52f50170a5cf2761e50a3d6efaca5fe64f) ([merge request](gitlab-org/security/gitlab!1900)) +- [Adding a '[redacted]' to mask private email addresses](gitlab-org/security/gitlab@6f2a2b2240eb7590bbc773f35d3927d4854a31b5) ([merge request](gitlab-org/security/gitlab!1894)) +- [Do not display the root password by default](gitlab-org/security/gitlab@87893548183fc4a111e12c0bdb3e409175a41668) ([merge request](gitlab-org/security/gitlab!1803)) +- [Set PipelineSchedules to inactive](gitlab-org/security/gitlab@0e77e1cd938f876f3e9c049a84486c8c90cd0f3f) ([merge request](gitlab-org/security/gitlab!1879)) +- [Remove external_webhook_token from exported project](gitlab-org/security/gitlab@1362f7481aad5e4295da11f0db53e31600c7c7b5) ([merge request](gitlab-org/security/gitlab!1866)) + ## 14.3.3 (2021-10-12) ### Fixed (3 changes) @@ -939,6 +975,24 @@ entry. - [Remove the FF ci_reset_bridge_with_subsequent_jobs](gitlab-org/gitlab@a4a75095b9b0250d0b1bdadea90c8a4cd24449b2) ([merge request](gitlab-org/gitlab!68295)) - [Removes ci_same_stage_job_needs ff](gitlab-org/gitlab@5e509cf7aa90041a541b19dda563120a359f0bf9) ([merge request](gitlab-org/gitlab!68041)) +## 14.2.6 (2021-10-28) + +### Security (13 changes) + +- [Highlight usage of unicode bidi characters](gitlab-org/security/gitlab@18a768bb3cd19b6dc780bb85d91a93605ec8aa4f) ([merge request](gitlab-org/security/gitlab!1939)) +- [Fix dompurify.js to prevent path traversal attacks](gitlab-org/security/gitlab@cfd7c715162c22060b9b80268ef501a9e604421a) ([merge request](gitlab-org/security/gitlab!1931)) +- [Refresh authorizations on transfer of groups having project shares](gitlab-org/security/gitlab@3fc08eb869156a090b015e78da79c8ced16a7162) ([merge request](gitlab-org/security/gitlab!1918)) +- [Do not allow Applications API to create apps with blank scopes](gitlab-org/security/gitlab@c4ffc8c0ee5356bcb9b76dbfa92517589b4225a8) ([merge request](gitlab-org/security/gitlab!1924)) +- [Don't allow author to resolve discussions when MR is locked via GraphQL](gitlab-org/security/gitlab@fe2d0b6f250b60619da97f162c93c9e645daf4af) ([merge request](gitlab-org/security/gitlab!1921)) +- [Workhorse: Allow uploading only a single file](gitlab-org/security/gitlab@89b04599592b7dfc0e4883cfde5d3ecd9ea855b2) ([merge request](gitlab-org/security/gitlab!1915)) +- [Group owners should see SCIM token only once](gitlab-org/security/gitlab@d52c1e41f38039db075a7a3418b8eb9ed8474c2a) ([merge request](gitlab-org/security/gitlab!1908)) **GitLab Enterprise Edition** +- [Respect visibility level settings when updating project via API](gitlab-org/security/gitlab@3051d6a00d1a56133a77ecd24313bafb4565d576) ([merge request](gitlab-org/security/gitlab!1905)) +- [Avoid decoding the whole tiff image on isTIFF check](gitlab-org/security/gitlab@bab7f45def8fc81fe4b0961a21b4c90a60358ff9) ([merge request](gitlab-org/security/gitlab!1901)) +- [Adding a '[redacted]' to mask private email addresses](gitlab-org/security/gitlab@8eb9749f40b87b9b49b034bceb263219a4d3b114) ([merge request](gitlab-org/security/gitlab!1895)) +- [Do not display the root password by default](gitlab-org/security/gitlab@4ccf08b6645b9f616657edd266d9d31e3602d170) ([merge request](gitlab-org/security/gitlab!1802)) +- [Set PipelineSchedules to inactive](gitlab-org/security/gitlab@ebee16945325d22ceb5c07b7ba48df6fd0b2f067) ([merge request](gitlab-org/security/gitlab!1878)) +- [Remove external_webhook_token from exported project](gitlab-org/security/gitlab@f3ef12185902f3ed5c9d62ffce07418fd704a753) ([merge request](gitlab-org/security/gitlab!1865)) + ## 14.2.5 (2021-09-30) ### Security (28 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c75d1df87c0..070f920c0f6 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -bf9abd731ba2dab65e8c275c51d578d95dd3d506 +80d41b90597e1ca027cc6f02a09a6d3607d75ba2 diff --git a/app/assets/javascripts/lib/dompurify.js b/app/assets/javascripts/lib/dompurify.js index d421d66981e..47ede8cb1bb 100644 --- a/app/assets/javascripts/lib/dompurify.js +++ b/app/assets/javascripts/lib/dompurify.js @@ -1,5 +1,5 @@ import { sanitize as dompurifySanitize, addHook } from 'dompurify'; -import { getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility'; +import { getNormalizedURL, getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility'; const defaultConfig = { // Safely allow SVG <use> tags @@ -11,12 +11,14 @@ const defaultConfig = { // Only icons urls from `gon` are allowed const getAllowedIconUrls = (gon = window.gon) => - [gon.sprite_file_icons, gon.sprite_icons].filter(Boolean); + [gon.sprite_file_icons, gon.sprite_icons] + .filter(Boolean) + .map((path) => relativePathToAbsolute(path, getBaseURL())); -const isUrlAllowed = (url) => getAllowedIconUrls().some((allowedUrl) => url.startsWith(allowedUrl)); +const isUrlAllowed = (url) => + getAllowedIconUrls().some((allowedUrl) => getNormalizedURL(url).startsWith(allowedUrl)); -const isHrefSafe = (url) => - isUrlAllowed(url) || isUrlAllowed(relativePathToAbsolute(url, getBaseURL())) || url.match(/^#/); +const isHrefSafe = (url) => url.match(/^#/) || isUrlAllowed(url); const removeUnsafeHref = (node, attr) => { if (!node.hasAttribute(attr)) { @@ -36,13 +38,14 @@ const removeUnsafeHref = (node, attr) => { * <use href="/assets/icons-xxx.svg#icon_name"></use> * </svg> * + * It validates both href & xlink:href attributes. + * Note that `xlink:href` is deprecated, but still in use + * https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href + * * @param {Object} node - Node to sanitize */ const sanitizeSvgIcon = (node) => { removeUnsafeHref(node, 'href'); - - // Note: `xlink:href` is deprecated, but still in use - // https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href removeUnsafeHref(node, 'xlink:href'); }; diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 1c22d21a313..c70d23d06ec 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -399,6 +399,24 @@ export function isSafeURL(url) { } } +/** + * Returns a normalized url + * + * https://gitlab.com/foo/../baz => https://gitlab.com/baz + * + * @param {String} url - URL to be transformed + * @param {String?} baseUrl - current base URL + * @returns {String} + */ +export const getNormalizedURL = (url, baseUrl) => { + const base = baseUrl || getBaseURL(); + try { + return new URL(url, base).href; + } catch (e) { + return ''; + } +}; + export function getWebSocketProtocol() { return window.location.protocol.replace('http', 'ws'); } diff --git a/app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue b/app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue index 73fb3656af1..13e5d7c3019 100644 --- a/app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue +++ b/app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue @@ -1,12 +1,13 @@ <script> import { GlAlert, GlFormGroup, GlFormInputGroup, GlSkeletonLoader, GlSprintf } from '@gitlab/ui'; -import { __ } from '~/locale'; +import { s__ } from '~/locale'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import TitleArea from '~/vue_shared/components/registry/title_area.vue'; import { DEPENDENCY_PROXY_SETTINGS_DESCRIPTION, DEPENDENCY_PROXY_DOCS_PATH, } from '~/packages_and_registries/settings/group/constants'; +import { GRAPHQL_PAGE_SIZE } from '~/packages_and_registries/dependency_proxy/constants'; import getDependencyProxyDetailsQuery from '~/packages_and_registries/dependency_proxy/graphql/queries/get_dependency_proxy_details.query.graphql'; @@ -22,11 +23,16 @@ export default { }, inject: ['groupPath', 'dependencyProxyAvailable'], i18n: { - proxyNotAvailableText: __('Dependency Proxy feature is limited to public groups for now.'), - proxyDisabledText: __('Dependency Proxy disabled. To enable it, contact the group owner.'), - proxyImagePrefix: __('Dependency Proxy image prefix'), - copyImagePrefixText: __('Copy prefix'), - blobCountAndSize: __('Contains %{count} blobs of images (%{size})'), + proxyNotAvailableText: s__( + 'DependencyProxy|Dependency Proxy feature is limited to public groups for now.', + ), + proxyDisabledText: s__( + 'DependencyProxy|Dependency Proxy disabled. To enable it, contact the group owner.', + ), + proxyImagePrefix: s__('DependencyProxy|Dependency Proxy image prefix'), + copyImagePrefixText: s__('DependencyProxy|Copy prefix'), + blobCountAndSize: s__('DependencyProxy|Contains %{count} blobs of images (%{size})'), + pageTitle: s__('DependencyProxy|Dependency Proxy'), }, data() { return { @@ -40,7 +46,7 @@ export default { return !this.dependencyProxyAvailable; }, variables() { - return { fullPath: this.groupPath }; + return { fullPath: this.groupPath, first: GRAPHQL_PAGE_SIZE }; }, }, }, @@ -62,7 +68,7 @@ export default { <template> <div> - <title-area :title="__('Dependency Proxy')" :info-messages="infoMessages" /> + <title-area :title="$options.i18n.pageTitle" :info-messages="infoMessages" /> <gl-alert v-if="!dependencyProxyAvailable" :dismissible="false" diff --git a/app/assets/javascripts/packages_and_registries/dependency_proxy/components/manifest_row.vue b/app/assets/javascripts/packages_and_registries/dependency_proxy/components/manifest_row.vue new file mode 100644 index 00000000000..78880b6e3f4 --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/dependency_proxy/components/manifest_row.vue @@ -0,0 +1,49 @@ +<script> +import { GlSprintf } from '@gitlab/ui'; +import ListItem from '~/vue_shared/components/registry/list_item.vue'; +import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import { s__ } from '~/locale'; + +export default { + name: 'ManifestRow', + components: { + GlSprintf, + ListItem, + TimeagoTooltip, + }, + props: { + manifest: { + type: Object, + required: true, + }, + }, + computed: { + name() { + return this.manifest?.imageName.split(':')[0]; + }, + version() { + return this.manifest?.imageName.split(':')[1]; + }, + }, + i18n: { + cachedAgoMessage: s__('DependencyProxy|Cached %{time}'), + }, +}; +</script> + +<template> + <list-item> + <template #left-primary> {{ name }} </template> + <template #left-secondary> {{ version }} </template> + <template #right-primary> </template> + <template #right-secondary> + <timeago-tooltip :time="manifest.createdAt" data-testid="cached-message"> + <template #default="{ timeAgo }"> + <gl-sprintf :message="$options.i18n.cachedAgoMessage"> + <template #time>{{ timeAgo }}</template> + </gl-sprintf> + </template> + </timeago-tooltip> + </template> + </list-item> +</template> diff --git a/app/assets/javascripts/packages_and_registries/dependency_proxy/components/manifests_list.vue b/app/assets/javascripts/packages_and_registries/dependency_proxy/components/manifests_list.vue new file mode 100644 index 00000000000..f3ac017268b --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/dependency_proxy/components/manifests_list.vue @@ -0,0 +1,46 @@ +<script> +import { GlKeysetPagination } from '@gitlab/ui'; +import { s__ } from '~/locale'; +import ManifestRow from '~/packages_and_registries/dependency_proxy/components/manifest_row.vue'; + +export default { + name: 'ManifestsLists', + components: { + ManifestRow, + GlKeysetPagination, + }, + props: { + manifests: { + type: Array, + required: false, + default: () => [], + }, + pagination: { + type: Object, + required: true, + }, + }, + i18n: { + listTitle: s__('DependencyProxy|Manifest list'), + }, +}; +</script> + +<template> + <div class="gl-mt-5"> + <h3 class="gl-font-base">{{ $options.i18n.listTitle }}</h3> + <div + class="gl-border-t-1 gl-border-gray-100 gl-border-t-solid gl-display-flex gl-flex-direction-column" + > + <manifest-row v-for="(manifest, index) in manifests" :key="index" :manifest="manifest" /> + </div> + <div class="gl-display-flex gl-justify-content-center"> + <gl-keyset-pagination + v-bind="pagination" + class="gl-mt-3" + @prev="$emit('prev-page')" + @next="$emit('next-page')" + /> + </div> + </div> +</template> diff --git a/app/assets/javascripts/packages_and_registries/dependency_proxy/constants.js b/app/assets/javascripts/packages_and_registries/dependency_proxy/constants.js new file mode 100644 index 00000000000..3c6ede6fdce --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/dependency_proxy/constants.js @@ -0,0 +1 @@ +export const GRAPHQL_PAGE_SIZE = 20; diff --git a/app/assets/javascripts/packages_and_registries/dependency_proxy/graphql/queries/get_dependency_proxy_details.query.graphql b/app/assets/javascripts/packages_and_registries/dependency_proxy/graphql/queries/get_dependency_proxy_details.query.graphql index 9058d349bf3..63d5469c955 100644 --- a/app/assets/javascripts/packages_and_registries/dependency_proxy/graphql/queries/get_dependency_proxy_details.query.graphql +++ b/app/assets/javascripts/packages_and_registries/dependency_proxy/graphql/queries/get_dependency_proxy_details.query.graphql @@ -1,4 +1,12 @@ -query getDependencyProxyDetails($fullPath: ID!) { +#import "~/graphql_shared/fragments/pageInfo.fragment.graphql" + +query getDependencyProxyDetails( + $fullPath: ID! + $first: Int + $last: Int + $after: String + $before: String +) { group(fullPath: $fullPath) { dependencyProxyBlobCount dependencyProxyTotalSize @@ -6,5 +14,14 @@ query getDependencyProxyDetails($fullPath: ID!) { dependencyProxySetting { enabled } + dependencyProxyManifests(after: $after, before: $before, first: $first, last: $last) { + nodes { + createdAt + imageName + } + pageInfo { + ...PageInfo + } + } } } diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/project_setting_row.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/project_setting_row.vue index b7546a6bed7..cc92a8cd476 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/project_setting_row.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/project_setting_row.vue @@ -1,10 +1,5 @@ <script> -import { GlIcon } from '@gitlab/ui'; - export default { - components: { - GlIcon, - }, props: { label: { type: String, @@ -29,10 +24,14 @@ export default { <div class="project-feature-row"> <label v-if="label" class="label-bold"> {{ label }} - <a v-if="helpPath" :href="helpPath" target="_blank"> - <gl-icon name="question-o" /> - </a> </label> - <span v-if="helpText" class="form-text text-muted"> {{ helpText }} </span> <slot></slot> + <div> + <span v-if="helpText" class="text-muted"> {{ helpText }} </span> + <span v-if="helpPath" + ><a :href="helpPath" target="_blank">{{ __('Learn more') }}</a + >.</span + > + </div> + <slot></slot> </div> </template> diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 261f7af7ef1..8deb955842c 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -37,6 +37,10 @@ export default { securityAndComplianceLabel: s__('ProjectSettings|Security & Compliance'), snippetsLabel: s__('ProjectSettings|Snippets'), wikiLabel: s__('ProjectSettings|Wiki'), + pucWarningLabel: s__('ProjectSettings|Warn about Potentially Unwanted Characters'), + pucWarningHelpText: s__( + 'ProjectSettings|Highlight the usage of hidden unicode characters. These have innocent uses for right-to-left languages, but can also be used in potential exploits.', + ), }, components: { @@ -178,6 +182,7 @@ export default { securityAndComplianceAccessLevel: featureAccessLevel.PROJECT_MEMBERS, operationsAccessLevel: featureAccessLevel.EVERYONE, containerRegistryAccessLevel: featureAccessLevel.EVERYONE, + warnAboutPotentiallyUnwantedCharacters: true, lfsEnabled: true, requestAccessEnabled: true, highlightChangesClass: false, @@ -395,6 +400,9 @@ export default { ref="project-visibility-settings" :help-path="visibilityHelpPath" :label="s__('ProjectSettings|Project visibility')" + :help-text=" + s__('ProjectSettings|Manage who can see the project in the public access directory.') + " > <div class="project-feature-controls gl-display-flex gl-align-items-center gl-my-3 gl-mx-0"> <div class="select-wrapper gl-flex-grow-1"> @@ -752,5 +760,19 @@ export default { }}</template> </gl-form-checkbox> </project-setting-row> + <project-setting-row class="gl-mb-5"> + <input + :value="warnAboutPotentiallyUnwantedCharacters" + type="hidden" + name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]" + /> + <gl-form-checkbox + v-model="warnAboutPotentiallyUnwantedCharacters" + name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]" + > + {{ $options.i18n.pucWarningLabel }} + <template #help>{{ $options.i18n.pucWarningHelpText }}</template> + </gl-form-checkbox> + </project-setting-row> </div> </template> diff --git a/app/assets/javascripts/snippets/components/show.vue b/app/assets/javascripts/snippets/components/show.vue index 46629a569ec..35d88d5ec8e 100644 --- a/app/assets/javascripts/snippets/components/show.vue +++ b/app/assets/javascripts/snippets/components/show.vue @@ -66,7 +66,13 @@ export default { data-qa-selector="clone_button" /> </div> - <snippet-blob v-for="blob in blobs" :key="blob.path" :snippet="snippet" :blob="blob" /> + <snippet-blob + v-for="blob in blobs" + :key="blob.path" + :snippet="snippet" + :blob="blob" + class="project-highlight-puc" + /> </template> </div> </template> diff --git a/app/assets/stylesheets/framework/highlight.scss b/app/assets/stylesheets/framework/highlight.scss index b4a1d9f9977..122c605e603 100644 --- a/app/assets/stylesheets/framework/highlight.scss +++ b/app/assets/stylesheets/framework/highlight.scss @@ -85,3 +85,9 @@ td.line-numbers { line-height: 1; } + +.project-highlight-puc .unicode-bidi::before { + content: '�'; + cursor: pointer; + text-decoration: underline wavy $red-500; +} diff --git a/app/controllers/jira_connect/application_controller.rb b/app/controllers/jira_connect/application_controller.rb index ecb23c326fe..1d1575f3a05 100644 --- a/app/controllers/jira_connect/application_controller.rb +++ b/app/controllers/jira_connect/application_controller.rb @@ -76,6 +76,6 @@ class JiraConnect::ApplicationController < ApplicationController end def signed_install_active? - Feature.enabled?(:jira_connect_asymmetric_jwt) + Feature.enabled?(:jira_connect_asymmetric_jwt, default_enabled: :yaml) end end diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index b75effc52d1..3b9dde68ded 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -105,8 +105,7 @@ class Projects::BranchesController < Projects::ApplicationController # rubocop: enable CodeReuse/ActiveRecord def destroy - @branch_name = Addressable::URI.unescape(params[:id]) - result = ::Branches::DeleteService.new(project, current_user).execute(@branch_name) + result = ::Branches::DeleteService.new(project, current_user).execute(params[:id]) respond_to do |format| format.html do diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 8e833a2f8dc..4888f40febc 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -409,6 +409,7 @@ class ProjectsController < Projects::ApplicationController show_default_award_emojis squash_option mr_default_target_self + warn_about_potentially_unwanted_characters ] end diff --git a/app/graphql/mutations/issues/set_severity.rb b/app/graphql/mutations/issues/set_severity.rb index 778563ba053..872a0e7b33d 100644 --- a/app/graphql/mutations/issues/set_severity.rb +++ b/app/graphql/mutations/issues/set_severity.rb @@ -8,6 +8,8 @@ module Mutations argument :severity, Types::IssuableSeverityEnum, required: true, description: 'Set the incident severity level.' + authorize :admin_issue + def resolve(project_path:, iid:, severity:) issue = authorized_find!(project_path: project_path, iid: iid) project = issue.project diff --git a/app/helpers/learn_gitlab_helper.rb b/app/helpers/learn_gitlab_helper.rb index 4fb7a05a0e9..4f69d1bba0e 100644 --- a/app/helpers/learn_gitlab_helper.rb +++ b/app/helpers/learn_gitlab_helper.rb @@ -10,7 +10,14 @@ module LearnGitlabHelper def onboarding_actions_data(project) attributes = onboarding_progress(project).attributes.symbolize_keys - action_urls.to_h do |action, url| + urls_to_use = nil + + experiment(:change_continuous_onboarding_link_urls) do |e| + e.use { urls_to_use = action_urls } + e.try { urls_to_use = new_action_urls(project) } + end + + urls_to_use.to_h do |action, url| [ action, url: url, @@ -46,6 +53,17 @@ module LearnGitlabHelper .merge(LearnGitlab::Onboarding::ACTION_DOC_URLS) end + def new_action_urls(project) + action_urls.merge( + issue_created: project_issues_path(project), + git_write: project_path(project), + pipeline_created: project_pipelines_path(project), + merge_request_created: project_merge_requests_path(project), + user_added: project_members_url(project), + security_scan_enabled: project_security_configuration_path(project) + ) + end + def learn_gitlab_project @learn_gitlab_project ||= LearnGitlab::Project.new(current_user).project end @@ -54,3 +72,5 @@ module LearnGitlabHelper OnboardingProgress.find_by(namespace: project.namespace) # rubocop: disable CodeReuse/ActiveRecord end end + +LearnGitlabHelper.prepend_mod_with('LearnGitlabHelper') diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index a93fc188a3e..ef4bdfb46cf 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -376,6 +376,12 @@ module ProjectsHelper } end + def project_classes(project) + return "project-highlight-puc" if project.warn_about_potentially_unwanted_characters? + + "" + end + private def tab_ability_map @@ -532,6 +538,7 @@ module ProjectsHelper metricsDashboardAccessLevel: feature.metrics_dashboard_access_level, operationsAccessLevel: feature.operations_access_level, showDefaultAwardEmojis: project.show_default_award_emojis?, + warnAboutPotentiallyUnwantedCharacters: project.warn_about_potentially_unwanted_characters?, securityAndComplianceAccessLevel: project.security_and_compliance_access_level, containerRegistryAccessLevel: feature.container_registry_access_level } diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb index 56568c02d65..ca68989002c 100644 --- a/app/models/ci/build_metadata.rb +++ b/app/models/ci/build_metadata.rb @@ -23,6 +23,7 @@ module Ci serialize :config_options, Serializers::SymbolizedJson # rubocop:disable Cop/ActiveRecordSerialize serialize :config_variables, Serializers::SymbolizedJson # rubocop:disable Cop/ActiveRecordSerialize + serialize :runtime_runner_features, Serializers::SymbolizedJson # rubocop:disable Cop/ActiveRecordSerialize chronic_duration_attr_reader :timeout_human_readable, :timeout @@ -47,6 +48,14 @@ module Ci update(timeout: timeout.value, timeout_source: timeout.source) end + def set_cancel_gracefully + runtime_runner_features.merge!( { cancel_gracefully: true } ) + end + + def cancel_gracefully? + runtime_runner_features[:cancel_gracefully] == true + end + private def set_build_project diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index ec86746ae54..611b27c722b 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -20,6 +20,8 @@ module Ci delegate :interruptible, to: :metadata, prefix: false, allow_nil: true delegate :has_exposed_artifacts?, to: :metadata, prefix: false, allow_nil: true delegate :environment_auto_stop_in, to: :metadata, prefix: false, allow_nil: true + delegate :set_cancel_gracefully, to: :metadata, prefix: false, allow_nil: false + delegate :cancel_gracefully?, to: :metadata, prefix: false, allow_nil: false before_create :ensure_metadata end diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb index 3e1e5faee54..60e1dde17b9 100644 --- a/app/models/concerns/resolvable_discussion.rb +++ b/app/models/concerns/resolvable_discussion.rb @@ -81,8 +81,7 @@ module ResolvableDiscussion return false unless current_user return false unless resolvable? - current_user == self.noteable.try(:author) || - current_user.can?(:resolve_note, self.project) + current_user.can?(:resolve_note, self.noteable) end def resolve!(current_user) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 1dfc0168ede..4b4e7f5cc1a 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -34,6 +34,8 @@ class Namespace < ApplicationRecord SHARED_RUNNERS_SETTINGS = [SR_DISABLED_AND_UNOVERRIDABLE, SR_DISABLED_WITH_OVERRIDE, SR_ENABLED].freeze URL_MAX_LENGTH = 255 + PATH_TRAILING_VIOLATIONS = %w[.git .atom .].freeze + cache_markdown_field :description, pipeline: :description has_many :projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -203,9 +205,14 @@ class Namespace < ApplicationRecord # Remove everything that's not in the list of allowed characters. path.gsub!(/[^a-zA-Z0-9_\-\.]/, "") # Remove trailing violations ('.atom', '.git', or '.') - path.gsub!(/(\.atom|\.git|\.)*\z/, "") + loop do + orig = path + PATH_TRAILING_VIOLATIONS.each { |ext| path = path.chomp(ext) } + break if orig == path + end + # Remove leading violations ('-') - path.gsub!(/\A\-+/, "") + path.gsub!(/\A\-+/, "") # Users with the great usernames of "." or ".." would end up with a blank username. # Work around that by setting their username to "blank", followed by a counter. @@ -531,21 +538,23 @@ class Namespace < ApplicationRecord # Until we compare the inconsistency rates of the new specialized worker and # the old approach, we still run AuthorizedProjectsWorker # but with some delay and lower urgency as a safety net. - Group - .joins(project_group_links: :project) - .where(projects: { namespace_id: id }) - .distinct - .find_each do |group| - group.refresh_members_authorized_projects( - blocking: false, - priority: UserProjectAccessChangedService::LOW_PRIORITY - ) - end + enqueue_jobs_for_groups_requiring_authorizations_refresh(priority: UserProjectAccessChangedService::LOW_PRIORITY) else - Group - .joins(project_group_links: :project) - .where(projects: { namespace_id: id }) - .find_each(&:refresh_members_authorized_projects) + enqueue_jobs_for_groups_requiring_authorizations_refresh(priority: UserProjectAccessChangedService::HIGH_PRIORITY) + end + end + + def enqueue_jobs_for_groups_requiring_authorizations_refresh(priority:) + groups_requiring_authorizations_refresh = Group + .joins(project_group_links: :project) + .where(projects: { namespace_id: id }) + .distinct + + groups_requiring_authorizations_refresh.find_each do |group| + group.refresh_members_authorized_projects( + blocking: false, + priority: priority + ) end end diff --git a/app/models/project.rb b/app/models/project.rb index 490e0e15af4..031dbc3e9bb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -423,8 +423,8 @@ class Project < ApplicationRecord :container_registry_access_level, :container_registry_enabled?, to: :project_feature, allow_nil: true alias_method :container_registry_enabled, :container_registry_enabled? - delegate :show_default_award_emojis, :show_default_award_emojis=, - :show_default_award_emojis?, + delegate :show_default_award_emojis, :show_default_award_emojis=, :show_default_award_emojis?, + :warn_about_potentially_unwanted_characters, :warn_about_potentially_unwanted_characters=, :warn_about_potentially_unwanted_characters?, to: :project_setting, allow_nil: true delegate :scheduled?, :started?, :in_progress?, :failed?, :finished?, prefix: :import, to: :import_state, allow_nil: true @@ -2724,8 +2724,23 @@ class Project < ApplicationRecord self.errors.add(:base, _("Could not change HEAD: branch '%{branch}' does not exist") % { branch: branch }) end + def visible_group_links(for_user:) + user = for_user + links = project_group_links_with_preload + user.max_member_access_for_group_ids(links.map(&:group_id)) if user && links.any? + + DeclarativePolicy.user_scope do + links.select { Ability.allowed?(user, :read_group, _1.group) } + end + end + private + # overridden in EE + def project_group_links_with_preload + project_group_links + end + def save_topics return if @topic_list.nil? diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb index d704f4c2c87..8394ebe1df4 100644 --- a/app/models/project_group_link.rb +++ b/app/models/project_group_link.rb @@ -15,6 +15,7 @@ class ProjectGroupLink < ApplicationRecord validate :different_group scope :non_guests, -> { where('group_access > ?', Gitlab::Access::GUEST) } + scope :in_group, -> (group_ids) { where(group_id: group_ids) } alias_method :shared_with_group, :group diff --git a/app/models/uploads/fog.rb b/app/models/uploads/fog.rb index b44e273e9ab..5d57b644dbe 100644 --- a/app/models/uploads/fog.rb +++ b/app/models/uploads/fog.rb @@ -15,13 +15,21 @@ module Uploads end def delete_keys(keys) - keys.each do |key| - connection.delete_object(bucket_name, key) - end + keys.each { |key| delete_object(key) } end private + def delete_object(key) + connection.delete_object(bucket_name, key) + + # So far, only GoogleCloudStorage raises an exception when the file is not found. + # Other providers support idempotent requests and does not raise an error + # when the file is missing. + rescue ::Google::Apis::ClientError => e + Gitlab::ErrorTracking.log_exception(e) + end + def object_store Gitlab.config.uploads.object_store end diff --git a/app/models/user.rb b/app/models/user.rb index 02011de8103..83558044c21 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1434,7 +1434,7 @@ class User < ApplicationRecord name: name, username: username, avatar_url: avatar_url(only_path: false), - email: email + email: public_email.presence || _('[REDACTED]') } end diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb index 61263e47d7c..39ce26526e6 100644 --- a/app/policies/issuable_policy.rb +++ b/app/policies/issuable_policy.rb @@ -11,6 +11,8 @@ class IssuablePolicy < BasePolicy @user && @subject.assignee_or_author?(@user) end + condition(:is_author) { @subject&.author == @user } + rule { can?(:guest_access) & assignee_or_author }.policy do enable :read_issue enable :update_issue @@ -20,6 +22,10 @@ class IssuablePolicy < BasePolicy enable :reopen_merge_request end + rule { is_author }.policy do + enable :resolve_note + end + rule { locked & ~is_project_member }.policy do prevent :create_note prevent :admin_note diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 59aa47beff9..87573c9ad13 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -221,6 +221,7 @@ class ProjectPolicy < BasePolicy enable :set_note_created_at enable :set_emails_disabled enable :set_show_default_award_emojis + enable :set_warn_about_potentially_unwanted_characters end rule { can?(:guest_access) }.policy do diff --git a/app/services/concerns/update_visibility_level.rb b/app/services/concerns/update_visibility_level.rb index b7a161f5089..4cd14a2fb53 100644 --- a/app/services/concerns/update_visibility_level.rb +++ b/app/services/concerns/update_visibility_level.rb @@ -1,13 +1,17 @@ # frozen_string_literal: true module UpdateVisibilityLevel + # check that user is allowed to set specified visibility_level def valid_visibility_level_change?(target, new_visibility) - # check that user is allowed to set specified visibility_level - if new_visibility && new_visibility.to_i != target.visibility_level + return true unless new_visibility + + new_visibility_level = Gitlab::VisibilityLevel.level_value(new_visibility) + + if new_visibility_level != target.visibility_level_value unless can?(current_user, :change_visibility_level, target) && - Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) + Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility_level) - deny_visibility_level(target, new_visibility) + deny_visibility_level(target, new_visibility_level) return false end end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index f0632780838..7e572cbb79b 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -182,6 +182,14 @@ module Groups # schedule refreshing projects for all the members of the group @group.refresh_members_authorized_projects + + # When a group is transferred, it also affects who gets access to the projects shared to + # the subgroups within its hierarchy, so we also schedule jobs that refresh authorizations for all such shared projects. + project_group_shares_within_the_hierarchy = ProjectGroupLink.in_group(group.self_and_descendants.select(:id)) + + project_group_shares_within_the_hierarchy.find_each do |project_group_link| + AuthorizedProjectUpdate::ProjectRecalculateWorker.perform_async(project_group_link.project_id) + end end def raise_transfer_error(message) diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 1ad43b051be..2d6334251ad 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -15,7 +15,7 @@ module Groups return false end - return false unless valid_visibility_level_change?(group, params[:visibility_level]) + return false unless valid_visibility_level_change?(group, group.visibility_attribute_value(params)) return false unless valid_share_with_group_lock_change? @@ -77,7 +77,7 @@ module Groups end def after_update - if group.previous_changes.include?(:visibility_level) && group.private? + if group.previous_changes.include?(group.visibility_level_field) && group.private? # don't enqueue immediately to prevent todos removal in case of a mistake TodosDestroyer::GroupPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, group.id) end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 59e521853de..2daf098b94a 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -142,6 +142,7 @@ class IssuableBaseService < ::BaseProjectService def filter_severity(issuable) severity = params.delete(:severity) return unless severity && issuable.supports_severity? + return unless can_admin_issuable?(issuable) severity = IssuableSeverity::DEFAULT unless IssuableSeverity.severities.key?(severity) return if severity == issuable.severity diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index a32e80af4b1..b34ecf06e52 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -49,7 +49,7 @@ module Projects private def validate! - unless valid_visibility_level_change?(project, params[:visibility_level]) + unless valid_visibility_level_change?(project, project.visibility_attribute_value(params)) raise ValidationError, s_('UpdateProject|New visibility level not allowed!') end diff --git a/app/views/clusters/clusters/aws/_new.html.haml b/app/views/clusters/clusters/aws/_new.html.haml index f6d50410e9a..7142dd83dce 100644 --- a/app/views/clusters/clusters/aws/_new.html.haml +++ b/app/views/clusters/clusters/aws/_new.html.haml @@ -11,7 +11,7 @@ 'external-id' => @aws_role.role_external_id, 'role-arn' => @aws_role.role_arn, 'instance-types' => @instance_types, - 'kubernetes-integration-help-path' => help_page_path('user/project/clusters/index'), + 'kubernetes-integration-help-path' => help_page_path('user/infrastructure/clusters/index.md'), 'account-and-external-ids-help-path' => help_page_path('user/project/clusters/add_eks_clusters.md', anchor: 'how-to-create-a-new-cluster-on-eks-through-cluster-certificates-deprecated'), 'create-role-arn-help-path' => help_page_path('user/project/clusters/add_eks_clusters.md', anchor: 'how-to-create-a-new-cluster-on-eks-through-cluster-certificates-deprecated'), 'external-link-icon' => sprite_icon('external-link') } } diff --git a/app/views/clusters/clusters/gcp/_form.html.haml b/app/views/clusters/clusters/gcp/_form.html.haml index c274fb9c427..173456926a5 100644 --- a/app/views/clusters/clusters/gcp/_form.html.haml +++ b/app/views/clusters/clusters/gcp/_form.html.haml @@ -2,7 +2,7 @@ - zones_link_url = 'https://cloud.google.com/compute/docs/regions-zones/regions-zones' - machine_type_link_url = 'https://cloud.google.com/compute/docs/machine-types' - pricing_link_url = 'https://cloud.google.com/compute/pricing#machinetype' -- kubernetes_integration_url = help_page_path('user/project/clusters/index') +- kubernetes_integration_url = help_page_path('user/infrastructure/clusters/index.md') - help_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe - help_link_end = ' %{external_link_icon}</a>'.html_safe % { external_link_icon: external_link_icon } diff --git a/app/views/clusters/clusters/show.html.haml b/app/views/clusters/clusters/show.html.haml index 2a09d8d8cc0..e4c8f225ed2 100644 --- a/app/views/clusters/clusters/show.html.haml +++ b/app/views/clusters/clusters/show.html.haml @@ -13,7 +13,7 @@ cluster_status: @cluster.status_name, cluster_status_reason: @cluster.status_reason, provider_type: @cluster.provider_type, - help_path: help_page_path('user/project/clusters/index.md'), + help_path: help_page_path('user/infrastructure/clusters/index.md'), environments_help_path: help_page_path('ci/environments/index.md', anchor: 'create-a-static-environment'), clusters_help_path: help_page_path('user/project/clusters/deploy_to_cluster.md'), deploy_boards_help_path: help_page_path('user/project/deploy_boards.md', anchor: 'enabling-deploy-boards'), diff --git a/app/views/groups/dependency_proxies/show.html.haml b/app/views/groups/dependency_proxies/show.html.haml index 8936c4dcbb4..83ab97f8e4f 100644 --- a/app/views/groups/dependency_proxies/show.html.haml +++ b/app/views/groups/dependency_proxies/show.html.haml @@ -1,4 +1,5 @@ - page_title _("Dependency Proxy") +- @content_class = "limit-container-width" unless fluid_layout - dependency_proxy_available = Feature.enabled?(:dependency_proxy_for_private_groups, default_enabled: true) || @group.public? #js-dependency-proxy{ data: { group_path: @group.full_path, diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml index 2df502d2899..a54e0351d2f 100644 --- a/app/views/layouts/project.html.haml +++ b/app/views/layouts/project.html.haml @@ -6,6 +6,7 @@ - display_subscription_banner! - display_namespace_storage_limit_alert! - @left_sidebar = true +- @content_class = [@content_class, project_classes(@project)].compact.join(" ") - content_for :project_javascripts do - project = @target_project || @project diff --git a/config/feature_flags/development/jira_connect_asymmetric_jwt.yml b/config/feature_flags/development/jira_connect_asymmetric_jwt.yml index e204a7d6fac..48684e80d0c 100644 --- a/config/feature_flags/development/jira_connect_asymmetric_jwt.yml +++ b/config/feature_flags/development/jira_connect_asymmetric_jwt.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/342808 milestone: '14.4' type: development group: group::integrations -default_enabled: false +default_enabled: true diff --git a/config/feature_flags/experiment/change_continuous_onboarding_link_urls.yml b/config/feature_flags/experiment/change_continuous_onboarding_link_urls.yml new file mode 100644 index 00000000000..e65d7cd8d94 --- /dev/null +++ b/config/feature_flags/experiment/change_continuous_onboarding_link_urls.yml @@ -0,0 +1,8 @@ +--- +name: change_continuous_onboarding_link_urls +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71408 +rollout_issue_url: +milestone: '14.5' +type: experiment +group: group::conversion +default_enabled: false diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 3d2acce9a69..bb69c215f8d 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -176,8 +176,10 @@ production: &base ## Application settings cache expiry in seconds (default: 60) # application_settings_cache_seconds: 60 - ## Print initial root password to stdout during initialization (default: true) - # display_initial_root_password: true + ## Print initial root password to stdout during initialization (default: false) + # WARNING: setting this to true means that the root password will be printed in + # plaintext. This can be a security risk. + # display_initial_root_password: false ## Reply by email # Allow users to comment on issues and merge requests by replying to notification emails. diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index b871aa92c7f..5138d8df9a1 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -218,8 +218,7 @@ Settings.gitlab['no_todos_messages'] ||= YAML.load_file(Rails.root.join('config' Settings.gitlab['impersonation_enabled'] ||= true if Settings.gitlab['impersonation_enabled'].nil? Settings.gitlab['usage_ping_enabled'] = true if Settings.gitlab['usage_ping_enabled'].nil? Settings.gitlab['max_request_duration_seconds'] ||= 57 - -Settings.gitlab['display_initial_root_password'] = true if Settings.gitlab['display_initial_root_password'].nil? +Settings.gitlab['display_initial_root_password'] = false if Settings.gitlab['display_initial_root_password'].nil? Gitlab.ee do Settings.gitlab['mirror_max_delay'] ||= 300 diff --git a/db/fixtures/production/002_admin.rb b/db/fixtures/production/002_admin.rb index b6a6da3a188..b4710bc3e97 100644 --- a/db/fixtures/production/002_admin.rb +++ b/db/fixtures/production/002_admin.rb @@ -26,7 +26,7 @@ if user.persisted? if ::Settings.gitlab['display_initial_root_password'] puts "password: #{user_args[:password]}".color(:green) else - puts "password: *** - You opted not to display initial root password to STDOUT." + puts "password: ******".color(:green) end else puts "password: You'll be prompted to create one on your first visit.".color(:green) diff --git a/db/migrate/20210929144453_add_warn_about_potentially_unwanted_characters_to_project_settings.rb b/db/migrate/20210929144453_add_warn_about_potentially_unwanted_characters_to_project_settings.rb new file mode 100644 index 00000000000..166afa13371 --- /dev/null +++ b/db/migrate/20210929144453_add_warn_about_potentially_unwanted_characters_to_project_settings.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddWarnAboutPotentiallyUnwantedCharactersToProjectSettings < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def up + add_column :project_settings, :warn_about_potentially_unwanted_characters, :boolean, null: false, default: true + end + + def down + remove_column :project_settings, :warn_about_potentially_unwanted_characters + end +end diff --git a/db/schema_migrations/20210929144453 b/db/schema_migrations/20210929144453 new file mode 100644 index 00000000000..753ea50c272 --- /dev/null +++ b/db/schema_migrations/20210929144453 @@ -0,0 +1 @@ +0f808c27d19e6a38d4aa31f2dd820fe226681af84e05c4af47213409b2043e5a
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index bef331f94ab..c4986f57306 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18262,6 +18262,7 @@ CREATE TABLE project_settings ( cve_id_request_enabled boolean DEFAULT true NOT NULL, mr_default_target_self boolean DEFAULT false NOT NULL, previous_default_branch text, + warn_about_potentially_unwanted_characters boolean DEFAULT true NOT NULL, CONSTRAINT check_3a03e7557a CHECK ((char_length(previous_default_branch) <= 4096)), CONSTRAINT check_bde223416c CHECK ((show_default_award_emojis IS NOT NULL)) ); diff --git a/doc/development/jh_features_review.md b/doc/development/jh_features_review.md index cb0f8ddbd13..858048c1e7c 100644 --- a/doc/development/jh_features_review.md +++ b/doc/development/jh_features_review.md @@ -23,6 +23,14 @@ We have two kinds of changes related to JH: If needed, review the corresponding JH merge request located at [JH repository](https://gitlab.com/gitlab-jh/gitlab) +## When to merge files to the GitLab Inc. repository + +Files that are added to the `gitlab-jh` repository outside of `jh/` must be mirrored in the GitLab Inc. repository. + +If code that is added to the GitLab Inc. repository references (for example, `render_if_exists`) any `gitlab-jh` file that does not +exist in the GitLab Inc. codebase, add a comment with a link to the JiHu merge request or file. This is to prevent +the reference from being misidentified as a missing partial and subsequently deleted in the `gitlab` codebase. + ## Process overview See the [merge request process](https://about.gitlab.com/handbook/ceo/chief-of-staff-team/jihu-support/#merge-request-process) diff --git a/doc/user/infrastructure/clusters/connect/index.md b/doc/user/infrastructure/clusters/connect/index.md index 636cb1bb457..21387998a17 100644 --- a/doc/user/infrastructure/clusters/connect/index.md +++ b/doc/user/infrastructure/clusters/connect/index.md @@ -6,62 +6,14 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Connect a cluster to GitLab **(FREE)** -You can create new or connect existing clusters to GitLab through different [levels](#cluster-levels), -using different [methods](#methods-to-connect-a-cluster-to-gitlab). +The [certificate-based Kubernetes integration with GitLab](../index.md) +was [deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) +in GitLab 14.5. To connect your clusters, use the [GitLab Kubernetes Agent](../../../clusters/agent/index.md). -Before getting started: - -1. Check the [supported Kubernetes cluster versions](#supported-cluster-versions). -1. Define the [cluster level](#cluster-levels) according to your case. - -After that: - -1. Choose the [method](#methods-to-connect-a-cluster-to-gitlab) -to connect your cluster according to your case. -1. [View your clusters](#view-your-clusters) connected to GitLab. - -## Methods to connect a cluster to GitLab - -GitLab offers three methods to connect existing and create new clusters: - -- **GitLab Kubernetes Agent**: the best solution to -[connect existing clusters](#connect-existing-clusters-to-gitlab). -- **Infrastructure as Code**: it's a broader infrastructure management -toolset that includes managing your cluster. It's the recommended -solution to [create a new cluster](#create-new-clusters-from-gitlab) -from GitLab. -- **Certificate-based method**: our first and legacy solution uses -cluster certificates to connect your cluster to GitLab. It is no longer -recommended for [security implications](#security-implications-for-clusters-connected-with-certificates). - -### Connect existing clusters to GitLab - -To safely connect and configure an existing cluster on the **project level**, -we **recommend** using the [GitLab Kubernetes Agent](../../../clusters/agent/index.md). -We are working to support [the Agent for connecting a cluster at the group level](https://gitlab.com/groups/gitlab-org/-/epics/5784). - -Alternatively, you can use [cluster certificates](../../../project/clusters/add_existing_cluster.md) -to connect clusters in all levels (projects, group, instance). However, -for [security implications](#security-implications-for-clusters-connected-with-certificates), -we don't recommend using this method. - -### Create new clusters from GitLab - -To safely create new clusters from GitLab, use -[Infrastructure as Code](../../iac/index.md#create-a-new-cluster-through-iac). - -The [certificate-based method to create a new cluster](../../../project/clusters/add_remove_clusters.md) -is still available through the GitLab UI but was **deprecated** in GitLab 14.0. -If possible, we don't recommend using this method. - -### Connect multiple clusters to a single project - -To connect multiple clusters to a single project in GitLab, -we **recommend** using the [GitLab Kubernetes Agent](../../../clusters/agent/index.md). - -You can also use the [certificate-based method](../../../project/clusters/multiple_kubernetes_clusters.md), -but, for [security implications](#security-implications-for-clusters-connected-with-certificates), -we don't recommend using this method. +<!-- TBA: (We need to resolve https://gitlab.com/gitlab-org/gitlab/-/issues/343660 before adding this line) +If you don't have a cluster yet, create one and connect it to GitLab through the Agent. +You can also create a new cluster from GitLab using [Infrastructure as Code](../../iac/index.md#create-a-new-cluster-through-iac). +--> ## Supported cluster versions @@ -85,7 +37,13 @@ Kubernetes version to any supported version at any time: Some GitLab features may support versions outside the range provided here. -## Cluster levels +## Cluster levels (DEPRECATED) + +> [Deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5. + +WARNING: +The [concept of cluster levels was deprecated](../index.md#cluster-levels) +in GitLab 14.5. Choose your cluster's level according to its purpose: @@ -118,6 +76,8 @@ your cluster's level. ## Security implications for clusters connected with certificates +> Connecting clusters to GitLab through cluster certificates was [deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5. + WARNING: The whole cluster security is based on a model where [developers](../../../permissions.md) are trusted, so **only trusted users should be allowed to control your clusters**. diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 6c6ae68b921..c16c6446acd 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -14,7 +14,7 @@ This feature was [deprecated](https://gitlab.com/groups/gitlab-org/configure/-/e in GitLab 14.5. To connect clusters to GitLab, use the [GitLab Kubernetes Agent](../../clusters/agent/index.md). -[Project-level Kubernetes clusters](../../infrastructure/clusters/index.md#cluster-levels) +[Project-level](../../infrastructure/clusters/connect/index.md#cluster-levels-deprecated) Kubernetes clusters allow you to connect a Kubernetes cluster to a project in GitLab. You can also [connect multiple clusters](multiple_kubernetes_clusters.md) diff --git a/lib/api/applications.rb b/lib/api/applications.rb index be482272b20..346bd6ccfe4 100644 --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -15,7 +15,7 @@ module API params do requires :name, type: String, desc: 'Application name' requires :redirect_uri, type: String, desc: 'Application redirect URI' - requires :scopes, type: String, desc: 'Application scopes' + requires :scopes, type: String, desc: 'Application scopes', allow_blank: false optional :confidential, type: Boolean, default: true, desc: 'Application will be used where the client secret is confidential' diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index df0c1d7a4c5..41320d184f9 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -100,7 +100,9 @@ module API expose :build_coverage_regex expose :ci_config_path, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) } expose :shared_with_groups do |project, options| - SharedGroupWithProject.represent(project.project_group_links, options) + user = options[:current_user] + + SharedGroupWithProject.represent(project.visible_group_links(for_user: user), options) end expose :only_allow_merge_if_pipeline_succeeds expose :allow_merge_on_skipped_pipeline diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index f66d689a4b9..d87680ab43e 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -39,6 +39,7 @@ module API optional :emails_disabled, type: Boolean, desc: 'Disable email notifications' optional :show_default_award_emojis, type: Boolean, desc: 'Show default award emojis' + optional :warn_about_potentially_unwanted_characters, type: Boolean, desc: 'Warn about Potentially Unwanted Characters' optional :shared_runners_enabled, type: Boolean, desc: 'Flag indication if shared runners are enabled for that project' optional :resolve_outdated_diff_discussions, type: Boolean, desc: 'Automatically resolve merge request diffs discussions on lines changed with a push' optional :remove_source_branch_after_merge, type: Boolean, desc: 'Remove the source branch by default after merge' diff --git a/lib/api/project_import.rb b/lib/api/project_import.rb index d43184ff75d..e7c532e2483 100644 --- a/lib/api/project_import.rb +++ b/lib/api/project_import.rb @@ -9,6 +9,8 @@ module API feature_category :importers + before { authenticate! unless route.settings[:skip_authentication] } + helpers do def import_params declared_params(include_missing: false) @@ -109,6 +111,7 @@ module API detail 'This feature was introduced in GitLab 10.6.' success Entities::ProjectImportStatus end + route_setting :skip_authentication, true get ':id/import' do present user_project, with: Entities::ProjectImportStatus end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index baa9db2b72c..57b67553fda 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -433,7 +433,7 @@ module API authorize_admin_project attrs = declared_params(include_missing: false) authorize! :rename_project, user_project if attrs[:name].present? - authorize! :change_visibility_level, user_project if attrs[:visibility].present? + authorize! :change_visibility_level, user_project if user_project.visibility_attribute_present?(attrs) attrs = translate_params_for_compatibility(attrs) filter_attributes_using_license!(attrs) diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb index fbbd6135959..b16af78841a 100644 --- a/lib/banzai/renderer.rb +++ b/lib/banzai/renderer.rb @@ -160,16 +160,40 @@ module Banzai def self.cacheless_render(text, context = {}) return text.to_s unless text.present? - Gitlab::Metrics.measure(:banzai_cacheless_render) do - result = render_result(text, context) + real_start = Gitlab::Metrics::System.monotonic_time + cpu_start = Gitlab::Metrics::System.cpu_time - output = result[:output] - if output.respond_to?(:to_html) - output.to_html - else - output.to_s - end - end + result = render_result(text, context) + + output = result[:output] + rendered = if output.respond_to?(:to_html) + output.to_html + else + output.to_s + end + + cpu_duration_histogram.observe({}, Gitlab::Metrics::System.cpu_time - cpu_start) + real_duration_histogram.observe({}, Gitlab::Metrics::System.monotonic_time - real_start) + + rendered + end + + def self.real_duration_histogram + Gitlab::Metrics.histogram( + :gitlab_banzai_cacheless_render_real_duration_seconds, + 'Duration of Banzai pipeline rendering in real time', + {}, + [0.01, 0.01, 0.05, 0.1, 0.5, 1, 2, 5, 10.0, 50, 100] + ) + end + + def self.cpu_duration_histogram + Gitlab::Metrics.histogram( + :gitlab_banzai_cacheless_render_cpu_duration_seconds, + 'Duration of Banzai pipeline rendering in cpu time', + {}, + Gitlab::Metrics::EXECUTION_MEASUREMENT_BUCKETS + ) end def self.full_cache_key(cache_key, pipeline_name) diff --git a/lib/bulk_imports/projects/pipelines/merge_requests_pipeline.rb b/lib/bulk_imports/projects/pipelines/merge_requests_pipeline.rb new file mode 100644 index 00000000000..264bda6e654 --- /dev/null +++ b/lib/bulk_imports/projects/pipelines/merge_requests_pipeline.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module BulkImports + module Projects + module Pipelines + class MergeRequestsPipeline + include NdjsonPipeline + + relation_name 'merge_requests' + + extractor ::BulkImports::Common::Extractors::NdjsonExtractor, relation: relation + + def after_run(_) + context.portable.merge_requests.set_latest_merge_request_diff_ids! + end + end + end + end +end diff --git a/lib/bulk_imports/projects/pipelines/repository_pipeline.rb b/lib/bulk_imports/projects/pipelines/repository_pipeline.rb index 86e696f87a4..6bbd4d0688b 100644 --- a/lib/bulk_imports/projects/pipelines/repository_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/repository_pipeline.rb @@ -17,10 +17,18 @@ module BulkImports def load(context, data) url = data['httpUrlToRepo'] url = url.sub("://", "://oauth2:#{context.configuration.access_token}@") + project = context.portable Gitlab::UrlBlocker.validate!(url, allow_local_network: allow_local_requests?, allow_localhost: allow_local_requests?) - context.portable.repository.import_repository(url) + project.ensure_repository + project.repository.fetch_as_mirror(url) + end + + # The initial fetch can bring in lots of loose refs and objects. + # Running a `git gc` will make importing merge requests faster. + def after_run(_) + ::Repositories::HousekeepingService.new(context.portable, :gc).execute end private diff --git a/lib/bulk_imports/projects/stage.rb b/lib/bulk_imports/projects/stage.rb index 380ae8ef504..83f7fddb81d 100644 --- a/lib/bulk_imports/projects/stage.rb +++ b/lib/bulk_imports/projects/stage.rb @@ -27,6 +27,10 @@ module BulkImports pipeline: BulkImports::Common::Pipelines::BoardsPipeline, stage: 4 }, + merge_requests: { + pipeline: BulkImports::Projects::Pipelines::MergeRequestsPipeline, + stage: 4 + }, uploads: { pipeline: BulkImports::Common::Pipelines::UploadsPipeline, stage: 5 diff --git a/lib/gitlab/background_migration/user_mentions/models/group.rb b/lib/gitlab/background_migration/user_mentions/models/group.rb index a8b4b59b06c..310723570c2 100644 --- a/lib/gitlab/background_migration/user_mentions/models/group.rb +++ b/lib/gitlab/background_migration/user_mentions/models/group.rb @@ -11,6 +11,10 @@ module Gitlab has_one :saml_provider + def root_saml_provider + root_ancestor.saml_provider + end + def self.declarative_policy_class "GroupPolicy" end diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index ebccb9678a6..71a8d747963 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -368,9 +368,11 @@ excluded_attributes: - :marked_for_deletion_by_user_id - :compliance_framework_setting - :show_default_award_emojis + - :warn_about_potentially_unwanted_characters - :services - :exported_protected_branches - :repository_size_limit + - :external_webhook_token namespaces: - :runners_token - :runners_token_encrypted @@ -583,6 +585,8 @@ excluded_attributes: system_note_metadata: - :description_version_id - :note_id + pipeline_schedules: + - :active methods: notes: - :type diff --git a/lib/gitlab/import_export/project/relation_factory.rb b/lib/gitlab/import_export/project/relation_factory.rb index 102fcedd2fc..888a5a10f2c 100644 --- a/lib/gitlab/import_export/project/relation_factory.rb +++ b/lib/gitlab/import_export/project/relation_factory.rb @@ -84,6 +84,7 @@ module Gitlab when :'Ci::Pipeline' then setup_pipeline when *BUILD_MODELS then setup_build when :issues then setup_issue + when :'Ci::PipelineSchedule' then setup_pipeline_schedule end update_project_references @@ -143,6 +144,10 @@ module Gitlab @relation_hash['relative_position'] = compute_relative_position end + def setup_pipeline_schedule + @relation_hash['active'] = false + end + def compute_relative_position return unless max_relative_position diff --git a/lib/gitlab/unicode.rb b/lib/gitlab/unicode.rb new file mode 100644 index 00000000000..b49c5647dab --- /dev/null +++ b/lib/gitlab/unicode.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + class Unicode + # Regular expression for identifying bidirectional control + # characters in UTF-8 strings + # + # Documentation on how this works: + # https://idiosyncratic-ruby.com/41-proper-unicoding.html + BIDI_REGEXP = /\p{Bidi Control}/.freeze + + class << self + # Warning message used to highlight bidi characters in the GUI + def bidi_warning + _("Potentially unwanted character detected: Unicode BiDi Control") + end + end + end +end diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index 64029d4d3fe..d378e558b8a 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -155,6 +155,14 @@ module Gitlab false end + def visibility_attribute_value(attributes) + visibility_level_attributes.each do |attr| + return attributes[attr] if attributes.has_key?(attr) + end + + nil + end + def visibility_level_attributes [visibility_level_field, visibility_level_field.to_s, :visibility, 'visibility'] diff --git a/lib/rouge/formatters/html_gitlab.rb b/lib/rouge/formatters/html_gitlab.rb index e0e9677fac7..9e76225fc54 100644 --- a/lib/rouge/formatters/html_gitlab.rb +++ b/lib/rouge/formatters/html_gitlab.rb @@ -21,12 +21,24 @@ module Rouge is_first = false yield %(<span id="LC#{@line_number}" class="line" lang="#{@tag}">) - line.each { |token, value| yield span(token, value.chomp! || value) } + + line.each do |token, value| + yield highlight_unicode_control_characters(span(token, value.chomp! || value)) + end + yield %(</span>) @line_number += 1 end end + + private + + def highlight_unicode_control_characters(text) + text.gsub(Gitlab::Unicode::BIDI_REGEXP) do |char| + %(<span class="unicode-bidi has-tooltip" data-toggle="tooltip" title="#{Gitlab::Unicode.bidi_warning}">#{char}</span>) + end + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b4701f0d3a9..3c6c84f763f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9178,9 +9178,6 @@ msgstr "" msgid "ContainerRegistry|You can add an image to this registry with the following commands:" msgstr "" -msgid "Contains %{count} blobs of images (%{size})" -msgstr "" - msgid "Content parsed with %{link}." msgstr "" @@ -9370,9 +9367,6 @@ msgstr "" msgid "Copy link to chart" msgstr "" -msgid "Copy prefix" -msgstr "" - msgid "Copy reference" msgstr "" @@ -11230,16 +11224,16 @@ msgstr "" msgid "Dependency Proxy" msgstr "" -msgid "Dependency Proxy disabled. To enable it, contact the group owner." +msgid "Dependency Scanning" msgstr "" -msgid "Dependency Proxy feature is limited to public groups for now." +msgid "DependencyProxy|Cached %{time}" msgstr "" -msgid "Dependency Proxy image prefix" +msgid "DependencyProxy|Contains %{count} blobs of images (%{size})" msgstr "" -msgid "Dependency Scanning" +msgid "DependencyProxy|Copy prefix" msgstr "" msgid "DependencyProxy|Create a local proxy for storing frequently used upstream images. %{docLinkStart}Learn more%{docLinkEnd} about dependency proxies." @@ -11248,9 +11242,21 @@ msgstr "" msgid "DependencyProxy|Dependency Proxy" msgstr "" +msgid "DependencyProxy|Dependency Proxy disabled. To enable it, contact the group owner." +msgstr "" + +msgid "DependencyProxy|Dependency Proxy feature is limited to public groups for now." +msgstr "" + +msgid "DependencyProxy|Dependency Proxy image prefix" +msgstr "" + msgid "DependencyProxy|Enable Proxy" msgstr "" +msgid "DependencyProxy|Manifest list" +msgstr "" + msgid "Depends on %d merge request being merged" msgid_plural "Depends on %d merge requests being merged" msgstr[0] "" @@ -25828,6 +25834,9 @@ msgstr "" msgid "Postman collection file path or URL" msgstr "" +msgid "Potentially unwanted character detected: Unicode BiDi Control" +msgstr "" + msgid "Pre-defined push rules." msgstr "" @@ -26953,6 +26962,9 @@ msgstr "" msgid "ProjectSettings|Global" msgstr "" +msgid "ProjectSettings|Highlight the usage of hidden unicode characters. These have innocent uses for right-to-left languages, but can also be used in potential exploits." +msgstr "" + msgid "ProjectSettings|Internal" msgstr "" @@ -26965,6 +26977,9 @@ msgstr "" msgid "ProjectSettings|LFS objects from this repository are available to forks. %{linkStart}How do I remove them?%{linkEnd}" msgstr "" +msgid "ProjectSettings|Manage who can see the project in the public access directory." +msgstr "" + msgid "ProjectSettings|Manages large files such as audio, video, and graphics files." msgstr "" @@ -27142,6 +27157,9 @@ msgstr "" msgid "ProjectSettings|Visualize the project's performance metrics." msgstr "" +msgid "ProjectSettings|Warn about Potentially Unwanted Characters" +msgstr "" + msgid "ProjectSettings|What are badges?" msgstr "" @@ -39836,6 +39854,9 @@ msgstr "" msgid "[No reason]" msgstr "" +msgid "[REDACTED]" +msgstr "" + msgid "[Redacted]" msgstr "" diff --git a/qa/qa/specs/features/browser_ui/5_package/npm_registry_spec.rb b/qa/qa/specs/features/browser_ui/5_package/npm_registry_spec.rb index 5a3b4388f0c..f87b324fe81 100644 --- a/qa/qa/specs/features/browser_ui/5_package/npm_registry_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/npm_registry_spec.rb @@ -159,7 +159,7 @@ module QA } end - it "push and pull a npm package via CI using a #{params[:token_name]}", testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1772' do + it "push and pull a npm package via CI using a #{params[:token_name]}" do Resource::Repository::Commit.fabricate_via_api! do |commit| commit.project = project commit.commit_message = 'Add .gitlab-ci.yml' diff --git a/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb index 8a6752ed817..0b4825715c1 100644 --- a/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb @@ -85,7 +85,7 @@ module QA end end - it "publishes a nuget package at the project level, installs and deletes it at the group level using a #{params[:token_name]}", testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1073' do + it "publishes a nuget package at the project level, installs and deletes it at the group level using a #{params[:token_name]}" do Flow::Login.sign_in Resource::Repository::Commit.fabricate_via_api! do |commit| diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 43e8bbd83cf..d9dedb04b0d 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -356,7 +356,7 @@ RSpec.describe Projects::BranchesController do context "valid branch name with encoded slashes" do let(:branch) { "improve%2Fawesome" } - it { expect(response).to have_gitlab_http_status(:ok) } + it { expect(response).to have_gitlab_http_status(:not_found) } it { expect(response.body).to be_blank } end @@ -396,10 +396,10 @@ RSpec.describe Projects::BranchesController do let(:branch) { 'improve%2Fawesome' } it 'returns JSON response with message' do - expect(json_response).to eql('message' => 'Branch was deleted') + expect(json_response).to eql('message' => 'No such branch') end - it { expect(response).to have_gitlab_http_status(:ok) } + it { expect(response).to have_gitlab_http_status(:not_found) } end context 'invalid branch name, valid ref' do diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 3d966848c5b..b34cfedb767 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -323,6 +323,34 @@ RSpec.describe ProjectsController do expect(response).to render_template('_files') expect(response.body).to have_content('LICENSE') # would be 'MIT license' if stub not works end + + describe "PUC highlighting" do + render_views + + before do + expect(controller).to receive(:find_routable!).and_return(public_project) + end + + context "option is enabled" do + it "adds the highlighting class" do + expect(public_project).to receive(:warn_about_potentially_unwanted_characters?).and_return(true) + + get_show + + expect(response.body).to have_css(".project-highlight-puc") + end + end + + context "option is disabled" do + it "doesn't add the highlighting class" do + expect(public_project).to receive(:warn_about_potentially_unwanted_characters?).and_return(false) + + get_show + + expect(response.body).not_to have_css(".project-highlight-puc") + end + end + end end context "when the url contains .atom" do diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 325f62f6028..8aa9654956e 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -15,6 +15,10 @@ FactoryBot.define do admin { true } end + trait :public_email do + public_email { email } + end + trait :blocked do after(:build) { |user, _| user.block! } end diff --git a/spec/frontend/lib/dompurify_spec.js b/spec/frontend/lib/dompurify_spec.js index 324441fa2c9..47a94a4dcde 100644 --- a/spec/frontend/lib/dompurify_spec.js +++ b/spec/frontend/lib/dompurify_spec.js @@ -22,12 +22,16 @@ const safeUrls = { const unsafeUrls = [ '/an/evil/url', '../../../evil/url', - 'https://evil.url/assets/icons-123a.svg', + 'https://evil.url/assets/icons-123a.svg#test', 'https://evil.url/assets/icons-456b.svg', `https://evil.url/${rootGon.sprite_icons}`, `https://evil.url/${rootGon.sprite_file_icons}`, `https://evil.url/${absoluteGon.sprite_icons}`, `https://evil.url/${absoluteGon.sprite_file_icons}`, + `${rootGon.sprite_icons}/../evil/path`, + `${rootGon.sprite_file_icons}/../../evil/path`, + `${absoluteGon.sprite_icons}/../evil/path`, + `${absoluteGon.sprite_file_icons}/../../https://evil.url`, ]; const forbiddenDataAttrs = ['data-remote', 'data-url', 'data-type', 'data-method']; diff --git a/spec/frontend/lib/utils/url_utility_spec.js b/spec/frontend/lib/utils/url_utility_spec.js index 18b68d91e01..36e1a453ef4 100644 --- a/spec/frontend/lib/utils/url_utility_spec.js +++ b/spec/frontend/lib/utils/url_utility_spec.js @@ -607,6 +607,27 @@ describe('URL utility', () => { }); }); + describe('getNormalizedURL', () => { + it.each` + url | base | result + ${'./foo'} | ${''} | ${'http://test.host/foo'} + ${'../john.md'} | ${''} | ${'http://test.host/john.md'} + ${'/images/img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/images/img.png'} + ${'/images/../img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/img.png'} + ${'/images/./img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/images/img.png'} + ${'./images/img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/user/images/img.png'} + ${'../images/../img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/img.png'} + ${'/images/img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/images/img.png'} + ${'/images/../img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/img.png'} + ${'/images/./img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/images/img.png'} + `( + 'converts url "$url" with base "$base" to normalized url => "expected"', + ({ url, base, result }) => { + expect(urlUtils.getNormalizedURL(url, base)).toBe(result); + }, + ); + }); + describe('getWebSocketProtocol', () => { it.each` protocol | expectation diff --git a/spec/frontend/packages_and_registries/dependency_proxy/components/manifest_list_spec.js b/spec/frontend/packages_and_registries/dependency_proxy/components/manifest_list_spec.js new file mode 100644 index 00000000000..2c3b4e96098 --- /dev/null +++ b/spec/frontend/packages_and_registries/dependency_proxy/components/manifest_list_spec.js @@ -0,0 +1,70 @@ +import { GlKeysetPagination } from '@gitlab/ui'; +import { stripTypenames } from 'helpers/graphql_helpers'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import ManifestRow from '~/packages_and_registries/dependency_proxy/components/manifest_row.vue'; + +import Component from '~/packages_and_registries/dependency_proxy/components/manifests_list.vue'; +import { + proxyManifests, + pagination, +} from 'jest/packages_and_registries/dependency_proxy/mock_data'; + +describe('Manifests List', () => { + let wrapper; + + const defaultProps = { + manifests: proxyManifests(), + pagination: stripTypenames(pagination()), + }; + + const createComponent = (propsData = defaultProps) => { + wrapper = shallowMountExtended(Component, { + propsData, + }); + }; + + const findRows = () => wrapper.findAllComponents(ManifestRow); + const findPagination = () => wrapper.findComponent(GlKeysetPagination); + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('has the correct title', () => { + expect(wrapper.text()).toContain(Component.i18n.listTitle); + }); + + it('shows a row for every manifest', () => { + expect(findRows().length).toBe(defaultProps.manifests.length); + }); + + it('binds a manifest to each row', () => { + expect(findRows().at(0).props()).toMatchObject({ + manifest: defaultProps.manifests[0], + }); + }); + + describe('pagination', () => { + it('has the correct props', () => { + expect(findPagination().props()).toMatchObject({ + ...defaultProps.pagination, + }); + }); + + it('emits the next-page event', () => { + findPagination().vm.$emit('next'); + + expect(wrapper.emitted('next-page')).toEqual([[]]); + }); + + it('emits the prev-page event', () => { + findPagination().vm.$emit('prev'); + + expect(wrapper.emitted('prev-page')).toEqual([[]]); + }); + }); +}); diff --git a/spec/frontend/packages_and_registries/dependency_proxy/components/manifest_row_spec.js b/spec/frontend/packages_and_registries/dependency_proxy/components/manifest_row_spec.js new file mode 100644 index 00000000000..b7cbd875497 --- /dev/null +++ b/spec/frontend/packages_and_registries/dependency_proxy/components/manifest_row_spec.js @@ -0,0 +1,59 @@ +import { GlSprintf } from '@gitlab/ui'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import ListItem from '~/vue_shared/components/registry/list_item.vue'; +import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import Component from '~/packages_and_registries/dependency_proxy/components/manifest_row.vue'; +import { proxyManifests } from 'jest/packages_and_registries/dependency_proxy/mock_data'; + +describe('Manifest Row', () => { + let wrapper; + + const defaultProps = { + manifest: proxyManifests()[0], + }; + + const createComponent = (propsData = defaultProps) => { + wrapper = shallowMountExtended(Component, { + propsData, + stubs: { + GlSprintf, + TimeagoTooltip, + ListItem, + }, + }); + }; + + const findListItem = () => wrapper.findComponent(ListItem); + const findCachedMessages = () => wrapper.findByTestId('cached-message'); + const findTimeAgoTooltip = () => wrapper.findComponent(TimeagoTooltip); + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('has a list item', () => { + expect(findListItem().exists()).toBe(true); + }); + + it('displays the name', () => { + expect(wrapper.text()).toContain('alpine'); + }); + + it('displays the version', () => { + expect(wrapper.text()).toContain('latest'); + }); + + it('displays the cached time', () => { + expect(findCachedMessages().text()).toContain('Cached'); + }); + + it('has a time ago tooltip component', () => { + expect(findTimeAgoTooltip().props()).toMatchObject({ + time: defaultProps.manifest.createdAt, + }); + }); +}); diff --git a/spec/frontend/packages_and_registries/dependency_proxy/mock_data.js b/spec/frontend/packages_and_registries/dependency_proxy/mock_data.js index 23d42e109f9..119ec995462 100644 --- a/spec/frontend/packages_and_registries/dependency_proxy/mock_data.js +++ b/spec/frontend/packages_and_registries/dependency_proxy/mock_data.js @@ -7,6 +7,20 @@ export const proxyData = () => ({ export const proxySettings = (extend = {}) => ({ enabled: true, ...extend }); +export const proxyManifests = () => [ + { createdAt: '2021-09-22T09:45:28Z', imageName: 'alpine:latest' }, + { createdAt: '2021-09-21T09:45:28Z', imageName: 'alpine:stable' }, +]; + +export const pagination = (extend) => ({ + endCursor: 'eyJpZCI6IjIwNSIsIm5hbWUiOiJteS9jb21wYW55L2FwcC9teS1hcHAifQ', + hasNextPage: true, + hasPreviousPage: true, + startCursor: 'eyJpZCI6IjI0NyIsIm5hbWUiOiJ2ZXJzaW9uX3Rlc3QxIn0', + __typename: 'PageInfo', + ...extend, +}); + export const proxyDetailsQuery = ({ extendSettings = {} } = {}) => ({ data: { group: { @@ -16,6 +30,10 @@ export const proxyDetailsQuery = ({ extendSettings = {} } = {}) => ({ ...proxySettings(extendSettings), __typename: 'DependencyProxySetting', }, + dependencyProxyManifests: { + nodes: proxyManifests(), + pageInfo: pagination(), + }, }, }, }); diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js index 1e562419f32..0020269e4e7 100644 --- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js +++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js @@ -27,6 +27,7 @@ const defaultProps = { emailsDisabled: false, packagesEnabled: true, showDefaultAwardEmojis: true, + warnAboutPotentiallyUnwantedCharacters: true, }, isGitlabCom: true, canDisableEmails: true, @@ -97,6 +98,10 @@ describe('Settings Panel', () => { const findEmailSettings = () => wrapper.find({ ref: 'email-settings' }); const findShowDefaultAwardEmojis = () => wrapper.find('input[name="project[project_setting_attributes][show_default_award_emojis]"]'); + const findWarnAboutPuc = () => + wrapper.find( + 'input[name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"]', + ); const findMetricsVisibilitySettings = () => wrapper.find({ ref: 'metrics-visibility-settings' }); const findOperationsSettings = () => wrapper.find({ ref: 'operations-settings' }); @@ -539,6 +544,14 @@ describe('Settings Panel', () => { }); }); + describe('Warn about potentially unwanted characters', () => { + it('should have a "Warn about Potentially Unwanted Characters" input', () => { + wrapper = mountComponent(); + + expect(findWarnAboutPuc().exists()).toBe(true); + }); + }); + describe('Metrics dashboard', () => { it('should show the metrics dashboard access toggle', () => { wrapper = mountComponent(); diff --git a/spec/graphql/mutations/discussions/toggle_resolve_spec.rb b/spec/graphql/mutations/discussions/toggle_resolve_spec.rb index 1ec38e3bc0a..2041b86d6e7 100644 --- a/spec/graphql/mutations/discussions/toggle_resolve_spec.rb +++ b/spec/graphql/mutations/discussions/toggle_resolve_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Mutations::Discussions::ToggleResolve do described_class.new(object: nil, context: { current_user: user }, field: nil) end + let_it_be(:author) { create(:user) } let_it_be(:project) { create(:project, :repository) } describe '#resolve' do @@ -136,20 +137,37 @@ RSpec.describe Mutations::Discussions::ToggleResolve do end end end + + context 'when user is the author and discussion is locked' do + let(:user) { author } + + before do + issuable.update!(discussion_locked: true) + end + + it 'raises an error' do + expect { mutation.resolve(id: id_arg, resolve: resolve_arg) }.to raise_error( + Gitlab::Graphql::Errors::ResourceNotAvailable, + "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + ) + end + end end context 'when discussion is on a merge request' do - let_it_be(:noteable) { create(:merge_request, source_project: project) } + let_it_be(:noteable) { create(:merge_request, source_project: project, author: author) } let(:discussion) { create(:diff_note_on_merge_request, noteable: noteable, project: project).to_discussion } + let(:issuable) { noteable } it_behaves_like 'a working resolve method' end context 'when discussion is on a design' do - let_it_be(:noteable) { create(:design, :with_file, issue: create(:issue, project: project)) } + let_it_be(:noteable) { create(:design, :with_file, issue: create(:issue, project: project, author: author)) } let(:discussion) { create(:diff_note_on_design, noteable: noteable, project: project).to_discussion } + let(:issuable) { noteable.issue } it_behaves_like 'a working resolve method' end diff --git a/spec/graphql/mutations/issues/set_severity_spec.rb b/spec/graphql/mutations/issues/set_severity_spec.rb index 7ce9c7f6621..84736fe7ee6 100644 --- a/spec/graphql/mutations/issues/set_severity_spec.rb +++ b/spec/graphql/mutations/issues/set_severity_spec.rb @@ -3,26 +3,58 @@ require 'spec_helper' RSpec.describe Mutations::Issues::SetSeverity do - let_it_be(:user) { create(:user) } - let_it_be(:issue) { create(:incident) } + let_it_be(:project) { create(:project) } + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:issue) { create(:incident, project: project) } let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } - specify { expect(described_class).to require_graphql_authorizations(:update_issue) } + specify { expect(described_class).to require_graphql_authorizations(:update_issue, :admin_issue) } + + before_all do + project.add_guest(guest) + project.add_reporter(reporter) + end describe '#resolve' do let(:severity) { 'critical' } - let(:mutated_incident) { subject[:issue] } - subject(:resolve) { mutation.resolve(project_path: issue.project.full_path, iid: issue.iid, severity: severity) } + subject(:resolve) do + mutation.resolve( + project_path: issue.project.full_path, + iid: issue.iid, + severity: severity + ) + end - it_behaves_like 'permission level for issue mutation is correctly verified' + context 'as guest' do + let(:user) { guest } - context 'when the user can update the issue' do - before do - issue.project.add_developer(user) + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + + context 'and also author' do + let!(:issue) { create(:incident, project: project, author: user) } + + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end end + context 'and also assignee' do + let!(:issue) { create(:incident, project: project, assignee_ids: [user.id]) } + + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + end + + context 'as reporter' do + let(:user) { reporter } + context 'when issue type is incident' do context 'when severity has a correct value' do it 'updates severity' do @@ -48,9 +80,9 @@ RSpec.describe Mutations::Issues::SetSeverity do end context 'when issue type is not incident' do - let!(:issue) { create(:issue) } + let!(:issue) { create(:issue, project: project) } - it 'does not updates the issue' do + it 'does not update the issue' do expect { resolve }.not_to change { issue.updated_at } end end diff --git a/spec/helpers/learn_gitlab_helper_spec.rb b/spec/helpers/learn_gitlab_helper_spec.rb index 1159fd96d59..7ff79f19eed 100644 --- a/spec/helpers/learn_gitlab_helper_spec.rb +++ b/spec/helpers/learn_gitlab_helper_spec.rb @@ -11,9 +11,6 @@ RSpec.describe LearnGitlabHelper do let_it_be(:namespace) { project.namespace } before do - project.add_developer(user) - - allow(helper).to receive(:user).and_return(user) allow_next_instance_of(LearnGitlab::Project) do |learn_gitlab| allow(learn_gitlab).to receive(:project).and_return(project) end @@ -22,38 +19,115 @@ RSpec.describe LearnGitlabHelper do OnboardingProgress.register(namespace, :git_write) end - describe '.onboarding_actions_data' do + describe '#onboarding_actions_data' do subject(:onboarding_actions_data) { helper.onboarding_actions_data(project) } - it 'has all actions' do - expect(onboarding_actions_data.keys).to contain_exactly( - :issue_created, - :git_write, - :pipeline_created, - :merge_request_created, - :user_added, - :trial_started, - :required_mr_approvals_enabled, - :code_owners_enabled, - :security_scan_enabled - ) + shared_examples 'has all actions' do + it 'has all actions' do + expect(onboarding_actions_data.keys).to contain_exactly( + :issue_created, + :git_write, + :pipeline_created, + :merge_request_created, + :user_added, + :trial_started, + :required_mr_approvals_enabled, + :code_owners_enabled, + :security_scan_enabled + ) + end end - it 'sets correct path and completion status' do - expect(onboarding_actions_data[:git_write]).to eq({ - url: project_issue_url(project, LearnGitlab::Onboarding::ACTION_ISSUE_IDS[:git_write]), - completed: true, - svg: helper.image_path("learn_gitlab/git_write.svg") + it_behaves_like 'has all actions' + + it 'sets correct paths' do + expect(onboarding_actions_data).to match({ + trial_started: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/2\z}) + ), + issue_created: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/4\z}) + ), + git_write: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/6\z}) + ), + pipeline_created: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/7\z}) + ), + user_added: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/8\z}) + ), + merge_request_created: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/9\z}) + ), + code_owners_enabled: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/10\z}) + ), + required_mr_approvals_enabled: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/11\z}) + ), + security_scan_enabled: a_hash_including( + url: a_string_matching(%r{docs\.gitlab\.com/ee/user/application_security/security_dashboard/#gitlab-security-dashboard-security-center-and-vulnerability-reports\z}) + ) }) - expect(onboarding_actions_data[:pipeline_created]).to eq({ - url: project_issue_url(project, LearnGitlab::Onboarding::ACTION_ISSUE_IDS[:pipeline_created]), - completed: false, - svg: helper.image_path("learn_gitlab/pipeline_created.svg") + end + + it 'sets correct completion statuses' do + expect(onboarding_actions_data).to match({ + issue_created: a_hash_including(completed: false), + git_write: a_hash_including(completed: true), + pipeline_created: a_hash_including(completed: false), + merge_request_created: a_hash_including(completed: false), + user_added: a_hash_including(completed: false), + trial_started: a_hash_including(completed: false), + required_mr_approvals_enabled: a_hash_including(completed: false), + code_owners_enabled: a_hash_including(completed: false), + security_scan_enabled: a_hash_including(completed: false) }) end + + context 'when in the new action URLs experiment' do + before do + stub_experiments(change_continuous_onboarding_link_urls: :candidate) + end + + it_behaves_like 'has all actions' + + it 'sets mostly new paths' do + expect(onboarding_actions_data).to match({ + trial_started: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/2\z}) + ), + issue_created: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues\z}) + ), + git_write: a_hash_including( + url: a_string_matching(%r{/learn_gitlab\z}) + ), + pipeline_created: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/pipelines\z}) + ), + user_added: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/project_members\z}) + ), + merge_request_created: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/merge_requests\z}) + ), + code_owners_enabled: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/10\z}) + ), + required_mr_approvals_enabled: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/issues/11\z}) + ), + security_scan_enabled: a_hash_including( + url: a_string_matching(%r{/learn_gitlab/-/security/configuration\z}) + ) + }) + end + end end - describe '.learn_gitlab_enabled?' do + describe '#learn_gitlab_enabled?' do using RSpec::Parameterized::TableSyntax let_it_be(:user) { create(:user) } @@ -89,7 +163,7 @@ RSpec.describe LearnGitlabHelper do end end - describe '.onboarding_sections_data' do + describe '#onboarding_sections_data' do subject(:sections) { helper.onboarding_sections_data } it 'has the right keys' do diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 1100f4a3ad5..5d52c9178cb 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -961,4 +961,26 @@ RSpec.describe ProjectsHelper do ) end end + + describe '#project_classes' do + subject { helper.project_classes(project) } + + it { is_expected.to be_a(String) } + + context 'PUC highlighting enabled' do + before do + project.warn_about_potentially_unwanted_characters = true + end + + it { is_expected.to include('project-highlight-puc') } + end + + context 'PUC highlighting disabled' do + before do + project.warn_about_potentially_unwanted_characters = false + end + + it { is_expected.not_to include('project-highlight-puc') } + end + end end diff --git a/spec/lib/api/entities/project_spec.rb b/spec/lib/api/entities/project_spec.rb new file mode 100644 index 00000000000..8d1c3aa878d --- /dev/null +++ b/spec/lib/api/entities/project_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::API::Entities::Project do + let(:project) { create(:project, :public) } + let(:current_user) { create(:user) } + let(:options) { { current_user: current_user } } + + let(:entity) do + ::API::Entities::Project.new(project, options) + end + + subject(:json) { entity.as_json } + + describe '.shared_with_groups' do + let(:group) { create(:group, :private) } + + before do + project.project_group_links.create!(group: group) + end + + context 'when the current user does not have access to the group' do + it 'is empty' do + expect(json[:shared_with_groups]).to be_empty + end + end + + context 'when the current user has access to the group' do + before do + group.add_guest(current_user) + end + + it 'contains information about the shared group' do + expect(json[:shared_with_groups]).to contain_exactly(include(group_id: group.id)) + end + end + end +end diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb index 52bf3087875..d487268da78 100644 --- a/spec/lib/banzai/renderer_spec.rb +++ b/spec/lib/banzai/renderer_spec.rb @@ -84,6 +84,24 @@ RSpec.describe Banzai::Renderer do end end + describe '#cacheless_render' do + context 'without cache' do + let(:object) { fake_object(fresh: false) } + let(:histogram) { double('prometheus histogram') } + + it 'returns cacheless render field' do + allow(renderer).to receive(:render_result).and_return(output: 'test') + allow(renderer).to receive(:real_duration_histogram).and_return(histogram) + allow(renderer).to receive(:cpu_duration_histogram).and_return(histogram) + + expect(renderer).to receive(:render_result).with('test', {}) + expect(histogram).to receive(:observe).twice + + renderer.cacheless_render('test') + end + end + end + describe '#post_process' do let(:context_options) { {} } let(:html) { 'Consequatur aperiam et nesciunt modi aut assumenda quo id. '} diff --git a/spec/lib/bulk_imports/projects/pipelines/merge_requests_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/merge_requests_pipeline_spec.rb new file mode 100644 index 00000000000..3f02356b41e --- /dev/null +++ b/spec/lib/bulk_imports/projects/pipelines/merge_requests_pipeline_spec.rb @@ -0,0 +1,297 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Projects::Pipelines::MergeRequestsPipeline do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:bulk_import) { create(:bulk_import, user: user) } + let_it_be(:entity) do + create( + :bulk_import_entity, + :project_entity, + project: project, + bulk_import: bulk_import, + source_full_path: 'source/full/path', + destination_name: 'My Destination Project', + destination_namespace: group.full_path + ) + end + + let_it_be(:tracker) { create(:bulk_import_tracker, entity: entity) } + let_it_be(:context) { BulkImports::Pipeline::Context.new(tracker) } + + let(:mr) do + { + 'iid' => 7, + 'author_id' => 22, + 'source_project_id' => 1234, + 'target_project_id' => 1234, + 'title' => 'Imported MR', + 'description' => 'Description', + 'state' => 'opened', + 'source_branch' => 'feature', + 'target_branch' => 'main', + 'source_branch_sha' => 'ABCD', + 'target_branch_sha' => 'DCBA', + 'created_at' => '2020-06-14T15:02:47.967Z', + 'updated_at' => '2020-06-14T15:03:47.967Z', + 'merge_request_diff' => { + 'state' => 'collected', + 'base_commit_sha' => 'ae73cb07c9eeaf35924a10f713b364d32b2dd34f', + 'head_commit_sha' => 'a97f74ddaa848b707bea65441c903ae4bf5d844d', + 'start_commit_sha' => '9eea46b5c72ead701c22f516474b95049c9d9462', + 'merge_request_diff_commits' => [ + { + 'sha' => 'COMMIT1', + 'relative_order' => 0, + 'message' => 'commit message', + 'authored_date' => '2014-08-06T08:35:52.000+02:00', + 'committed_date' => '2014-08-06T08:35:52.000+02:00', + 'commit_author' => { + 'name' => 'Commit Author', + 'email' => 'gitlab@example.com' + }, + 'committer' => { + 'name' => 'Committer', + 'email' => 'committer@example.com' + } + } + ], + 'merge_request_diff_files' => [ + { + 'relative_order' => 0, + 'utf8_diff' => '--- a/.gitignore\n+++ b/.gitignore\n@@ -1 +1 @@ test\n', + 'new_path' => '.gitignore', + 'old_path' => '.gitignore', + 'a_mode' => '100644', + 'b_mode' => '100644', + 'new_file' => false, + 'renamed_file' => false, + 'deleted_file' => false, + 'too_large' => false + } + ] + } + }.merge(attributes) + end + + let(:attributes) { {} } + let(:imported_mr) { project.merge_requests.find_by_title(mr['title']) } + + subject(:pipeline) { described_class.new(context) } + + describe '#run' do + before do + group.add_owner(user) + + allow_next_instance_of(BulkImports::Common::Extractors::NdjsonExtractor) do |extractor| + allow(extractor).to receive(:remove_tmp_dir) + allow(extractor).to receive(:extract).and_return(BulkImports::Pipeline::ExtractedData.new(data: [[mr, 0]])) + end + + allow(project.repository).to receive(:fetch_source_branch!).and_return(true) + allow(project.repository).to receive(:branch_exists?).and_return(false) + allow(project.repository).to receive(:create_branch) + + pipeline.run + end + + it 'imports a merge request' do + expect(project.merge_requests.count).to eq(1) + expect(imported_mr.title).to eq(mr['title']) + expect(imported_mr.description).to eq(mr['description']) + expect(imported_mr.state).to eq(mr['state']) + expect(imported_mr.iid).to eq(mr['iid']) + expect(imported_mr.created_at).to eq(mr['created_at']) + expect(imported_mr.updated_at).to eq(mr['updated_at']) + expect(imported_mr.author).to eq(user) + end + + context 'merge request state' do + context 'when mr is closed' do + let(:attributes) { { 'state' => 'closed' } } + + it 'imported mr as closed' do + expect(imported_mr.state).to eq(attributes['state']) + end + end + + context 'when mr is merged' do + let(:attributes) { { 'state' => 'merged' } } + + it 'imported mr as merged' do + expect(imported_mr.state).to eq(attributes['state']) + end + end + end + + context 'source & target project' do + it 'has the new project as target' do + expect(imported_mr.target_project).to eq(project) + end + + it 'has the new project as source' do + expect(imported_mr.source_project).to eq(project) + end + + context 'when source/target projects differ' do + let(:attributes) { { 'source_project_id' => 4321 } } + + it 'has no source' do + expect(imported_mr.source_project).to be_nil + end + + context 'when diff_head_sha is present' do + let(:attributes) { { 'diff_head_sha' => 'HEAD', 'source_project_id' => 4321 } } + + it 'has the new project as source' do + expect(imported_mr.source_project).to eq(project) + end + end + end + end + + context 'resource label events' do + let(:attributes) { { 'resource_label_events' => [{ 'action' => 'add', 'user_id' => 1 }] } } + + it 'restores resource label events' do + expect(imported_mr.resource_label_events.first.action).to eq('add') + end + end + + context 'award emoji' do + let(:attributes) { { 'award_emoji' => [{ 'name' => 'tada', 'user_id' => 22 }] } } + + it 'has award emoji' do + expect(imported_mr.award_emoji.first.name).to eq(attributes['award_emoji'].first['name']) + end + end + + context 'notes' do + let(:note) { imported_mr.notes.first } + let(:attributes) do + { + 'notes' => [ + { + 'note' => 'Issue note', + 'note_html' => '<p>something else entirely</p>', + 'cached_markdown_version' => 917504, + 'author_id' => 22, + 'author' => { 'name' => 'User 22' }, + 'created_at' => '2016-06-14T15:02:56.632Z', + 'updated_at' => '2016-06-14T15:02:47.770Z', + 'award_emoji' => [{ 'name' => 'clapper', 'user_id' => 22 }] + } + ] + } + end + + it 'imports mr note' do + expect(note).to be_present + expect(note.note).to include('By User 22') + expect(note.note).to include(attributes['notes'].first['note']) + expect(note.author).to eq(user) + end + + it 'has award emoji' do + emoji = note.award_emoji.first + + expect(emoji.name).to eq('clapper') + expect(emoji.user).to eq(user) + end + + it 'does not import note_html' do + expect(note.note_html).to match(attributes['notes'].first['note']) + expect(note.note_html).not_to match(attributes['notes'].first['note_html']) + end + end + + context 'system note metadata' do + let(:attributes) do + { + 'notes' => [ + { + 'note' => 'added 3 commits', + 'system' => true, + 'author_id' => 22, + 'author' => { 'name' => 'User 22' }, + 'created_at' => '2016-06-14T15:02:56.632Z', + 'updated_at' => '2016-06-14T15:02:47.770Z', + 'system_note_metadata' => { 'action' => 'commit', 'commit_count' => 3 } + } + ] + } + end + + it 'restores system note metadata' do + note = imported_mr.notes.first + + expect(note.system).to eq(true) + expect(note.noteable_type).to eq('MergeRequest') + expect(note.system_note_metadata.action).to eq('commit') + expect(note.system_note_metadata.commit_count).to eq(3) + end + end + + context 'diffs' do + it 'imports merge request diff' do + expect(imported_mr.merge_request_diff).to be_present + end + + it 'has the correct data for merge request latest_merge_request_diff' do + expect(imported_mr.latest_merge_request_diff_id).to eq(imported_mr.merge_request_diffs.maximum(:id)) + end + + it 'imports diff files' do + expect(imported_mr.merge_request_diff.merge_request_diff_files.count).to eq(1) + end + + context 'diff commits' do + it 'imports diff commits' do + expect(imported_mr.merge_request_diff.merge_request_diff_commits.count).to eq(1) + end + + it 'assigns committer and author details to diff commits' do + commit = imported_mr.merge_request_diff.merge_request_diff_commits.first + + expect(commit.commit_author_id).not_to be_nil + expect(commit.committer_id).not_to be_nil + end + + it 'assigns the correct commit users to diff commits' do + commit = MergeRequestDiffCommit.find_by(sha: 'COMMIT1') + + expect(commit.commit_author.name).to eq('Commit Author') + expect(commit.commit_author.email).to eq('gitlab@example.com') + expect(commit.committer.name).to eq('Committer') + expect(commit.committer.email).to eq('committer@example.com') + end + end + end + + context 'labels' do + let(:attributes) do + { + 'label_links' => [ + { 'label' => { 'title' => 'imported label 1', 'type' => 'ProjectLabel' } }, + { 'label' => { 'title' => 'imported label 2', 'type' => 'ProjectLabel' } } + ] + } + end + + it 'imports labels' do + expect(imported_mr.labels.pluck(:title)).to contain_exactly('imported label 1', 'imported label 2') + end + end + + context 'milestone' do + let(:attributes) { { 'milestone' => { 'title' => 'imported milestone' } } } + + it 'imports milestone' do + expect(imported_mr.milestone.title).to eq(attributes.dig('milestone', 'title')) + end + end + end +end diff --git a/spec/lib/bulk_imports/projects/pipelines/repository_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/repository_pipeline_spec.rb index af39ec7a11c..583485faf8d 100644 --- a/spec/lib/bulk_imports/projects/pipelines/repository_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/repository_pipeline_spec.rb @@ -3,71 +3,72 @@ require 'spec_helper' RSpec.describe BulkImports::Projects::Pipelines::RepositoryPipeline do - describe '#run' do - let_it_be(:user) { create(:user) } - let_it_be(:parent) { create(:project) } - let_it_be(:bulk_import) { create(:bulk_import, user: user) } - let_it_be(:bulk_import_configuration) { create(:bulk_import_configuration, bulk_import: bulk_import) } - - let_it_be(:entity) do - create( - :bulk_import_entity, - :project_entity, - bulk_import: bulk_import, - source_full_path: 'source/full/path', - destination_name: 'My Destination Repository', - destination_namespace: parent.full_path, - project: parent - ) - end + let_it_be(:user) { create(:user) } + let_it_be(:parent) { create(:project) } + let_it_be(:bulk_import) { create(:bulk_import, user: user) } + let_it_be(:bulk_import_configuration) { create(:bulk_import_configuration, bulk_import: bulk_import) } + + let_it_be(:entity) do + create( + :bulk_import_entity, + :project_entity, + bulk_import: bulk_import, + source_full_path: 'source/full/path', + destination_name: 'My Destination Repository', + destination_namespace: parent.full_path, + project: parent + ) + end - let_it_be(:tracker) { create(:bulk_import_tracker, entity: entity) } - let_it_be(:context) { BulkImports::Pipeline::Context.new(tracker) } + let_it_be(:tracker) { create(:bulk_import_tracker, entity: entity) } + let_it_be(:context) { BulkImports::Pipeline::Context.new(tracker) } - context 'successfully imports repository' do - let(:project_data) do - { - 'httpUrlToRepo' => 'http://test.git' - } - end + let(:extracted_data) { BulkImports::Pipeline::ExtractedData.new(data: project_data) } - subject { described_class.new(context) } + subject(:pipeline) { described_class.new(context) } + + before do + allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor| + allow(extractor).to receive(:extract).and_return(extracted_data) + end + end + + describe '#run' do + context 'successfully imports repository' do + let(:project_data) { { 'httpUrlToRepo' => 'http://test.git' } } it 'imports new repository into destination project' do - allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor| - allow(extractor).to receive(:extract).and_return(BulkImports::Pipeline::ExtractedData.new(data: project_data)) - end + url = project_data['httpUrlToRepo'].sub("://", "://oauth2:#{bulk_import_configuration.access_token}@") - expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| - url = project_data['httpUrlToRepo'].sub("://", "://oauth2:#{bulk_import_configuration.access_token}@") - expect(repository_service).to receive(:import_repository).with(url).and_return 0 - end + expect(context.portable).to receive(:ensure_repository) + expect(context.portable.repository).to receive(:fetch_as_mirror).with(url) - subject.run + pipeline.run end end context 'blocked local networks' do - let(:project_data) do - { - 'httpUrlToRepo' => 'http://localhost/foo.git' - } - end + let(:project_data) { { 'httpUrlToRepo' => 'http://localhost/foo.git' } } - before do + it 'imports new repository into destination project' do allow(Gitlab.config.gitlab).to receive(:host).and_return('notlocalhost.gitlab.com') allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(false) - allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor| - allow(extractor).to receive(:extract).and_return(BulkImports::Pipeline::ExtractedData.new(data: project_data)) - end - end - subject { described_class.new(context) } + pipeline.run - it 'imports new repository into destination project' do - subject.run - expect(context.entity.failed?).to be_truthy + expect(context.entity.failed?).to eq(true) end end end + + describe '#after_run' do + it 'executes housekeeping service after import' do + service = instance_double(Repositories::HousekeepingService) + + expect(Repositories::HousekeepingService).to receive(:new).with(context.portable, :gc).and_return(service) + expect(service).to receive(:execute) + + pipeline.after_run(context) + end + end end diff --git a/spec/lib/bulk_imports/projects/stage_spec.rb b/spec/lib/bulk_imports/projects/stage_spec.rb index 8667651bde2..8b35ed8bc38 100644 --- a/spec/lib/bulk_imports/projects/stage_spec.rb +++ b/spec/lib/bulk_imports/projects/stage_spec.rb @@ -10,6 +10,7 @@ RSpec.describe BulkImports::Projects::Stage do [2, BulkImports::Common::Pipelines::LabelsPipeline], [3, BulkImports::Projects::Pipelines::IssuesPipeline], [4, BulkImports::Common::Pipelines::BoardsPipeline], + [4, BulkImports::Projects::Pipelines::MergeRequestsPipeline], [5, BulkImports::Common::Pipelines::UploadsPipeline], [6, BulkImports::Common::Pipelines::EntityFinisher] ] diff --git a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb index a46b7db256f..f9313f0ff28 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb @@ -280,7 +280,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do end context 'metrics' do - let(:histogram) { double(:histogram) } + let(:histogram) { double(:histogram).as_null_object } let(:counter) { double('counter', increment: true) } before do @@ -315,7 +315,6 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do ) expect(counter).to receive(:increment) - allow(histogram).to receive(:observe).with({ importer: :bitbucket_server_importer }, anything) subject.execute end diff --git a/spec/lib/gitlab/data_builder/build_spec.rb b/spec/lib/gitlab/data_builder/build_spec.rb index 325fdb90929..9cee0802e87 100644 --- a/spec/lib/gitlab/data_builder/build_spec.rb +++ b/spec/lib/gitlab/data_builder/build_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Gitlab::DataBuilder::Build do let!(:tag_names) { %w(tag-1 tag-2) } let(:runner) { create(:ci_runner, :instance, tag_list: tag_names.map { |n| ActsAsTaggableOn::Tag.create!(name: n)}) } - let(:user) { create(:user) } + let(:user) { create(:user, :public_email) } let(:build) { create(:ci_build, :running, runner: runner, user: user) } describe '.build' do diff --git a/spec/lib/gitlab/data_builder/pipeline_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb index 0e574c7aa84..8b57da8e60b 100644 --- a/spec/lib/gitlab/data_builder/pipeline_spec.rb +++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::DataBuilder::Pipeline do - let_it_be(:user) { create(:user) } + let_it_be(:user) { create(:user, :public_email) } let_it_be(:project) { create(:project, :repository) } let_it_be_with_reload(:pipeline) do @@ -46,7 +46,7 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do name: user.name, username: user.username, avatar_url: user.avatar_url(only_path: false), - email: user.email + email: user.public_email }) end diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index 88bd71d3d64..49df2313924 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -267,6 +267,35 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_ end end + context 'pipeline_schedule' do + let(:relation_sym) { :pipeline_schedules } + let(:relation_hash) do + { + "id": 3, + "created_at": "2016-07-22T08:55:44.161Z", + "updated_at": "2016-07-22T08:55:44.161Z", + "description": "pipeline schedule", + "ref": "main", + "cron": "0 4 * * 0", + "cron_timezone": "UTC", + "active": value, + "project_id": project.id + } + end + + subject { created_object.active } + + [true, false].each do |v| + context "when relation_hash has active set to #{v}" do + let(:value) { v } + + it "the created object is not active" do + expect(created_object.active).to eq(false) + end + end + end + end + # `project_id`, `described_class.USER_REFERENCES`, noteable_id, target_id, and some project IDs are already # re-assigned by described_class. context 'Potentially hazardous foreign keys' do diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index 45c34d46c77..cd3d29f1a51 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -375,7 +375,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do expect(pipeline_schedule.ref).to eq('master') expect(pipeline_schedule.cron).to eq('0 4 * * 0') expect(pipeline_schedule.cron_timezone).to eq('UTC') - expect(pipeline_schedule.active).to eq(true) + expect(pipeline_schedule.active).to eq(false) end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 287be24d11f..4b125cab49b 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -556,7 +556,6 @@ Project: - merge_requests_rebase_enabled - jobs_cache_index - external_authorization_classification_label -- external_webhook_token - pages_https_only - merge_requests_disable_committers_approval - require_password_to_approve diff --git a/spec/lib/gitlab/unicode_spec.rb b/spec/lib/gitlab/unicode_spec.rb new file mode 100644 index 00000000000..68f3266ecc7 --- /dev/null +++ b/spec/lib/gitlab/unicode_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Gitlab::Unicode do + describe described_class::BIDI_REGEXP do + using RSpec::Parameterized::TableSyntax + + where(:bidi_string, :match) do + "\u2066" | true # left-to-right isolate + "\u2067" | true # right-to-left isolate + "\u2068" | true # first strong isolate + "\u2069" | true # pop directional isolate + "\u202a" | true # left-to-right embedding + "\u202b" | true # right-to-left embedding + "\u202c" | true # pop directional formatting + "\u202d" | true # left-to-right override + "\u202e" | true # right-to-left override + "\u2066foobar" | true + "" | false + "foo" | false + "\u2713" | false # checkmark + end + + with_them do + let(:utf8_string) { bidi_string.encode("utf-8") } + + it "matches only the bidi characters" do + expect(utf8_string.match?(subject)).to eq(match) + end + end + end +end diff --git a/spec/lib/rouge/formatters/html_gitlab_spec.rb b/spec/lib/rouge/formatters/html_gitlab_spec.rb index 4bc9b256dce..7c92c62e30b 100644 --- a/spec/lib/rouge/formatters/html_gitlab_spec.rb +++ b/spec/lib/rouge/formatters/html_gitlab_spec.rb @@ -36,5 +36,26 @@ RSpec.describe Rouge::Formatters::HTMLGitlab do is_expected.to eq(code) end end + + context 'when unicode control characters are used' do + let(:lang) { 'javascript' } + let(:tokens) { lexer.lex(code, continue: false) } + let(:code) do + <<~JS + #!/usr/bin/env node + + var accessLevel = "user"; + if (accessLevel != "user // Check if admin ") { + console.log("You are an admin."); + } + JS + end + + it 'highlights the control characters' do + message = "Potentially unwanted character detected: Unicode BiDi Control" + + is_expected.to include(%{<span class="unicode-bidi has-tooltip" data-toggle="tooltip" title="#{message}">}).exactly(4).times + end + end end end diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 069864fa765..b2ffb34da1d 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -121,4 +121,16 @@ RSpec.describe Ci::BuildMetadata do end end end + + describe 'set_cancel_gracefully' do + it 'sets cancel_gracefully' do + build.set_cancel_gracefully + + expect(build.cancel_gracefully?).to be true + end + + it 'returns false' do + expect(build.cancel_gracefully?).to be false + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 8e3fb7d94e7..50978ac017c 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -35,6 +35,8 @@ RSpec.describe Ci::Build do it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } + it { is_expected.to respond_to(:set_cancel_gracefully) } + it { is_expected.to respond_to(:cancel_gracefully?) } it { is_expected.to delegate_method(:merge_request?).to(:pipeline) } it { is_expected.to delegate_method(:merge_request_ref?).to(:pipeline) } @@ -5386,4 +5388,23 @@ RSpec.describe Ci::Build do create(:ci_build) end end + + describe '#runner_features' do + subject do + build.save! + build.cancel_gracefully? + end + + let_it_be(:build) { create(:ci_build, pipeline: pipeline) } + + it 'cannot cancel gracefully' do + expect(subject).to be false + end + + it 'can cancel gracefully' do + build.set_cancel_gracefully + + expect(subject).to be true + end + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 8505b19258c..8d25502ebb5 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do include StubRequests include Ci::SourcePipelineHelpers - let_it_be(:user) { create(:user) } + let_it_be(:user) { create(:user, :public_email) } let_it_be(:namespace) { create_default(:namespace).freeze } let_it_be(:project) { create_default(:project, :repository).freeze } diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb index c0e5ddc23b1..fc154738f11 100644 --- a/spec/models/concerns/resolvable_discussion_spec.rb +++ b/spec/models/concerns/resolvable_discussion_spec.rb @@ -188,6 +188,16 @@ RSpec.describe Discussion, ResolvableDiscussion do it "returns true" do expect(subject.can_resolve?(current_user)).to be true end + + context 'when noteable is locked' do + before do + allow(subject.noteable).to receive(:discussion_locked?).and_return(true) + end + + it 'returns false' do + expect(subject.can_resolve?(current_user)).to be_falsey + end + end end context "when the signed in user can push to the project" do @@ -200,8 +210,11 @@ RSpec.describe Discussion, ResolvableDiscussion do end context "when the noteable has no author" do + before do + subject.noteable.author = nil + end + it "returns true" do - expect(noteable).to receive(:author).and_return(nil) expect(subject.can_resolve?(current_user)).to be true end end @@ -213,8 +226,11 @@ RSpec.describe Discussion, ResolvableDiscussion do end context "when the noteable has no author" do + before do + subject.noteable.author = nil + end + it "returns false" do - expect(noteable).to receive(:author).and_return(nil) expect(subject.can_resolve?(current_user)).to be false end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index ab20e6cb020..808343ffae9 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1341,6 +1341,7 @@ RSpec.describe Namespace do context 'refreshing project access on updating share_with_group_lock' do let(:group) { create(:group, share_with_group_lock: false) } let(:project) { create(:project, :private, group: group) } + let(:another_project) { create(:project, :private, group: group) } let_it_be(:shared_with_group_one) { create(:group) } let_it_be(:shared_with_group_two) { create(:group) } @@ -1353,6 +1354,7 @@ RSpec.describe Namespace do shared_with_group_one.add_developer(group_one_user) shared_with_group_two.add_developer(group_two_user) create(:project_group_link, group: shared_with_group_one, project: project) + create(:project_group_link, group: shared_with_group_one, project: another_project) create(:project_group_link, group: shared_with_group_two, project: project) end @@ -1360,6 +1362,9 @@ RSpec.describe Namespace do expect(AuthorizedProjectUpdate::ProjectRecalculateWorker) .to receive(:perform_async).with(project.id).once + expect(AuthorizedProjectUpdate::ProjectRecalculateWorker) + .to receive(:perform_async).with(another_project.id).once + execute_update end @@ -1392,11 +1397,23 @@ RSpec.describe Namespace do stub_feature_flags(specialized_worker_for_group_lock_update_auth_recalculation: false) end - it 'refreshes the permissions of the members of the old and new namespace' do + it 'updates authorizations leading to users from shared groups losing access', :sidekiq_inline do expect { execute_update } .to change { group_one_user.authorized_projects.include?(project) }.from(true).to(false) .and change { group_two_user.authorized_projects.include?(project) }.from(true).to(false) end + + it 'updates the authorizations in a non-blocking manner' do + expect(AuthorizedProjectsWorker).to( + receive(:bulk_perform_async) + .with([[group_one_user.id]])).once + + expect(AuthorizedProjectsWorker).to( + receive(:bulk_perform_async) + .with([[group_two_user.id]])).once + + execute_update + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 157d2e55536..4b3a6c731d5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -681,6 +681,19 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to delegate_method(:container_registry_enabled?).to(:project_feature) } it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) } + describe 'project settings' do + %i( + show_default_award_emojis + show_default_award_emojis= + show_default_award_emojis? + warn_about_potentially_unwanted_characters + warn_about_potentially_unwanted_characters= + warn_about_potentially_unwanted_characters? + ).each do |method| + it { is_expected.to delegate_method(method).to(:project_setting).with_arguments(allow_nil: true) } + end + end + include_examples 'ci_cd_settings delegation' do # Skip attributes defined in EE code let(:exclude_attributes) do diff --git a/spec/models/uploads/fog_spec.rb b/spec/models/uploads/fog_spec.rb index 899e6f2064c..1ffe7c6c43b 100644 --- a/spec/models/uploads/fog_spec.rb +++ b/spec/models/uploads/fog_spec.rb @@ -40,7 +40,9 @@ RSpec.describe Uploads::Fog do end describe '#delete_keys' do + let(:connection) { ::Fog::Storage.new(FileUploader.object_store_credentials) } let(:keys) { data_store.keys(relation) } + let(:paths) { relation.pluck(:path) } let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) } subject { data_store.delete_keys(keys) } @@ -50,17 +52,32 @@ RSpec.describe Uploads::Fog do end it 'deletes multiple data' do - paths = relation.pluck(:path) + paths.each do |path| + expect(connection.get_object('uploads', path)[:body]).not_to be_nil + end + + subject + + paths.each do |path| + expect { connection.get_object('uploads', path)[:body] }.to raise_error(Excon::Error::NotFound) + end + end - ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection| + context 'when one of keys is missing' do + let(:keys) { ['unknown'] + super() } + + it 'deletes only existing keys' do paths.each do |path| expect(connection.get_object('uploads', path)[:body]).not_to be_nil end - end - subject + expect_next_instance_of(::Fog::Storage) do |storage| + allow(storage).to receive(:delete_object).and_call_original + expect(storage).to receive(:delete_object).with('uploads', keys.first).and_raise(::Google::Apis::ClientError, 'NotFound') + end + + subject - ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection| paths.each do |path| expect { connection.get_object('uploads', path)[:body] }.to raise_error(Excon::Error::NotFound) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index db805a804c8..21c5aea514a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5679,16 +5679,29 @@ RSpec.describe User do end describe '#hook_attrs' do - it 'includes id, name, username, avatar_url, and email' do - user = create(:user) - user_attributes = { + let(:user) { create(:user) } + let(:user_attributes) do + { id: user.id, name: user.name, username: user.username, - avatar_url: user.avatar_url(only_path: false), - email: user.email + avatar_url: user.avatar_url(only_path: false) } - expect(user.hook_attrs).to eq(user_attributes) + end + + context 'with a public email' do + it 'includes id, name, username, avatar_url, and email' do + user.public_email = "hello@hello.com" + user_attributes[:email] = user.public_email + expect(user.hook_attrs).to eq(user_attributes) + end + end + + context 'without a public email' do + it "does not include email if user's email is private" do + user_attributes[:email] = "[REDACTED]" + expect(user.hook_attrs).to eq(user_attributes) + end end end diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb index 86b04ccda57..eeb298e853e 100644 --- a/spec/policies/issuable_policy_spec.rb +++ b/spec/policies/issuable_policy_spec.rb @@ -13,6 +13,10 @@ RSpec.describe IssuablePolicy, models: true do let(:merge_request) { create(:merge_request, source_project: project, author: user) } let(:policies) { described_class.new(user, merge_request) } + it 'allows resolving notes' do + expect(policies).to be_allowed(:resolve_note) + end + context 'when user is able to read project' do it 'enables user to read and update issuables' do expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request) @@ -43,16 +47,24 @@ RSpec.describe IssuablePolicy, models: true do it 'can not create a note nor award emojis' do expect(policies).to be_disallowed(:create_note, :award_emoji) end + + it 'does not allow resolving note' do + expect(policies).to be_disallowed(:resolve_note) + end end context 'when the user is a project member' do before do - project.add_guest(user) + project.add_developer(user) end it 'can create a note and award emojis' do expect(policies).to be_allowed(:create_note, :award_emoji) end + + it 'allows resolving notes' do + expect(policies).to be_allowed(:resolve_note) + end end end end diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index 959e68e6a0d..022451553ee 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -5,13 +5,14 @@ require 'spec_helper' RSpec.describe API::Applications, :api do let(:admin_user) { create(:user, admin: true) } let(:user) { create(:user, admin: false) } - let!(:application) { create(:application, name: 'another_application', owner: nil, redirect_uri: 'http://other_application.url', scopes: '') } + let(:scopes) { 'api' } + let!(:application) { create(:application, name: 'another_application', owner: nil, redirect_uri: 'http://other_application.url', scopes: scopes) } describe 'POST /applications' do context 'authenticated and authorized user' do it 'creates and returns an OAuth application' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '' } + post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes } end.to change { Doorkeeper::Application.count }.by 1 application = Doorkeeper::Application.find_by(name: 'application_name', redirect_uri: 'http://application.url') @@ -22,11 +23,12 @@ RSpec.describe API::Applications, :api do expect(json_response['secret']).to eq application.secret expect(json_response['callback_url']).to eq application.redirect_uri expect(json_response['confidential']).to eq application.confidential + expect(application.scopes.to_s).to eq('api') end it 'does not allow creating an application with the wrong redirect_uri format' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://', scopes: '' } + post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -36,7 +38,7 @@ RSpec.describe API::Applications, :api do it 'does not allow creating an application with a forbidden URI format' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'javascript://alert()', scopes: '' } + post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'javascript://alert()', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -46,7 +48,7 @@ RSpec.describe API::Applications, :api do it 'does not allow creating an application without a name' do expect do - post api('/applications', admin_user), params: { redirect_uri: 'http://application.url', scopes: '' } + post api('/applications', admin_user), params: { redirect_uri: 'http://application.url', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -56,7 +58,7 @@ RSpec.describe API::Applications, :api do it 'does not allow creating an application without a redirect_uri' do expect do - post api('/applications', admin_user), params: { name: 'application_name', scopes: '' } + post api('/applications', admin_user), params: { name: 'application_name', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -64,19 +66,59 @@ RSpec.describe API::Applications, :api do expect(json_response['error']).to eq('redirect_uri is missing') end - it 'does not allow creating an application without scopes' do + it 'does not allow creating an application without specifying `scopes`' do expect do post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url' } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response).to be_a Hash - expect(json_response['error']).to eq('scopes is missing') + expect(json_response['error']).to eq('scopes is missing, scopes is empty') + end + + it 'does not allow creating an application with blank `scopes`' do + expect do + post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '' } + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('scopes is empty') + end + + it 'does not allow creating an application with invalid `scopes`' do + expect do + post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'non_existent_scope' } + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['scopes'][0]).to eq('doesn\'t match configured on the server.') + end + + context 'multiple scopes' do + it 'creates an application with multiple `scopes` when each scope specified is seperated by a space' do + expect do + post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api read_user' } + end.to change { Doorkeeper::Application.count }.by 1 + + application = Doorkeeper::Application.last + + expect(response).to have_gitlab_http_status(:created) + expect(application.scopes.to_s).to eq('api read_user') + end + + it 'does not allow creating an application with multiple `scopes` when one of the scopes is invalid' do + expect do + post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api non_existent_scope' } + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['scopes'][0]).to eq('doesn\'t match configured on the server.') + end end it 'defaults to creating an application with confidential' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '', confidential: nil } + post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes, confidential: nil } end.to change { Doorkeeper::Application.count }.by(1) expect(response).to have_gitlab_http_status(:created) @@ -89,7 +131,7 @@ RSpec.describe API::Applications, :api do context 'authorized user without authorization' do it 'does not create application' do expect do - post api('/applications', user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '' } + post api('/applications', user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:forbidden) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index fc596bff87e..75f5a974d22 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -882,6 +882,15 @@ RSpec.describe API::Groups do expect(json_response['prevent_sharing_groups_outside_hierarchy']).to eq(true) end + it 'does not update visibility_level if it is restricted' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + + put api("/groups/#{group1.id}", user1), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['visibility_level']).to include('internal has been restricted by your GitLab administrator') + end + context 'updating the `default_branch_protection` attribute' do subject do put api("/groups/#{group1.id}", user1), params: { default_branch_protection: ::Gitlab::Access::PROTECTION_NONE } @@ -969,6 +978,15 @@ RSpec.describe API::Groups do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(new_group_name) end + + it 'ignores visibility level restrictions' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + + put api("/groups/#{group1.id}", admin), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['visibility']).to eq('internal') + end end context 'when authenticated as an user that can see the group' do diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 9174356f123..dd00d413664 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -139,6 +139,7 @@ project_setting: - has_confluence - has_vulnerabilities - prevent_merge_without_jira_issue + - warn_about_potentially_unwanted_characters - previous_default_branch - project_id - push_rule_id diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index d3b24eb3832..0c9e125cc90 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -16,6 +16,16 @@ RSpec.describe API::ProjectImport do namespace.add_owner(user) end + shared_examples 'requires authentication' do + let(:user) { nil } + + it 'returns 401' do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + describe 'POST /projects/import' do subject { upload_archive(file_upload, workhorse_headers, params) } @@ -32,6 +42,8 @@ RSpec.describe API::ProjectImport do allow(ImportExportUploader).to receive(:workhorse_upload_path).and_return('/') end + it_behaves_like 'requires authentication' + it 'executes a limited number of queries' do control_count = ActiveRecord::QueryRecorder.new { subject }.count @@ -281,6 +293,10 @@ RSpec.describe API::ProjectImport do end describe 'POST /projects/remote-import' do + subject do + post api('/projects/remote-import', user), params: params + end + let(:params) do { path: 'test-import', @@ -288,10 +304,12 @@ RSpec.describe API::ProjectImport do } end + it_behaves_like 'requires authentication' + it 'returns NOT FOUND when the feature is disabled' do stub_feature_flags(import_project_from_remote_file: false) - post api('/projects/remote-import', user), params: params + subject expect(response).to have_gitlab_http_status(:not_found) end @@ -315,7 +333,7 @@ RSpec.describe API::ProjectImport do .to receive(:execute) .and_return(service_response) - post api('/projects/remote-import', user), params: params + subject expect(response).to have_gitlab_http_status(:created) expect(json_response).to include({ @@ -338,7 +356,7 @@ RSpec.describe API::ProjectImport do .to receive(:execute) .and_return(service_response) - post api('/projects/remote-import', user), params: params + subject expect(response).to have_gitlab_http_status(:bad_request) expect(json_response).to eq({ @@ -350,6 +368,14 @@ RSpec.describe API::ProjectImport do end describe 'GET /projects/:id/import' do + it 'public project accessible for an unauthenticated user' do + project = create(:project, :public) + + get api("/projects/#{project.id}/import", nil) + + expect(response).to have_gitlab_http_status(:ok) + end + it 'returns the import status' do project = create(:project, :import_started) project.add_maintainer(user) @@ -376,6 +402,8 @@ RSpec.describe API::ProjectImport do describe 'POST /projects/import/authorize' do subject { post api('/projects/import/authorize', user), headers: workhorse_headers } + it_behaves_like 'requires authentication' + it 'authorizes importing project with workhorse header' do subject diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index da7ccef851e..4f84e6f2562 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -991,7 +991,7 @@ RSpec.describe API::Projects do expect do get api('/projects', admin) - end.not_to exceed_query_limit(control.count) + end.not_to exceed_query_limit(control) end end end @@ -3232,6 +3232,15 @@ RSpec.describe API::Projects do expect(json_response['visibility']).to eq('private') end + it 'does not update visibility_level if it is restricted' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + + put api("/projects/#{project3.id}", user), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['visibility_level']).to include('internal has been restricted by your GitLab administrator') + end + it 'does not update name to existing name' do project_param = { name: project3.name } @@ -3555,6 +3564,19 @@ RSpec.describe API::Projects do end end + context 'when authenticated as the admin' do + let_it_be(:admin) { create(:admin) } + + it 'ignores visibility level restrictions' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + + put api("/projects/#{project3.id}", admin), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['visibility']).to eq('internal') + end + end + context 'when updating repository storage' do let(:unknown_storage) { 'new-storage' } let(:new_project) { create(:project, :repository, namespace: user.namespace) } diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index a75c5bff912..0b032d6a903 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Groups::TransferService do +RSpec.describe Groups::TransferService, :sidekiq_inline do shared_examples 'project namespace path is in sync with project path' do it 'keeps project and project namespace attributes in sync' do projects_with_project_namespace.each do |project| @@ -653,6 +653,59 @@ RSpec.describe Groups::TransferService do transfer_service.execute(new_parent_group) end end + + context 'transferring groups with shared_projects' do + let_it_be_with_reload(:shared_project) { create(:project, :public) } + + shared_examples_for 'drops the authorizations of ancestor members from the old hierarchy' do + it 'drops the authorizations of ancestor members from the old hierarchy' do + expect { transfer_service.execute(new_parent_group) }.to change { + ProjectAuthorization.where(project: shared_project, user: old_group_member).size + }.from(1).to(0) + end + end + + context 'when the group that has existing project share is transferred' do + before do + create(:project_group_link, :maintainer, project: shared_project, group: group) + end + + it_behaves_like 'drops the authorizations of ancestor members from the old hierarchy' + end + + context 'when the group whose subgroup has an existing project share is transferred' do + let_it_be_with_reload(:subgroup) { create(:group, :private, parent: group) } + + before do + create(:project_group_link, :maintainer, project: shared_project, group: subgroup) + end + + it_behaves_like 'drops the authorizations of ancestor members from the old hierarchy' + end + end + + context 'when a group that has existing group share is transferred' do + let(:shared_with_group) { group } + + let_it_be(:member_of_shared_with_group) { create(:user) } + let_it_be(:shared_group) { create(:group, :private) } + let_it_be(:project_in_shared_group) { create(:project, namespace: shared_group) } + + before do + shared_with_group.add_developer(member_of_shared_with_group) + create(:group_group_link, :maintainer, shared_group: shared_group, shared_with_group: shared_with_group) + shared_with_group.refresh_members_authorized_projects + end + + it 'retains the authorizations of direct members' do + expect { transfer_service.execute(new_parent_group) }.not_to change { + ProjectAuthorization.where( + project: project_in_shared_group, + user: member_of_shared_with_group, + access_level: Gitlab::Access::DEVELOPER).size + }.from(1) + end + end end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 44ffece7f3b..85b8fef685e 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Issues::UpdateService, :mailer do let_it_be(:user) { create(:user) } let_it_be(:user2) { create(:user) } let_it_be(:user3) { create(:user) } + let_it_be(:guest) { create(:user) } let_it_be(:group) { create(:group, :public) } let_it_be(:project, reload: true) { create(:project, :repository, group: group) } let_it_be(:label) { create(:label, project: project) } @@ -24,6 +25,7 @@ RSpec.describe Issues::UpdateService, :mailer do project.add_maintainer(user) project.add_developer(user2) project.add_developer(user3) + project.add_guest(guest) end describe 'execute' do @@ -95,9 +97,7 @@ RSpec.describe Issues::UpdateService, :mailer do end context 'user is a guest' do - before do - project.add_guest(user) - end + let(:user) { guest } it 'does not assign the sentry error' do update_issue(opts) @@ -258,11 +258,7 @@ RSpec.describe Issues::UpdateService, :mailer do context 'from issue to restricted issue types' do context 'without sufficient permissions' do - let(:user) { create(:user) } - - before do - project.add_guest(user) - end + let(:user) { guest } it 'does nothing to the labels' do expect { update_issue(issue_type: 'issue') }.not_to change(issue.labels, :count) @@ -407,12 +403,6 @@ RSpec.describe Issues::UpdateService, :mailer do end context 'when current user cannot admin issues in the project' do - let(:guest) { create(:user) } - - before do - project.add_guest(guest) - end - it 'filters out params that cannot be set without the :admin_issue permission' do described_class.new( project: project, current_user: guest, params: opts.merge( @@ -1113,6 +1103,24 @@ RSpec.describe Issues::UpdateService, :mailer do it_behaves_like 'does not change the severity' end + + context 'as guest' do + let(:user) { guest } + + it_behaves_like 'does not change the severity' + + context 'and also author' do + let(:issue) { create(:incident, project: project, author: user) } + + it_behaves_like 'does not change the severity' + end + + context 'and also assignee' do + let(:issue) { create(:incident, project: project, assignee_ids: [user.id]) } + + it_behaves_like 'does not change the severity' + end + end end context 'when severity has been set before' do diff --git a/workhorse/internal/artifacts/artifacts_upload_test.go b/workhorse/internal/artifacts/artifacts_upload_test.go index 3e8a52be1a1..df1c30dcff0 100644 --- a/workhorse/internal/artifacts/artifacts_upload_test.go +++ b/workhorse/internal/artifacts/artifacts_upload_test.go @@ -270,7 +270,7 @@ func TestUploadHandlerForMultipleFiles(t *testing.T) { require.NoError(t, s.writer.Close()) response := testUploadArtifacts(t, s.writer.FormDataContentType(), s.url, s.buffer) - require.Equal(t, http.StatusInternalServerError, response.Code) + require.Equal(t, http.StatusBadRequest, response.Code) } func TestUploadFormProcessing(t *testing.T) { diff --git a/workhorse/internal/upload/exif/testdata/takes_lot_of_memory_to_decode.tiff b/workhorse/internal/upload/exif/testdata/takes_lot_of_memory_to_decode.tiff Binary files differnew file mode 100644 index 00000000000..6935cb130db --- /dev/null +++ b/workhorse/internal/upload/exif/testdata/takes_lot_of_memory_to_decode.tiff diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go index 7945fa1f34d..3dfab120188 100644 --- a/workhorse/internal/upload/rewrite.go +++ b/workhorse/internal/upload/rewrite.go @@ -25,7 +25,10 @@ import ( ) // ErrInjectedClientParam means that the client sent a parameter that overrides one of our own fields -var ErrInjectedClientParam = errors.New("injected client parameter") +var ( + ErrInjectedClientParam = errors.New("injected client parameter") + ErrMultipleFilesUploaded = errors.New("upload request contains more than one file") +) var ( multipartUploadRequests = promauto.NewCounterVec( @@ -114,6 +117,10 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr } func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipart.Part, opts *filestore.SaveFileOpts) error { + if rew.filter.Count() > 0 { + return ErrMultipleFilesUploaded + } + multipartFiles.WithLabelValues(rew.filter.Name()).Inc() filename := filepath.Base(p.FileName()) @@ -226,7 +233,7 @@ func handleExifUpload(ctx context.Context, r io.Reader, filename string, imageTy } func isTIFF(r io.Reader) bool { - _, err := tiff.Decode(r) + _, err := tiff.DecodeConfig(r) if err == nil { return true } diff --git a/workhorse/internal/upload/rewrite_test.go b/workhorse/internal/upload/rewrite_test.go index 6fc41c3fefd..e3f33a02489 100644 --- a/workhorse/internal/upload/rewrite_test.go +++ b/workhorse/internal/upload/rewrite_test.go @@ -2,6 +2,7 @@ package upload import ( "os" + "runtime" "testing" "github.com/stretchr/testify/require" @@ -29,6 +30,10 @@ func TestImageTypeRecongition(t *testing.T) { filename: "exif/testdata/sample_exif_invalid.jpg", isJPEG: false, isTIFF: false, + }, { + filename: "exif/testdata/takes_lot_of_memory_to_decode.tiff", // File from https://gitlab.com/gitlab-org/gitlab/-/issues/341363 + isJPEG: false, + isTIFF: true, }, } @@ -36,8 +41,16 @@ func TestImageTypeRecongition(t *testing.T) { t.Run(test.filename, func(t *testing.T) { input, err := os.Open(test.filename) require.NoError(t, err) + + var m runtime.MemStats + runtime.ReadMemStats(&m) + start := m.TotalAlloc + require.Equal(t, test.isJPEG, isJPEG(input)) require.Equal(t, test.isTIFF, isTIFF(input)) + + runtime.ReadMemStats(&m) + require.Less(t, m.TotalAlloc-start, uint64(50000), "must take reasonable amount of memory to recognise the type") }) } } diff --git a/workhorse/internal/upload/uploads.go b/workhorse/internal/upload/uploads.go index 7a93fa4a9d8..8f4c8bb95f0 100644 --- a/workhorse/internal/upload/uploads.go +++ b/workhorse/internal/upload/uploads.go @@ -21,6 +21,7 @@ type MultipartFormProcessor interface { ProcessField(ctx context.Context, formName string, writer *multipart.Writer) error Finalize(ctx context.Context) error Name() string + Count() int } func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, preauth *api.Response, filter MultipartFormProcessor, opts *filestore.SaveFileOpts) { @@ -34,6 +35,8 @@ func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, p switch err { case ErrInjectedClientParam: helper.CaptureAndFail(w, r, err, "Bad Request", http.StatusBadRequest) + case ErrMultipleFilesUploaded: + helper.CaptureAndFail(w, r, err, "Uploading multiple files is not allowed", http.StatusBadRequest) case http.ErrNotMultipart: h.ServeHTTP(w, r) case filestore.ErrEntityTooLarge: diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go index 29acf849e05..e82dcdcbc69 100644 --- a/workhorse/internal/upload/uploads_test.go +++ b/workhorse/internal/upload/uploads_test.go @@ -18,7 +18,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/filestore" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" "gitlab.com/gitlab-org/gitlab/workhorse/internal/objectstore/test" "gitlab.com/gitlab-org/gitlab/workhorse/internal/proxy" @@ -28,11 +27,7 @@ import ( var nilHandler = http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) -type testFormProcessor struct{} - -func (a *testFormProcessor) ProcessFile(ctx context.Context, formName string, file *filestore.FileHandler, writer *multipart.Writer) error { - return nil -} +type testFormProcessor struct{ SavedFileTracker } func (a *testFormProcessor) ProcessField(ctx context.Context, formName string, writer *multipart.Writer) error { if formName != "token" && !strings.HasPrefix(formName, "file.") && !strings.HasPrefix(formName, "other.") { @@ -45,10 +40,6 @@ func (a *testFormProcessor) Finalize(ctx context.Context) error { return nil } -func (a *testFormProcessor) Name() string { - return "" -} - func TestUploadTempPathRequirement(t *testing.T) { apiResponse := &api.Response{} preparer := &DefaultPreparer{} @@ -259,6 +250,38 @@ func TestUploadProcessingField(t *testing.T) { require.Equal(t, 500, response.Code) } +func TestUploadingMultipleFiles(t *testing.T) { + testhelper.ConfigureSecret() + + tempPath, err := ioutil.TempDir("", "uploads") + require.NoError(t, err) + defer os.RemoveAll(tempPath) + + var buffer bytes.Buffer + + writer := multipart.NewWriter(&buffer) + _, err = writer.CreateFormFile("file", "my.file") + require.NoError(t, err) + _, err = writer.CreateFormFile("file", "my.file") + require.NoError(t, err) + require.NoError(t, writer.Close()) + + httpRequest, err := http.NewRequest("PUT", "/url/path", &buffer) + require.NoError(t, err) + httpRequest.Header.Set("Content-Type", writer.FormDataContentType()) + + response := httptest.NewRecorder() + apiResponse := &api.Response{TempPath: tempPath} + preparer := &DefaultPreparer{} + opts, _, err := preparer.Prepare(apiResponse) + require.NoError(t, err) + + HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts) + + require.Equal(t, 400, response.Code) + require.Equal(t, "Uploading multiple files is not allowed\n", response.Body.String()) +} + func TestUploadProcessingFile(t *testing.T) { tempPath, err := ioutil.TempDir("", "uploads") require.NoError(t, err) |