diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-01 00:08:16 +0300 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-01 00:08:16 +0300 |
| commit | 32bbedbc214c30979168c8005c83259feb468540 (patch) | |
| tree | 4fdc4817df2ce77440cd4894759100bbb22df806 | |
| parent | e7fb61499317b3c044845817b991e13aea276955 (diff) | |
Add latest changes from gitlab-org/gitlab@master
158 files changed, 2465 insertions, 1443 deletions
diff --git a/.eslintrc.yml b/.eslintrc.yml index f5814639b36..7f45fd912a9 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -93,6 +93,10 @@ rules: group: internal alphabetize: order: ignore + 'no-restricted-syntax': + - error + - selector: ImportSpecifier[imported.name='GlSkeletonLoading'] + message: 'Migrate to GlSkeletonLoader, or import GlDeprecatedSkeletonLoading.' overrides: - files: - '{,ee/,jh/}spec/frontend*/**/*' @@ -107,6 +111,8 @@ overrides: message: 'Using $nextTick from a component instance is discouraged. Import nextTick directly from the Vue package.' - selector: Identifier[name='setImmediate'] message: 'Prefer explicit waitForPromises (or equivalent), or jest.runAllTimers (or equivalent) to vague setImmediate calls.' + - selector: ImportSpecifier[imported.name='GlSkeletonLoading'] + message: 'Migrate to GlSkeletonLoader, or import GlDeprecatedSkeletonLoading.' - files: - 'config/**/*' - 'scripts/**/*' diff --git a/.gitlab/merge_request_templates/Documentation.md b/.gitlab/merge_request_templates/Documentation.md index d3ea9682d34..49d1d0f79bf 100644 --- a/.gitlab/merge_request_templates/Documentation.md +++ b/.gitlab/merge_request_templates/Documentation.md @@ -8,17 +8,17 @@ ## Author's checklist -- [ ] Consider taking [the GitLab Technical Writing Fundamentals course](https://gitlab.edcast.com/pathways/ECL-02528ee2-c334-4e16-abf3-e9d8b8260de4). +- [ ] Optional. Consider taking [the GitLab Technical Writing Fundamentals course](https://gitlab.edcast.com/pathways/ECL-02528ee2-c334-4e16-abf3-e9d8b8260de4). - [ ] Follow the: - [Documentation process](https://docs.gitlab.com/ee/development/documentation/workflow.html). - [Documentation guidelines](https://docs.gitlab.com/ee/development/documentation/). - [Style Guide](https://docs.gitlab.com/ee/development/documentation/styleguide/). -- [ ] Ensure that the [product tier badge](https://docs.gitlab.com/ee/development/documentation/styleguide/index.html#product-tier-badges) is added to topic's `h1`. -- [ ] [Request a review](https://docs.gitlab.com/ee/development/code_review.html#dogfooding-the-reviewers-feature) based on: - - The documentation page's [metadata](https://docs.gitlab.com/ee/development/documentation/#metadata). - - The [associated Technical Writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments). +- [ ] If you're adding or changing the main heading of the page (H1), ensure that the [product tier badge](https://docs.gitlab.com/ee/development/documentation/styleguide/index.html#product-tier-badges) is added. +- [ ] If you are a GitLab team member, [request a review](https://docs.gitlab.com/ee/development/code_review.html#dogfooding-the-attention-request-feature) based on: + - The documentation page's [metadata](https://docs.gitlab.com/ee/development/documentation/#metadata). + - The [associated Technical Writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments). -If you are only adding documentation, do not add any of the following labels: +If you are a GitLab team member and only adding documentation, do not add any of the following labels: - `~"frontend"` - `~"backend"` @@ -27,7 +27,7 @@ If you are only adding documentation, do not add any of the following labels: These labels cause the MR to be added to code verification QA issues. -## Review checklist +## Reviewer's checklist Documentation-related MRs should be reviewed by a Technical Writer for a non-blocking review, based on [Documentation Guidelines](https://docs.gitlab.com/ee/development/documentation/) and the [Style Guide](https://docs.gitlab.com/ee/development/documentation/styleguide/). @@ -35,13 +35,13 @@ Documentation-related MRs should be reviewed by a Technical Writer for a non-blo - Technical writer review items: - [ ] Ensure docs metadata is present and up-to-date. - [ ] Ensure the appropriate [labels](https://about.gitlab.com/handbook/engineering/ux/technical-writing/workflow/#labels) are added to this MR. + - [ ] Ensure a release milestone is set. - If relevant to this MR, ensure [content topic type](https://docs.gitlab.com/ee/development/documentation/structure.html) principles are in use, including: - [ ] The headings should be something you'd do a Google search for. Instead of `Default behavior`, say something like `Default behavior when you close an issue`. - [ ] The headings (other than the page title) should be active. Instead of `Configuring GDK`, say something like `Configure GDK`. - [ ] Any task steps should be written as a numbered list. - If the content still needs to be edited for topic types, you can create a follow-up issue with the ~"docs-technical-debt" label. - [ ] Review by assigned maintainer, who can always request/require the reviews above. Maintainer's review can occur before or after a technical writer review. -- [ ] Ensure a release milestone is set. -/label ~documentation ~"type::maintenance" +/label ~documentation ~"type::maintenance" ~"docs::improvement" /assign me diff --git a/CHANGELOG.md b/CHANGELOG.md index 529a3904a0a..add5cacce7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,31 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.9.2 (2022-03-31) + +### Security (20 changes) + +- [Quarantine UsageDataNonSqlMetrics failing test](gitlab-org/security/gitlab@123fc00ff9f407284ce05007ddc373e1bd0aeede) ([merge request](gitlab-org/security/gitlab!2364)) +- [Disallow login if password matches a fixed list](gitlab-org/security/gitlab@1a128ae3fb17b3d83974bb08034e4ba7a7d54e3b) ([merge request](gitlab-org/security/gitlab!2357)) +- [Update devise-two-factor to 4.0.2](gitlab-org/security/gitlab@17c70b13dcd437c05de63b3286245af8e6f42210) ([merge request](gitlab-org/security/gitlab!2349)) +- [Limit the number of tags associated with a CI runner](gitlab-org/security/gitlab@ed5daced882a0206e050c4f676a888ac1c2417b1) ([merge request](gitlab-org/security/gitlab!2303)) +- [GitLab Pages Security Updates for 14.9](gitlab-org/security/gitlab@79709cabf71a57a336f490636a7e32a208fe0229) ([merge request](gitlab-org/security/gitlab!2327)) +- [Upgrade swagger-ui dependency](gitlab-org/security/gitlab@14280c1d844be3ffc2f30f5321a818a7b6c51770) ([merge request](gitlab-org/security/gitlab!2336)) +- [Modify release link format check to avoid regex if string is too long](gitlab-org/security/gitlab@f516d883b46e1441410476dc140d69fde51cdf0f) ([merge request](gitlab-org/security/gitlab!2307)) +- [Masks variables in error messages](gitlab-org/security/gitlab@9cf62118390c0cfba3d36a4231a30a7836f06e2f) ([merge request](gitlab-org/security/gitlab!2308)) +- [Escape user provided string to prevent XSS](gitlab-org/security/gitlab@2da3502aef64ed1b01c13d82418950cf284098c6) ([merge request](gitlab-org/security/gitlab!2313)) +- [Monkey patch of RDoc to prevent Ruby segfault](gitlab-org/security/gitlab@0ae4925089a1b5fd7c9abeeb0756b3a50e05799a) ([merge request](gitlab-org/security/gitlab!2321)) +- [Project import maps members' created_by_id users based on source user ID](gitlab-org/security/gitlab@3826f2a7c652d3f74e45bfef8888601ca1c86ba1) ([merge request](gitlab-org/security/gitlab!2301)) +- [Redact InvalidURIError error messages](gitlab-org/security/gitlab@59b60e9cf8f79d6f41000d34a4434c5a04988030) ([merge request](gitlab-org/security/gitlab!2295)) +- [Fix access for approval rules API](gitlab-org/security/gitlab@7890215aa29624cd67c5bc8ac25175f2866479b7) ([merge request](gitlab-org/security/gitlab!2322)) +- [Fix kroki exploit](gitlab-org/security/gitlab@b2a44b407ab85ca056a271ba4e708128ef08d25f) ([merge request](gitlab-org/security/gitlab!2306)) +- [Fix blind SSRF when looking up SSH host keys for mirroring](gitlab-org/security/gitlab@5a9509b52584302c508bd6dff1454f80aae371ea) ([merge request](gitlab-org/security/gitlab!2309)) +- [Escape original content in reference redactor](gitlab-org/security/gitlab@b33b170a2c2df8285999f3631e8a53d35e0eed22) ([merge request](gitlab-org/security/gitlab!2317)) +- [Security fix for CI/CD analytics visibility](gitlab-org/security/gitlab@f3febd00b440475b2aca0b9bd6728fa5f8750288) ([merge request](gitlab-org/security/gitlab!2304)) +- [Latest commit exposed through fork of a private project](gitlab-org/security/gitlab@3f20d4f294a12ceb33bec19d86790f582fb7fb48) ([merge request](gitlab-org/security/gitlab!2294)) +- [Fix Asana integration restricted branch filter](gitlab-org/security/gitlab@08aa0f55b1b715f7311ee6502cd6f8a1b875f878) ([merge request](gitlab-org/security/gitlab!2300)) +- [Revert "JH need more complex passwords"](gitlab-org/security/gitlab@e2fb87ec5d4e235d6b83454980cec9c049849a1c) ([merge request](gitlab-org/security/gitlab!2352)) + ## 14.9.1 (2022-03-23) ### Fixed (1 change) @@ -604,6 +629,32 @@ entry. - [Clean up issue_boards_filtered_search feature flag](gitlab-org/gitlab@a97ed09ffb0d88007b21a314ab48b2e50d7c4bfa) ([merge request](gitlab-org/gitlab!80771)) - [Add table for storing issue tsvector](gitlab-org/gitlab@ceabf5a8ad0d67768b05a58a84b242495645a57c) ([merge request](gitlab-org/gitlab!71913)) +## 14.8.5 (2022-03-31) + +### Security (21 changes) + +- [Update to commonmarker 0.23.4](gitlab-org/security/gitlab@51532ccc5f1b6b053d4ca6c54496607e62f8f25c) ([merge request](gitlab-org/security/gitlab!2282)) +- [Revert merge request approval groups behavior](gitlab-org/security/gitlab@dd9724e429033974da6c3852dc6fd33f0f2b0a46) ([merge request](gitlab-org/security/gitlab!2334)) +- [Disallow login if password matches a fixed list](gitlab-org/security/gitlab@6779d5f2948425a7ad7f19a6e10f82cc10b80989) ([merge request](gitlab-org/security/gitlab!2358)) +- [Update devise-two-factor to 4.0.2](gitlab-org/security/gitlab@0329d2d82a9064c0bae36e7b993ee40df7c999bc) ([merge request](gitlab-org/security/gitlab!2350)) +- [Limit the number of tags associated with a CI runner](gitlab-org/security/gitlab@8d5938c08fe66c22f1bc54ff76cc9daf2de86b1a) ([merge request](gitlab-org/security/gitlab!2302)) +- [GitLab Pages Security Updates for 14.9](gitlab-org/security/gitlab@5a5a862c8a9e37ca2ea84133f92b216eaa7cd148) ([merge request](gitlab-org/security/gitlab!2328)) +- [Upgrade swagger-ui dependency](gitlab-org/security/gitlab@afcb570867db61347bb6a4e243bb2557340191be) ([merge request](gitlab-org/security/gitlab!2337)) +- [Modify release link format check to avoid regex if string is too long](gitlab-org/security/gitlab@a3ab0ff9c470c1c6e5b4fd055ddd02dffce32652) ([merge request](gitlab-org/security/gitlab!2243)) +- [Masks variables in error messages](gitlab-org/security/gitlab@94236bbdb8eef6600562bdc4e242e07eaed8c50f) ([merge request](gitlab-org/security/gitlab!2291)) +- [Escape user provided string to prevent XSS](gitlab-org/security/gitlab@03e695d4c34546582b503b3f7712246206b56b99) ([merge request](gitlab-org/security/gitlab!2314)) +- [Monkey patch of RDoc to prevent Ruby segfault](gitlab-org/security/gitlab@14eec4487387bc0c999f1c48b046a3ed3848c5a1) ([merge request](gitlab-org/security/gitlab!2232)) +- [Project import maps members' created_by_id users based on source user ID](gitlab-org/security/gitlab@7fd7ab3f57e8d8b4e0aed42aebe9a8b7436a6255) ([merge request](gitlab-org/security/gitlab!2238)) +- [Redact InvalidURIError error messages](gitlab-org/security/gitlab@0592c182bfd60aee501c4c66f47a71c9469f2bcd) ([merge request](gitlab-org/security/gitlab!2296)) +- [Fix access for approval rules API](gitlab-org/security/gitlab@987e06bacba224519adf94cda73b5a8b2e7b917a) ([merge request](gitlab-org/security/gitlab!2323)) +- [Fix kroki exploit](gitlab-org/security/gitlab@bf056c683af25ec4b94c0efa7166eea399ed6502) ([merge request](gitlab-org/security/gitlab!2277)) +- [Fix blind SSRF when looking up SSH host keys for mirroring](gitlab-org/security/gitlab@3c853a32a73aba15e309d05111b744455a360cca) ([merge request](gitlab-org/security/gitlab!2310)) +- [Escape original content in reference redactor](gitlab-org/security/gitlab@00ee99bc3834d9d59572272064c9ad6abeae5975) ([merge request](gitlab-org/security/gitlab!2318)) +- [Security fix for CI/CD analytics visibility](gitlab-org/security/gitlab@691d69be77ae3c8e0a2598b75ccf336b672fd540) ([merge request](gitlab-org/security/gitlab!2273)) +- [Latest commit exposed through fork of a private project](gitlab-org/security/gitlab@6ca7a3b040edac06b23a697bfc2bf46f457d6b81) ([merge request](gitlab-org/security/gitlab!2271)) +- [Fix Asana integration restricted branch filter](gitlab-org/security/gitlab@4c1db692b4e99fab6cdbb818cf02fb879f6d4886) ([merge request](gitlab-org/security/gitlab!2218)) +- [Revert "JH need more complex passwords"](gitlab-org/security/gitlab@919aa2b28645d49fb71508362a0c61da39893c69) ([merge request](gitlab-org/security/gitlab!2353)) + ## 14.8.4 (2022-03-16) ### Added (1 change) @@ -1319,6 +1370,32 @@ entry. - [Use `ssh_data` gem instead of `net-ssh` and `sshkey` where possible](gitlab-org/gitlab@59a0ee8605d509753c9aec719f8e0da77bcc679d) ([merge request](gitlab-org/gitlab!77424)) - [Remove feature flag already default enabled](gitlab-org/gitlab@9b7059a4bf9dc2ecdce1910a931cc6967d05b5ad) ([merge request](gitlab-org/gitlab!78238)) **GitLab Enterprise Edition** +## 14.7.7 (2022-03-31) + +### Security (21 changes) + +- [Update to commonmarker 0.23.4](gitlab-org/security/gitlab@eb4b231173c86901f93b5b7781716b1f7706dad1) ([merge request](gitlab-org/security/gitlab!2283)) +- [Revert merge request approval groups behavior](gitlab-org/security/gitlab@08e3ecced649f6ad241db6de7050b1502f7bef21) ([merge request](gitlab-org/security/gitlab!2333)) +- [Disallow login if password matches a fixed list](gitlab-org/security/gitlab@02a69ab32da1ac67d855de3ee388d0bd2bb6586e) ([merge request](gitlab-org/security/gitlab!2359)) +- [Update devise-two-factor to 4.0.2](gitlab-org/security/gitlab@c9fde96c7780f5b883cd1ac63d7ac3d5f4d78dc6) ([merge request](gitlab-org/security/gitlab!2351)) +- [Limit the number of tags associated with a CI runner](gitlab-org/security/gitlab@00124d5f8ba0d7437d1f6f19b029754bf481185b) ([merge request](gitlab-org/security/gitlab!2305)) +- [GitLab Pages Security Updates for 14.9](gitlab-org/security/gitlab@d335917e233658fa9d4452053469c3582ef38368) ([merge request](gitlab-org/security/gitlab!2325)) +- [Upgrade swagger-ui dependency](gitlab-org/security/gitlab@7a8ce32f70fd0338817705651ee0dbe0a277d5f1) ([merge request](gitlab-org/security/gitlab!2338)) +- [Modify release link format check to avoid regex if string is too long](gitlab-org/security/gitlab@e18dc2be245bca7e192c8536d1ba7de2ad798c43) ([merge request](gitlab-org/security/gitlab!2244)) +- [Masks variables in error messages](gitlab-org/security/gitlab@1706c5cf9b939a6ab0682db7b8945feb851a3f8b) ([merge request](gitlab-org/security/gitlab!2292)) +- [Escape user provided string to prevent XSS](gitlab-org/security/gitlab@c57edf9ab52810d455e41d71bad4e4d12c098cad) ([merge request](gitlab-org/security/gitlab!2315)) +- [Monkey patch of RDoc to prevent Ruby segfault](gitlab-org/security/gitlab@f9e5597d1864d03bf1f0103787becbc84886968d) ([merge request](gitlab-org/security/gitlab!2233)) +- [Project import maps members' created_by_id users based on source user ID](gitlab-org/security/gitlab@3ea1e477e0596f15e040f42b59fa86953d057128) ([merge request](gitlab-org/security/gitlab!2239)) +- [Redact InvalidURIError error messages](gitlab-org/security/gitlab@a42ede835e32f44b68c1affe78a7ee48332bb30a) ([merge request](gitlab-org/security/gitlab!2297)) +- [Fix access for approval rules API](gitlab-org/security/gitlab@b8c3997763d1e041dc2b82e464a99a5b2f15a798) ([merge request](gitlab-org/security/gitlab!2324)) +- [Fix kroki exploit](gitlab-org/security/gitlab@ad123e33510103af4fb00378ef1fc8dae4cacb21) ([merge request](gitlab-org/security/gitlab!2278)) +- [Fix blind SSRF when looking up SSH host keys for mirroring](gitlab-org/security/gitlab@0209f44cb4876f0a9ef13d4c8875a95a0cda1e2f) ([merge request](gitlab-org/security/gitlab!2311)) +- [Escape original content in reference redactor](gitlab-org/security/gitlab@f63861d8fe7b2b8d161162063e7995782cbfada8) ([merge request](gitlab-org/security/gitlab!2319)) +- [Security fix for CI/CD analytics visibility](gitlab-org/security/gitlab@fea6a4ff80862f9dba493405d03d82cf129e8854) ([merge request](gitlab-org/security/gitlab!2274)) +- [Latest commit exposed through fork of a private project](gitlab-org/security/gitlab@b573cea38cdce020e5f25fb9de60e0e506c87a9b) ([merge request](gitlab-org/security/gitlab!2272)) +- [Fix Asana integration restricted branch filter](gitlab-org/security/gitlab@56e2d9ae3de4f587d2c8a5aa111c2922553d6b7b) ([merge request](gitlab-org/security/gitlab!2214)) +- [Revert "JH need more complex passwords"](gitlab-org/security/gitlab@2419522b02700ce98e0c4d6e7bfd4d28b6464506) ([merge request](gitlab-org/security/gitlab!2354)) + ## 14.7.6 (2022-03-24) ### Added (1 change) diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 3ebf789f5a8..43c989b5531 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.56.0 +1.56.1 @@ -67,7 +67,7 @@ gem 'akismet', '~> 3.0' gem 'invisible_captcha', '~> 1.1.0' # Two-factor authentication -gem 'devise-two-factor', '~> 4.0.0' +gem 'devise-two-factor', '~> 4.0.2' gem 'rqrcode-rails3', '~> 0.1.7' gem 'attr_encrypted', '~> 3.1.0' gem 'u2f', '~> 0.2.1' diff --git a/Gemfile.lock b/Gemfile.lock index 7fc00cd0004..344810f92a2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -270,11 +270,11 @@ GEM railties (>= 4.1.0) responders warden (~> 1.2.3) - devise-two-factor (4.0.0) - activesupport (< 6.2) + devise-two-factor (4.0.2) + activesupport (< 7.1) attr_encrypted (>= 1.3, < 4, != 2) devise (~> 4.0) - railties (< 6.2) + railties (< 7.1) rotp (~> 6.0) diff-lcs (1.4.4) diff_match_patch (0.1.0) @@ -1458,7 +1458,7 @@ DEPENDENCIES derailed_benchmarks device_detector devise (~> 4.7.2) - devise-two-factor (~> 4.0.0) + devise-two-factor (~> 4.0.2) diff_match_patch (~> 0.1.0) diffy (~> 3.3) discordrb-webhooks (~> 3.4) diff --git a/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue b/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue new file mode 100644 index 00000000000..31b2682b546 --- /dev/null +++ b/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue @@ -0,0 +1,25 @@ +<script> +import { GlAlert } from '@gitlab/ui'; +import { __ } from '~/locale'; + +export default { + i18n: { + bodyText: __('Warning: Displaying this diagram might cause performance issues on this page.'), + buttonText: __('Display'), + }, + components: { + GlAlert, + }, +}; +</script> + +<template> + <gl-alert + :primary-button-text="$options.i18n.buttonText" + variant="warning" + @dismiss="$emit('closeAlert')" + @primaryAction="$emit('showImage')" + > + {{ $options.i18n.bodyText }} + </gl-alert> +</template> diff --git a/app/assets/javascripts/behaviors/markdown/constants.js b/app/assets/javascripts/behaviors/markdown/constants.js index b4545d6c6c6..13f8d9ef0cf 100644 --- a/app/assets/javascripts/behaviors/markdown/constants.js +++ b/app/assets/javascripts/behaviors/markdown/constants.js @@ -1,3 +1,19 @@ // https://prosemirror.net/docs/ref/#model.ParseRule.priority export const DEFAULT_PARSE_RULE_PRIORITY = 50; export const HIGHER_PARSE_RULE_PRIORITY = 1 + DEFAULT_PARSE_RULE_PRIORITY; + +export const unrestrictedPages = [ + // Group wiki + 'groups:wikis:show', + 'groups:wikis:edit', + 'groups:wikis:create', + + // Project wiki + 'projects:wikis:show', + 'projects:wikis:edit', + 'projects:wikis:create', + + // Project files + 'projects:show', + 'projects:blob:show', +]; diff --git a/app/assets/javascripts/behaviors/markdown/render_gfm.js b/app/assets/javascripts/behaviors/markdown/render_gfm.js index 4bfce12c7c5..5079da9aa02 100644 --- a/app/assets/javascripts/behaviors/markdown/render_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/render_gfm.js @@ -1,6 +1,7 @@ import $ from 'jquery'; import syntaxHighlight from '~/syntax_highlight'; import highlightCurrentUser from './highlight_current_user'; +import { renderKroki } from './render_kroki'; import renderMath from './render_math'; import renderMermaid from './render_mermaid'; import renderSandboxedMermaid from './render_sandboxed_mermaid'; @@ -12,6 +13,7 @@ import renderMetrics from './render_metrics'; // $.fn.renderGFM = function renderGFM() { syntaxHighlight(this.find('.js-syntax-highlight').get()); + renderKroki(this.find('.js-render-kroki[hidden]').get()); renderMath(this.find('.js-render-math')); if (gon.features?.sandboxedMermaid) { renderSandboxedMermaid(this.find('.js-render-mermaid')); diff --git a/app/assets/javascripts/behaviors/markdown/render_kroki.js b/app/assets/javascripts/behaviors/markdown/render_kroki.js new file mode 100644 index 00000000000..abe71694d73 --- /dev/null +++ b/app/assets/javascripts/behaviors/markdown/render_kroki.js @@ -0,0 +1,63 @@ +import Vue from 'vue'; +import DiagramPerformanceWarning from '../components/diagram_performance_warning.vue'; +import { unrestrictedPages } from './constants'; + +/** + * Create alert element. + * + * @param {Element} krokiImage Kroki `img` element + * @return {Element} Alert element + */ +function createAlert(krokiImage) { + const app = new Vue({ + el: document.createElement('div'), + name: 'DiagramPerformanceWarningRoot', + render(createElement) { + return createElement(DiagramPerformanceWarning, { + on: { + closeAlert() { + app.$destroy(); + app.$el.remove(); + }, + showImage() { + krokiImage.removeAttribute('hidden'); + app.$destroy(); + app.$el.remove(); + }, + }, + }); + }, + }); + + return app.$el; +} + +/** + * Add warning alert to hidden Kroki images, + * or show Kroki image if on an unrestricted page. + * + * Kroki images are given a hidden attribute by the + * backend when the original markdown source is large. + * + * @param {Array<Element>} krokiImages Array of hidden Kroki `img` elements + */ +export function renderKroki(krokiImages) { + const pageName = document.querySelector('body').dataset.page; + const isUnrestrictedPage = unrestrictedPages.includes(pageName); + + krokiImages.forEach((krokiImage) => { + if (isUnrestrictedPage) { + krokiImage.removeAttribute('hidden'); + return; + } + + const parent = krokiImage.parentElement; + + // A single Kroki image is processed multiple times for some reason, + // so this condition ensures we only create one alert per Kroki image + if (!parent.hasAttribute('data-kroki-processed')) { + parent.setAttribute('data-kroki-processed', 'true'); + parent.after(createAlert(krokiImage)); + } + }); +} diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index d78c456ed5b..f9cf3af98bb 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -3,6 +3,7 @@ import { once, countBy } from 'lodash'; import createFlash from '~/flash'; import { darkModeEnabled } from '~/lib/utils/color_utils'; import { __, sprintf } from '~/locale'; +import { unrestrictedPages } from './constants'; // Renders diagrams and flowcharts from text using Mermaid in any element with the // `js-render-mermaid` class. @@ -30,24 +31,6 @@ let renderedMermaidBlocks = 0; let mermaidModule = {}; -// Whitelist pages where we won't impose any restrictions -// on mermaid rendering -const WHITELISTED_PAGES = [ - // Group wiki - 'groups:wikis:show', - 'groups:wikis:edit', - 'groups:wikis:create', - - // Project wiki - 'projects:wikis:show', - 'projects:wikis:edit', - 'projects:wikis:create', - - // Project files - 'projects:show', - 'projects:blob:show', -]; - export function initMermaid(mermaid) { let theme = 'neutral'; @@ -163,7 +146,7 @@ function renderMermaids($els) { * up the entire thread and causing a DoS. */ if ( - !WHITELISTED_PAGES.includes(pageName) && + !unrestrictedPages.includes(pageName) && ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT || renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT || diff --git a/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js index 85a991a1ec9..6922ec9c5a5 100644 --- a/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js @@ -9,6 +9,7 @@ import { } from '~/lib/utils/url_utility'; import { darkModeEnabled } from '~/lib/utils/color_utils'; import { setAttributes } from '~/lib/utils/dom_utils'; +import { unrestrictedPages } from './constants'; // Renders diagrams and flowcharts from text using Mermaid in any element with the // `js-render-mermaid` class. @@ -36,23 +37,6 @@ const BUFFER_IFRAME_HEIGHT = 10; const elsProcessingMap = new WeakMap(); let renderedMermaidBlocks = 0; -// Pages without any restrictions on mermaid rendering -const PAGES_WITHOUT_RESTRICTIONS = [ - // Group wiki - 'groups:wikis:show', - 'groups:wikis:edit', - 'groups:wikis:create', - - // Project wiki - 'projects:wikis:show', - 'projects:wikis:edit', - 'projects:wikis:create', - - // Project files - 'projects:show', - 'projects:blob:show', -]; - function shouldLazyLoadMermaidBlock(source) { /** * If source contains `&`, which means that it might @@ -149,7 +133,7 @@ function renderMermaids($els) { * up the entire thread and causing a DoS. */ if ( - !PAGES_WITHOUT_RESTRICTIONS.includes(pageName) && + !unrestrictedPages.includes(pageName) && ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT || renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT || diff --git a/app/assets/javascripts/blob/openapi/index.js b/app/assets/javascripts/blob/openapi/index.js index b19cc19cb8c..a04da98ff77 100644 --- a/app/assets/javascripts/blob/openapi/index.js +++ b/app/assets/javascripts/blob/openapi/index.js @@ -1,6 +1,5 @@ import { SwaggerUIBundle } from 'swagger-ui-dist'; import createFlash from '~/flash'; -import { removeParams, updateHistory } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; export default () => { @@ -8,14 +7,10 @@ export default () => { Promise.all([import(/* webpackChunkName: 'openapi' */ 'swagger-ui-dist/swagger-ui.css')]) .then(() => { - // Temporary fix to prevent an XSS attack due to "useUnsafeMarkdown" - // Once we upgrade Swagger to "4.0.0", we can safely remove this as it will be deprecated - // Follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/339696 - updateHistory({ url: removeParams(['useUnsafeMarkdown']), replace: true }); SwaggerUIBundle({ url: el.dataset.endpoint, dom_id: '#js-openapi-viewer', - useUnsafeMarkdown: false, + deepLinking: true, }); }) .catch((error) => { diff --git a/app/assets/javascripts/crm/components/contact_form.vue b/app/assets/javascripts/crm/components/contact_form.vue deleted file mode 100644 index 81ae5c246be..00000000000 --- a/app/assets/javascripts/crm/components/contact_form.vue +++ /dev/null @@ -1,224 +0,0 @@ -<script> -import { GlAlert, GlButton, GlDrawer, GlFormGroup, GlFormInput } from '@gitlab/ui'; -import { produce } from 'immer'; -import { __, s__ } from '~/locale'; -import { convertToGraphQLId } from '~/graphql_shared/utils'; -import { TYPE_GROUP } from '~/graphql_shared/constants'; -import createContactMutation from './queries/create_contact.mutation.graphql'; -import updateContactMutation from './queries/update_contact.mutation.graphql'; -import getGroupContactsQuery from './queries/get_group_contacts.query.graphql'; - -export default { - components: { - GlAlert, - GlButton, - GlDrawer, - GlFormGroup, - GlFormInput, - }, - inject: ['groupFullPath', 'groupId'], - props: { - drawerOpen: { - type: Boolean, - required: true, - }, - contact: { - type: Object, - required: false, - default: () => {}, - }, - }, - data() { - return { - firstName: '', - lastName: '', - phone: '', - email: '', - description: '', - submitting: false, - errorMessages: [], - }; - }, - computed: { - invalid() { - const { firstName, lastName, email } = this; - - return firstName.trim() === '' || lastName.trim() === '' || email.trim() === ''; - }, - editMode() { - return Boolean(this.contact); - }, - title() { - return this.editMode ? this.$options.i18n.editTitle : this.$options.i18n.newTitle; - }, - buttonLabel() { - return this.editMode - ? this.$options.i18n.editButtonLabel - : this.$options.i18n.createButtonLabel; - }, - mutation() { - return this.editMode ? updateContactMutation : createContactMutation; - }, - variables() { - const { contact, firstName, lastName, phone, email, description, editMode, groupId } = this; - - const variables = { - input: { - firstName, - lastName, - phone, - email, - description, - }, - }; - - if (editMode) { - variables.input.id = contact.id; - } else { - variables.input.groupId = convertToGraphQLId(TYPE_GROUP, groupId); - } - - return variables; - }, - }, - mounted() { - if (this.editMode) { - const { contact } = this; - - this.firstName = contact.firstName || ''; - this.lastName = contact.lastName || ''; - this.phone = contact.phone || ''; - this.email = contact.email || ''; - this.description = contact.description || ''; - } - }, - methods: { - save() { - const { mutation, variables, updateCache, close } = this; - - this.submitting = true; - - return this.$apollo - .mutate({ - mutation, - variables, - update: updateCache, - }) - .then(({ data }) => { - if ( - data.customerRelationsContactCreate?.errors.length === 0 || - data.customerRelationsContactUpdate?.errors.length === 0 - ) { - close(true); - } - - this.submitting = false; - }) - .catch(() => { - this.errorMessages = [this.$options.i18n.somethingWentWrong]; - this.submitting = false; - }); - }, - close(success) { - this.$emit('close', success); - }, - updateCache(store, { data }) { - const mutationData = - data.customerRelationsContactCreate || data.customerRelationsContactUpdate; - - if (mutationData?.errors.length > 0) { - this.errorMessages = mutationData.errors; - return; - } - - const queryArgs = { - query: getGroupContactsQuery, - variables: { groupFullPath: this.groupFullPath }, - }; - - const sourceData = store.readQuery(queryArgs); - - queryArgs.data = produce(sourceData, (draftState) => { - draftState.group.contacts.nodes = [ - ...sourceData.group.contacts.nodes.filter(({ id }) => id !== this.contact?.id), - mutationData.contact, - ]; - }); - - store.writeQuery(queryArgs); - }, - getDrawerHeaderHeight() { - const wrapperEl = document.querySelector('.content-wrapper'); - - if (wrapperEl) { - return `${wrapperEl.offsetTop}px`; - } - - return ''; - }, - }, - i18n: { - createButtonLabel: s__('Crm|Create new contact'), - editButtonLabel: __('Save changes'), - cancel: __('Cancel'), - firstName: s__('Crm|First name'), - lastName: s__('Crm|Last name'), - email: s__('Crm|Email'), - phone: s__('Crm|Phone number (optional)'), - description: s__('Crm|Description (optional)'), - newTitle: s__('Crm|New contact'), - editTitle: s__('Crm|Edit contact'), - somethingWentWrong: __('Something went wrong. Please try again.'), - }, -}; -</script> - -<template> - <gl-drawer - class="gl-drawer-responsive" - :open="drawerOpen" - :header-height="getDrawerHeaderHeight()" - @close="close(false)" - > - <template #title> - <h3>{{ title }}</h3> - </template> - <gl-alert v-if="errorMessages.length" variant="danger" @dismiss="errorMessages = []"> - <ul class="gl-mb-0! gl-ml-5"> - <li v-for="error in errorMessages" :key="error"> - {{ error }} - </li> - </ul> - </gl-alert> - <form @submit.prevent="save"> - <gl-form-group :label="$options.i18n.firstName" label-for="contact-first-name"> - <gl-form-input id="contact-first-name" v-model="firstName" /> - </gl-form-group> - <gl-form-group :label="$options.i18n.lastName" label-for="contact-last-name"> - <gl-form-input id="contact-last-name" v-model="lastName" /> - </gl-form-group> - <gl-form-group :label="$options.i18n.email" label-for="contact-email"> - <gl-form-input id="contact-email" v-model="email" /> - </gl-form-group> - <gl-form-group :label="$options.i18n.phone" label-for="contact-phone"> - <gl-form-input id="contact-phone" v-model="phone" /> - </gl-form-group> - <gl-form-group :label="$options.i18n.description" label-for="contact-description"> - <gl-form-input id="contact-description" v-model="description" /> - </gl-form-group> - <span class="gl-float-right"> - <gl-button data-testid="cancel-button" @click="close(false)"> - {{ $options.i18n.cancel }} - </gl-button> - <gl-button - variant="confirm" - :disabled="invalid" - :loading="submitting" - data-testid="save-contact-button" - type="submit" - >{{ buttonLabel }}</gl-button - > - </span> - </form> - </gl-drawer> -</template> diff --git a/app/assets/javascripts/crm/components/form.vue b/app/assets/javascripts/crm/components/form.vue index b24de1e95e8..4f94898ff63 100644 --- a/app/assets/javascripts/crm/components/form.vue +++ b/app/assets/javascripts/crm/components/form.vue @@ -61,11 +61,6 @@ export default { required: false, default: null, }, - existingModel: { - type: Object, - required: false, - default: () => ({}), - }, additionalCreateParams: { type: Object, required: false, @@ -76,25 +71,42 @@ export default { required: false, default: () => MSG_SAVE_CHANGES, }, + existingId: { + type: String, + required: false, + default: null, + }, }, data() { - const initialModel = this.fields.reduce( - (map, field) => - Object.assign(map, { - [field.name]: this.existingModel ? this.existingModel[field.name] : null, - }), - {}, - ); - return { - model: initialModel, + model: null, submitting: false, errorMessages: [], + records: [], + loading: true, }; }, + apollo: { + records: { + query() { + return this.getQuery.query; + }, + variables() { + return this.getQuery.variables; + }, + update(data) { + this.records = getPropValueByPath(data, this.getQueryNodePath).nodes || []; + this.setInitialModel(); + this.loading = false; + }, + error() { + this.errorMessages = [MSG_ERROR]; + }, + }, + }, computed: { isEditMode() { - return this.existingModel?.id; + return this.existingId; }, isInvalid() { const { fields, model } = this; @@ -115,13 +127,24 @@ export default { ); if (isEditMode) { - return { input: { id: this.existingModel.id, ...variables } }; + return { input: { id: this.existingId, ...variables } }; } return { input: { ...additionalCreateParams, ...variables } }; }, }, methods: { + setInitialModel() { + const existingModel = this.records.find(({ id }) => id === this.existingId); + + this.model = this.fields.reduce( + (map, field) => + Object.assign(map, { + [field.name]: !this.isEditMode || !existingModel ? null : existingModel[field.name], + }), + {}, + ); + }, formatValue(model, field) { if (!isEmpty(model[field.name]) && field.input?.type === 'number') { return parseFloat(model[field.name]); @@ -173,7 +196,7 @@ export default { const sourceData = store.readQuery(getQuery); const newData = produce(sourceData, (draftState) => { - getPropValueByPath(draftState, getQueryNodePath).nodes.push(getFirstPropertyValue(result)); + getPropValueByPath(draftState, getQueryNodePath).nodes.push(this.getPayload(result)); }); store.writeQuery({ @@ -185,6 +208,14 @@ export default { const optionalSuffix = field.required ? '' : ` ${MSG_OPTIONAL}`; return field.label + optionalSuffix; }, + getPayload(data) { + if (!data) return null; + + const keys = Object.keys(data); + if (keys[0] === '__typename') return data[keys[1]]; + + return data[keys[0]]; + }, }, MSG_CANCEL, INDEX_ROUTE_NAME, @@ -192,7 +223,7 @@ export default { </script> <template> - <mounting-portal mount-to="#js-crm-form-portal" append> + <mounting-portal v-if="!loading" mount-to="#js-crm-form-portal" append> <gl-drawer class="gl-drawer-responsive gl-absolute" :open="drawerOpen" @close="close(false)"> <template #title> <h3>{{ title }}</h3> diff --git a/app/assets/javascripts/crm/components/new_organization_form.vue b/app/assets/javascripts/crm/components/new_organization_form.vue deleted file mode 100644 index 3b11edc6935..00000000000 --- a/app/assets/javascripts/crm/components/new_organization_form.vue +++ /dev/null @@ -1,164 +0,0 @@ -<script> -import { GlAlert, GlButton, GlDrawer, GlFormGroup, GlFormInput } from '@gitlab/ui'; -import { produce } from 'immer'; -import { __, s__ } from '~/locale'; -import { convertToGraphQLId } from '~/graphql_shared/utils'; -import { TYPE_GROUP } from '~/graphql_shared/constants'; -import createOrganization from './queries/create_organization.mutation.graphql'; -import getGroupOrganizationsQuery from './queries/get_group_organizations.query.graphql'; - -export default { - components: { - GlAlert, - GlButton, - GlDrawer, - GlFormGroup, - GlFormInput, - }, - inject: ['groupFullPath', 'groupId'], - props: { - drawerOpen: { - type: Boolean, - required: true, - }, - }, - data() { - return { - name: '', - defaultRate: null, - description: '', - submitting: false, - errorMessages: [], - }; - }, - computed: { - invalid() { - return this.name.trim() === ''; - }, - }, - methods: { - save() { - this.submitting = true; - return this.$apollo - .mutate({ - mutation: createOrganization, - variables: { - input: { - groupId: convertToGraphQLId(TYPE_GROUP, this.groupId), - name: this.name, - defaultRate: this.defaultRate ? parseFloat(this.defaultRate) : null, - description: this.description, - }, - }, - update: this.updateCache, - }) - .then(({ data }) => { - if (data.customerRelationsOrganizationCreate.errors.length === 0) this.close(true); - - this.submitting = false; - }) - .catch(() => { - this.errorMessages = [this.$options.i18n.somethingWentWrong]; - this.submitting = false; - }); - }, - close(success) { - this.$emit('close', success); - }, - updateCache(store, { data: { customerRelationsOrganizationCreate } }) { - if (customerRelationsOrganizationCreate.errors.length > 0) { - this.errorMessages = customerRelationsOrganizationCreate.errors; - return; - } - - const variables = { - groupFullPath: this.groupFullPath, - }; - const sourceData = store.readQuery({ - query: getGroupOrganizationsQuery, - variables, - }); - - const data = produce(sourceData, (draftState) => { - draftState.group.organizations.nodes = [ - ...sourceData.group.organizations.nodes, - customerRelationsOrganizationCreate.organization, - ]; - }); - - store.writeQuery({ - query: getGroupOrganizationsQuery, - variables, - data, - }); - }, - getDrawerHeaderHeight() { - const wrapperEl = document.querySelector('.content-wrapper'); - - if (wrapperEl) { - return `${wrapperEl.offsetTop}px`; - } - - return ''; - }, - }, - i18n: { - buttonLabel: s__('Crm|Create organization'), - cancel: __('Cancel'), - name: __('Name'), - defaultRate: s__('Crm|Default rate (optional)'), - description: __('Description (optional)'), - title: s__('Crm|New Organization'), - somethingWentWrong: __('Something went wrong. Please try again.'), - }, -}; -</script> - -<template> - <gl-drawer - class="gl-drawer-responsive" - :open="drawerOpen" - :header-height="getDrawerHeaderHeight()" - @close="close(false)" - > - <template #title> - <h4>{{ $options.i18n.title }}</h4> - </template> - <gl-alert v-if="errorMessages.length" variant="danger" @dismiss="errorMessages = []"> - <ul class="gl-mb-0! gl-ml-5"> - <li v-for="error in errorMessages" :key="error"> - {{ error }} - </li> - </ul> - </gl-alert> - <form @submit.prevent="save"> - <gl-form-group :label="$options.i18n.name" label-for="organization-name"> - <gl-form-input id="organization-name" v-model="name" /> - </gl-form-group> - <gl-form-group :label="$options.i18n.defaultRate" label-for="organization-default-rate"> - <gl-form-input - id="organization-default-rate" - v-model="defaultRate" - type="number" - step="0.01" - /> - </gl-form-group> - <gl-form-group :label="$options.i18n.description" label-for="organization-description"> - <gl-form-input id="organization-description" v-model="description" /> - </gl-form-group> - <span class="gl-float-right"> - <gl-button data-testid="cancel-button" @click="close(false)"> - {{ $options.i18n.cancel }} - </gl-button> - <gl-button - variant="confirm" - :disabled="invalid" - :loading="submitting" - data-testid="create-new-organization-button" - type="submit" - >{{ $options.i18n.buttonLabel }}</gl-button - > - </span> - </form> - </gl-drawer> -</template> diff --git a/app/assets/javascripts/crm/contacts_bundle.js b/app/assets/javascripts/crm/contacts/bundle.js index f49ec64210f..f49ec64210f 100644 --- a/app/assets/javascripts/crm/contacts_bundle.js +++ b/app/assets/javascripts/crm/contacts/bundle.js diff --git a/app/assets/javascripts/crm/contacts/components/contact_form_wrapper.vue b/app/assets/javascripts/crm/contacts/components/contact_form_wrapper.vue new file mode 100644 index 00000000000..58eaabfbb7f --- /dev/null +++ b/app/assets/javascripts/crm/contacts/components/contact_form_wrapper.vue @@ -0,0 +1,78 @@ +<script> +import { s__, __ } from '~/locale'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import { TYPE_CRM_CONTACT, TYPE_GROUP } from '~/graphql_shared/constants'; +import ContactForm from '../../components/form.vue'; +import getGroupContactsQuery from './graphql/get_group_contacts.query.graphql'; +import createContactMutation from './graphql/create_contact.mutation.graphql'; +import updateContactMutation from './graphql/update_contact.mutation.graphql'; + +export default { + components: { + ContactForm, + }, + inject: ['groupFullPath', 'groupId'], + props: { + isEditMode: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + contactGraphQLId() { + if (!this.isEditMode) return null; + + return convertToGraphQLId(TYPE_CRM_CONTACT, this.$route.params.id); + }, + groupGraphQLId() { + return convertToGraphQLId(TYPE_GROUP, this.groupId); + }, + mutation() { + if (this.isEditMode) return updateContactMutation; + + return createContactMutation; + }, + getQuery() { + return { + query: getGroupContactsQuery, + variables: { groupFullPath: this.groupFullPath }, + }; + }, + title() { + if (this.isEditMode) return s__('Crm|Edit contact'); + + return s__('Crm|New contact'); + }, + successMessage() { + if (this.isEditMode) return s__('Crm|Contact has been updated.'); + + return s__('Crm|Contact has been added.'); + }, + additionalCreateParams() { + return { groupId: this.groupGraphQLId }; + }, + }, + fields: [ + { name: 'firstName', label: __('First name'), required: true }, + { name: 'lastName', label: __('Last name'), required: true }, + { name: 'email', label: __('Email'), required: true }, + { name: 'phone', label: __('Phone') }, + { name: 'description', label: __('Description') }, + ], +}; +</script> + +<template> + <contact-form + :drawer-open="true" + :get-query="getQuery" + get-query-node-path="group.contacts" + :mutation="mutation" + :additional-create-params="additionalCreateParams" + :existing-id="contactGraphQLId" + :fields="$options.fields" + :title="title" + :success-message="successMessage" + /> +</template> diff --git a/app/assets/javascripts/crm/components/contacts_root.vue b/app/assets/javascripts/crm/contacts/components/contacts_root.vue index 178ce84c64d..17be3800256 100644 --- a/app/assets/javascripts/crm/components/contacts_root.vue +++ b/app/assets/javascripts/crm/contacts/components/contacts_root.vue @@ -2,11 +2,9 @@ import { GlAlert, GlButton, GlLoadingIcon, GlTable, GlTooltipDirective } from '@gitlab/ui'; import { parseBoolean } from '~/lib/utils/common_utils'; import { s__, __ } from '~/locale'; -import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; -import { TYPE_CRM_CONTACT } from '~/graphql_shared/constants'; -import { INDEX_ROUTE_NAME, NEW_ROUTE_NAME, EDIT_ROUTE_NAME } from '../constants'; -import getGroupContactsQuery from './queries/get_group_contacts.query.graphql'; -import ContactForm from './contact_form.vue'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; +import { EDIT_ROUTE_NAME, NEW_ROUTE_NAME } from '../../constants'; +import getGroupContactsQuery from './graphql/get_group_contacts.query.graphql'; export default { components: { @@ -14,12 +12,11 @@ export default { GlButton, GlLoadingIcon, GlTable, - ContactForm, }, directives: { GlTooltip: GlTooltipDirective, }, - inject: ['groupFullPath', 'groupIssuesPath', 'canAdminCrmContact'], + inject: ['canAdminCrmContact', 'groupFullPath', 'groupIssuesPath'], data() { return { contacts: [], @@ -48,50 +45,20 @@ export default { isLoading() { return this.$apollo.queries.contacts.loading; }, - showNewForm() { - return this.$route.name === NEW_ROUTE_NAME; - }, - showEditForm() { - return !this.isLoading && this.$route.name === EDIT_ROUTE_NAME; - }, canAdmin() { return parseBoolean(this.canAdminCrmContact); }, - editingContact() { - return this.contacts.find( - (contact) => contact.id === convertToGraphQLId(TYPE_CRM_CONTACT, this.$route.params.id), - ); - }, }, methods: { extractContacts(data) { const contacts = data?.group?.contacts?.nodes || []; return contacts.slice().sort((a, b) => a.firstName.localeCompare(b.firstName)); }, - displayNewForm() { - if (this.showNewForm) return; - - this.$router.push({ name: NEW_ROUTE_NAME }); - }, - hideNewForm(success) { - if (success) this.$toast.show(s__('Crm|Contact has been added')); - - this.$router.replace({ name: INDEX_ROUTE_NAME }); - }, - hideEditForm(success) { - if (success) this.$toast.show(s__('Crm|Contact has been updated')); - - this.editingContactId = 0; - this.$router.replace({ name: INDEX_ROUTE_NAME }); - }, getIssuesPath(path, value) { return `${path}?scope=all&state=opened&crm_contact_id=${value}`; }, - edit(value) { - if (this.showEditForm) return; - - this.editingContactId = value; - this.$router.push({ name: EDIT_ROUTE_NAME, params: { id: value } }); + getEditRoute(id) { + return { name: this.$options.EDIT_ROUTE_NAME, params: { id } }; }, }, fields: [ @@ -119,10 +86,12 @@ export default { emptyText: s__('Crm|No contacts found'), issuesButtonLabel: __('View issues'), editButtonLabel: __('Edit'), - title: s__('Crm|Customer Relations Contacts'), + title: s__('Crm|Customer relations contacts'), newContact: s__('Crm|New contact'), errorText: __('Something went wrong. Please try again.'), }, + EDIT_ROUTE_NAME, + NEW_ROUTE_NAME, }; </script> @@ -137,24 +106,15 @@ export default { <h2 class="gl-font-size-h2 gl-my-0"> {{ $options.i18n.title }} </h2> - <div class="gl-display-none gl-md-display-flex gl-align-items-center gl-justify-content-end"> - <gl-button - v-if="canAdmin" - variant="confirm" - data-testid="new-contact-button" - @click="displayNewForm" - > - {{ $options.i18n.newContact }} - </gl-button> + <div v-if="canAdmin"> + <router-link :to="{ name: $options.NEW_ROUTE_NAME }"> + <gl-button variant="confirm" data-testid="new-contact-button"> + {{ $options.i18n.newContact }} + </gl-button> + </router-link> </div> </div> - <contact-form v-if="showNewForm" :drawer-open="showNewForm" @close="hideNewForm" /> - <contact-form - v-if="showEditForm" - :contact="editingContact" - :drawer-open="showEditForm" - @close="hideEditForm" - /> + <router-view /> <gl-loading-icon v-if="isLoading" class="gl-mt-5" size="lg" /> <gl-table v-else @@ -164,23 +124,24 @@ export default { :empty-text="$options.i18n.emptyText" show-empty > - <template #cell(id)="data"> + <template #cell(id)="{ value: id }"> <gl-button v-gl-tooltip.hover.bottom="$options.i18n.issuesButtonLabel" class="gl-mr-3" data-testid="issues-link" icon="issues" :aria-label="$options.i18n.issuesButtonLabel" - :href="getIssuesPath(groupIssuesPath, data.value)" - /> - <gl-button - v-if="canAdmin" - v-gl-tooltip.hover.bottom="$options.i18n.editButtonLabel" - data-testid="edit-contact-button" - icon="pencil" - :aria-label="$options.i18n.editButtonLabel" - @click="edit(data.value)" + :href="getIssuesPath(groupIssuesPath, id)" /> + <router-link :to="getEditRoute(id)"> + <gl-button + v-if="canAdmin" + v-gl-tooltip.hover.bottom="$options.i18n.editButtonLabel" + data-testid="edit-contact-button" + icon="pencil" + :aria-label="$options.i18n.editButtonLabel" + /> + </router-link> </template> </gl-table> </div> diff --git a/app/assets/javascripts/crm/components/queries/create_contact.mutation.graphql b/app/assets/javascripts/crm/contacts/components/graphql/create_contact.mutation.graphql index e0192459609..e0192459609 100644 --- a/app/assets/javascripts/crm/components/queries/create_contact.mutation.graphql +++ b/app/assets/javascripts/crm/contacts/components/graphql/create_contact.mutation.graphql diff --git a/app/assets/javascripts/crm/components/queries/crm_contact_fields.fragment.graphql b/app/assets/javascripts/crm/contacts/components/graphql/crm_contact_fields.fragment.graphql index cef4083b446..cef4083b446 100644 --- a/app/assets/javascripts/crm/components/queries/crm_contact_fields.fragment.graphql +++ b/app/assets/javascripts/crm/contacts/components/graphql/crm_contact_fields.fragment.graphql diff --git a/app/assets/javascripts/crm/components/queries/get_group_contacts.query.graphql b/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts.query.graphql index 2a8150e42e3..2a8150e42e3 100644 --- a/app/assets/javascripts/crm/components/queries/get_group_contacts.query.graphql +++ b/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts.query.graphql diff --git a/app/assets/javascripts/crm/components/queries/update_contact.mutation.graphql b/app/assets/javascripts/crm/contacts/components/graphql/update_contact.mutation.graphql index f55f6a10e0a..f55f6a10e0a 100644 --- a/app/assets/javascripts/crm/components/queries/update_contact.mutation.graphql +++ b/app/assets/javascripts/crm/contacts/components/graphql/update_contact.mutation.graphql diff --git a/app/assets/javascripts/crm/routes.js b/app/assets/javascripts/crm/contacts/routes.js index 12aa17d73b6..18768e1c775 100644 --- a/app/assets/javascripts/crm/routes.js +++ b/app/assets/javascripts/crm/contacts/routes.js @@ -1,4 +1,5 @@ -import { INDEX_ROUTE_NAME, NEW_ROUTE_NAME, EDIT_ROUTE_NAME } from './constants'; +import { INDEX_ROUTE_NAME, NEW_ROUTE_NAME, EDIT_ROUTE_NAME } from '../constants'; +import ContactFormWrapper from './components/contact_form_wrapper.vue'; export default [ { @@ -8,9 +9,12 @@ export default [ { name: NEW_ROUTE_NAME, path: '/new', + component: ContactFormWrapper, }, { name: EDIT_ROUTE_NAME, path: '/:id/edit', + component: ContactFormWrapper, + props: { isEditMode: true }, }, ]; diff --git a/app/assets/javascripts/crm/organizations_bundle.js b/app/assets/javascripts/crm/organizations/bundle.js index 828d7cd426c..828d7cd426c 100644 --- a/app/assets/javascripts/crm/organizations_bundle.js +++ b/app/assets/javascripts/crm/organizations/bundle.js diff --git a/app/assets/javascripts/crm/components/queries/create_organization.mutation.graphql b/app/assets/javascripts/crm/organizations/components/graphql/create_organization.mutation.graphql index 2cc7e53ee9b..2cc7e53ee9b 100644 --- a/app/assets/javascripts/crm/components/queries/create_organization.mutation.graphql +++ b/app/assets/javascripts/crm/organizations/components/graphql/create_organization.mutation.graphql diff --git a/app/assets/javascripts/crm/components/queries/crm_organization_fields.fragment.graphql b/app/assets/javascripts/crm/organizations/components/graphql/crm_organization_fields.fragment.graphql index 4adc5742d3a..4adc5742d3a 100644 --- a/app/assets/javascripts/crm/components/queries/crm_organization_fields.fragment.graphql +++ b/app/assets/javascripts/crm/organizations/components/graphql/crm_organization_fields.fragment.graphql diff --git a/app/assets/javascripts/crm/components/queries/get_group_organizations.query.graphql b/app/assets/javascripts/crm/organizations/components/graphql/get_group_organizations.query.graphql index e8d8109431e..e8d8109431e 100644 --- a/app/assets/javascripts/crm/components/queries/get_group_organizations.query.graphql +++ b/app/assets/javascripts/crm/organizations/components/graphql/get_group_organizations.query.graphql diff --git a/app/assets/javascripts/crm/organizations/components/graphql/update_organization.mutation.graphql b/app/assets/javascripts/crm/organizations/components/graphql/update_organization.mutation.graphql new file mode 100644 index 00000000000..a4c46d1f0fa --- /dev/null +++ b/app/assets/javascripts/crm/organizations/components/graphql/update_organization.mutation.graphql @@ -0,0 +1,10 @@ +#import "./crm_organization_fields.fragment.graphql" + +mutation updateOrganization($input: CustomerRelationsOrganizationUpdateInput!) { + customerRelationsOrganizationUpdate(input: $input) { + organization { + ...OrganizationFragment + } + errors + } +} diff --git a/app/assets/javascripts/crm/organizations/components/organization_form_wrapper.vue b/app/assets/javascripts/crm/organizations/components/organization_form_wrapper.vue new file mode 100644 index 00000000000..38468e1f4e4 --- /dev/null +++ b/app/assets/javascripts/crm/organizations/components/organization_form_wrapper.vue @@ -0,0 +1,80 @@ +<script> +import { s__, __ } from '~/locale'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import { TYPE_CRM_ORGANIZATION, TYPE_GROUP } from '~/graphql_shared/constants'; +import OrganizationForm from '../../components/form.vue'; +import getGroupOrganizationsQuery from './graphql/get_group_organizations.query.graphql'; +import createOrganizationMutation from './graphql/create_organization.mutation.graphql'; +import updateOrganizationMutation from './graphql/update_organization.mutation.graphql'; + +export default { + components: { + OrganizationForm, + }, + inject: ['groupFullPath', 'groupId'], + props: { + isEditMode: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + organizationGraphQLId() { + if (!this.isEditMode) return null; + + return convertToGraphQLId(TYPE_CRM_ORGANIZATION, this.$route.params.id); + }, + groupGraphQLId() { + return convertToGraphQLId(TYPE_GROUP, this.groupId); + }, + mutation() { + if (this.isEditMode) return updateOrganizationMutation; + + return createOrganizationMutation; + }, + getQuery() { + return { + query: getGroupOrganizationsQuery, + variables: { groupFullPath: this.groupFullPath }, + }; + }, + title() { + if (this.isEditMode) return s__('Crm|Edit organization'); + + return s__('Crm|New organization'); + }, + successMessage() { + if (this.isEditMode) return s__('Crm|Organization has been updated.'); + + return s__('Crm|Organization has been added.'); + }, + additionalCreateParams() { + return { groupId: this.groupGraphQLId }; + }, + }, + fields: [ + { name: 'name', label: __('Name'), required: true }, + { + name: 'defaultRate', + label: s__('Crm|Default rate'), + input: { type: 'number', step: '0.01' }, + }, + { name: 'description', label: __('Description') }, + ], +}; +</script> + +<template> + <organization-form + :drawer-open="true" + :get-query="getQuery" + get-query-node-path="group.organizations" + :mutation="mutation" + :additional-create-params="additionalCreateParams" + :existing-id="organizationGraphQLId" + :fields="$options.fields" + :title="title" + :success-message="successMessage" + /> +</template> diff --git a/app/assets/javascripts/crm/components/organizations_root.vue b/app/assets/javascripts/crm/organizations/components/organizations_root.vue index 9370c6377e9..522e29eb2af 100644 --- a/app/assets/javascripts/crm/components/organizations_root.vue +++ b/app/assets/javascripts/crm/organizations/components/organizations_root.vue @@ -3,9 +3,8 @@ import { GlAlert, GlButton, GlLoadingIcon, GlTable, GlTooltipDirective } from '@ import { parseBoolean } from '~/lib/utils/common_utils'; import { s__, __ } from '~/locale'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; -import { INDEX_ROUTE_NAME, NEW_ROUTE_NAME } from '../constants'; -import getGroupOrganizationsQuery from './queries/get_group_organizations.query.graphql'; -import NewOrganizationForm from './new_organization_form.vue'; +import { EDIT_ROUTE_NAME, NEW_ROUTE_NAME } from '../../constants'; +import getGroupOrganizationsQuery from './graphql/get_group_organizations.query.graphql'; export default { components: { @@ -13,7 +12,6 @@ export default { GlButton, GlLoadingIcon, GlTable, - NewOrganizationForm, }, directives: { GlTooltip: GlTooltipDirective, @@ -21,8 +19,8 @@ export default { inject: ['canAdminCrmOrganization', 'groupFullPath', 'groupIssuesPath'], data() { return { - error: false, organizations: [], + error: false, }; }, apollo: { @@ -47,10 +45,7 @@ export default { isLoading() { return this.$apollo.queries.organizations.loading; }, - showNewForm() { - return this.$route.name === NEW_ROUTE_NAME; - }, - canCreateNew() { + canAdmin() { return parseBoolean(this.canAdminCrmOrganization); }, }, @@ -62,15 +57,8 @@ export default { getIssuesPath(path, value) { return `${path}?scope=all&state=opened&crm_organization_id=${value}`; }, - displayNewForm() { - if (this.showNewForm) return; - - this.$router.push({ name: NEW_ROUTE_NAME }); - }, - hideNewForm(success) { - if (success) this.$toast.show(this.$options.i18n.organizationAdded); - - this.$router.replace({ name: INDEX_ROUTE_NAME }); + getEditRoute(id) { + return { name: this.$options.EDIT_ROUTE_NAME, params: { id } }; }, }, fields: [ @@ -79,7 +67,7 @@ export default { { key: 'description', sortable: true }, { key: 'id', - label: __('Issues'), + label: '', formatter: (id) => { return getIdFromGraphQLId(id); }, @@ -88,11 +76,13 @@ export default { i18n: { emptyText: s__('Crm|No organizations found'), issuesButtonLabel: __('View issues'), - title: s__('Crm|Customer Relations Organizations'), + editButtonLabel: __('Edit'), + title: s__('Crm|Customer relations organizations'), newOrganization: s__('Crm|New organization'), errorText: __('Something went wrong. Please try again.'), - organizationAdded: s__('Crm|Organization has been added'), }, + EDIT_ROUTE_NAME, + NEW_ROUTE_NAME, }; </script> @@ -108,15 +98,17 @@ export default { {{ $options.i18n.title }} </h2> <div - v-if="canCreateNew" + v-if="canAdmin" class="gl-display-none gl-md-display-flex gl-align-items-center gl-justify-content-end" > - <gl-button variant="confirm" data-testid="new-organization-button" @click="displayNewForm"> - {{ $options.i18n.newOrganization }} - </gl-button> + <router-link :to="{ name: $options.NEW_ROUTE_NAME }"> + <gl-button variant="confirm" data-testid="new-organization-button"> + {{ $options.i18n.newOrganization }} + </gl-button> + </router-link> </div> </div> - <new-organization-form v-if="showNewForm" :drawer-open="showNewForm" @close="hideNewForm" /> + <router-view /> <gl-loading-icon v-if="isLoading" class="gl-mt-5" size="lg" /> <gl-table v-else @@ -126,14 +118,24 @@ export default { :empty-text="$options.i18n.emptyText" show-empty > - <template #cell(id)="data"> + <template #cell(id)="{ value: id }"> <gl-button v-gl-tooltip.hover.bottom="$options.i18n.issuesButtonLabel" + class="gl-mr-3" data-testid="issues-link" icon="issues" :aria-label="$options.i18n.issuesButtonLabel" - :href="getIssuesPath(groupIssuesPath, data.value)" + :href="getIssuesPath(groupIssuesPath, id)" /> + <router-link :to="getEditRoute(id)"> + <gl-button + v-if="canAdmin" + v-gl-tooltip.hover.bottom="$options.i18n.editButtonLabel" + data-testid="edit-organization-button" + icon="pencil" + :aria-label="$options.i18n.editButtonLabel" + /> + </router-link> </template> </gl-table> </div> diff --git a/app/assets/javascripts/crm/organizations/routes.js b/app/assets/javascripts/crm/organizations/routes.js new file mode 100644 index 00000000000..85bd3b32877 --- /dev/null +++ b/app/assets/javascripts/crm/organizations/routes.js @@ -0,0 +1,20 @@ +import { INDEX_ROUTE_NAME, NEW_ROUTE_NAME, EDIT_ROUTE_NAME } from '../constants'; +import OrganizationFormWrapper from './components/organization_form_wrapper.vue'; + +export default [ + { + name: INDEX_ROUTE_NAME, + path: '/', + }, + { + name: NEW_ROUTE_NAME, + path: '/new', + component: OrganizationFormWrapper, + }, + { + name: EDIT_ROUTE_NAME, + path: '/:id/edit', + component: OrganizationFormWrapper, + props: { isEditMode: true }, + }, +]; diff --git a/app/assets/javascripts/graphql_shared/constants.js b/app/assets/javascripts/graphql_shared/constants.js index 3726743c032..22fa2912881 100644 --- a/app/assets/javascripts/graphql_shared/constants.js +++ b/app/assets/javascripts/graphql_shared/constants.js @@ -3,6 +3,7 @@ export const MINIMUM_SEARCH_LENGTH = 3; export const TYPE_BOARD = 'Board'; export const TYPE_CI_RUNNER = 'Ci::Runner'; export const TYPE_CRM_CONTACT = 'CustomerRelations::Contact'; +export const TYPE_CRM_ORGANIZATION = 'CustomerRelations::Organization'; export const TYPE_DISCUSSION = 'Discussion'; export const TYPE_EPIC = 'Epic'; export const TYPE_EPIC_BOARD = 'Boards::EpicBoard'; diff --git a/app/assets/javascripts/lib/utils/text_markdown.js b/app/assets/javascripts/lib/utils/text_markdown.js index ac2eb34260c..18eca11ac42 100644 --- a/app/assets/javascripts/lib/utils/text_markdown.js +++ b/app/assets/javascripts/lib/utils/text_markdown.js @@ -9,7 +9,7 @@ const LINK_TAG_PATTERN = '[{text}](url)'; // a bullet point character (*+-) and an optional checkbox ([ ] [x]) // OR a number with a . after it and an optional checkbox ([ ] [x]) // followed by one or more whitespace characters -const LIST_LINE_HEAD_PATTERN = /^(?<indent>\s*)(?<leader>((?<isUl>[*+-])|(?<isOl>\d+\.))( \[([x ])\])?\s)(?<content>.)?/; +const LIST_LINE_HEAD_PATTERN = /^(?<indent>\s*)(?<leader>((?<isUl>[*+-])|(?<isOl>\d+\.))( \[([xX ])\])?\s)(?<content>.)?/; function selectedText(text, textarea) { return text.substring(textarea.selectionStart, textarea.selectionEnd); @@ -391,6 +391,8 @@ function handleContinueList(e, textArea) { itemToInsert = `${indent}${leader}`; } + itemToInsert = itemToInsert.replace(/\[x\]/i, '[ ]'); + e.preventDefault(); updateText({ diff --git a/app/assets/javascripts/pages/groups/crm/contacts/index.js b/app/assets/javascripts/pages/groups/crm/contacts/index.js index a595246957f..6af47621c1d 100644 --- a/app/assets/javascripts/pages/groups/crm/contacts/index.js +++ b/app/assets/javascripts/pages/groups/crm/contacts/index.js @@ -1,3 +1,3 @@ -import initCrmContactsApp from '~/crm/contacts_bundle'; +import initCrmContactsApp from '~/crm/contacts/bundle'; initCrmContactsApp(); diff --git a/app/assets/javascripts/pages/groups/crm/organizations/index.js b/app/assets/javascripts/pages/groups/crm/organizations/index.js index 16479b43d52..2ad0904688e 100644 --- a/app/assets/javascripts/pages/groups/crm/organizations/index.js +++ b/app/assets/javascripts/pages/groups/crm/organizations/index.js @@ -1,3 +1,3 @@ -import initCrmOrganizationsApp from '~/crm/organizations_bundle'; +import initCrmOrganizationsApp from '~/crm/organizations/bundle'; initCrmOrganizationsApp(); diff --git a/app/assets/javascripts/runner/components/runner_jobs.vue b/app/assets/javascripts/runner/components/runner_jobs.vue index eb77babcc57..b25d92d049e 100644 --- a/app/assets/javascripts/runner/components/runner_jobs.vue +++ b/app/assets/javascripts/runner/components/runner_jobs.vue @@ -1,5 +1,5 @@ <script> -import { GlSkeletonLoading } from '@gitlab/ui'; +import { GlDeprecatedSkeletonLoading as GlSkeletonLoading } from '@gitlab/ui'; import { createAlert } from '~/flash'; import runnerJobsQuery from '../graphql/details/runner_jobs.query.graphql'; import { I18N_FETCH_ERROR, I18N_NO_JOBS_FOUND, RUNNER_DETAILS_JOBS_PAGE_SIZE } from '../constants'; diff --git a/app/assets/javascripts/runner/components/runner_projects.vue b/app/assets/javascripts/runner/components/runner_projects.vue index f8ec29b8a24..d080d34fdd3 100644 --- a/app/assets/javascripts/runner/components/runner_projects.vue +++ b/app/assets/javascripts/runner/components/runner_projects.vue @@ -1,5 +1,5 @@ <script> -import { GlSkeletonLoading } from '@gitlab/ui'; +import { GlDeprecatedSkeletonLoading as GlSkeletonLoading } from '@gitlab/ui'; import { sprintf, formatNumber } from '~/locale'; import { createAlert } from '~/flash'; import runnerProjectsQuery from '../graphql/details/runner_projects.query.graphql'; diff --git a/app/assets/javascripts/security_configuration/components/feature_card.vue b/app/assets/javascripts/security_configuration/components/feature_card.vue index cd5ad86e1a8..4ce3c7cbc9a 100644 --- a/app/assets/javascripts/security_configuration/components/feature_card.vue +++ b/app/assets/javascripts/security_configuration/components/feature_card.vue @@ -2,6 +2,7 @@ import { GlButton, GlCard, GlIcon, GlLink } from '@gitlab/ui'; import { __, s__, sprintf } from '~/locale'; import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue'; +import { REPORT_TYPE_SAST_IAC } from '~/vue_shared/security_reports/constants'; export default { components: { @@ -61,6 +62,12 @@ export default { const { name, description, configurationText } = this.feature.secondary ?? {}; return Boolean(name && description && configurationText); }, + // This condition is a temporary hack to not display any wrong information + // until this BE Bug is fixed: https://gitlab.com/gitlab-org/gitlab/-/issues/350307. + // More Information: https://gitlab.com/gitlab-org/gitlab/-/issues/350307#note_825447417 + isNotSastIACTemporaryHack() { + return this.feature.type !== REPORT_TYPE_SAST_IAC; + }, }, methods: { onError(message) { @@ -85,6 +92,7 @@ export default { <h3 class="gl-font-lg gl-m-0 gl-mr-3">{{ feature.name }}</h3> <div + v-if="isNotSastIACTemporaryHack" :class="statusClasses" data-testid="feature-status" :data-qa-selector="`${feature.type}_status`" @@ -109,7 +117,7 @@ export default { <gl-link :href="feature.helpPath">{{ $options.i18n.learnMore }}</gl-link> </p> - <template v-if="available"> + <template v-if="available && isNotSastIACTemporaryHack"> <gl-button v-if="feature.configurationPath" :href="feature.configurationPath" diff --git a/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue index 028d48e7e8a..20f178dfb7d 100644 --- a/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue +++ b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue @@ -1,5 +1,10 @@ <script> -import { GlAlert, GlKeysetPagination, GlSkeletonLoading, GlPagination } from '@gitlab/ui'; +import { + GlAlert, + GlKeysetPagination, + GlDeprecatedSkeletonLoading as GlSkeletonLoading, + GlPagination, +} from '@gitlab/ui'; import { uniqueId } from 'lodash'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { updateHistory, setUrlParams } from '~/lib/utils/url_utility'; diff --git a/app/controllers/groups/crm/organizations_controller.rb b/app/controllers/groups/crm/organizations_controller.rb index f8536b4f538..846995ecba5 100644 --- a/app/controllers/groups/crm/organizations_controller.rb +++ b/app/controllers/groups/crm/organizations_controller.rb @@ -10,6 +10,10 @@ class Groups::Crm::OrganizationsController < Groups::ApplicationController render action: "index" end + def edit + render action: "index" + end + private def authorize_read_crm_organization! diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index 88337242fcd..93e2298ca98 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -81,7 +81,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap def branch_to @target_project = selected_target_project - if @target_project && params[:ref].present? + if @target_project && params[:ref].present? && Ability.allowed?(current_user, :create_merge_request_in, @target_project) @ref = params[:ref] @commit = @target_project.commit(Gitlab::Git::BRANCH_REF_PREFIX + @ref) end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 15b19dc8d7c..0703c6d1df3 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -824,6 +824,8 @@ module Ci variables.append(key: 'CI_OPEN_MERGE_REQUESTS', value: open_merge_requests_refs.join(',')) end + variables.append(key: 'CI_GITLAB_FIPS_MODE', value: 'true') if Gitlab::FIPS.enabled? + variables.append(key: 'CI_KUBERNETES_ACTIVE', value: 'true') if has_kubernetes_active? variables.append(key: 'CI_DEPLOY_FREEZE', value: 'true') if freeze_period? diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 4228da279a4..b9ba9d8e1b0 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -65,6 +65,8 @@ module Ci FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze MINUTES_COST_FACTOR_FIELDS = %i[public_projects_minutes_cost_factor private_projects_minutes_cost_factor].freeze + TAG_LIST_MAX_LENGTH = 50 + has_many :builds has_many :runner_projects, inverse_of: :runner, autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :projects, through: :runner_projects, disable_joins: true @@ -520,6 +522,11 @@ module Ci errors.add(:tags_list, 'can not be empty when runner is not allowed to pick untagged jobs') end + + if tag_list_changed? && tag_list.count > TAG_LIST_MAX_LENGTH + errors.add(:tags_list, + "Too many tags specified. Please limit the number of tags to #{TAG_LIST_MAX_LENGTH}") + end end def no_projects diff --git a/app/models/integrations/asana.rb b/app/models/integrations/asana.rb index 054f0606dd2..e7634f51ec4 100644 --- a/app/models/integrations/asana.rb +++ b/app/models/integrations/asana.rb @@ -59,12 +59,9 @@ module Integrations def execute(data) return unless supported_events.include?(data[:object_kind]) - # check the branch restriction is poplulated and branch is not included branch = Gitlab::Git.ref_name(data[:ref]) - branch_restriction = restrict_to_branch.to_s - if branch_restriction.present? && branch_restriction.index(branch).nil? - return - end + + return unless branch_allowed?(branch) user = data[:user_name] project_name = project.full_name @@ -103,5 +100,13 @@ module Integrations end end end + + private + + def branch_allowed?(branch_name) + return true if restrict_to_branch.blank? + + restrict_to_branch.to_s.gsub(/\s+/, '').split(',').include?(branch_name) + end end end diff --git a/app/models/releases/link.rb b/app/models/releases/link.rb index acc56d3980a..347adbdf96a 100644 --- a/app/models/releases/link.rb +++ b/app/models/releases/link.rb @@ -9,10 +9,20 @@ module Releases # See https://gitlab.com/gitlab-org/gitlab/-/issues/218753 # Regex modified to prevent catastrophic backtracking FILEPATH_REGEX = %r{\A\/[^\/](?!.*\/\/.*)[\-\.\w\/]+[\da-zA-Z]+\z}.freeze + FILEPATH_MAX_LENGTH = 128 validates :url, presence: true, addressable_url: { schemes: %w(http https ftp) }, uniqueness: { scope: :release } validates :name, presence: true, uniqueness: { scope: :release } - validates :filepath, uniqueness: { scope: :release }, format: { with: FILEPATH_REGEX }, allow_blank: true, length: { maximum: 128 } + validates :filepath, uniqueness: { scope: :release }, allow_blank: true + validate :filepath_format_valid? + + # we use a custom validator here to prevent running the regex if the string is too long + # see https://gitlab.com/gitlab-org/gitlab/-/issues/273771 + def filepath_format_valid? + return if filepath.nil? # valid use case + return errors.add(:filepath, "is too long (maximum is #{FILEPATH_MAX_LENGTH} characters)") if filepath.length > FILEPATH_MAX_LENGTH + return errors.add(:filepath, 'is in an invalid format') unless FILEPATH_REGEX.match? filepath + end scope :sorted, -> { order(created_at: :desc) } diff --git a/app/models/ssh_host_key.rb b/app/models/ssh_host_key.rb index bb928118edf..ac7ba9530dd 100644 --- a/app/models/ssh_host_key.rb +++ b/app/models/ssh_host_key.rb @@ -46,11 +46,11 @@ class SshHostKey .select(&:valid?) end - attr_reader :project, :url, :compare_host_keys + attr_reader :project, :url, :ip, :compare_host_keys def initialize(project:, url:, compare_host_keys: nil) @project = project - @url = normalize_url(url) + @url, @ip = normalize_url(url) @compare_host_keys = compare_host_keys end @@ -90,9 +90,11 @@ class SshHostKey end def calculate_reactive_cache + input = [ip, url.hostname].compact.join(' ') + known_hosts, errors, status = Open3.popen3({}, *%W[ssh-keyscan -T 5 -p #{url.port} -f-]) do |stdin, stdout, stderr, wait_thr| - stdin.puts(url.host) + stdin.puts(input) stdin.close [ @@ -127,11 +129,31 @@ class SshHostKey end def normalize_url(url) - full_url = ::Addressable::URI.parse(url) - raise ArgumentError, "Invalid URL" unless full_url&.scheme == 'ssh' + url, real_hostname = Gitlab::UrlBlocker.validate!( + url, + schemes: %w[ssh], + allow_localhost: allow_local_requests?, + allow_local_network: allow_local_requests?, + dns_rebind_protection: Gitlab::CurrentSettings.dns_rebinding_protection_enabled? + ) + + # When DNS rebinding protection is required, the hostname is replaced by the + # resolved IP. However, `url` is used in `id`, so we can't change it. Track + # the resolved IP separately instead. + if real_hostname + ip = url.hostname + url.hostname = real_hostname + end + + # Ensure ssh://foo and ssh://foo:22 share the same cache + url.port = url.inferred_port - Addressable::URI.parse("ssh://#{full_url.host}:#{full_url.inferred_port}") - rescue Addressable::URI::InvalidURIError + [url, ip] + rescue Gitlab::UrlBlocker::BlockedUrlError raise ArgumentError, "Invalid URL" end + + def allow_local_requests? + Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? + end end diff --git a/app/models/user.rb b/app/models/user.rb index 281e52b90b0..b15daeb0c4a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -879,6 +879,23 @@ class User < ApplicationRecord reset_password_sent_at.present? && reset_password_sent_at >= 1.minute.ago end + # See https://gitlab.com/gitlab-org/security/gitlab/-/issues/638 + DISALLOWED_PASSWORDS = %w[123qweQWE!@#000000000].freeze + + # Overwrites valid_password? from Devise::Models::DatabaseAuthenticatable + # In constant-time, check both that the password isn't on a denylist AND + # that the password is the user's password + def valid_password?(password) + password_allowed = true + DISALLOWED_PASSWORDS.each do |disallowed_password| + password_allowed = false if Devise.secure_compare(password, disallowed_password) + end + + original_result = super + + password_allowed && original_result + end + def remember_me! super if ::Gitlab::Database.read_write? end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index f5e3f30c61b..a417ea35673 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -240,7 +240,6 @@ class ProjectPolicy < BasePolicy rule { can?(:guest_access) }.policy do enable :read_project - enable :create_merge_request_in enable :read_issue_board enable :read_issue_board_list enable :read_wiki @@ -497,7 +496,7 @@ class ProjectPolicy < BasePolicy prevent(*create_read_update_admin_destroy(:issue_board_list)) end - rule { merge_requests_disabled | repository_disabled }.policy do + rule { merge_requests_disabled | repository_disabled | ~can?(:download_code) }.policy do prevent :create_merge_request_in prevent :create_merge_request_from prevent(*create_read_update_admin_destroy(:merge_request)) @@ -600,13 +599,14 @@ class ProjectPolicy < BasePolicy enable :read_cycle_analytics enable :read_pages_content enable :read_analytics - enable :read_ci_cd_analytics enable :read_insights # NOTE: may be overridden by IssuePolicy enable :read_issue end + rule { can?(:public_access) & public_builds }.enable :read_ci_cd_analytics + rule { public_builds }.policy do enable :read_build end @@ -664,6 +664,10 @@ class ProjectPolicy < BasePolicy enable :read_security_configuration end + rule { can?(:guest_access) & can?(:read_commit_status) }.policy do + enable :create_merge_request_in + end + # Design abilities could also be prevented in the issue policy. rule { design_management_disabled }.policy do prevent :read_design diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 9184c99c039..47f4b9c6898 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -77,7 +77,10 @@ module QuickActions # want to also handle bare usernames. The ReferenceExtractor also has # different behaviour, and will return all group members for groups named # using a user-style reference, which is not in scope here. + # + # nb: underscores may be passed in escaped to protect them from markdown rendering args = params.split(/\s|,/).select(&:present?).uniq - ['and'] + args.map! { _1.gsub(/\\_/, '_') } usernames = (args - ['me']).map { _1.delete_prefix('@') } found = User.by_username(usernames).to_a.select { can?(:read_user, _1) } found_names = found.map(&:username).to_set diff --git a/app/views/groups/crm/contacts/index.html.haml b/app/views/groups/crm/contacts/index.html.haml index 81293937f77..8a971e451a4 100644 --- a/app/views/groups/crm/contacts/index.html.haml +++ b/app/views/groups/crm/contacts/index.html.haml @@ -1,4 +1,8 @@ -- breadcrumb_title _('Customer Relations Contacts') -- page_title _('Customer Relations Contacts') +- breadcrumb_title _('Customer relations contacts') +- page_title _('Customer relations contacts') +- @content_wrapper_class = "gl-relative" + += content_for :after_content do + #js-crm-form-portal #js-crm-contacts-app{ data: { group_full_path: @group.full_path, group_issues_path: issues_group_path(@group), group_id: @group.id, can_admin_crm_contact: can?(current_user, :admin_crm_contact, @group).to_s, base_path: group_crm_contacts_path(@group) } } diff --git a/app/views/groups/crm/organizations/index.html.haml b/app/views/groups/crm/organizations/index.html.haml index 1647805b976..ff1ba678de0 100644 --- a/app/views/groups/crm/organizations/index.html.haml +++ b/app/views/groups/crm/organizations/index.html.haml @@ -1,4 +1,8 @@ -- breadcrumb_title _('Customer Relations Organizations') -- page_title _('Customer Relations Organizations') +- breadcrumb_title _('Customer relations organizations') +- page_title _('Customer relations organizations') +- @content_wrapper_class = "gl-relative" + += content_for :after_content do + #js-crm-form-portal #js-crm-organizations-app{ data: { base_path: group_crm_organizations_path(@group), can_admin_crm_organization: can?(current_user, :admin_crm_organization, @group).to_s, group_full_path: @group.full_path, group_id: @group.id, group_issues_path: issues_group_path(@group) } } diff --git a/config/initializers/rdoc_segfault_patch.rb b/config/initializers/rdoc_segfault_patch.rb new file mode 100644 index 00000000000..2494d7ef421 --- /dev/null +++ b/config/initializers/rdoc_segfault_patch.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# Monkey patch of RDoc to prevent Ruby segfault due to +# stack buffer overflow Ruby bug - +# https://bugs.ruby-lang.org/issues/16376 +# +# Safe to remove once GitLab upgrades to Ruby 3.0 +# or once the fix is backported to 2.7.x and +# GitLab upgrades. +# https://gitlab.com/gitlab-org/gitlab/-/issues/351179 +class RDoc::Markup::ToHtml + def parseable?(_) + false + end +end + +class RDoc::Markup::Verbatim + def ruby? + false + end +end diff --git a/config/routes/group.rb b/config/routes/group.rb index 7252bdc251d..bf6094ff2f1 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -135,7 +135,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do namespace :crm do resources :contacts, only: [:index, :new, :edit] - resources :organizations, only: [:index, :new] + resources :organizations, only: [:index, :new, :edit] end end diff --git a/db/fixtures/development/18_abuse_reports.rb b/db/fixtures/development/18_abuse_reports.rb index b06beca35e9..88d2f784852 100644 --- a/db/fixtures/development/18_abuse_reports.rb +++ b/db/fixtures/development/18_abuse_reports.rb @@ -11,7 +11,7 @@ module Db name: FFaker::Name.name, email: FFaker::Internet.email, confirmed_at: DateTime.now, - password: Gitlab::Password.test_default + password: '12345678' ) ::AbuseReport.create(reporter: ::User.take, user: reported_user, message: 'User sends spam') diff --git a/doc/ci/variables/predefined_variables.md b/doc/ci/variables/predefined_variables.md index 44a8e631660..3b504b02fae 100644 --- a/doc/ci/variables/predefined_variables.md +++ b/doc/ci/variables/predefined_variables.md @@ -58,6 +58,7 @@ There are also a number of [variables you can use to configure runner behavior]( | `CI_ENVIRONMENT_URL` | 9.3 | all | The URL of the environment for this job. Available if [`environment:url`](../yaml/index.md#environmenturl) is set. | | `CI_ENVIRONMENT_ACTION` | 13.11 | all | The action annotation specified for this job's environment. Available if [`environment:action`](../yaml/index.md#environmentaction) is set. Can be `start`, `prepare`, or `stop`. | | `CI_ENVIRONMENT_TIER` | 14.0 | all | The [deployment tier of the environment](../environments/index.md#deployment-tier-of-environments) for this job. | +| `CI_GITLAB_FIPS_MODE` | 14.10 | all | The configuration setting for whether FIPS mode is enabled in the GitLab instance. | | `CI_HAS_OPEN_REQUIREMENTS` | 13.1 | all | Only available if the pipeline's project has an open [requirement](../../user/project/requirements/index.md). `true` when available. | | `CI_JOB_ID` | 9.0 | all | The internal ID of the job, unique across all jobs in the GitLab instance. | | `CI_JOB_IMAGE` | 12.9 | 12.9 | The name of the Docker image running the job. | diff --git a/doc/user/crm/crm_contacts_v14_10.png b/doc/user/crm/crm_contacts_v14_10.png Binary files differnew file mode 100644 index 00000000000..f82878f9d4b --- /dev/null +++ b/doc/user/crm/crm_contacts_v14_10.png diff --git a/doc/user/crm/crm_contacts_v14_6.png b/doc/user/crm/crm_contacts_v14_6.png Binary files differdeleted file mode 100644 index 37a615f3926..00000000000 --- a/doc/user/crm/crm_contacts_v14_6.png +++ /dev/null diff --git a/doc/user/crm/crm_organizations_v14_10.png b/doc/user/crm/crm_organizations_v14_10.png Binary files differnew file mode 100644 index 00000000000..256c9c3f83b --- /dev/null +++ b/doc/user/crm/crm_organizations_v14_10.png diff --git a/doc/user/crm/crm_organizations_v14_6.png b/doc/user/crm/crm_organizations_v14_6.png Binary files differdeleted file mode 100644 index 2bde3823cc7..00000000000 --- a/doc/user/crm/crm_organizations_v14_6.png +++ /dev/null diff --git a/doc/user/crm/index.md b/doc/user/crm/index.md index 201f6f710b9..7fc11add2cd 100644 --- a/doc/user/crm/index.md +++ b/doc/user/crm/index.md @@ -49,7 +49,7 @@ To view a group's contacts: 1. On the top bar, select **Menu > Groups** and find your group. 1. On the left sidebar, select **Customer relations > Contacts**. - + ### Create a contact @@ -86,7 +86,7 @@ To view a group's organizations: 1. On the top bar, select **Menu > Groups** and find your group. 1. On the left sidebar, select **Customer relations > Organizations**. - + ### Create an organization diff --git a/lib/banzai/filter/kroki_filter.rb b/lib/banzai/filter/kroki_filter.rb index 75ccf998f60..845c7f2bc0a 100644 --- a/lib/banzai/filter/kroki_filter.rb +++ b/lib/banzai/filter/kroki_filter.rb @@ -6,8 +6,10 @@ require "asciidoctor/extensions/asciidoctor_kroki/extension" module Banzai module Filter # HTML that replaces all diagrams supported by Kroki with the corresponding img tags. - # + # If the source content is large then the hidden attribute is added to the img tag. class KrokiFilter < HTML::Pipeline::Filter + MAX_CHARACTER_LIMIT = 2000 + def call return doc unless settings.kroki_enabled @@ -21,10 +23,16 @@ module Banzai diagram_format = "svg" doc.xpath(xpath).each do |node| diagram_type = node.parent['lang'] - img_tag = Nokogiri::HTML::DocumentFragment.parse(%(<img src="#{create_image_src(diagram_type, diagram_format, node.content)}"/>)) + diagram_src = node.content + image_src = create_image_src(diagram_type, diagram_format, diagram_src) + img_tag = Nokogiri::HTML::DocumentFragment.parse(%(<img src="#{image_src}" />)) img_tag = img_tag.children.first unless img_tag.nil? + lazy_load = diagram_src.length > MAX_CHARACTER_LIMIT + img_tag.set_attribute('hidden', '') if lazy_load + img_tag.set_attribute('class', 'js-render-kroki') + img_tag.set_attribute('data-diagram', node.parent['lang']) img_tag.set_attribute('data-diagram-src', "data:text/plain;base64,#{Base64.strict_encode64(node.content)}") diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 07f82c98666..bcd9f39d1dc 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -56,7 +56,7 @@ module Banzai retry end - sourcepos_attr = sourcepos ? "data-sourcepos=\"#{sourcepos}\"" : '' + sourcepos_attr = sourcepos ? "data-sourcepos=\"#{escape_once(sourcepos)}\"" : '' highlighted = %(<div class="gl-relative markdown-code-block js-markdown-code"><pre #{sourcepos_attr} class="#{css_classes}" lang="#{language}" diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index f4964f779ca..5e7c2f64c92 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -12,10 +12,10 @@ module Banzai def self.filters @filters ||= FilterArray[ Filter::PlantumlFilter, - Filter::KrokiFilter, # Must always be before the SanitizationFilter to prevent XSS attacks Filter::SpacedLinkFilter, Filter::SanitizationFilter, + Filter::KrokiFilter, Filter::AssetProxyFilter, Filter::SyntaxHighlightFilter, Filter::MathFilter, diff --git a/lib/banzai/reference_redactor.rb b/lib/banzai/reference_redactor.rb index 81e4fd45966..c19f992078a 100644 --- a/lib/banzai/reference_redactor.rb +++ b/lib/banzai/reference_redactor.rb @@ -65,16 +65,15 @@ module Banzai # def redacted_node_content(node) original_content = node.attr('data-original') - link_reference = node.attr('data-link-reference') + original_content = CGI.escape_html(original_content) if original_content # Build the raw <a> tag just with a link as href and content if # it's originally a link pattern. We shouldn't return a plain text href. original_link = - if link_reference == 'true' + if node.attr('data-link-reference') == 'true' href = node.attr('href') - content = original_content - %(<a href="#{href}">#{content}</a>) + %(<a href="#{href}">#{original_content}</a>) end # The reference should be replaced by the original link's content, diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb index 200f1a843e6..d9efb6b8d2d 100644 --- a/lib/gitlab/auth/o_auth/user.rb +++ b/lib/gitlab/auth/o_auth/user.rb @@ -239,8 +239,8 @@ module Gitlab name: name.strip.presence || valid_username, username: valid_username, email: email, - password: Gitlab::Password.test_default(21), - password_confirmation: Gitlab::Password.test_default(21), + password: auth_hash.password, + password_confirmation: auth_hash.password, password_automatically_set: true } end diff --git a/lib/gitlab/ci/config/external/context.rb b/lib/gitlab/ci/config/external/context.rb index 308414af47d..512cfdde474 100644 --- a/lib/gitlab/ci/config/external/context.rb +++ b/lib/gitlab/ci/config/external/context.rb @@ -70,6 +70,16 @@ module Gitlab } end + def mask_variables_from(location) + variables.reduce(location.dup) do |loc, variable| + if variable[:masked] + Gitlab::Ci::MaskSecret.mask!(loc, variable[:value]) + else + loc + end + end + end + protected attr_writer :expandset, :execution_deadline, :logger diff --git a/lib/gitlab/ci/config/external/file/artifact.rb b/lib/gitlab/ci/config/external/file/artifact.rb index e6ff33d6f79..4f79e64ca9a 100644 --- a/lib/gitlab/ci/config/external/file/artifact.rb +++ b/lib/gitlab/ci/config/external/file/artifact.rb @@ -37,7 +37,7 @@ module Gitlab def validate_content! return unless ensure_preconditions_satisfied! - errors.push("File `#{location}` is empty!") unless content.present? + errors.push("File `#{masked_location}` is empty!") unless content.present? end def ensure_preconditions_satisfied! diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index 7c45961b738..4d7cf4af9f5 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -79,21 +79,21 @@ module Gitlab def validate_location! if invalid_location_type? - errors.push("Included file `#{location}` needs to be a string") + errors.push("Included file `#{masked_location}` needs to be a string") elsif invalid_extension? - errors.push("Included file `#{location}` does not have YAML extension!") + errors.push("Included file `#{masked_location}` does not have YAML extension!") end end def validate_content! if content.blank? - errors.push("Included file `#{location}` is empty or does not exist!") + errors.push("Included file `#{masked_location}` is empty or does not exist!") end end def validate_hash! if to_hash.blank? - errors.push("Included file `#{location}` does not have valid YAML syntax!") + errors.push("Included file `#{masked_location}` does not have valid YAML syntax!") end end @@ -104,6 +104,12 @@ module Gitlab def expand_context_attrs {} end + + def masked_location + strong_memoize(:masked_location) do + context.mask_variables_from(location) + end + end end end end diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb index 3839c43bd53..3aa665c7d18 100644 --- a/lib/gitlab/ci/config/external/file/local.rb +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -23,11 +23,11 @@ module Gitlab def validate_content! if context.project&.repository.nil? - errors.push("Local file `#{location}` does not have project!") + errors.push("Local file `#{masked_location}` does not have project!") elsif content.nil? - errors.push("Local file `#{location}` does not exist!") + errors.push("Local file `#{masked_location}` does not exist!") elsif content.blank? - errors.push("Local file `#{location}` is empty!") + errors.push("Local file `#{masked_location}` is empty!") end end diff --git a/lib/gitlab/ci/config/external/file/project.rb b/lib/gitlab/ci/config/external/file/project.rb index b719a1675cd..1fc22bf3780 100644 --- a/lib/gitlab/ci/config/external/file/project.rb +++ b/lib/gitlab/ci/config/external/file/project.rb @@ -35,9 +35,9 @@ module Gitlab elsif sha.nil? errors.push("Project `#{project_name}` reference `#{ref_name}` does not exist!") elsif content.nil? - errors.push("Project `#{project_name}` file `#{location}` does not exist!") + errors.push("Project `#{project_name}` file `#{masked_location}` does not exist!") elsif content.blank? - errors.push("Project `#{project_name}` file `#{location}` is empty!") + errors.push("Project `#{project_name}` file `#{masked_location}` is empty!") end end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb index 4bd8e250d7a..8335a9ef625 100644 --- a/lib/gitlab/ci/config/external/file/remote.rb +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -24,7 +24,7 @@ module Gitlab super unless ::Gitlab::UrlSanitizer.valid?(location) - errors.push("Remote file `#{location}` does not have a valid address!") + errors.push("Remote file `#{masked_location}` does not have a valid address!") end end @@ -32,17 +32,17 @@ module Gitlab begin response = Gitlab::HTTP.get(location) rescue SocketError - errors.push("Remote file `#{location}` could not be fetched because of a socket error!") + errors.push("Remote file `#{masked_location}` could not be fetched because of a socket error!") rescue Timeout::Error - errors.push("Remote file `#{location}` could not be fetched because of a timeout error!") + errors.push("Remote file `#{masked_location}` could not be fetched because of a timeout error!") rescue Gitlab::HTTP::Error - errors.push("Remote file `#{location}` could not be fetched because of HTTP error!") + errors.push("Remote file `#{masked_location}` could not be fetched because of HTTP error!") rescue Gitlab::HTTP::BlockedUrlError => e errors.push("Remote file could not be fetched because #{e}!") end if response&.code.to_i >= 400 - errors.push("Remote file `#{location}` could not be fetched because of HTTP code `#{response.code}` error!") + errors.push("Remote file `#{masked_location}` could not be fetched because of HTTP code `#{response.code}` error!") end response.body if errors.none? diff --git a/lib/gitlab/ci/config/external/file/template.rb b/lib/gitlab/ci/config/external/file/template.rb index 47441fa3818..c3d120dfdce 100644 --- a/lib/gitlab/ci/config/external/file/template.rb +++ b/lib/gitlab/ci/config/external/file/template.rb @@ -26,7 +26,7 @@ module Gitlab super unless template_name_valid? - errors.push("Template file `#{location}` is not a valid location!") + errors.push("Template file `#{masked_location}` is not a valid location!") end end diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb index fb62b48c076..eae0277314b 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -143,7 +143,7 @@ module Gitlab file_class.new(location, context) end.select(&:matching?) - raise AmbigiousSpecificationError, "Include `#{location.to_json}` needs to match exactly one accessor!" unless matching.one? + raise AmbigiousSpecificationError, "Include `#{masked_location(location.to_json)}` needs to match exactly one accessor!" unless matching.one? matching.first end @@ -182,6 +182,10 @@ module Gitlab def expand(data) ExpandVariables.expand(data, -> { context.variables_hash }) end + + def masked_location(location) + context.mask_variables_from(location) + end end end end diff --git a/lib/gitlab/database/background_migration/batched_migration_wrapper.rb b/lib/gitlab/database/background_migration/batched_migration_wrapper.rb index 057f856d859..e9f77d10fea 100644 --- a/lib/gitlab/database/background_migration/batched_migration_wrapper.rb +++ b/lib/gitlab/database/background_migration/batched_migration_wrapper.rb @@ -4,10 +4,9 @@ module Gitlab module Database module BackgroundMigration class BatchedMigrationWrapper - extend Gitlab::Utils::StrongMemoize - - def initialize(connection: ApplicationRecord.connection) + def initialize(connection: ApplicationRecord.connection, metrics: PrometheusMetrics.new) @connection = connection + @metrics = metrics end # Wraps the execution of a batched_background_migration. @@ -28,12 +27,12 @@ module Gitlab raise ensure - track_prometheus_metrics(batch_tracking_record) + metrics.track(batch_tracking_record) end private - attr_reader :connection + attr_reader :connection, :metrics def start_tracking_execution(tracking_record) tracking_record.run! @@ -63,80 +62,6 @@ module Gitlab job_class.new end end - - def track_prometheus_metrics(tracking_record) - migration = tracking_record.batched_migration - base_labels = migration.prometheus_labels - - metric_for(:gauge_batch_size).set(base_labels, tracking_record.batch_size) - metric_for(:gauge_sub_batch_size).set(base_labels, tracking_record.sub_batch_size) - metric_for(:gauge_interval).set(base_labels, tracking_record.batched_migration.interval) - metric_for(:gauge_job_duration).set(base_labels, (tracking_record.finished_at - tracking_record.started_at).to_i) - metric_for(:counter_updated_tuples).increment(base_labels, tracking_record.batch_size) - metric_for(:gauge_migrated_tuples).set(base_labels, tracking_record.batched_migration.migrated_tuple_count) - metric_for(:gauge_total_tuple_count).set(base_labels, tracking_record.batched_migration.total_tuple_count) - metric_for(:gauge_last_update_time).set(base_labels, Time.current.to_i) - - if metrics = tracking_record.metrics - metrics['timings']&.each do |key, timings| - summary = metric_for(:histogram_timings) - labels = base_labels.merge(operation: key) - - timings.each do |timing| - summary.observe(labels, timing) - end - end - end - end - - def metric_for(name) - self.class.metrics[name] - end - - def self.metrics - strong_memoize(:metrics) do - { - gauge_batch_size: Gitlab::Metrics.gauge( - :batched_migration_job_batch_size, - 'Batch size for a batched migration job' - ), - gauge_sub_batch_size: Gitlab::Metrics.gauge( - :batched_migration_job_sub_batch_size, - 'Sub-batch size for a batched migration job' - ), - gauge_interval: Gitlab::Metrics.gauge( - :batched_migration_job_interval_seconds, - 'Interval for a batched migration job' - ), - gauge_job_duration: Gitlab::Metrics.gauge( - :batched_migration_job_duration_seconds, - 'Duration for a batched migration job' - ), - counter_updated_tuples: Gitlab::Metrics.counter( - :batched_migration_job_updated_tuples_total, - 'Number of tuples updated by batched migration job' - ), - gauge_migrated_tuples: Gitlab::Metrics.gauge( - :batched_migration_migrated_tuples_total, - 'Total number of tuples migrated by a batched migration' - ), - histogram_timings: Gitlab::Metrics.histogram( - :batched_migration_job_query_duration_seconds, - 'Query timings for a batched migration job', - {}, - [0.1, 0.25, 0.5, 1, 5].freeze - ), - gauge_total_tuple_count: Gitlab::Metrics.gauge( - :batched_migration_total_tuple_count, - 'Total tuple count the migration needs to touch' - ), - gauge_last_update_time: Gitlab::Metrics.gauge( - :batched_migration_last_update_time_seconds, - 'Unix epoch time in seconds' - ) - } - end - end end end end diff --git a/lib/gitlab/database/background_migration/prometheus_metrics.rb b/lib/gitlab/database/background_migration/prometheus_metrics.rb new file mode 100644 index 00000000000..ce1da4c59eb --- /dev/null +++ b/lib/gitlab/database/background_migration/prometheus_metrics.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module BackgroundMigration + class PrometheusMetrics + extend Gitlab::Utils::StrongMemoize + + QUERY_TIMING_BUCKETS = [0.1, 0.25, 0.5, 1, 5].freeze + + def track(job_record) + migration_record = job_record.batched_migration + base_labels = migration_record.prometheus_labels + + metric_for(:gauge_batch_size).set(base_labels, job_record.batch_size) + metric_for(:gauge_sub_batch_size).set(base_labels, job_record.sub_batch_size) + metric_for(:gauge_interval).set(base_labels, job_record.batched_migration.interval) + metric_for(:gauge_job_duration).set(base_labels, (job_record.finished_at - job_record.started_at).to_i) + metric_for(:counter_updated_tuples).increment(base_labels, job_record.batch_size) + metric_for(:gauge_migrated_tuples).set(base_labels, migration_record.migrated_tuple_count) + metric_for(:gauge_total_tuple_count).set(base_labels, migration_record.total_tuple_count) + metric_for(:gauge_last_update_time).set(base_labels, Time.current.to_i) + + track_timing_metrics(base_labels, job_record.metrics) + end + + def self.metrics + strong_memoize(:metrics) do + { + gauge_batch_size: Gitlab::Metrics.gauge( + :batched_migration_job_batch_size, + 'Batch size for a batched migration job' + ), + gauge_sub_batch_size: Gitlab::Metrics.gauge( + :batched_migration_job_sub_batch_size, + 'Sub-batch size for a batched migration job' + ), + gauge_interval: Gitlab::Metrics.gauge( + :batched_migration_job_interval_seconds, + 'Interval for a batched migration job' + ), + gauge_job_duration: Gitlab::Metrics.gauge( + :batched_migration_job_duration_seconds, + 'Duration for a batched migration job' + ), + counter_updated_tuples: Gitlab::Metrics.counter( + :batched_migration_job_updated_tuples_total, + 'Number of tuples updated by batched migration job' + ), + gauge_migrated_tuples: Gitlab::Metrics.gauge( + :batched_migration_migrated_tuples_total, + 'Total number of tuples migrated by a batched migration' + ), + histogram_timings: Gitlab::Metrics.histogram( + :batched_migration_job_query_duration_seconds, + 'Query timings for a batched migration job', + {}, + QUERY_TIMING_BUCKETS + ), + gauge_total_tuple_count: Gitlab::Metrics.gauge( + :batched_migration_total_tuple_count, + 'Total tuple count the migration needs to touch' + ), + gauge_last_update_time: Gitlab::Metrics.gauge( + :batched_migration_last_update_time_seconds, + 'Unix epoch time in seconds' + ) + } + end + end + + private + + def track_timing_metrics(base_labels, metrics) + return unless metrics && metrics['timings'] + + metrics['timings'].each do |key, timings| + summary = metric_for(:histogram_timings) + labels = base_labels.merge(operation: key) + + timings.each do |timing| + summary.observe(labels, timing) + end + end + end + + def metric_for(name) + self.class.metrics[name] + end + end + end + end +end diff --git a/lib/gitlab/error_tracking.rb b/lib/gitlab/error_tracking.rb index 259b430a73c..d71f9b5e7cf 100644 --- a/lib/gitlab/error_tracking.rb +++ b/lib/gitlab/error_tracking.rb @@ -19,7 +19,8 @@ module Gitlab PROCESSORS = [ ::Gitlab::ErrorTracking::Processor::SidekiqProcessor, ::Gitlab::ErrorTracking::Processor::GrpcErrorProcessor, - ::Gitlab::ErrorTracking::Processor::ContextPayloadProcessor + ::Gitlab::ErrorTracking::Processor::ContextPayloadProcessor, + ::Gitlab::ErrorTracking::Processor::SanitizeErrorMessageProcessor ].freeze class << self diff --git a/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb new file mode 100644 index 00000000000..4b6c69a8b33 --- /dev/null +++ b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + module Processor + module Concerns + module ProcessesExceptions + private + + def extract_exceptions_from(event) + exceptions = if event.is_a?(Raven::Event) + event.instance_variable_get(:@interfaces)[:exception]&.values + else + event&.exception&.instance_variable_get(:@values) + end + + Array.wrap(exceptions) + end + + def set_exception_message(exception, message) + if exception.respond_to?(:value=) + exception.value = message + else + exception.instance_variable_set(:@value, message) + end + end + + def valid_exception?(exception) + case exception + when Raven::SingleExceptionInterface, Sentry::SingleExceptionInterface + exception&.value.present? + else + false + end + end + end + end + end + end +end diff --git a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb index 045a18f4110..ab0df39e512 100644 --- a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb +++ b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb @@ -4,6 +4,8 @@ module Gitlab module ErrorTracking module Processor module GrpcErrorProcessor + extend Gitlab::ErrorTracking::Processor::Concerns::ProcessesExceptions + DEBUG_ERROR_STRING_REGEX = RE2('(.*) debug_error_string:(.*)') class << self @@ -19,9 +21,6 @@ module Gitlab def process_first_exception_value(event) # Better in new version, will be event.exception.values exceptions = extract_exceptions_from(event) - - return unless exceptions.is_a?(Array) - exception = exceptions.first return unless valid_exception?(exception) @@ -39,11 +38,7 @@ module Gitlab exceptions.each do |exception| next unless valid_exception?(exception) - if exception.respond_to?(:value=) - exception.value = message - else - exception.instance_variable_set(:@value, message) - end + set_exception_message(exception, message) end end @@ -59,16 +54,6 @@ module Gitlab fingerprint[1] = message if message end - private - - def extract_exceptions_from(event) - if event.is_a?(Raven::Event) - event.instance_variable_get(:@interfaces)[:exception]&.values - else - event.exception&.instance_variable_get(:@values) - end - end - def custom_grpc_fingerprint?(fingerprint) fingerprint.is_a?(Array) && fingerprint.length == 2 && fingerprint[0].start_with?('GRPC::') end @@ -82,15 +67,6 @@ module Gitlab [match[1], match[2]] end - - def valid_exception?(exception) - case exception - when Raven::SingleExceptionInterface, Sentry::SingleExceptionInterface - exception&.value - else - false - end - end end end end diff --git a/lib/gitlab/error_tracking/processor/sanitize_error_message_processor.rb b/lib/gitlab/error_tracking/processor/sanitize_error_message_processor.rb new file mode 100644 index 00000000000..1d6547256c7 --- /dev/null +++ b/lib/gitlab/error_tracking/processor/sanitize_error_message_processor.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + module Processor + module SanitizeErrorMessageProcessor + extend Gitlab::ErrorTracking::Processor::Concerns::ProcessesExceptions + + class << self + def call(event) + exceptions = extract_exceptions_from(event) + + exceptions.each do |exception| + next unless valid_exception?(exception) + + message = Gitlab::Sanitizers::ExceptionMessage.clean(exception.type, exception.value) + + set_exception_message(exception, message) + end + + event + end + end + end + end + end +end diff --git a/lib/gitlab/exception_log_formatter.rb b/lib/gitlab/exception_log_formatter.rb index 315574fed31..ce802b562f0 100644 --- a/lib/gitlab/exception_log_formatter.rb +++ b/lib/gitlab/exception_log_formatter.rb @@ -10,7 +10,7 @@ module Gitlab # Use periods to flatten the fields. payload.merge!( 'exception.class' => exception.class.name, - 'exception.message' => exception.message + 'exception.message' => sanitize_message(exception) ) if exception.backtrace @@ -38,6 +38,10 @@ module Gitlab rescue PgQuery::ParseError sql end + + def sanitize_message(exception) + Gitlab::Sanitizers::ExceptionMessage.clean(exception.class.name, exception.message) + end end end end diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index dd7ec361dd8..d3b1bb6a57d 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -19,9 +19,8 @@ module Gitlab @exported_members.inject(missing_keys_tracking_hash) do |hash, member| if member['user'] old_user_id = member['user']['id'] - old_user_email = member.dig('user', 'public_email') || member.dig('user', 'email') - existing_user = User.find_by(find_user_query(old_user_email)) if old_user_email - hash[old_user_id] = existing_user.id if existing_user && add_team_member(member, existing_user) + existing_user_id = existing_users_email_map[get_email(member)] + hash[old_user_id] = existing_user_id if existing_user_id && add_team_member(member, existing_user_id) else add_team_member(member) end @@ -72,11 +71,45 @@ module Gitlab member&.user == @user && member.access_level >= highest_access_level end - def add_team_member(member, existing_user = nil) - return true if existing_user && @importable.members.exists?(user_id: existing_user.id) + # Returns {email => user_id} hash where user_id is an ID at current instance + def existing_users_email_map + @existing_users_email_map ||= begin + emails = @exported_members.map { |member| get_email(member) } + + User.by_user_email(emails).pluck(:email, :id).to_h + end + end + + # Returns {user_id => email} hash where user_id is an ID at source "old" instance + def exported_members_email_map + @exported_members_email_map ||= begin + result = {} + @exported_members.each do |member| + email = get_email(member) + + next unless email + + result[member.dig('user', 'id')] = email + end + + result + end + end + + def get_email(member_data) + return unless member_data['user'] + + member_data.dig('user', 'public_email') || member_data.dig('user', 'email') + end + + def add_team_member(member, existing_user_id = nil) + return true if existing_user_id && @importable.members.exists?(user_id: existing_user_id) - member['user'] = existing_user member_hash = member_hash(member) + if existing_user_id + member_hash.delete('user') + member_hash['user_id'] = existing_user_id + end member = relation_class.create(member_hash) @@ -92,11 +125,19 @@ module Gitlab end def member_hash(member) - parsed_hash(member).merge( + result = parsed_hash(member).merge( 'source_id' => @importable.id, 'importing' => true, 'access_level' => [member['access_level'], highest_access_level].min ).except('user_id') + + if result['created_by_id'] + created_by_email = exported_members_email_map[result['created_by_id']] + + result['created_by_id'] = existing_users_email_map[created_by_email] + end + + result end def parsed_hash(member) @@ -104,14 +145,6 @@ module Gitlab relation_class: relation_class) end - def find_user_query(email) - user_arel[:email].eq(email) - end - - def user_arel - @user_arel ||= User.arel_table - end - def relation_class case @importable when ::Project @@ -143,7 +176,7 @@ module Gitlab def base_log_params(member_hash) { - user_id: member_hash['user']&.id, + user_id: member_hash['user_id'], access_level: member_hash['access_level'], importable_type: @importable.class.to_s, importable_id: @importable.id, diff --git a/lib/gitlab/password.rb b/lib/gitlab/password.rb deleted file mode 100644 index 00aef8754d6..00000000000 --- a/lib/gitlab/password.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -# This module is used to return fake strong password for tests - -module Gitlab - module Password - DEFAULT_LENGTH = 12 - TEST_DEFAULT = "123qweQWE!@#" + "0" * (User.password_length.max - DEFAULT_LENGTH) - def self.test_default(length = 12) - password_length = [[User.password_length.min, length].max, User.password_length.max].min - TEST_DEFAULT[...password_length] - end - end -end diff --git a/lib/gitlab/sanitizers/exception_message.rb b/lib/gitlab/sanitizers/exception_message.rb new file mode 100644 index 00000000000..11c91093d88 --- /dev/null +++ b/lib/gitlab/sanitizers/exception_message.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + module Sanitizers + module ExceptionMessage + FILTERED_STRING = '[FILTERED]' + EXCEPTION_NAMES = %w(URI::InvalidURIError Addressable::URI::InvalidURIError).freeze + MESSAGE_REGEX = %r{(\A[^:]+:\s).*\Z}.freeze + + class << self + def clean(exception_name, message) + return message unless exception_name.in?(EXCEPTION_NAMES) + + message.sub(MESSAGE_REGEX, '\1' + FILTERED_STRING) + end + end + end + end +end diff --git a/lib/tasks/gitlab/seed/group_seed.rake b/lib/tasks/gitlab/seed/group_seed.rake index 491cf782985..a9a350fb6c3 100644 --- a/lib/tasks/gitlab/seed/group_seed.rake +++ b/lib/tasks/gitlab/seed/group_seed.rake @@ -125,7 +125,7 @@ class GroupSeeder name: FFaker::Name.name, email: FFaker::Internet.email, confirmed_at: DateTime.now, - password: Gitlab::Password.test_default + password: Devise.friendly_token ) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0d9c0c70d1e..ef0acfb4533 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10829,43 +10829,25 @@ msgstr "" msgid "Critical vulnerabilities present" msgstr "" -msgid "Crm|Contact has been added" +msgid "Crm|Contact has been added." msgstr "" -msgid "Crm|Contact has been updated" +msgid "Crm|Contact has been updated." msgstr "" -msgid "Crm|Create new contact" +msgid "Crm|Customer relations contacts" msgstr "" -msgid "Crm|Create organization" +msgid "Crm|Customer relations organizations" msgstr "" -msgid "Crm|Customer Relations Contacts" -msgstr "" - -msgid "Crm|Customer Relations Organizations" -msgstr "" - -msgid "Crm|Default rate (optional)" -msgstr "" - -msgid "Crm|Description (optional)" +msgid "Crm|Default rate" msgstr "" msgid "Crm|Edit contact" msgstr "" -msgid "Crm|Email" -msgstr "" - -msgid "Crm|First name" -msgstr "" - -msgid "Crm|Last name" -msgstr "" - -msgid "Crm|New Organization" +msgid "Crm|Edit organization" msgstr "" msgid "Crm|New contact" @@ -10880,10 +10862,10 @@ msgstr "" msgid "Crm|No organizations found" msgstr "" -msgid "Crm|Organization has been added" +msgid "Crm|Organization has been added." msgstr "" -msgid "Crm|Phone number (optional)" +msgid "Crm|Organization has been updated." msgstr "" msgid "Cron Timezone" @@ -10994,16 +10976,16 @@ msgstr "" msgid "Custom range (UTC)" msgstr "" -msgid "Customer Relations Contacts" +msgid "Customer experience improvement and third-party offers" msgstr "" -msgid "Customer Relations Organizations" +msgid "Customer relations" msgstr "" -msgid "Customer experience improvement and third-party offers" +msgid "Customer relations contacts" msgstr "" -msgid "Customer relations" +msgid "Customer relations organizations" msgstr "" msgid "Customize CI/CD settings, including Auto DevOps, shared runners, and job artifacts." @@ -13223,6 +13205,9 @@ msgstr "" msgid "Dismissed on pipeline %{pipelineLink} at %{projectLink}" msgstr "" +msgid "Display" +msgstr "" + msgid "Display alerts from all configured monitoring tools." msgstr "" @@ -27269,6 +27254,9 @@ msgstr "" msgid "Phabricator Tasks" msgstr "" +msgid "Phone" +msgstr "" + msgid "Pick a name" msgstr "" diff --git a/package.json b/package.json index 99db42bdc8f..794321f4cb6 100644 --- a/package.json +++ b/package.json @@ -170,7 +170,7 @@ "sortablejs": "^1.10.2", "string-hash": "1.1.3", "style-loader": "^2.0.0", - "swagger-ui-dist": "^3.52.3", + "swagger-ui-dist": "4.8.0", "three": "^0.84.0", "three-orbit-controls": "^82.1.0", "three-stl-loader": "^1.0.4", diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index c52223d4758..c46a12680a2 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -612,8 +612,8 @@ RSpec.describe Admin::UsersController do end context 'when the new password does not match the password confirmation' do - let(:password) { Gitlab::Password.test_default } - let(:password_confirmation) { "not" + Gitlab::Password.test_default } + let(:password) { 'some_password' } + let(:password_confirmation) { 'not_same_as_password' } it 'shows the edit page again' do update_password(user, password, password_confirmation) diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 3c650988b4f..a061a14c7b1 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -186,6 +186,7 @@ RSpec.describe Projects::MergeRequests::CreationsController do it 'fetches the commit if a user has access' do expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true } + expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { true }.at_least(:once) get :branch_to, params: { @@ -199,8 +200,25 @@ RSpec.describe Projects::MergeRequests::CreationsController do expect(response).to have_gitlab_http_status(:ok) end + it 'does not load the commit when the user cannot create_merge_request_in' do + expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true } + expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { false }.at_least(:once) + + get :branch_to, + params: { + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id, + ref: 'master' + } + + expect(assigns(:commit)).to be_nil + expect(response).to have_gitlab_http_status(:ok) + end + it 'does not load the commit when the user cannot read the project' do expect(Ability).to receive(:allowed?).with(user, :read_project, project) { false } + expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { true }.at_least(:once) get :branch_to, params: { diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index 7bc86d7c583..686effd799e 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -177,6 +177,7 @@ RSpec.describe Projects::MirrorsController do INVALID git@example.com:foo/bar.git ssh://git@example.com:foo/bar.git + ssh://127.0.0.1/foo/bar.git ].each do |url| it "returns an error with a 400 response for URL #{url.inspect}" do do_get(project, url) diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index af34ae2f69b..caff7bcfc7b 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -521,7 +521,7 @@ RSpec.describe RegistrationsController do end it 'succeeds if password is confirmed' do - post :destroy, params: { password: Gitlab::Password.test_default } + post :destroy, params: { password: '12345678' } expect_success end @@ -562,7 +562,7 @@ RSpec.describe RegistrationsController do end it 'fails' do - delete :destroy, params: { password: Gitlab::Password.test_default } + delete :destroy, params: { password: '12345678' } expect_failure(s_('Profiles|You must transfer ownership or delete groups you are an owner of before you can delete your account')) end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index eb89cb0a40a..70b0af8a36c 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -5,7 +5,7 @@ FactoryBot.define do email { generate(:email) } name { generate(:name) } username { generate(:username) } - password { Gitlab::Password.test_default } + password { "12345678" } role { 'software_developer' } confirmed_at { Time.now } confirmation_token { nil } @@ -27,6 +27,10 @@ FactoryBot.define do after(:build) { |user, _| user.block! } end + trait :disallowed_password do + password { User::DISALLOWED_PASSWORDS.first } + end + trait :blocked_pending_approval do after(:build) { |user, _| user.block_pending_approval! } end diff --git a/spec/features/markdown/kroki_spec.rb b/spec/features/markdown/kroki_spec.rb new file mode 100644 index 00000000000..f02f5d44244 --- /dev/null +++ b/spec/features/markdown/kroki_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'kroki rendering', :js do + let_it_be(:project) { create(:project, :public) } + + before do + stub_application_setting(kroki_enabled: true, kroki_url: 'http://localhost:8000') + end + + it 'shows kroki image' do + plain_text = 'This text length is ignored. ' * 300 + + description = <<~KROKI + #{plain_text} + ```plantuml + A -> A: T + ``` + KROKI + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + within('.description') do + expect(page).to have_css('img') + expect(page).not_to have_text 'Warning: Displaying this diagram might cause performance issues on this page.' + end + end + + it 'hides kroki image and shows warning alert when kroki source size is large' do + plantuml_text = 'A -> A: T ' * 300 + + description = <<~KROKI + ```plantuml + #{plantuml_text} + ``` + KROKI + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + within('.description') do + expect(page).not_to have_css('img') + expect(page).to have_text 'Warning: Displaying this diagram might cause performance issues on this page.' + + click_button 'Display' + + expect(page).to have_css('img') + expect(page).not_to have_text 'Warning: Displaying this diagram might cause performance issues on this page.' + end + end +end diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb index a4e167a3e75..f89e19f5361 100644 --- a/spec/features/password_reset_spec.rb +++ b/spec/features/password_reset_spec.rb @@ -44,8 +44,8 @@ RSpec.describe 'Password reset' do visit(edit_user_password_path(reset_password_token: token)) - fill_in 'New password', with: "new" + Gitlab::Password.test_default - fill_in 'Confirm new password', with: "new" + Gitlab::Password.test_default + fill_in 'New password', with: 'hello1234' + fill_in 'Confirm new password', with: 'hello1234' click_button 'Change your password' diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb index 34eb07d78f1..36657406303 100644 --- a/spec/features/profile_spec.rb +++ b/spec/features/profile_spec.rb @@ -29,7 +29,7 @@ RSpec.describe 'Profile account page', :js do it 'deletes user', :js, :sidekiq_might_not_need_inline do click_button 'Delete account' - fill_in 'password', with: Gitlab::Password.test_default + fill_in 'password', with: '12345678' page.within '.modal' do click_button 'Delete account' diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb index 2181285f771..7eadb74d2d4 100644 --- a/spec/features/profiles/password_spec.rb +++ b/spec/features/profiles/password_spec.rb @@ -39,7 +39,7 @@ RSpec.describe 'Profile > Password' do describe 'User puts the same passwords in the field and in the confirmation' do it 'shows a success message' do - fill_passwords(Gitlab::Password.test_default, Gitlab::Password.test_default) + fill_passwords('mypassword', 'mypassword') page.within('[data-testid="alert-info"]') do expect(page).to have_content('Password was successfully updated. Please sign in again.') @@ -79,7 +79,7 @@ RSpec.describe 'Profile > Password' do end context 'Change password' do - let(:new_password) { "new" + Gitlab::Password.test_default } + let(:new_password) { '22233344' } before do sign_in(user) @@ -170,8 +170,8 @@ RSpec.describe 'Profile > Password' do expect(page).to have_current_path new_profile_password_path, ignore_query: true fill_in :user_password, with: user.password - fill_in :user_new_password, with: Gitlab::Password.test_default - fill_in :user_password_confirmation, with: Gitlab::Password.test_default + fill_in :user_new_password, with: '12345678' + fill_in :user_password_confirmation, with: '12345678' click_button 'Set new password' expect(page).to have_current_path new_user_session_path, ignore_query: true diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 05fd72a8932..76baad63cc2 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -948,6 +948,53 @@ RSpec.describe 'File blob', :js do end end end + + context 'openapi.yml' do + before do + file_name = 'openapi.yml' + + create_file(file_name, ' + swagger: \'2.0\' + info: + title: Classic API Resource Documentation + description: | + <div class="foo-bar" style="background-color: red;" data-foo-bar="baz"> + <h1>Swagger API documentation</h1> + </div> + version: production + basePath: /JSSResource/ + produces: + - application/xml + - application/json + consumes: + - application/xml + - application/json + security: + - basicAuth: [] + paths: + /accounts: + get: + responses: + \'200\': + description: No response was specified + tags: + - accounts + operationId: findAccounts + summary: Finds all accounts + ') + visit_blob(file_name, useUnsafeMarkdown: '1') + click_button('Display rendered file') + + wait_for_requests + end + + it 'removes `style`, `class`, and `data-*`` attributes from HTML' do + expect(page).to have_css('h1', text: 'Swagger API documentation') + expect(page).not_to have_css('.foo-bar') + expect(page).not_to have_css('[style="background-color: red;"]') + expect(page).not_to have_css('[data-foo-bar="baz"]') + end + end end context 'realtime pipelines' do diff --git a/spec/features/refactor_blob_viewer_disabled/projects/blobs/blob_show_spec.rb b/spec/features/refactor_blob_viewer_disabled/projects/blobs/blob_show_spec.rb index 659014c922b..5574b4da383 100644 --- a/spec/features/refactor_blob_viewer_disabled/projects/blobs/blob_show_spec.rb +++ b/spec/features/refactor_blob_viewer_disabled/projects/blobs/blob_show_spec.rb @@ -1050,6 +1050,53 @@ RSpec.describe 'File blob', :js do end end end + + context 'openapi.yml' do + before do + file_name = 'openapi.yml' + + create_file(file_name, ' + swagger: \'2.0\' + info: + title: Classic API Resource Documentation + description: | + <div class="foo-bar" style="background-color: red;" data-foo-bar="baz"> + <h1>Swagger API documentation</h1> + </div> + version: production + basePath: /JSSResource/ + produces: + - application/xml + - application/json + consumes: + - application/xml + - application/json + security: + - basicAuth: [] + paths: + /accounts: + get: + responses: + \'200\': + description: No response was specified + tags: + - accounts + operationId: findAccounts + summary: Finds all accounts + ') + visit_blob(file_name, useUnsafeMarkdown: '1') + click_button('Display rendered file') + + wait_for_requests + end + + it 'removes `style`, `class`, and `data-*`` attributes from HTML' do + expect(page).to have_css('h1', text: 'Swagger API documentation') + expect(page).not_to have_css('.foo-bar') + expect(page).not_to have_css('[style="background-color: red;"]') + expect(page).not_to have_css('[data-foo-bar="baz"]') + end + end end context 'realtime pipelines' do diff --git a/spec/features/users/anonymous_sessions_spec.rb b/spec/features/users/anonymous_sessions_spec.rb index f9b23626397..6b21412ae3d 100644 --- a/spec/features/users/anonymous_sessions_spec.rb +++ b/spec/features/users/anonymous_sessions_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Session TTLs', :clean_gitlab_redis_shared_state do visit new_user_session_path # The session key only gets created after a post fill_in 'user_login', with: 'non-existant@gitlab.org' - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' expect(page).to have_content('Invalid login or password') diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 4d06415e203..8610cae58a4 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -49,15 +49,15 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do expect(page).to have_current_path edit_user_password_path, ignore_query: true expect(page).to have_content('Please create a password for your new account.') - fill_in 'user_password', with: Gitlab::Password.test_default - fill_in 'user_password_confirmation', with: Gitlab::Password.test_default + fill_in 'user_password', with: 'password' + fill_in 'user_password_confirmation', with: 'password' click_button 'Change your password' expect(page).to have_current_path new_user_session_path, ignore_query: true expect(page).to have_content(I18n.t('devise.passwords.updated_not_active')) fill_in 'user_login', with: user.username - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: 'password' click_button 'Sign in' expect_single_session_with_authenticated_ttl @@ -150,6 +150,27 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do end end + describe 'with a disallowed password' do + let(:user) { create(:user, :disallowed_password) } + + before do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + end + + it 'disallows login' do + gitlab_sign_in(user, password: user.password) + + expect(page).to have_content('Invalid login or password.') + end + + it 'does not update Devise trackable attributes' do + expect { gitlab_sign_in(user, password: user.password) } + .not_to change { User.ghost.reload.sign_in_count } + end + end + describe 'with the ghost user' do it 'disallows login' do expect(authentication_metrics) @@ -210,7 +231,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do end it 'does not allow sign-in if the user password is updated before entering a one-time code' do - user.update!(password: "new" + Gitlab::Password.test_default) + user.update!(password: 'new_password') enter_code(user.current_otp) @@ -447,7 +468,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' expect(page).to have_current_path(new_profile_password_path, ignore_query: true) @@ -456,7 +477,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do end context 'with invalid username and password' do - let(:user) { create(:user, password: "not" + Gitlab::Password.test_default) } + let(:user) { create(:user, password: 'not-the-default') } it 'blocks invalid login' do expect(authentication_metrics) @@ -767,7 +788,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' @@ -788,7 +809,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' @@ -810,7 +831,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' @@ -845,7 +866,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' fill_in 'user_otp_attempt', with: user.reload.current_otp @@ -871,7 +892,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' expect_to_be_on_terms_page @@ -879,7 +900,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do expect(page).to have_current_path(new_profile_password_path, ignore_query: true) - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' fill_in 'user_new_password', with: 'new password' fill_in 'user_password_confirmation', with: 'new password' click_button 'Set new password' diff --git a/spec/fixtures/markdown/markdown_golden_master_examples.yml b/spec/fixtures/markdown/markdown_golden_master_examples.yml index 70a23d09c62..bdd7c13c1a3 100644 --- a/spec/fixtures/markdown/markdown_golden_master_examples.yml +++ b/spec/fixtures/markdown/markdown_golden_master_examples.yml @@ -391,7 +391,7 @@ ] ``` html: |- - <a class="no-attachment-icon" href="http://localhost:8000/nomnoml/svg/eNp1jbsOwjAMRfd-haUuIJQBBlRFVZb2L1CGkBqpgtpR6oEhH0_CW6hsts-9xwD1LJHPqKF2zX67ayqAQ3uKbkLTo-fohCMEJ4KRUoYFu2MuOS-m4ykwIUlKG-CAOT0yrdb2EewuY2YWBgxIwwxKmXx8dZ6h95ekgPAqGv4miuk-YnEVFfmIgr-Fzw6tVt-CZb7osdUNUAReJA==" target="_blank" rel="noopener noreferrer" data-diagram="nomnoml" data-diagram-src="data:text/plain;base64,ICAjc3Ryb2tlOiAjYTg2MTI4CiAgWzxmcmFtZT5EZWNvcmF0b3IgcGF0dGVybnwKICAgIFs8YWJzdHJhY3Q+Q29tcG9uZW50fHwrIG9wZXJhdGlvbigpXQogICAgW0NsaWVudF0gZGVwZW5kcyAtLT4gW0NvbXBvbmVudF0KICAgIFtEZWNvcmF0b3J8LSBuZXh0OiBDb21wb25lbnRdCiAgICBbRGVjb3JhdG9yXSBkZWNvcmF0ZXMgLS0gW0NvbmNyZXRlQ29tcG9uZW50XQogICAgW0NvbXBvbmVudF0gPDotIFtEZWNvcmF0b3JdCiAgICBbQ29tcG9uZW50XSA8Oi0gW0NvbmNyZXRlQ29tcG9uZW50XQogIF0K"><img src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" class="lazy" data-src="http://localhost:8000/nomnoml/svg/eNp1jbsOwjAMRfd-haUuIJQBBlRFVZb2L1CGkBqpgtpR6oEhH0_CW6hsts-9xwD1LJHPqKF2zX67ayqAQ3uKbkLTo-fohCMEJ4KRUoYFu2MuOS-m4ykwIUlKG-CAOT0yrdb2EewuY2YWBgxIwwxKmXx8dZ6h95ekgPAqGv4miuk-YnEVFfmIgr-Fzw6tVt-CZb7osdUNUAReJA=="></a> + <a class="no-attachment-icon" href="http://localhost:8000/nomnoml/svg/eNp1jbsOwjAMRfd-haUuIJQBBlRFVZb2L1CGkBqpgtpR6oEhH0_CW6hsts-9xwD1LJHPqKF2zX67ayqAQ3uKbkLTo-fohCMEJ4KRUoYFu2MuOS-m4ykwIUlKG-CAOT0yrdb2EewuY2YWBgxIwwxKmXx8dZ6h95ekgPAqGv4miuk-YnEVFfmIgr-Fzw6tVt-CZb7osdUNUAReJA==" target="_blank" rel="noopener noreferrer" data-diagram="nomnoml" data-diagram-src="data:text/plain;base64,ICAjc3Ryb2tlOiAjYTg2MTI4CiAgWzxmcmFtZT5EZWNvcmF0b3IgcGF0dGVybnwKICAgIFs8YWJzdHJhY3Q+Q29tcG9uZW50fHwrIG9wZXJhdGlvbigpXQogICAgW0NsaWVudF0gZGVwZW5kcyAtLT4gW0NvbXBvbmVudF0KICAgIFtEZWNvcmF0b3J8LSBuZXh0OiBDb21wb25lbnRdCiAgICBbRGVjb3JhdG9yXSBkZWNvcmF0ZXMgLS0gW0NvbmNyZXRlQ29tcG9uZW50XQogICAgW0NvbXBvbmVudF0gPDotIFtEZWNvcmF0b3JdCiAgICBbQ29tcG9uZW50XSA8Oi0gW0NvbmNyZXRlQ29tcG9uZW50XQogIF0K"><img src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" class="js-render-kroki lazy" data-src="http://localhost:8000/nomnoml/svg/eNp1jbsOwjAMRfd-haUuIJQBBlRFVZb2L1CGkBqpgtpR6oEhH0_CW6hsts-9xwD1LJHPqKF2zX67ayqAQ3uKbkLTo-fohCMEJ4KRUoYFu2MuOS-m4ykwIUlKG-CAOT0yrdb2EewuY2YWBgxIwwxKmXx8dZ6h95ekgPAqGv4miuk-YnEVFfmIgr-Fzw6tVt-CZb7osdUNUAReJA=="></a> - name: diagram_plantuml markdown: |- diff --git a/spec/frontend/behaviors/components/diagram_performance_warning_spec.js b/spec/frontend/behaviors/components/diagram_performance_warning_spec.js new file mode 100644 index 00000000000..c58c2bc55a9 --- /dev/null +++ b/spec/frontend/behaviors/components/diagram_performance_warning_spec.js @@ -0,0 +1,40 @@ +import { GlAlert } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import DiagramPerformanceWarning from '~/behaviors/components/diagram_performance_warning.vue'; + +describe('DiagramPerformanceWarning component', () => { + let wrapper; + + const findAlert = () => wrapper.findComponent(GlAlert); + + beforeEach(() => { + wrapper = shallowMount(DiagramPerformanceWarning); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('renders warning alert with button', () => { + expect(findAlert().props()).toMatchObject({ + primaryButtonText: DiagramPerformanceWarning.i18n.buttonText, + variant: 'warning', + }); + }); + + it('renders warning message', () => { + expect(findAlert().text()).toBe(DiagramPerformanceWarning.i18n.bodyText); + }); + + it('emits event when closing alert', () => { + findAlert().vm.$emit('dismiss'); + + expect(wrapper.emitted('closeAlert')).toEqual([[]]); + }); + + it('emits event when accepting alert', () => { + findAlert().vm.$emit('primaryAction'); + + expect(wrapper.emitted('showImage')).toEqual([[]]); + }); +}); diff --git a/spec/frontend/crm/contact_form_spec.js b/spec/frontend/crm/contact_form_spec.js deleted file mode 100644 index 0edab4f5ec5..00000000000 --- a/spec/frontend/crm/contact_form_spec.js +++ /dev/null @@ -1,157 +0,0 @@ -import { GlAlert } from '@gitlab/ui'; -import Vue from 'vue'; -import VueApollo from 'vue-apollo'; -import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import createMockApollo from 'helpers/mock_apollo_helper'; -import waitForPromises from 'helpers/wait_for_promises'; -import ContactForm from '~/crm/components/contact_form.vue'; -import createContactMutation from '~/crm/components/queries/create_contact.mutation.graphql'; -import updateContactMutation from '~/crm/components/queries/update_contact.mutation.graphql'; -import getGroupContactsQuery from '~/crm/components/queries/get_group_contacts.query.graphql'; -import { - createContactMutationErrorResponse, - createContactMutationResponse, - getGroupContactsQueryResponse, - updateContactMutationErrorResponse, - updateContactMutationResponse, -} from './mock_data'; - -describe('Customer relations contact form component', () => { - Vue.use(VueApollo); - let wrapper; - let fakeApollo; - let mutation; - let queryHandler; - - const findSaveContactButton = () => wrapper.findByTestId('save-contact-button'); - const findCancelButton = () => wrapper.findByTestId('cancel-button'); - const findForm = () => wrapper.find('form'); - const findError = () => wrapper.findComponent(GlAlert); - - const mountComponent = ({ mountFunction = shallowMountExtended, editForm = false } = {}) => { - fakeApollo = createMockApollo([[mutation, queryHandler]]); - fakeApollo.clients.defaultClient.cache.writeQuery({ - query: getGroupContactsQuery, - variables: { groupFullPath: 'flightjs' }, - data: getGroupContactsQueryResponse.data, - }); - const propsData = { drawerOpen: true }; - if (editForm) - propsData.contact = { firstName: 'First', lastName: 'Last', email: 'email@example.com' }; - wrapper = mountFunction(ContactForm, { - provide: { groupId: 26, groupFullPath: 'flightjs' }, - apolloProvider: fakeApollo, - propsData, - }); - }; - - beforeEach(() => { - mutation = createContactMutation; - queryHandler = jest.fn().mockResolvedValue(createContactMutationResponse); - }); - - afterEach(() => { - wrapper.destroy(); - fakeApollo = null; - }); - - describe('Save contact button', () => { - it('should be disabled when required fields are empty', () => { - mountComponent(); - - expect(findSaveContactButton().props('disabled')).toBe(true); - }); - - it('should not be disabled when required fields have values', async () => { - mountComponent(); - - wrapper.find('#contact-first-name').vm.$emit('input', 'A'); - wrapper.find('#contact-last-name').vm.$emit('input', 'B'); - wrapper.find('#contact-email').vm.$emit('input', 'C'); - await waitForPromises(); - - expect(findSaveContactButton().props('disabled')).toBe(false); - }); - }); - - it("should emit 'close' when cancel button is clicked", () => { - mountComponent(); - - findCancelButton().vm.$emit('click'); - - expect(wrapper.emitted().close).toBeTruthy(); - }); - - describe('when create mutation is successful', () => { - it("should emit 'close'", async () => { - mountComponent(); - - findForm().trigger('submit'); - await waitForPromises(); - - expect(wrapper.emitted().close).toBeTruthy(); - }); - }); - - describe('when create mutation fails', () => { - it('should show error on reject', async () => { - queryHandler = jest.fn().mockRejectedValue('ERROR'); - mountComponent(); - - findForm().trigger('submit'); - await waitForPromises(); - - expect(findError().exists()).toBe(true); - }); - - it('should show error on error response', async () => { - queryHandler = jest.fn().mockResolvedValue(createContactMutationErrorResponse); - mountComponent(); - - findForm().trigger('submit'); - await waitForPromises(); - - expect(findError().exists()).toBe(true); - expect(findError().text()).toBe('create contact is invalid.'); - }); - }); - - describe('when update mutation is successful', () => { - it("should emit 'close'", async () => { - mutation = updateContactMutation; - queryHandler = jest.fn().mockResolvedValue(updateContactMutationResponse); - mountComponent({ editForm: true }); - - findForm().trigger('submit'); - await waitForPromises(); - - expect(wrapper.emitted().close).toBeTruthy(); - }); - }); - - describe('when update mutation fails', () => { - beforeEach(() => { - mutation = updateContactMutation; - }); - - it('should show error on reject', async () => { - queryHandler = jest.fn().mockRejectedValue('ERROR'); - mountComponent({ editForm: true }); - findForm().trigger('submit'); - await waitForPromises(); - - expect(findError().exists()).toBe(true); - }); - - it('should show error on error response', async () => { - queryHandler = jest.fn().mockResolvedValue(updateContactMutationErrorResponse); - mountComponent({ editForm: true }); - - findForm().trigger('submit'); - await waitForPromises(); - - expect(findError().exists()).toBe(true); - expect(findError().text()).toBe('update contact is invalid.'); - }); - }); -}); diff --git a/spec/frontend/crm/contact_form_wrapper_spec.js b/spec/frontend/crm/contact_form_wrapper_spec.js new file mode 100644 index 00000000000..6307889a7aa --- /dev/null +++ b/spec/frontend/crm/contact_form_wrapper_spec.js @@ -0,0 +1,88 @@ +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import ContactFormWrapper from '~/crm/contacts/components/contact_form_wrapper.vue'; +import ContactForm from '~/crm/components/form.vue'; +import getGroupContactsQuery from '~/crm/contacts/components/graphql/get_group_contacts.query.graphql'; +import createContactMutation from '~/crm/contacts/components/graphql/create_contact.mutation.graphql'; +import updateContactMutation from '~/crm/contacts/components/graphql/update_contact.mutation.graphql'; + +describe('Customer relations contact form wrapper', () => { + let wrapper; + + const findContactForm = () => wrapper.findComponent(ContactForm); + + const $apollo = { + queries: { + contacts: { + loading: false, + }, + }, + }; + const $route = { + params: { + id: 7, + }, + }; + const contacts = [{ id: 'gid://gitlab/CustomerRelations::Contact/7' }]; + + const mountComponent = ({ isEditMode = false } = {}) => { + wrapper = shallowMountExtended(ContactFormWrapper, { + propsData: { + isEditMode, + }, + provide: { + groupFullPath: 'flightjs', + groupId: 26, + }, + mocks: { + $apollo, + $route, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('in edit mode', () => { + it('should render contact form with correct props', () => { + mountComponent({ isEditMode: true }); + + const contactForm = findContactForm(); + expect(contactForm.props('fields')).toHaveLength(5); + expect(contactForm.props('title')).toBe('Edit contact'); + expect(contactForm.props('successMessage')).toBe('Contact has been updated.'); + expect(contactForm.props('mutation')).toBe(updateContactMutation); + expect(contactForm.props('getQuery')).toMatchObject({ + query: getGroupContactsQuery, + variables: { groupFullPath: 'flightjs' }, + }); + expect(contactForm.props('getQueryNodePath')).toBe('group.contacts'); + expect(contactForm.props('existingId')).toBe(contacts[0].id); + expect(contactForm.props('additionalCreateParams')).toMatchObject({ + groupId: 'gid://gitlab/Group/26', + }); + }); + }); + + describe('in create mode', () => { + it('should render contact form with correct props', () => { + mountComponent(); + + const contactForm = findContactForm(); + expect(contactForm.props('fields')).toHaveLength(5); + expect(contactForm.props('title')).toBe('New contact'); + expect(contactForm.props('successMessage')).toBe('Contact has been added.'); + expect(contactForm.props('mutation')).toBe(createContactMutation); + expect(contactForm.props('getQuery')).toMatchObject({ + query: getGroupContactsQuery, + variables: { groupFullPath: 'flightjs' }, + }); + expect(contactForm.props('getQueryNodePath')).toBe('group.contacts'); + expect(contactForm.props('existingId')).toBeNull(); + expect(contactForm.props('additionalCreateParams')).toMatchObject({ + groupId: 'gid://gitlab/Group/26', + }); + }); + }); +}); diff --git a/spec/frontend/crm/contacts_root_spec.js b/spec/frontend/crm/contacts_root_spec.js index b30349305a3..b02d94e9cb1 100644 --- a/spec/frontend/crm/contacts_root_spec.js +++ b/spec/frontend/crm/contacts_root_spec.js @@ -5,11 +5,9 @@ import VueRouter from 'vue-router'; import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import ContactsRoot from '~/crm/components/contacts_root.vue'; -import ContactForm from '~/crm/components/contact_form.vue'; -import getGroupContactsQuery from '~/crm/components/queries/get_group_contacts.query.graphql'; -import { NEW_ROUTE_NAME, EDIT_ROUTE_NAME } from '~/crm/constants'; -import routes from '~/crm/routes'; +import ContactsRoot from '~/crm/contacts/components/contacts_root.vue'; +import getGroupContactsQuery from '~/crm/contacts/components/graphql/get_group_contacts.query.graphql'; +import routes from '~/crm/contacts/routes'; import { getGroupContactsQueryResponse } from './mock_data'; describe('Customer relations contacts root app', () => { @@ -23,8 +21,6 @@ describe('Customer relations contacts root app', () => { const findRowByName = (rowName) => wrapper.findAllByRole('row', { name: rowName }); const findIssuesLinks = () => wrapper.findAllByTestId('issues-link'); const findNewContactButton = () => wrapper.findByTestId('new-contact-button'); - const findEditContactButton = () => wrapper.findByTestId('edit-contact-button'); - const findContactForm = () => wrapper.findComponent(ContactForm); const findError = () => wrapper.findComponent(GlAlert); const successQueryHandler = jest.fn().mockResolvedValue(getGroupContactsQueryResponse); @@ -40,8 +36,8 @@ describe('Customer relations contacts root app', () => { router, provide: { groupFullPath: 'flightjs', - groupIssuesPath: '/issues', groupId: 26, + groupIssuesPath: '/issues', canAdminCrmContact, }, apolloProvider: fakeApollo, @@ -82,71 +78,6 @@ describe('Customer relations contacts root app', () => { }); }); - describe('contact form', () => { - it('should not exist by default', async () => { - mountComponent(); - await waitForPromises(); - - expect(findContactForm().exists()).toBe(false); - }); - - it('should exist when user clicks new contact button', async () => { - mountComponent(); - - findNewContactButton().vm.$emit('click'); - await waitForPromises(); - - expect(findContactForm().exists()).toBe(true); - }); - - it('should exist when user navigates directly to `new` route', async () => { - router.replace({ name: NEW_ROUTE_NAME }); - mountComponent(); - await waitForPromises(); - - expect(findContactForm().exists()).toBe(true); - }); - - it('should exist when user clicks edit contact button', async () => { - mountComponent({ mountFunction: mountExtended }); - await waitForPromises(); - - findEditContactButton().vm.$emit('click'); - await waitForPromises(); - - expect(findContactForm().exists()).toBe(true); - }); - - it('should exist when user navigates directly to `edit` route', async () => { - router.replace({ name: EDIT_ROUTE_NAME, params: { id: 16 } }); - mountComponent(); - await waitForPromises(); - - expect(findContactForm().exists()).toBe(true); - }); - - it('should not exist when new form emits close', async () => { - router.replace({ name: NEW_ROUTE_NAME }); - mountComponent(); - - findContactForm().vm.$emit('close'); - await waitForPromises(); - - expect(findContactForm().exists()).toBe(false); - }); - - it('should not exist when edit form emits close', async () => { - router.replace({ name: EDIT_ROUTE_NAME, params: { id: 16 } }); - mountComponent(); - await waitForPromises(); - - findContactForm().vm.$emit('close'); - await waitForPromises(); - - expect(findContactForm().exists()).toBe(false); - }); - }); - describe('error', () => { it('should exist on reject', async () => { mountComponent({ queryHandler: jest.fn().mockRejectedValue('ERROR') }); diff --git a/spec/frontend/crm/form_spec.js b/spec/frontend/crm/form_spec.js index 0e3abc05c37..5c349b24ea1 100644 --- a/spec/frontend/crm/form_spec.js +++ b/spec/frontend/crm/form_spec.js @@ -6,12 +6,12 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import Form from '~/crm/components/form.vue'; -import routes from '~/crm/routes'; -import createContactMutation from '~/crm/components/queries/create_contact.mutation.graphql'; -import updateContactMutation from '~/crm/components/queries/update_contact.mutation.graphql'; -import getGroupContactsQuery from '~/crm/components/queries/get_group_contacts.query.graphql'; -import createOrganizationMutation from '~/crm/components/queries/create_organization.mutation.graphql'; -import getGroupOrganizationsQuery from '~/crm/components/queries/get_group_organizations.query.graphql'; +import routes from '~/crm/contacts/routes'; +import createContactMutation from '~/crm/contacts/components/graphql/create_contact.mutation.graphql'; +import updateContactMutation from '~/crm/contacts/components/graphql/update_contact.mutation.graphql'; +import getGroupContactsQuery from '~/crm/contacts/components/graphql/get_group_contacts.query.graphql'; +import createOrganizationMutation from '~/crm/organizations/components/graphql/create_organization.mutation.graphql'; +import getGroupOrganizationsQuery from '~/crm/organizations/components/graphql/get_group_organizations.query.graphql'; import { createContactMutationErrorResponse, createContactMutationResponse, @@ -101,6 +101,11 @@ describe('Reusable form component', () => { { name: 'phone', label: 'Phone' }, { name: 'description', label: 'Description' }, ], + getQuery: { + query: getGroupContactsQuery, + variables: { groupFullPath: 'flightjs' }, + }, + getQueryNodePath: 'group.contacts', ...propsData, }); }; @@ -108,13 +113,8 @@ describe('Reusable form component', () => { const mountContactCreate = () => { const propsData = { title: 'New contact', - successMessage: 'Contact has been added', + successMessage: 'Contact has been added.', buttonLabel: 'Create contact', - getQuery: { - query: getGroupContactsQuery, - variables: { groupFullPath: 'flightjs' }, - }, - getQueryNodePath: 'group.contacts', mutation: createContactMutation, additionalCreateParams: { groupId: 'gid://gitlab/Group/26' }, }; @@ -124,14 +124,9 @@ describe('Reusable form component', () => { const mountContactUpdate = () => { const propsData = { title: 'Edit contact', - successMessage: 'Contact has been updated', + successMessage: 'Contact has been updated.', mutation: updateContactMutation, - existingModel: { - id: 'gid://gitlab/CustomerRelations::Contact/12', - firstName: 'First', - lastName: 'Last', - email: 'email@example.com', - }, + existingId: 'gid://gitlab/CustomerRelations::Contact/12', }; mountContact({ propsData }); }; @@ -143,6 +138,11 @@ describe('Reusable form component', () => { { name: 'defaultRate', label: 'Default rate', input: { type: 'number', step: '0.01' } }, { name: 'description', label: 'Description' }, ], + getQuery: { + query: getGroupOrganizationsQuery, + variables: { groupFullPath: 'flightjs' }, + }, + getQueryNodePath: 'group.organizations', ...propsData, }); }; @@ -150,13 +150,8 @@ describe('Reusable form component', () => { const mountOrganizationCreate = () => { const propsData = { title: 'New organization', - successMessage: 'Organization has been added', + successMessage: 'Organization has been added.', buttonLabel: 'Create organization', - getQuery: { - query: getGroupOrganizationsQuery, - variables: { groupFullPath: 'flightjs' }, - }, - getQueryNodePath: 'group.organizations', mutation: createOrganizationMutation, additionalCreateParams: { groupId: 'gid://gitlab/Group/26' }, }; @@ -167,17 +162,17 @@ describe('Reusable form component', () => { [FORM_CREATE_CONTACT]: { mountFunction: mountContactCreate, mutationErrorResponse: createContactMutationErrorResponse, - toastMessage: 'Contact has been added', + toastMessage: 'Contact has been added.', }, [FORM_UPDATE_CONTACT]: { mountFunction: mountContactUpdate, mutationErrorResponse: updateContactMutationErrorResponse, - toastMessage: 'Contact has been updated', + toastMessage: 'Contact has been updated.', }, [FORM_CREATE_ORG]: { mountFunction: mountOrganizationCreate, mutationErrorResponse: createOrganizationMutationErrorResponse, - toastMessage: 'Organization has been added', + toastMessage: 'Organization has been added.', }, }; const asTestParams = (...keys) => keys.map((name) => [name, forms[name]]); diff --git a/spec/frontend/crm/mock_data.js b/spec/frontend/crm/mock_data.js index e351e101b29..35bc7fb69b4 100644 --- a/spec/frontend/crm/mock_data.js +++ b/spec/frontend/crm/mock_data.js @@ -157,3 +157,28 @@ export const createOrganizationMutationErrorResponse = { }, }, }; + +export const updateOrganizationMutationResponse = { + data: { + customerRelationsOrganizationUpdate: { + __typeName: 'CustomerRelationsOrganizationUpdatePayload', + organization: { + __typename: 'CustomerRelationsOrganization', + id: 'gid://gitlab/CustomerRelations::Organization/2', + name: 'A', + defaultRate: null, + description: null, + }, + errors: [], + }, + }, +}; + +export const updateOrganizationMutationErrorResponse = { + data: { + customerRelationsOrganizationUpdate: { + organization: null, + errors: ['Description is invalid.'], + }, + }, +}; diff --git a/spec/frontend/crm/new_organization_form_spec.js b/spec/frontend/crm/new_organization_form_spec.js deleted file mode 100644 index 0a7909774c9..00000000000 --- a/spec/frontend/crm/new_organization_form_spec.js +++ /dev/null @@ -1,109 +0,0 @@ -import { GlAlert } from '@gitlab/ui'; -import Vue from 'vue'; -import VueApollo from 'vue-apollo'; -import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import createMockApollo from 'helpers/mock_apollo_helper'; -import waitForPromises from 'helpers/wait_for_promises'; -import NewOrganizationForm from '~/crm/components/new_organization_form.vue'; -import createOrganizationMutation from '~/crm/components/queries/create_organization.mutation.graphql'; -import getGroupOrganizationsQuery from '~/crm/components/queries/get_group_organizations.query.graphql'; -import { - createOrganizationMutationErrorResponse, - createOrganizationMutationResponse, - getGroupOrganizationsQueryResponse, -} from './mock_data'; - -describe('Customer relations organizations root app', () => { - Vue.use(VueApollo); - let wrapper; - let fakeApollo; - let queryHandler; - - const findCreateNewOrganizationButton = () => - wrapper.findByTestId('create-new-organization-button'); - const findCancelButton = () => wrapper.findByTestId('cancel-button'); - const findForm = () => wrapper.find('form'); - const findError = () => wrapper.findComponent(GlAlert); - - const mountComponent = () => { - fakeApollo = createMockApollo([[createOrganizationMutation, queryHandler]]); - fakeApollo.clients.defaultClient.cache.writeQuery({ - query: getGroupOrganizationsQuery, - variables: { groupFullPath: 'flightjs' }, - data: getGroupOrganizationsQueryResponse.data, - }); - wrapper = shallowMountExtended(NewOrganizationForm, { - provide: { groupId: 26, groupFullPath: 'flightjs' }, - apolloProvider: fakeApollo, - propsData: { drawerOpen: true }, - }); - }; - - beforeEach(() => { - queryHandler = jest.fn().mockResolvedValue(createOrganizationMutationResponse); - }); - - afterEach(() => { - wrapper.destroy(); - fakeApollo = null; - }); - - describe('Create new organization button', () => { - it('should be disabled by default', () => { - mountComponent(); - - expect(findCreateNewOrganizationButton().attributes('disabled')).toBeTruthy(); - }); - - it('should not be disabled when first, last and email have values', async () => { - mountComponent(); - - wrapper.find('#organization-name').vm.$emit('input', 'A'); - await waitForPromises(); - - expect(findCreateNewOrganizationButton().attributes('disabled')).toBeFalsy(); - }); - }); - - it("should emit 'close' when cancel button is clicked", () => { - mountComponent(); - - findCancelButton().vm.$emit('click'); - - expect(wrapper.emitted().close).toBeTruthy(); - }); - - describe('when query is successful', () => { - it("should emit 'close'", async () => { - mountComponent(); - - findForm().trigger('submit'); - await waitForPromises(); - - expect(wrapper.emitted().close).toBeTruthy(); - }); - }); - - describe('when query fails', () => { - it('should show error on reject', async () => { - queryHandler = jest.fn().mockRejectedValue('ERROR'); - mountComponent(); - - findForm().trigger('submit'); - await waitForPromises(); - - expect(findError().exists()).toBe(true); - }); - - it('should show error on error response', async () => { - queryHandler = jest.fn().mockResolvedValue(createOrganizationMutationErrorResponse); - mountComponent(); - - findForm().trigger('submit'); - await waitForPromises(); - - expect(findError().exists()).toBe(true); - expect(findError().text()).toBe('create organization is invalid.'); - }); - }); -}); diff --git a/spec/frontend/crm/organization_form_wrapper_spec.js b/spec/frontend/crm/organization_form_wrapper_spec.js new file mode 100644 index 00000000000..1a5a7c6ca5d --- /dev/null +++ b/spec/frontend/crm/organization_form_wrapper_spec.js @@ -0,0 +1,88 @@ +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import OrganizationFormWrapper from '~/crm/organizations/components/organization_form_wrapper.vue'; +import OrganizationForm from '~/crm/components/form.vue'; +import getGroupOrganizationsQuery from '~/crm/organizations/components/graphql/get_group_organizations.query.graphql'; +import createOrganizationMutation from '~/crm/organizations/components/graphql/create_organization.mutation.graphql'; +import updateOrganizationMutation from '~/crm/organizations/components/graphql/update_organization.mutation.graphql'; + +describe('Customer relations organization form wrapper', () => { + let wrapper; + + const findOrganizationForm = () => wrapper.findComponent(OrganizationForm); + + const $apollo = { + queries: { + organizations: { + loading: false, + }, + }, + }; + const $route = { + params: { + id: 7, + }, + }; + const organizations = [{ id: 'gid://gitlab/CustomerRelations::Organization/7' }]; + + const mountComponent = ({ isEditMode = false } = {}) => { + wrapper = shallowMountExtended(OrganizationFormWrapper, { + propsData: { + isEditMode, + }, + provide: { + groupFullPath: 'flightjs', + groupId: 26, + }, + mocks: { + $apollo, + $route, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('in edit mode', () => { + it('should render organization form with correct props', () => { + mountComponent({ isEditMode: true }); + + const organizationForm = findOrganizationForm(); + expect(organizationForm.props('fields')).toHaveLength(3); + expect(organizationForm.props('title')).toBe('Edit organization'); + expect(organizationForm.props('successMessage')).toBe('Organization has been updated.'); + expect(organizationForm.props('mutation')).toBe(updateOrganizationMutation); + expect(organizationForm.props('getQuery')).toMatchObject({ + query: getGroupOrganizationsQuery, + variables: { groupFullPath: 'flightjs' }, + }); + expect(organizationForm.props('getQueryNodePath')).toBe('group.organizations'); + expect(organizationForm.props('existingId')).toBe(organizations[0].id); + expect(organizationForm.props('additionalCreateParams')).toMatchObject({ + groupId: 'gid://gitlab/Group/26', + }); + }); + }); + + describe('in create mode', () => { + it('should render organization form with correct props', () => { + mountComponent(); + + const organizationForm = findOrganizationForm(); + expect(organizationForm.props('fields')).toHaveLength(3); + expect(organizationForm.props('title')).toBe('New organization'); + expect(organizationForm.props('successMessage')).toBe('Organization has been added.'); + expect(organizationForm.props('mutation')).toBe(createOrganizationMutation); + expect(organizationForm.props('getQuery')).toMatchObject({ + query: getGroupOrganizationsQuery, + variables: { groupFullPath: 'flightjs' }, + }); + expect(organizationForm.props('getQueryNodePath')).toBe('group.organizations'); + expect(organizationForm.props('existingId')).toBeNull(); + expect(organizationForm.props('additionalCreateParams')).toMatchObject({ + groupId: 'gid://gitlab/Group/26', + }); + }); + }); +}); diff --git a/spec/frontend/crm/organizations_root_spec.js b/spec/frontend/crm/organizations_root_spec.js index aef417964f4..231208d938e 100644 --- a/spec/frontend/crm/organizations_root_spec.js +++ b/spec/frontend/crm/organizations_root_spec.js @@ -5,11 +5,9 @@ import VueRouter from 'vue-router'; import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import OrganizationsRoot from '~/crm/components/organizations_root.vue'; -import NewOrganizationForm from '~/crm/components/new_organization_form.vue'; -import { NEW_ROUTE_NAME } from '~/crm/constants'; -import routes from '~/crm/routes'; -import getGroupOrganizationsQuery from '~/crm/components/queries/get_group_organizations.query.graphql'; +import OrganizationsRoot from '~/crm/organizations/components/organizations_root.vue'; +import routes from '~/crm/organizations/routes'; +import getGroupOrganizationsQuery from '~/crm/organizations/components/graphql/get_group_organizations.query.graphql'; import { getGroupOrganizationsQueryResponse } from './mock_data'; describe('Customer relations organizations root app', () => { @@ -23,7 +21,6 @@ describe('Customer relations organizations root app', () => { const findRowByName = (rowName) => wrapper.findAllByRole('row', { name: rowName }); const findIssuesLinks = () => wrapper.findAllByTestId('issues-link'); const findNewOrganizationButton = () => wrapper.findByTestId('new-organization-button'); - const findNewOrganizationForm = () => wrapper.findComponent(NewOrganizationForm); const findError = () => wrapper.findComponent(GlAlert); const successQueryHandler = jest.fn().mockResolvedValue(getGroupOrganizationsQueryResponse); @@ -37,7 +34,11 @@ describe('Customer relations organizations root app', () => { fakeApollo = createMockApollo([[getGroupOrganizationsQuery, queryHandler]]); wrapper = mountFunction(OrganizationsRoot, { router, - provide: { canAdminCrmOrganization, groupFullPath: 'flightjs', groupIssuesPath: '/issues' }, + provide: { + canAdminCrmOrganization, + groupFullPath: 'flightjs', + groupIssuesPath: '/issues', + }, apolloProvider: fakeApollo, }); }; @@ -76,42 +77,6 @@ describe('Customer relations organizations root app', () => { }); }); - describe('new organization form', () => { - it('should not exist by default', async () => { - mountComponent(); - await waitForPromises(); - - expect(findNewOrganizationForm().exists()).toBe(false); - }); - - it('should exist when user clicks new contact button', async () => { - mountComponent(); - - findNewOrganizationButton().vm.$emit('click'); - await waitForPromises(); - - expect(findNewOrganizationForm().exists()).toBe(true); - }); - - it('should exist when user navigates directly to /new', async () => { - router.replace({ name: NEW_ROUTE_NAME }); - mountComponent(); - await waitForPromises(); - - expect(findNewOrganizationForm().exists()).toBe(true); - }); - - it('should not exist when form emits close', async () => { - router.replace({ name: NEW_ROUTE_NAME }); - mountComponent(); - - findNewOrganizationForm().vm.$emit('close'); - await waitForPromises(); - - expect(findNewOrganizationForm().exists()).toBe(false); - }); - }); - it('should render error message on reject', async () => { mountComponent({ queryHandler: jest.fn().mockRejectedValue('ERROR') }); await waitForPromises(); diff --git a/spec/frontend/ide/components/ide_side_bar_spec.js b/spec/frontend/ide/components/ide_side_bar_spec.js index 34f14ef23a4..ace8988b8c9 100644 --- a/spec/frontend/ide/components/ide_side_bar_spec.js +++ b/spec/frontend/ide/components/ide_side_bar_spec.js @@ -1,4 +1,4 @@ -import { GlSkeletonLoading } from '@gitlab/ui'; +import { GlDeprecatedSkeletonLoading as GlSkeletonLoading } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import Vuex from 'vuex'; diff --git a/spec/frontend/lib/utils/text_markdown_spec.js b/spec/frontend/lib/utils/text_markdown_spec.js index a5877aa6e3e..55c18de0845 100644 --- a/spec/frontend/lib/utils/text_markdown_spec.js +++ b/spec/frontend/lib/utils/text_markdown_spec.js @@ -179,11 +179,13 @@ describe('init markdown', () => { text | expected ${'- item'} | ${'- item\n- '} ${'- [ ] item'} | ${'- [ ] item\n- [ ] '} - ${'- [x] item'} | ${'- [x] item\n- [x] '} + ${'- [x] item'} | ${'- [x] item\n- [ ] '} + ${'- [X] item'} | ${'- [X] item\n- [ ] '} ${'- item\n - second'} | ${'- item\n - second\n - '} ${'1. item'} | ${'1. item\n2. '} ${'1. [ ] item'} | ${'1. [ ] item\n2. [ ] '} - ${'1. [x] item'} | ${'1. [x] item\n2. [x] '} + ${'1. [x] item'} | ${'1. [x] item\n2. [ ] '} + ${'1. [X] item'} | ${'1. [X] item\n2. [ ] '} ${'108. item'} | ${'108. item\n109. '} ${'108. item\n - second'} | ${'108. item\n - second\n - '} ${'108. item\n 1. second'} | ${'108. item\n 1. second\n 2. '} @@ -207,10 +209,12 @@ describe('init markdown', () => { ${'- item\n- '} | ${'- item\n'} ${'- [ ] item\n- [ ] '} | ${'- [ ] item\n'} ${'- [x] item\n- [x] '} | ${'- [x] item\n'} + ${'- [X] item\n- [X] '} | ${'- [X] item\n'} ${'- item\n - second\n - '} | ${'- item\n - second\n'} ${'1. item\n2. '} | ${'1. item\n'} ${'1. [ ] item\n2. [ ] '} | ${'1. [ ] item\n'} ${'1. [x] item\n2. [x] '} | ${'1. [x] item\n'} + ${'1. [X] item\n2. [X] '} | ${'1. [X] item\n'} ${'108. item\n109. '} | ${'108. item\n'} ${'108. item\n - second\n - '} | ${'108. item\n - second\n'} ${'108. item\n 1. second\n 1. '} | ${'108. item\n 1. second\n'} diff --git a/spec/frontend/runner/components/runner_jobs_spec.js b/spec/frontend/runner/components/runner_jobs_spec.js index 9abb2861005..9e40e911448 100644 --- a/spec/frontend/runner/components/runner_jobs_spec.js +++ b/spec/frontend/runner/components/runner_jobs_spec.js @@ -1,4 +1,4 @@ -import { GlSkeletonLoading } from '@gitlab/ui'; +import { GlDeprecatedSkeletonLoading as GlSkeletonLoading } from '@gitlab/ui'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; diff --git a/spec/frontend/runner/components/runner_projects_spec.js b/spec/frontend/runner/components/runner_projects_spec.js index 96de8d11bca..62ebc6539e2 100644 --- a/spec/frontend/runner/components/runner_projects_spec.js +++ b/spec/frontend/runner/components/runner_projects_spec.js @@ -1,4 +1,4 @@ -import { GlSkeletonLoading } from '@gitlab/ui'; +import { GlDeprecatedSkeletonLoading as GlSkeletonLoading } from '@gitlab/ui'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; diff --git a/spec/frontend/vue_shared/issuable/list/components/issuable_list_root_spec.js b/spec/frontend/vue_shared/issuable/list/components/issuable_list_root_spec.js index 64823cd4c6c..058cb30c1d5 100644 --- a/spec/frontend/vue_shared/issuable/list/components/issuable_list_root_spec.js +++ b/spec/frontend/vue_shared/issuable/list/components/issuable_list_root_spec.js @@ -1,4 +1,9 @@ -import { GlAlert, GlKeysetPagination, GlSkeletonLoading, GlPagination } from '@gitlab/ui'; +import { + GlAlert, + GlKeysetPagination, + GlDeprecatedSkeletonLoading as GlSkeletonLoading, + GlPagination, +} from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import VueDraggable from 'vuedraggable'; diff --git a/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb b/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb index ccc861baae5..66a5f0277fb 100644 --- a/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb +++ b/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do let(:current_user) { reporter } - before_all do + before do project.add_guest(guest) project.add_reporter(reporter) end @@ -20,13 +20,8 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do expect(described_class).to have_nullable_graphql_type(::Types::Ci::AnalyticsType) end - def resolve_statistics(project, args) - ctx = { current_user: current_user } - resolve(described_class, obj: project, args: args, ctx: ctx) - end - - describe '#resolve' do - it 'returns the pipelines statistics for a given project' do + shared_examples 'returns the pipelines statistics for a given project' do + it do result = resolve_statistics(project, {}) expect(result.keys).to contain_exactly( :week_pipelines_labels, @@ -42,14 +37,67 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do :pipeline_times_values ) end + end + + shared_examples 'it returns nils' do + it do + result = resolve_statistics(project, {}) + + expect(result).to be_nil + end + end + + def resolve_statistics(project, args) + ctx = { current_user: current_user } + resolve(described_class, obj: project, args: args, ctx: ctx) + end + + describe '#resolve' do + it_behaves_like 'returns the pipelines statistics for a given project' context 'when the user does not have access to the CI/CD analytics data' do let(:current_user) { guest } - it 'returns nil' do - result = resolve_statistics(project, {}) + it_behaves_like 'it returns nils' + end + + context 'when the project is public' do + let_it_be(:project) { create(:project, :public) } + + context 'public pipelines are disabled' do + before do + project.update!(public_builds: false) + end + + context 'user is not a member' do + let(:current_user) { create(:user) } + + it_behaves_like 'it returns nils' + end + + context 'user is a guest' do + let(:current_user) { guest } + + it_behaves_like 'it returns nils' + end + + context 'user is a reporter or above' do + let(:current_user) { reporter } + + it_behaves_like 'returns the pipelines statistics for a given project' + end + end + + context 'public pipelines are enabled' do + before do + project.update!(public_builds: true) + end + + context 'user is not a member' do + let(:current_user) { create(:user) } - expect(result).to be_nil + it_behaves_like 'returns the pipelines statistics for a given project' + end end end end diff --git a/spec/initializers/rdoc_segfault_patch_spec.rb b/spec/initializers/rdoc_segfault_patch_spec.rb new file mode 100644 index 00000000000..f9630295052 --- /dev/null +++ b/spec/initializers/rdoc_segfault_patch_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +RSpec.describe 'RDoc segfault patch fix' do + describe 'RDoc::Markup::ToHtml' do + describe '#parseable?' do + it 'returns false' do + to_html = RDoc::Markup::ToHtml.new( nil) + + expect(to_html.parseable?('"def foo; end"')).to eq(false) + end + end + end + + describe 'RDoc::Markup::Verbatim' do + describe 'ruby?' do + it 'returns false' do + verbatim = RDoc::Markup::Verbatim.new('def foo; end') + verbatim.format = :ruby + + expect(verbatim.ruby?).to eq(false) + end + end + end +end diff --git a/spec/lib/banzai/filter/kroki_filter_spec.rb b/spec/lib/banzai/filter/kroki_filter_spec.rb index d9e48754559..1fb61ad1991 100644 --- a/spec/lib/banzai/filter/kroki_filter_spec.rb +++ b/spec/lib/banzai/filter/kroki_filter_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Banzai::Filter::KrokiFilter do stub_application_setting(kroki_enabled: true, kroki_url: "http://localhost:8000") doc = filter("<pre lang='nomnoml'><code>[Pirate|eyeCount: Int|raid();pillage()|\n [beard]--[parrot]\n [beard]-:>[foul mouth]\n]</code></pre>") - expect(doc.to_s).to eq '<img src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KzhUlCITkpNLEqJ1dWNLkgsKsoviUUSs7KLTssvzVHIzS8tyYjligUAMhEd0g==" data-diagram="nomnoml" data-diagram-src="data:text/plain;base64,W1BpcmF0ZXxleWVDb3VudDogSW50fHJhaWQoKTtwaWxsYWdlKCl8CiAgW2JlYXJkXS0tW3BhcnJvdF0KICBbYmVhcmRdLTo+W2ZvdWwgbW91dGhdCl0=">' + expect(doc.to_s).to eq '<img src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KzhUlCITkpNLEqJ1dWNLkgsKsoviUUSs7KLTssvzVHIzS8tyYjligUAMhEd0g==" class="js-render-kroki" data-diagram="nomnoml" data-diagram-src="data:text/plain;base64,W1BpcmF0ZXxleWVDb3VudDogSW50fHJhaWQoKTtwaWxsYWdlKCl8CiAgW2JlYXJkXS0tW3BhcnJvdF0KICBbYmVhcmRdLTo+W2ZvdWwgbW91dGhdCl0=">' end it 'replaces nomnoml pre tag with img tag if both kroki and plantuml are enabled' do @@ -19,7 +19,7 @@ RSpec.describe Banzai::Filter::KrokiFilter do plantuml_url: "http://localhost:8080") doc = filter("<pre lang='nomnoml'><code>[Pirate|eyeCount: Int|raid();pillage()|\n [beard]--[parrot]\n [beard]-:>[foul mouth]\n]</code></pre>") - expect(doc.to_s).to eq '<img src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KzhUlCITkpNLEqJ1dWNLkgsKsoviUUSs7KLTssvzVHIzS8tyYjligUAMhEd0g==" data-diagram="nomnoml" data-diagram-src="data:text/plain;base64,W1BpcmF0ZXxleWVDb3VudDogSW50fHJhaWQoKTtwaWxsYWdlKCl8CiAgW2JlYXJkXS0tW3BhcnJvdF0KICBbYmVhcmRdLTo+W2ZvdWwgbW91dGhdCl0=">' + expect(doc.to_s).to eq '<img src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KzhUlCITkpNLEqJ1dWNLkgsKsoviUUSs7KLTssvzVHIzS8tyYjligUAMhEd0g==" class="js-render-kroki" data-diagram="nomnoml" data-diagram-src="data:text/plain;base64,W1BpcmF0ZXxleWVDb3VudDogSW50fHJhaWQoKTtwaWxsYWdlKCl8CiAgW2JlYXJkXS0tW3BhcnJvdF0KICBbYmVhcmRdLTo+W2ZvdWwgbW91dGhdCl0=">' end it 'does not replace nomnoml pre tag with img tag if kroki is disabled' do @@ -38,4 +38,12 @@ RSpec.describe Banzai::Filter::KrokiFilter do expect(doc.to_s).to eq '<pre lang="plantuml"><code>Bob->Alice : hello</code></pre>' end + + it 'adds hidden attribute when content size is large' do + stub_application_setting(kroki_enabled: true, kroki_url: "http://localhost:8000") + text = '[Pirate|eyeCount: Int|raid();pillage()|\n [beard]--[parrot]\n [beard]-:>[foul mouth]\n]' * 25 + doc = filter("<pre lang='nomnoml'><code>#{text}</code></pre>") + + expect(doc.to_s).to start_with '<img src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KyJyVNQiE5KTSxKidXVjS5ILCrKL4lFFrSyi07LL81RyM0vLckAysRGjxo8avCowaMGjxo8avCowaMGU8lgAE7mIdc=" hidden="" class="js-render-kroki" data-diagram="nomnoml" data-diagram-src="data:text/plain;base64,W1BpcmF0ZXxleWVDb3VudDog' + end end diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index aee4bd93207..16c958ec10b 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -132,6 +132,12 @@ RSpec.describe Banzai::Filter::SyntaxHighlightFilter do expect(result.to_html.delete("\n")).to eq('<div class="gl-relative markdown-code-block js-markdown-code"><pre data-sourcepos="1:1-3:3" class="code highlight js-syntax-highlight language-plaintext" lang="plaintext" v-pre="true"><code><span id="LC1" class="line" lang="plaintext">This is a test</span></code></pre><copy-code></copy-code></div>') end + + it "escape sourcepos metadata to prevent XSS" do + result = filter('<pre data-sourcepos=""%22 href="x"></pre><base href=http://unsafe-website.com/><pre x=""><code></code></pre>') + + expect(result.to_html.delete("\n")).to eq('<div class="gl-relative markdown-code-block js-markdown-code"><pre data-sourcepos=\'"%22 href="x"></pre><base href=http://unsafe-website.com/><pre x="\' class="code highlight js-syntax-highlight language-plaintext" lang="plaintext" v-pre="true"><code></code></pre><copy-code></copy-code></div>') + end end context "when Rouge lexing fails" do diff --git a/spec/lib/banzai/reference_redactor_spec.rb b/spec/lib/banzai/reference_redactor_spec.rb index 45e14032a98..344b8988296 100644 --- a/spec/lib/banzai/reference_redactor_spec.rb +++ b/spec/lib/banzai/reference_redactor_spec.rb @@ -35,7 +35,7 @@ RSpec.describe Banzai::ReferenceRedactor do end context 'when data-original attribute provided' do - let(:original_content) { '<code>foo</code>' } + let(:original_content) { '<script>alert(1);</script>' } it 'replaces redacted reference with original content' do doc = Nokogiri::HTML.fragment("<a class='gfm' href='https://www.gitlab.com' data-reference-type='issue' data-original='#{original_content}'>bar</a>") diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 706344831b8..f5a74956174 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -87,7 +87,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end context 'when IP is already banned' do - subject { gl_auth.find_for_git_client('username', Gitlab::Password.test_default, project: nil, ip: 'ip') } + subject { gl_auth.find_for_git_client('username', 'password', project: nil, ip: 'ip') } before do expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter| @@ -219,16 +219,16 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end it 'recognizes master passwords' do - user = create(:user, password: Gitlab::Password.test_default) + user = create(:user, password: 'password') - expect(gl_auth.find_for_git_client(user.username, Gitlab::Password.test_default, project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) + expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) end include_examples 'user login operation with unique ip limit' do - let(:user) { create(:user, password: Gitlab::Password.test_default) } + let(:user) { create(:user, password: 'password') } def operation - expect(gl_auth.find_for_git_client(user.username, Gitlab::Password.test_default, project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) + expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) end end @@ -492,7 +492,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do :user, :blocked, username: 'normal_user', - password: Gitlab::Password.test_default + password: 'my-secret' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -501,7 +501,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when 2fa is enabled globally' do let_it_be(:user) do - create(:user, username: 'normal_user', password: Gitlab::Password.test_default, otp_grace_period_started_at: 1.day.ago) + create(:user, username: 'normal_user', password: 'my-secret', otp_grace_period_started_at: 1.day.ago) end before do @@ -525,7 +525,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when 2fa is enabled personally' do let(:user) do - create(:user, :two_factor, username: 'normal_user', password: Gitlab::Password.test_default, otp_grace_period_started_at: 1.day.ago) + create(:user, :two_factor, username: 'normal_user', password: 'my-secret', otp_grace_period_started_at: 1.day.ago) end it 'fails' do @@ -538,7 +538,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do user = create( :user, username: 'normal_user', - password: Gitlab::Password.test_default + password: 'my-secret' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -549,7 +549,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do user = create( :user, username: 'oauth2', - password: Gitlab::Password.test_default + password: 'my-secret' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -624,7 +624,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when deploy token and user have the same username' do let(:username) { 'normal_user' } - let(:user) { create(:user, username: username, password: Gitlab::Password.test_default) } + let(:user) { create(:user, username: username, password: 'my-secret') } let(:deploy_token) { create(:deploy_token, username: username, read_registry: false, projects: [project]) } it 'succeeds for the token' do @@ -637,7 +637,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'succeeds for the user' do auth_success = { actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities } - expect(gl_auth.find_for_git_client(username, Gitlab::Password.test_default, project: project, ip: 'ip')) + expect(gl_auth.find_for_git_client(username, 'my-secret', project: project, ip: 'ip')) .to have_attributes(auth_success) end end @@ -831,7 +831,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end let(:username) { 'John' } # username isn't lowercase, test this - let(:password) { Gitlab::Password.test_default } + let(:password) { 'my-secret' } it "finds user by valid login/password" do expect(gl_auth.find_with_user_password(username, password)).to eql user @@ -956,13 +956,13 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it "does not find user by using ldap as fallback to for authentication" do expect(Gitlab::Auth::Ldap::Authentication).to receive(:login).and_return(nil) - expect(gl_auth.find_with_user_password('ldap_user', Gitlab::Password.test_default)).to be_nil + expect(gl_auth.find_with_user_password('ldap_user', 'password')).to be_nil end it "find new user by using ldap as fallback to for authentication" do expect(Gitlab::Auth::Ldap::Authentication).to receive(:login).and_return(user) - expect(gl_auth.find_with_user_password('ldap_user', Gitlab::Password.test_default)).to eq(user) + expect(gl_auth.find_with_user_password('ldap_user', 'password')).to eq(user) end end diff --git a/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb b/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb index 3678b46c8ff..0f1dae27293 100644 --- a/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb @@ -128,6 +128,12 @@ RSpec.describe Gitlab::Ci::Config::External::File::Artifact do let!(:metadata) { create(:ci_job_artifact, :metadata, job: generator_job) } context 'when file is empty' do + let(:params) { { artifact: 'secret_stuff/generated.yml', job: 'generator' } } + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_stuff', 'masked' => true }]) } + let(:context) do + Gitlab::Ci::Config::External::Context.new(parent_pipeline: parent_pipeline, variables: variables) + end + before do allow_next_instance_of(Gitlab::Ci::ArtifactFileReader) do |reader| allow(reader).to receive(:read).and_return('') @@ -135,7 +141,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Artifact do end let(:expected_error) do - 'File `generated.yml` is empty!' + 'File `xxxxxxxxxxxx/generated.yml` is empty!' end it_behaves_like 'is invalid' diff --git a/spec/lib/gitlab/ci/config/external/file/base_spec.rb b/spec/lib/gitlab/ci/config/external/file/base_spec.rb index a3a319313c4..71b1a58c82e 100644 --- a/spec/lib/gitlab/ci/config/external/file/base_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/base_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Config::External::File::Base do - let(:context_params) { { sha: 'HEAD' } } + let(:variables) { } + let(:context_params) { { sha: 'HEAD', variables: variables } } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:test_class) do @@ -81,7 +82,8 @@ RSpec.describe Gitlab::Ci::Config::External::File::Base do end context 'when there are YAML syntax errors' do - let(:location) { 'some/file/config.yml' } + let(:location) { 'some/file/secret_file_name.yml' } + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_file_name', 'masked' => true }]) } before do allow_any_instance_of(test_class) @@ -90,7 +92,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Base do it 'is not a valid file' do expect(valid?).to be_falsy - expect(file.error_message).to match /does not have valid YAML syntax/ + expect(file.error_message).to eq('Included file `some/file/xxxxxxxxxxxxxxxx.yml` does not have valid YAML syntax!') end end end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index a13e4410207..f4f713ee46c 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do let_it_be(:user) { create(:user) } let(:sha) { '12345' } + let(:variables) { project.predefined_variables.to_runner_variables } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:params) { { local: location } } let(:local_file) { described_class.new(params, context) } @@ -18,7 +19,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do sha: sha, user: user, parent_pipeline: parent_pipeline, - variables: project.predefined_variables.to_runner_variables + variables: variables } end @@ -69,18 +70,29 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do it { is_expected.to be_truthy } end - context 'when is not a valid local path' do + context 'when it is not a valid local path' do let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } it { is_expected.to be_falsy } end - context 'when is not a yaml file' do + context 'when it is not a yaml file' do let(:location) { '/config/application.rb' } it { is_expected.to be_falsy } end + context 'when it is an empty file' do + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret', 'masked' => true }]) } + let(:location) { '/lib/gitlab/ci/templates/secret/existent-file.yml' } + + it 'returns false and adds an error message about an empty file' do + allow_any_instance_of(described_class).to receive(:fetch_local_content).and_return("") + local_file.validate! + expect(local_file.errors).to include("Local file `/lib/gitlab/ci/templates/xxxxxx/existent-file.yml` is empty!") + end + end + context 'when the given sha is not valid' do let(:location) { '/lib/gitlab/ci/templates/existent-file.yml' } let(:sha) { ':' } @@ -125,14 +137,15 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do end describe '#error_message' do - let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } + let(:location) { '/lib/gitlab/ci/templates/secret_file.yml' } + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_file', 'masked' => true }]) } before do local_file.validate! end it 'returns an error message' do - expect(local_file.error_message).to eq("Local file `#{location}` does not exist!") + expect(local_file.error_message).to eq("Local file `/lib/gitlab/ci/templates/xxxxxxxxxxx.yml` does not exist!") end end diff --git a/spec/lib/gitlab/ci/config/external/file/project_spec.rb b/spec/lib/gitlab/ci/config/external/file/project_spec.rb index 96d67b65abe..aa46fa61bf6 100644 --- a/spec/lib/gitlab/ci/config/external/file/project_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/project_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do let(:parent_pipeline) { double(:parent_pipeline) } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:project_file) { described_class.new(params, context) } + let(:variables) { project.predefined_variables.to_runner_variables } let(:context_params) do { @@ -18,7 +19,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do sha: '12345', user: context_user, parent_pipeline: parent_pipeline, - variables: project.predefined_variables.to_runner_variables + variables: variables } end @@ -109,18 +110,19 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do context 'when an empty file is used' do let(:params) do - { project: project.full_path, file: '/file.yml' } + { project: project.full_path, file: '/secret_file.yml' } end + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_file', 'masked' => true }]) } let(:root_ref_sha) { project.repository.root_ref_sha } before do - stub_project_blob(root_ref_sha, '/file.yml') { '' } + stub_project_blob(root_ref_sha, '/secret_file.yml') { '' } end it 'returns false' do expect(valid?).to be_falsy - expect(project_file.error_message).to include("Project `#{project.full_path}` file `/file.yml` is empty!") + expect(project_file.error_message).to include("Project `#{project.full_path}` file `/xxxxxxxxxxx.yml` is empty!") end end @@ -136,13 +138,15 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do end context 'when non-existing file is requested' do + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret-invalid-file', 'masked' => true }]) } + let(:params) do - { project: project.full_path, file: '/invalid-file.yml' } + { project: project.full_path, file: '/secret-invalid-file.yml' } end it 'returns false' do expect(valid?).to be_falsy - expect(project_file.error_message).to include("Project `#{project.full_path}` file `/invalid-file.yml` does not exist!") + expect(project_file.error_message).to include("Project `#{project.full_path}` file `/xxxxxxxxxxxxxxxxxxx.yml` does not exist!") end end diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index 56113532787..5f148f6c411 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -5,11 +5,12 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Config::External::File::Remote do include StubRequests - let(:context_params) { { sha: '12345' } } + let(:variables) {Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_file', 'masked' => true }]) } + let(:context_params) { { sha: '12345', variables: variables } } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:params) { { remote: location } } let(:remote_file) { described_class.new(params, context) } - let(:location) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } + let(:location) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.secret_file.yml' } let(:remote_file_content) do <<~HEREDOC before_script: @@ -142,10 +143,10 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do end context 'when remote file location is not valid' do - let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } + let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-foss/blob/1234/?secret_file.yml' } it 'returns an error message describing invalid address' do - expect(subject).to match /does not have a valid address!/ + expect(subject).to eq('Remote file `not-valid://gitlab.com/gitlab-org/gitlab-foss/blob/1234/?xxxxxxxxxxx.yml` does not have a valid address!') end end @@ -155,7 +156,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do end it 'returns error message about a timeout' do - expect(subject).to match /could not be fetched because of a timeout error!/ + expect(subject).to eq('Remote file `https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.xxxxxxxxxxx.yml` could not be fetched because of a timeout error!') end end @@ -165,7 +166,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do end it 'returns error message about a HTTP error' do - expect(subject).to match /could not be fetched because of HTTP error!/ + expect(subject).to eq('Remote file `https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.xxxxxxxxxxx.yml` could not be fetched because of HTTP error!') end end @@ -175,7 +176,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do end it 'returns error message about a timeout' do - expect(subject).to match /could not be fetched because of HTTP code `404` error!/ + expect(subject).to eq('Remote file `https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.xxxxxxxxxxx.yml` could not be fetched because of HTTP code `404` error!') end end diff --git a/spec/lib/gitlab/ci/config/external/file/template_spec.rb b/spec/lib/gitlab/ci/config/external/file/template_spec.rb index 2bbaa46a8c4..88740d492e8 100644 --- a/spec/lib/gitlab/ci/config/external/file/template_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/template_spec.rb @@ -57,11 +57,13 @@ RSpec.describe Gitlab::Ci::Config::External::File::Template do end context 'with invalid template name' do - let(:template) { 'Template.yml' } + let(:template) { 'SecretTemplate.yml' } + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'SecretTemplate', 'masked' => true }]) } + let(:context_params) { { project: project, sha: '12345', user: user, variables: variables } } it 'returns false' do expect(valid?).to be_falsy - expect(template_file.error_message).to include('Template file `Template.yml` is not a valid location!') + expect(template_file.error_message).to include('`xxxxxxxxxxxxxx.yml` is not a valid location!') end end diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index f8754d7e124..f69feba5e59 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -11,7 +11,8 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do let(:local_file) { '/lib/gitlab/ci/templates/non-existent-file.yml' } let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } let(:template_file) { 'Auto-DevOps.gitlab-ci.yml' } - let(:context_params) { { project: project, sha: '123456', user: user, variables: project.predefined_variables } } + let(:variables) { project.predefined_variables } + let(:context_params) { { project: project, sha: '123456', user: user, variables: variables } } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:file_content) do @@ -92,13 +93,16 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do end context 'when the key is a hash of file and remote' do + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret-file', 'masked' => true }]) } + let(:local_file) { 'secret-file.yml' } + let(:remote_url) { 'https://gitlab.com/secret-file.yml' } let(:values) do { include: { 'local' => local_file, 'remote' => remote_url }, image: 'ruby:2.7' } end it 'returns ambigious specification error' do - expect { subject }.to raise_error(described_class::AmbigiousSpecificationError) + expect { subject }.to raise_error(described_class::AmbigiousSpecificationError, 'Include `{"local":"xxxxxxxxxxx.yml","remote":"https://gitlab.com/xxxxxxxxxxx.yml"}` needs to match exactly one accessor!') end end diff --git a/spec/lib/gitlab/database/background_migration/batched_migration_wrapper_spec.rb b/spec/lib/gitlab/database/background_migration/batched_migration_wrapper_spec.rb index d6c984c7adb..ba2a67f77ee 100644 --- a/spec/lib/gitlab/database/background_migration/batched_migration_wrapper_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_migration_wrapper_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, '#perform' do - subject { described_class.new.perform(job_record) } + subject { described_class.new(metrics: metrics_tracker).perform(job_record) } + let(:metrics_tracker) { instance_double('::Gitlab::Database::BackgroundMigration::PrometheusMetrics', track: nil) } let(:job_class) { Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJob } let_it_be(:pause_ms) { 250 } @@ -78,86 +79,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, ' end end - context 'reporting prometheus metrics' do - let(:labels) { job_record.batched_migration.prometheus_labels } - - before do - allow(job_instance).to receive(:perform) - end - - it 'reports batch_size' do - expect(described_class.metrics[:gauge_batch_size]).to receive(:set).with(labels, job_record.batch_size) - - subject - end - - it 'reports sub_batch_size' do - expect(described_class.metrics[:gauge_sub_batch_size]).to receive(:set).with(labels, job_record.sub_batch_size) - - subject - end - - it 'reports interval' do - expect(described_class.metrics[:gauge_interval]).to receive(:set).with(labels, job_record.batched_migration.interval) - - subject - end - - it 'reports updated tuples (currently based on batch_size)' do - expect(described_class.metrics[:counter_updated_tuples]).to receive(:increment).with(labels, job_record.batch_size) - - subject - end - - it 'reports migrated tuples' do - count = double - expect(job_record.batched_migration).to receive(:migrated_tuple_count).and_return(count) - expect(described_class.metrics[:gauge_migrated_tuples]).to receive(:set).with(labels, count) - - subject - end - - it 'reports summary of query timings' do - metrics = { 'timings' => { 'update_all' => [1, 2, 3, 4, 5] } } - - expect(job_instance).to receive(:batch_metrics).and_return(metrics) - - metrics['timings'].each do |key, timings| - summary_labels = labels.merge(operation: key) - timings.each do |timing| - expect(described_class.metrics[:histogram_timings]).to receive(:observe).with(summary_labels, timing) - end - end - - subject - end - - it 'reports job duration' do - freeze_time do - expect(Time).to receive(:current).and_return(Time.zone.now - 5.seconds).ordered - allow(Time).to receive(:current).and_call_original - - expect(described_class.metrics[:gauge_job_duration]).to receive(:set).with(labels, 5.seconds) - - subject - end - end - - it 'reports the total tuple count for the migration' do - expect(described_class.metrics[:gauge_total_tuple_count]).to receive(:set).with(labels, job_record.batched_migration.total_tuple_count) - - subject - end - - it 'reports last updated at timestamp' do - freeze_time do - expect(described_class.metrics[:gauge_last_update_time]).to receive(:set).with(labels, Time.current.to_i) - - subject - end - end - end - context 'when the migration job does not raise an error' do it 'marks the tracking record as succeeded' do expect(job_instance).to receive(:perform).with(1, 10, 'events', 'id', 1, pause_ms, 'id', 'other_id') @@ -171,6 +92,13 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, ' expect(reloaded_job_record.finished_at).to eq(Time.current) end end + + it 'tracks metrics of the execution' do + expect(job_instance).to receive(:perform) + expect(metrics_tracker).to receive(:track).with(job_record) + + subject + end end context 'when the migration job raises an error' do @@ -189,6 +117,13 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, ' expect(reloaded_job_record.finished_at).to eq(Time.current) end end + + it 'tracks metrics of the execution' do + expect(job_instance).to receive(:perform).and_raise(error_class) + expect(metrics_tracker).to receive(:track).with(job_record) + + expect { subject }.to raise_error(error_class) + end end it_behaves_like 'an error is raised', RuntimeError.new('Something broke!') diff --git a/spec/lib/gitlab/database/background_migration/prometheus_metrics_spec.rb b/spec/lib/gitlab/database/background_migration/prometheus_metrics_spec.rb new file mode 100644 index 00000000000..1f256de35ec --- /dev/null +++ b/spec/lib/gitlab/database/background_migration/prometheus_metrics_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::BackgroundMigration::PrometheusMetrics, :prometheus do + describe '#track' do + let(:job_record) do + build(:batched_background_migration_job, :succeeded, + started_at: Time.current - 2.minutes, + finished_at: Time.current - 1.minute, + updated_at: Time.current, + metrics: { 'timings' => { 'update_all' => [0.05, 0.2, 0.4, 0.9, 4] } }) + end + + let(:labels) { job_record.batched_migration.prometheus_labels } + + subject(:track_job_record_metrics) { described_class.new.track(job_record) } + + it 'reports batch_size' do + track_job_record_metrics + + expect(metric_for_job_by_name(:gauge_batch_size)).to eq(job_record.batch_size) + end + + it 'reports sub_batch_size' do + track_job_record_metrics + + expect(metric_for_job_by_name(:gauge_sub_batch_size)).to eq(job_record.sub_batch_size) + end + + it 'reports interval' do + track_job_record_metrics + + expect(metric_for_job_by_name(:gauge_interval)).to eq(job_record.batched_migration.interval) + end + + it 'reports job duration' do + freeze_time do + track_job_record_metrics + + expect(metric_for_job_by_name(:gauge_job_duration)).to eq(1.minute) + end + end + + it 'increments updated tuples (currently based on batch_size)' do + expect(described_class.metrics[:counter_updated_tuples]).to receive(:increment) + .with(labels, job_record.batch_size) + .twice + .and_call_original + + track_job_record_metrics + + expect(metric_for_job_by_name(:counter_updated_tuples)).to eq(job_record.batch_size) + + described_class.new.track(job_record) + + expect(metric_for_job_by_name(:counter_updated_tuples)).to eq(job_record.batch_size * 2) + end + + it 'reports migrated tuples' do + expect(job_record.batched_migration).to receive(:migrated_tuple_count).and_return(20) + + track_job_record_metrics + + expect(metric_for_job_by_name(:gauge_migrated_tuples)).to eq(20) + end + + it 'reports the total tuple count for the migration' do + track_job_record_metrics + + expect(metric_for_job_by_name(:gauge_total_tuple_count)).to eq(job_record.batched_migration.total_tuple_count) + end + + it 'reports last updated at timestamp' do + freeze_time do + track_job_record_metrics + + expect(metric_for_job_by_name(:gauge_last_update_time)).to eq(Time.current.to_i) + end + end + + it 'reports summary of query timings' do + summary_labels = labels.merge(operation: 'update_all') + + job_record.metrics['timings']['update_all'].each do |timing| + expect(described_class.metrics[:histogram_timings]).to receive(:observe) + .with(summary_labels, timing) + .and_call_original + end + + track_job_record_metrics + + expect(metric_for_job_by_name(:histogram_timings, job_labels: summary_labels)) + .to eq({ 0.1 => 1.0, 0.25 => 2.0, 0.5 => 3.0, 1 => 4.0, 5 => 5.0 }) + end + + context 'when the tracking record does not having timing metrics' do + before do + job_record.metrics = {} + end + + it 'does not attempt to report query timings' do + summary_labels = labels.merge(operation: 'update_all') + + expect(described_class.metrics[:histogram_timings]).not_to receive(:observe) + + track_job_record_metrics + + expect(metric_for_job_by_name(:histogram_timings, job_labels: summary_labels)) + .to eq({ 0.1 => 0.0, 0.25 => 0.0, 0.5 => 0.0, 1 => 0.0, 5 => 0.0 }) + end + end + + def metric_for_job_by_name(name, job_labels: labels) + described_class.metrics[name].values[job_labels].get + end + end +end diff --git a/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb new file mode 100644 index 00000000000..5ec73233e77 --- /dev/null +++ b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::ErrorTracking::Processor::SanitizeErrorMessageProcessor, :sentry do + describe '.call' do + let(:exception) { StandardError.new('raw error') } + let(:result_hash) { described_class.call(event).to_hash } + + shared_examples 'processes the exception' do + it 'cleans the exception message' do + expect(Gitlab::Sanitizers::ExceptionMessage).to receive(:clean).with('StandardError', 'raw error').and_return('cleaned') + + expect(result_hash[:exception][:values].first).to include( + type: 'StandardError', + value: 'cleaned' + ) + end + end + + context 'with Raven event' do + let(:raven_required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } + end + + let(:event) { Raven::Event.from_exception(exception, raven_required_options) } + + it_behaves_like 'processes the exception' + end + + context 'with Sentry event' do + let(:event) { Sentry.get_current_client.event_from_exception(exception) } + + it_behaves_like 'processes the exception' + end + + context 'with invalid event' do + let(:event) { instance_double('Sentry::Event', to_hash: { invalid: true }) } + + it 'does nothing' do + extracted_exception = instance_double('Sentry::SingleExceptionInterface', value: nil) + allow(described_class).to receive(:extract_exceptions_from).and_return([extracted_exception]) + + expect(Gitlab::Sanitizers::ExceptionMessage).not_to receive(:clean) + expect(result_hash).to eq(invalid: true) + end + end + end +end diff --git a/spec/lib/gitlab/error_tracking_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb index 936954fc1b6..1ade3a51c55 100644 --- a/spec/lib/gitlab/error_tracking_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -368,5 +368,61 @@ RSpec.describe Gitlab::ErrorTracking do end end end + + context 'when processing invalid URI exceptions' do + let(:invalid_uri) { 'http://foo:bar' } + let(:raven_exception_values) { raven_event['exception']['values'] } + let(:sentry_exception_values) { sentry_event.exception.to_hash[:values] } + + context 'when the error is a URI::InvalidURIError' do + let(:exception) do + URI.parse(invalid_uri) + rescue URI::InvalidURIError => error + error + end + + it 'filters the URI from the error message' do + track_exception + + expect(raven_exception_values).to include( + hash_including( + 'type' => 'URI::InvalidURIError', + 'value' => 'bad URI(is not URI?): [FILTERED]' + ) + ) + expect(sentry_exception_values).to include( + hash_including( + type: 'URI::InvalidURIError', + value: 'bad URI(is not URI?): [FILTERED]' + ) + ) + end + end + + context 'when the error is a Addressable::URI::InvalidURIError' do + let(:exception) do + Addressable::URI.parse(invalid_uri) + rescue Addressable::URI::InvalidURIError => error + error + end + + it 'filters the URI from the error message' do + track_exception + + expect(raven_exception_values).to include( + hash_including( + 'type' => 'Addressable::URI::InvalidURIError', + 'value' => 'Invalid port number: [FILTERED]' + ) + ) + expect(sentry_exception_values).to include( + hash_including( + type: 'Addressable::URI::InvalidURIError', + value: 'Invalid port number: [FILTERED]' + ) + ) + end + end + end end end diff --git a/spec/lib/gitlab/exception_log_formatter_spec.rb b/spec/lib/gitlab/exception_log_formatter_spec.rb index beeeeb2b64c..7dda56f0bf5 100644 --- a/spec/lib/gitlab/exception_log_formatter_spec.rb +++ b/spec/lib/gitlab/exception_log_formatter_spec.rb @@ -22,6 +22,14 @@ RSpec.describe Gitlab::ExceptionLogFormatter do expect(payload['exception.sql']).to be_nil end + it 'cleans the exception message' do + expect(Gitlab::Sanitizers::ExceptionMessage).to receive(:clean).with('RuntimeError', 'bad request').and_return('cleaned') + + described_class.format!(exception, payload) + + expect(payload['exception.message']).to eq('cleaned') + end + context 'when exception is ActiveRecord::StatementInvalid' do let(:exception) { ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1') } diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 8d9bff9c610..87ca899a87d 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do "notification_level" => 3, "created_at" => "2016-03-11T10:21:44.822Z", "updated_at" => "2016-03-11T10:21:44.822Z", - "created_by_id" => nil, + "created_by_id" => 1, "invite_email" => nil, "invite_token" => nil, "invite_accepted_at" => nil, @@ -38,10 +38,24 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do "notification_level" => 3, "created_at" => "2016-03-11T10:21:44.822Z", "updated_at" => "2016-03-11T10:21:44.822Z", - "created_by_id" => 1, + "created_by_id" => 2, "invite_email" => 'invite@test.com', "invite_token" => 'token', "invite_accepted_at" => nil + }, + { + "id" => 3, + "access_level" => 40, + "source_id" => 14, + "source_type" => source_type, + "user_id" => nil, + "notification_level" => 3, + "created_at" => "2016-03-11T10:21:44.822Z", + "updated_at" => "2016-03-11T10:21:44.822Z", + "created_by_id" => nil, + "invite_email" => 'invite2@test.com', + "invite_token" => 'token', + "invite_accepted_at" => nil }] end @@ -68,12 +82,37 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do expect(member_class.find_by_invite_email('invite@test.com')).not_to be_nil end - it 'removes old user_id from member_hash to avoid conflict with user key' do + it 'maps created_by_id to user on new instance' do + expect(member_class) + .to receive(:create) + .once + .with(hash_including('user_id' => user2.id, 'created_by_id' => nil)) + .and_call_original + expect(member_class) + .to receive(:create) + .once + .with(hash_including('invite_email' => 'invite@test.com', 'created_by_id' => nil)) + .and_call_original + expect(member_class) + .to receive(:create) + .once + .with(hash_including('invite_email' => 'invite2@test.com', 'created_by_id' => nil)) + .and_call_original + + members_mapper.map + end + + it 'replaced user_id with user_id from new instance' do + expect(member_class) + .to receive(:create) + .once + .with(hash_including('user_id' => user2.id)) + .and_call_original expect(member_class) .to receive(:create) - .twice - .with(hash_excluding('user_id')) - .and_call_original + .twice + .with(hash_excluding('user_id')) + .and_call_original members_mapper.map end @@ -99,7 +138,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end expect(logger).to receive(:info).with(hash_including(expected_log_params.call(user2.id))).once - expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).once + expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).twice members_mapper.map end diff --git a/spec/lib/gitlab/sanitizers/exception_message_spec.rb b/spec/lib/gitlab/sanitizers/exception_message_spec.rb new file mode 100644 index 00000000000..8b54b353235 --- /dev/null +++ b/spec/lib/gitlab/sanitizers/exception_message_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' + +RSpec.describe Gitlab::Sanitizers::ExceptionMessage do + describe '.clean' do + let(:exception_name) { exception.class.name } + let(:exception_message) { exception.message } + + subject { described_class.clean(exception_name, exception_message) } + + context 'when error is a URI::InvalidURIError' do + let(:exception) do + URI.parse('http://foo:bar') + rescue URI::InvalidURIError => error + error + end + + it { is_expected.to eq('bad URI(is not URI?): [FILTERED]') } + end + + context 'when error is an Addressable::URI::InvalidURIError' do + using RSpec::Parameterized::TableSyntax + + let(:exception) do + Addressable::URI.parse(uri) + rescue Addressable::URI::InvalidURIError => error + error + end + + where(:uri, :result) do + 'http://foo:bar' | 'Invalid port number: [FILTERED]' + 'http://foo:%eb' | 'Invalid encoding in port' + 'ht%0atp://foo' | 'Invalid scheme format: [FILTERED]' + 'http:' | 'Absolute URI missing hierarchical segment: [FILTERED]' + '::http' | 'Cannot assemble URI string with ambiguous path: [FILTERED]' + 'http://foo bar' | 'Invalid character in host: [FILTERED]' + end + + with_them do + it { is_expected.to eq(result) } + end + end + + context 'with any other exception' do + let(:exception) { StandardError.new('Error message: http://foo@bar:baz@ex:ample.com') } + + it 'is not invoked and does nothing' do + is_expected.to eq('Error message: http://foo@bar:baz@ex:ample.com') + end + end + end +end diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index 88efdbd77be..f4483f7e8f5 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -49,7 +49,7 @@ RSpec.describe Emails::Profile do describe 'for users that signed up, the email' do let(:example_site_path) { root_path } - let(:new_user) { create(:user, email: new_user_address, password: Gitlab::Password.test_default) } + let(:new_user) { create(:user, email: new_user_address, password: "securePassword") } subject { Notify.new_user_email(new_user.id) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b3e811de0c2..c6a27b4b687 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1146,6 +1146,28 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end end + + describe 'variable CI_GITLAB_FIPS_MODE' do + context 'when FIPS flag is enabled' do + before do + allow(Gitlab::FIPS).to receive(:enabled?).and_return(true) + end + + it "is included with value 'true'" do + expect(subject.to_hash).to include('CI_GITLAB_FIPS_MODE' => 'true') + end + end + + context 'when FIPS flag is disabled' do + before do + allow(Gitlab::FIPS).to receive(:enabled?).and_return(false) + end + + it 'is not included' do + expect(subject.to_hash).not_to have_key('CI_GITLAB_FIPS_MODE') + end + end + end end describe '#protected_ref?' do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 0620bb1ffc5..42187c3ef99 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -41,15 +41,25 @@ RSpec.describe Ci::Runner do context 'when runner is not allowed to pick untagged jobs' do context 'when runner does not have tags' do + let(:runner) { build(:ci_runner, tag_list: [], run_untagged: false) } + + it 'is not valid' do + expect(runner).to be_invalid + end + end + + context 'when runner has too many tags' do + let(:runner) { build(:ci_runner, tag_list: (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" }, run_untagged: false) } + it 'is not valid' do - runner = build(:ci_runner, tag_list: [], run_untagged: false) expect(runner).to be_invalid end end context 'when runner has tags' do + let(:runner) { build(:ci_runner, tag_list: ['tag'], run_untagged: false) } + it 'is valid' do - runner = build(:ci_runner, tag_list: ['tag'], run_untagged: false) expect(runner).to be_valid end end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index a3d36058b74..bf69c7219a8 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -37,7 +37,7 @@ RSpec.describe SystemHook do let(:project) { create(:project, namespace: user.namespace) } let(:group) { create(:group) } let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: Gitlab::Password.test_default } + { name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: 'mydummypass' } end before do diff --git a/spec/models/integrations/asana_spec.rb b/spec/models/integrations/asana_spec.rb index b6602964182..43e876a4f47 100644 --- a/spec/models/integrations/asana_spec.rb +++ b/spec/models/integrations/asana_spec.rb @@ -20,11 +20,13 @@ RSpec.describe Integrations::Asana do let(:gid) { "123456789ABCD" } let(:asana_task) { double(::Asana::Resources::Task) } let(:asana_integration) { described_class.new } + let(:ref) { 'main' } + let(:restrict_to_branch) { nil } let(:data) do { object_kind: 'push', - ref: 'master', + ref: ref, user_name: user.name, commits: [ { @@ -40,16 +42,44 @@ RSpec.describe Integrations::Asana do project: project, project_id: project.id, api_key: 'verySecret', - restrict_to_branch: 'master' + restrict_to_branch: restrict_to_branch ) end subject(:execute_integration) { asana_integration.execute(data) } + context 'with restrict_to_branch' do + let(:restrict_to_branch) { 'feature-branch, main' } + let(:message) { 'fix #456789' } + + context 'when ref is in scope of restriced branches' do + let(:ref) { 'main' } + + it 'calls the Asana integration' do + expect(asana_task).to receive(:add_comment) + expect(asana_task).to receive(:update).with(completed: true) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456789').once.and_return(asana_task) + + execute_integration + end + end + + context 'when ref is not in scope of restricted branches' do + let(:ref) { 'mai' } + + it 'does not call the Asana integration' do + expect(asana_task).not_to receive(:add_comment) + expect(::Asana::Resources::Task).not_to receive(:find_by_id) + + execute_integration + end + end + end + context 'when creating a story' do let(:message) { "Message from commit. related to ##{gid}" } let(:expected_message) do - "#{user.name} pushed to branch master of #{project.full_name} ( https://gitlab.com/ ): #{message}" + "#{user.name} pushed to branch main of #{project.full_name} ( https://gitlab.com/ ): #{message}" end it 'calls Asana integration to create a story' do diff --git a/spec/models/releases/link_spec.rb b/spec/models/releases/link_spec.rb index 4dc1e53d59e..74ef38f482b 100644 --- a/spec/models/releases/link_spec.rb +++ b/spec/models/releases/link_spec.rb @@ -113,6 +113,17 @@ RSpec.describe Releases::Link do end end + describe 'when filepath is greater than max length' do + let!(:invalid_link) { build(:release_link, filepath: 'x' * (Releases::Link::FILEPATH_MAX_LENGTH + 1), release: release) } + + it 'will not execute regex' do + invalid_link.filepath_format_valid? + + expect(invalid_link.errors[:filepath].size).to eq(1) + expect(invalid_link.errors[:filepath].first).to start_with("is too long") + end + end + describe 'FILEPATH_REGEX with table' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/ssh_host_key_spec.rb b/spec/models/ssh_host_key_spec.rb index 4d729d5585f..4b756846598 100644 --- a/spec/models/ssh_host_key_spec.rb +++ b/spec/models/ssh_host_key_spec.rb @@ -4,7 +4,9 @@ require 'spec_helper' RSpec.describe SshHostKey do using RSpec::Parameterized::TableSyntax + include ReactiveCachingHelpers + include StubRequests let(:key1) do 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC3UpyF2iLqy1d63M6k3jH1vuEnq/NWtE+o' \ @@ -35,6 +37,7 @@ RSpec.describe SshHostKey do let(:extra) { known_hosts + "foo\nbar\n" } let(:reversed) { known_hosts.lines.reverse.join } + let(:url) { 'ssh://example.com:2222' } let(:compare_host_keys) { nil } def stub_ssh_keyscan(args, status: true, stdout: "", stderr: "") @@ -50,7 +53,7 @@ RSpec.describe SshHostKey do let(:project) { build(:project) } - subject(:ssh_host_key) { described_class.new(project: project, url: 'ssh://example.com:2222', compare_host_keys: compare_host_keys) } + subject(:ssh_host_key) { described_class.new(project: project, url: url, compare_host_keys: compare_host_keys) } describe '.primary_key' do it 'returns a symbol' do @@ -191,5 +194,45 @@ RSpec.describe SshHostKey do is_expected.to eq(error: 'Failed to detect SSH host keys') end end + + context 'DNS rebinding protection enabled' do + before do + stub_application_setting(dns_rebinding_protection_enabled: true) + end + + it 'sends an address as well as hostname to ssh-keyscan' do + stub_dns(url, ip_address: '1.2.3.4') + + stdin = stub_ssh_keyscan(%w[-T 5 -p 2222 -f-]) + + cache + + expect(stdin.string).to eq("1.2.3.4 example.com\n") + end + end + end + + describe 'URL validation' do + let(:url) { 'ssh://127.0.0.1' } + + context 'when local requests are not allowed' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + end + + it 'forbids scanning localhost' do + expect { ssh_host_key }.to raise_error(/Invalid URL/) + end + end + + context 'when local requests are allowed' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + end + + it 'permits scanning localhost' do + expect(ssh_host_key.url.to_s).to eq('ssh://127.0.0.1:22') + end + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e9657de6ed1..1a1b92d3a9c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1757,9 +1757,9 @@ RSpec.describe User do describe '#generate_password' do it 'does not generate password by default' do - user = create(:user, password: Gitlab::Password.test_default) + user = create(:user, password: 'abcdefghe') - expect(user.password).to eq(Gitlab::Password.test_default) + expect(user.password).to eq('abcdefghe') end end @@ -5723,6 +5723,36 @@ RSpec.describe User do end end + describe '#valid_password?' do + subject { user.valid_password?(password) } + + context 'user with password not in disallowed list' do + let(:user) { create(:user) } + let(:password) { user.password } + + it { is_expected.to be_truthy } + + context 'using a wrong password' do + let(:password) { 'WRONG PASSWORD' } + + it { is_expected.to be_falsey } + end + end + + context 'user with disallowed password' do + let(:user) { create(:user, :disallowed_password) } + let(:password) { user.password } + + it { is_expected.to be_falsey } + + context 'using a wrong password' do + let(:password) { 'WRONG PASSWORD' } + + it { is_expected.to be_falsey } + end + end + end + describe '#password_expired?' do let(:user) { build(:user, password_expires_at: password_expires_at) } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 310454c227f..bde83d647db 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -81,25 +81,62 @@ RSpec.describe ProjectPolicy do context 'merge requests feature' do let(:current_user) { owner } + let(:mr_permissions) do + [:create_merge_request_from, :read_merge_request, :update_merge_request, + :admin_merge_request, :create_merge_request_in] + end it 'disallows all permissions when the feature is disabled' do project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED) - mr_permissions = [:create_merge_request_from, :read_merge_request, - :update_merge_request, :admin_merge_request, - :create_merge_request_in] - expect_disallowed(*mr_permissions) end + + context 'for a guest in a private project' do + let(:current_user) { guest } + let(:project) { private_project } + + it 'disallows the guest from all merge request permissions' do + expect_disallowed(*mr_permissions) + end + end end - context 'for a guest in a private project' do - let(:current_user) { guest } - let(:project) { private_project } + context 'creating_merge_request_in' do + context 'when project is public' do + let(:project) { public_project } + + context 'when the current_user is guest' do + let(:current_user) { guest } + + it { is_expected.to be_allowed(:create_merge_request_in) } + end + end + + context 'when project is internal' do + let(:project) { internal_project } + + context 'when the current_user is guest' do + let(:current_user) { guest } + + it { is_expected.to be_allowed(:create_merge_request_in) } + end + end + + context 'when project is private' do + let(:project) { private_project } + + context 'when the current_user is guest' do + let(:current_user) { guest } + + it { is_expected.not_to be_allowed(:create_merge_request_in) } + end + + context 'when the current_user is reporter or above' do + let(:current_user) { reporter } - it 'disallows the guest from reading the merge request and merge request iid' do - expect_disallowed(:read_merge_request) - expect_disallowed(:read_merge_request_iid) + it { is_expected.to be_allowed(:create_merge_request_in) } + end end end @@ -1346,6 +1383,110 @@ RSpec.describe ProjectPolicy do end end + describe 'read_ci_cd_analytics' do + context 'public project' do + let(:project) { create(:project, :public, :analytics_enabled) } + let(:current_user) { create(:user) } + + context 'when public pipelines are disabled for the project' do + before do + project.update!(public_builds: false) + end + + context 'project member' do + %w(guest reporter developer maintainer).each do |role| + context role do + before do + project.add_user(current_user, role.to_sym) + end + + if role == 'guest' + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + else + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + end + end + end + + context 'non member' do + let(:current_user) { non_member } + + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + + context 'anonymous' do + let(:current_user) { anonymous } + + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + end + + context 'when public pipelines are enabled for the project' do + before do + project.update!(public_builds: true) + end + + context 'project member' do + %w(guest reporter developer maintainer).each do |role| + context role do + before do + project.add_user(current_user, role.to_sym) + end + + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + end + end + + context 'non member' do + let(:current_user) { non_member } + + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + + context 'anonymous' do + let(:current_user) { anonymous } + + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + end + end + + context 'private project' do + let(:project) { create(:project, :private, :analytics_enabled) } + let(:current_user) { create(:user) } + + context 'project member' do + %w(guest reporter developer maintainer).each do |role| + context role do + before do + project.add_user(current_user, role.to_sym) + end + + if role == 'guest' + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + else + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + end + end + end + + context 'non member' do + let(:current_user) { non_member } + + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + + context 'anonymous' do + let(:current_user) { anonymous } + + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + end + end + it_behaves_like 'Self-managed Core resource access tokens' describe 'operations feature' do diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 1d553751eea..50ace7adccb 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -130,7 +130,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do } end - let_it_be(:new_runner) { create(:ci_runner) } + let_it_be(:new_runner) { build(:ci_runner) } it 'uses active value in registration' do expect_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| @@ -183,6 +183,54 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:created) expect(::Ci::Runner.last.ip_address).to eq('123.111.123.111') end + + context 'when tags parameter is provided' do + def request + post api('/runners'), params: { + token: registration_token, + tag_list: tag_list + } + end + + context 'with number of tags above limit' do + let(:tag_list) { (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" } } + + it 'uses tag_list value in registration and returns error' do + expect_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| + expected_params = { tag_list: tag_list }.stringify_keys + + expect(service).to receive(:execute) + .once + .with(registration_token, a_hash_including(expected_params)) + .and_call_original + end + + request + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response.dig('message', 'tags_list')).to contain_exactly("Too many tags specified. Please limit the number of tags to #{::Ci::Runner::TAG_LIST_MAX_LENGTH}") + end + end + + context 'with number of tags below limit' do + let(:tag_list) { (1..20).map { |i| "tag#{i}" } } + + it 'uses tag_list value in registration and successfully creates runner' do + expect_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| + expected_params = { tag_list: tag_list }.stringify_keys + + expect(service).to receive(:execute) + .once + .with(registration_token, a_hash_including(expected_params)) + .and_call_original + end + + request + + expect(response).to have_gitlab_http_status(:created) + end + end + end end end end diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index a1fda68b77b..d6ebc197ab0 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -396,6 +396,19 @@ RSpec.describe API::Ci::Runners do expect(shared_runner.reload.tag_list).to include('ruby2.1', 'pgsql', 'mysql') end + it 'unrelated runner attribute on an existing runner with too many tags' do + # This test ensures that it is possible to update any attribute on a runner that currently fails the + # validation that ensures that there aren't too many tags associated with a runner + existing_invalid_shared_runner = build(:ci_runner, :instance, tag_list: (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" } ) + existing_invalid_shared_runner.save!(validate: false) + + active = existing_invalid_shared_runner.active + update_runner(existing_invalid_shared_runner.id, admin, paused: active) + + expect(response).to have_gitlab_http_status(:ok) + expect(existing_invalid_shared_runner.reload.active).to eq(!active) + end + it 'runner untagged flag' do # Ensure tag list is non-empty before setting untagged to false. update_runner(shared_runner.id, admin, tag_list: ['ruby2.1', 'pgsql', 'mysql']) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index adbbd173189..518b0fd23c2 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1073,7 +1073,7 @@ RSpec.describe API::Users do post api('/users', admin), params: { email: 'invalid email', - password: Gitlab::Password.test_default, + password: 'password', name: 'test' } expect(response).to have_gitlab_http_status(:bad_request) @@ -1139,7 +1139,7 @@ RSpec.describe API::Users do post api('/users', admin), params: { email: 'test@example.com', - password: Gitlab::Password.test_default, + password: 'password', username: 'test', name: 'foo' } @@ -1151,7 +1151,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'test@example.com', - password: Gitlab::Password.test_default, + password: 'password', username: 'foo' } end.to change { User.count }.by(0) @@ -1165,7 +1165,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'foo@example.com', - password: Gitlab::Password.test_default, + password: 'password', username: 'test' } end.to change { User.count }.by(0) @@ -1179,7 +1179,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'foo@example.com', - password: Gitlab::Password.test_default, + password: 'password', username: 'TEST' } end.to change { User.count }.by(0) @@ -1524,8 +1524,8 @@ RSpec.describe API::Users do context "with existing user" do before do - post api("/users", admin), params: { email: 'test@example.com', password: Gitlab::Password.test_default, username: 'test', name: 'test' } - post api("/users", admin), params: { email: 'foo@bar.com', password: Gitlab::Password.test_default, username: 'john', name: 'john' } + post api("/users", admin), params: { email: 'test@example.com', password: 'password', username: 'test', name: 'test' } + post api("/users", admin), params: { email: 'foo@bar.com', password: 'password', username: 'john', name: 'john' } @user = User.all.last end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 9f9e1cfd90e..38c8d43376e 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -319,7 +319,7 @@ RSpec.describe 'Git HTTP requests' do context 'when user is using credentials with special characters' do context 'with password with special characters' do before do - user.update!(password: Gitlab::Password.test_default) + user.update!(password: 'RKszEwéC5kFnû∆f243fycGu§Gh9ftDj!U') end it 'allows clones' do @@ -1716,7 +1716,7 @@ RSpec.describe 'Git HTTP requests' do context 'when user is using credentials with special characters' do context 'with password with special characters' do before do - user.update!(password: Gitlab::Password.test_default) + user.update!(password: 'RKszEwéC5kFnû∆f243fycGu§Gh9ftDj!U') end it 'allows clones' do diff --git a/spec/requests/groups/crm/contacts_controller_spec.rb b/spec/requests/groups/crm/contacts_controller_spec.rb index 4d8ca0fcd60..0ee72233418 100644 --- a/spec/requests/groups/crm/contacts_controller_spec.rb +++ b/spec/requests/groups/crm/contacts_controller_spec.rb @@ -85,28 +85,19 @@ RSpec.describe Groups::Crm::ContactsController do end describe 'GET #index' do - subject do - get group_crm_contacts_path(group) - response - end + subject { get group_crm_contacts_path(group) } it_behaves_like 'ok response with index template if authorized' end describe 'GET #new' do - subject do - get new_group_crm_contact_path(group) - response - end + subject { get new_group_crm_contact_path(group) } it_behaves_like 'ok response with index template if authorized' end describe 'GET #edit' do - subject do - get edit_group_crm_contact_path(group, id: 1) - response - end + subject { get edit_group_crm_contact_path(group, id: 1) } it_behaves_like 'ok response with index template if authorized' end diff --git a/spec/requests/groups/crm/organizations_controller_spec.rb b/spec/requests/groups/crm/organizations_controller_spec.rb index 37ffac71772..410fc979262 100644 --- a/spec/requests/groups/crm/organizations_controller_spec.rb +++ b/spec/requests/groups/crm/organizations_controller_spec.rb @@ -85,18 +85,19 @@ RSpec.describe Groups::Crm::OrganizationsController do end describe 'GET #index' do - subject do - get group_crm_organizations_path(group) - response - end + subject { get group_crm_organizations_path(group) } it_behaves_like 'ok response with index template if authorized' end describe 'GET #new' do - subject do - get new_group_crm_organization_path(group) - end + subject { get new_group_crm_organization_path(group) } + + it_behaves_like 'ok response with index template if authorized' + end + + describe 'GET #edit' do + subject { get edit_group_crm_organization_path(group, id: 1) } it_behaves_like 'ok response with index template if authorized' end diff --git a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb index e62a94b6df8..a920b90b97d 100644 --- a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb +++ b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb @@ -53,6 +53,24 @@ RSpec.describe Ci::CreatePipelineService do end context 'when failed to create the pipeline' do + context 'when errors are raised and masked variables are involved' do + let_it_be(:variable) { create(:ci_variable, project: project, key: 'GL_TOKEN', value: 'test_value', masked: true) } + + let(:config) do + <<~YAML + include: + - local: $GL_TOKEN/gitlab-ci.txt + YAML + end + + it 'contains errors and masks variables' do + error_message = "Included file `xxxxxxxxxx/gitlab-ci.txt` does not have YAML extension!" + expect(pipeline.yaml_errors).to eq(error_message) + expect(pipeline.error_messages.map(&:content)).to contain_exactly(error_message) + expect(pipeline.errors.full_messages).to contain_exactly(error_message) + end + end + context 'when warnings are raised' do let(:config) do <<~YAML diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index f7daa1d62ac..85dbc39edcf 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -671,6 +671,19 @@ RSpec.describe QuickActions::InterpretService do end shared_examples 'assign command' do + it 'assigns to users with escaped underscores' do + user = create(:user) + base = user.username + user.update!(username: "#{base}_") + issuable.project.add_developer(user) + + cmd = "/assign @#{base}\\_" + + _, updates, _ = service.execute(cmd, issuable) + + expect(updates).to eq(assignee_ids: [user.id]) + end + it 'assigns to a single user' do _, updates, _ = service.execute(content, issuable) diff --git a/spec/services/users/create_service_spec.rb b/spec/services/users/create_service_spec.rb index ab9da82e91c..74340bac055 100644 --- a/spec/services/users/create_service_spec.rb +++ b/spec/services/users/create_service_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Users::CreateService do context 'when required parameters are provided' do let(:params) do - { name: 'John Doe', username: 'jduser', email: email, password: Gitlab::Password.test_default } + { name: 'John Doe', username: 'jduser', email: email, password: 'mydummypass' } end it 'returns a persisted user' do @@ -82,13 +82,13 @@ RSpec.describe Users::CreateService do context 'when force_random_password parameter is true' do let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: Gitlab::Password.test_default, force_random_password: true } + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', force_random_password: true } end it 'generates random password' do user = service.execute - expect(user.password).not_to eq Gitlab::Password.test_default + expect(user.password).not_to eq 'mydummypass' expect(user.password).to be_present end end @@ -99,7 +99,7 @@ RSpec.describe Users::CreateService do name: 'John Doe', username: 'jduser', email: 'jd@example.com', - password: Gitlab::Password.test_default, + password: 'mydummypass', password_automatically_set: true } end @@ -121,7 +121,7 @@ RSpec.describe Users::CreateService do context 'when skip_confirmation parameter is true' do let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: Gitlab::Password.test_default, skip_confirmation: true } + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', skip_confirmation: true } end it 'confirms the user' do @@ -131,7 +131,7 @@ RSpec.describe Users::CreateService do context 'when reset_password parameter is true' do let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: Gitlab::Password.test_default, reset_password: true } + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', reset_password: true } end it 'resets password even if a password parameter is given' do @@ -152,7 +152,7 @@ RSpec.describe Users::CreateService do context 'with nil user' do let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: Gitlab::Password.test_default, skip_confirmation: true } + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', skip_confirmation: true } end let(:service) { described_class.new(nil, params) } diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index c0734bae375..7666d71f13c 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -91,11 +91,12 @@ module LoginHelpers # user - User instance to login with # remember - Whether or not to check "Remember me" (default: false) # two_factor_auth - If two-factor authentication is enabled (default: false) - def gitlab_sign_in_with(user, remember: false, two_factor_auth: false) + # password - password to attempt to login with + def gitlab_sign_in_with(user, remember: false, two_factor_auth: false, password: nil) visit new_user_session_path fill_in "user_login", with: user.email - fill_in "user_password", with: Gitlab::Password.test_default + fill_in "user_password", with: (password || "12345678") check 'user_remember_me' if remember click_button "Sign in" diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index 3641edc845a..a78953e8199 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -15,7 +15,7 @@ RSpec.shared_context 'ProjectPolicy context' do let(:base_guest_permissions) do %i[ - award_emoji create_issue create_merge_request_in create_note + award_emoji create_issue create_note create_project read_issue_board read_issue read_issue_iid read_issue_link read_label read_planning_hierarchy read_issue_board_list read_milestone read_note read_project read_project_for_iids read_project_member read_release read_snippet @@ -26,12 +26,12 @@ RSpec.shared_context 'ProjectPolicy context' do let(:base_reporter_permissions) do %i[ admin_issue admin_issue_link admin_label admin_issue_board_list - create_snippet create_incident daily_statistics download_code + create_snippet create_incident daily_statistics create_merge_request_in download_code download_wiki_code fork_project metrics_dashboard read_build read_commit_status read_confidential_issues read_container_image read_deployment read_environment read_merge_request read_metrics_dashboard_annotation read_pipeline read_prometheus - read_sentry_issue update_issue + read_sentry_issue update_issue create_merge_request_in ] end @@ -66,7 +66,7 @@ RSpec.shared_context 'ProjectPolicy context' do let(:public_permissions) do %i[ - build_download_code build_read_container_image download_code + build_download_code build_read_container_image create_merge_request_in download_code download_wiki_code fork_project read_commit_status read_container_image read_pipeline read_release ] diff --git a/spec/support/shared_examples/policies/project_policy_shared_examples.rb b/spec/support/shared_examples/policies/project_policy_shared_examples.rb index a4243db6bc9..63e4d458ad4 100644 --- a/spec/support/shared_examples/policies/project_policy_shared_examples.rb +++ b/spec/support/shared_examples/policies/project_policy_shared_examples.rb @@ -107,6 +107,19 @@ RSpec.shared_examples 'deploy token does not get confused with user' do end RSpec.shared_examples 'project policies as guest' do + context 'abilities for public projects' do + let(:project) { public_project } + let(:current_user) { guest } + + it do + expect_allowed(*guest_permissions) + expect_allowed(*public_permissions) + expect_disallowed(*developer_permissions) + expect_disallowed(*maintainer_permissions) + expect_disallowed(*owner_permissions) + end + end + context 'abilities for non-public projects' do let(:project) { private_project } let(:current_user) { guest } diff --git a/spec/tasks/gitlab/password_rake_spec.rb b/spec/tasks/gitlab/password_rake_spec.rb index ec18d713351..65bba836024 100644 --- a/spec/tasks/gitlab/password_rake_spec.rb +++ b/spec/tasks/gitlab/password_rake_spec.rb @@ -3,7 +3,7 @@ require 'rake_helper' RSpec.describe 'gitlab:password rake tasks', :silence_stdout do - let_it_be(:user_1) { create(:user, username: 'foobar', password: Gitlab::Password.test_default) } + let_it_be(:user_1) { create(:user, username: 'foobar', password: 'initial_password') } def stub_username(username) allow(Gitlab::TaskHelpers).to receive(:prompt).with('Enter username: ').and_return(username) @@ -19,14 +19,14 @@ RSpec.describe 'gitlab:password rake tasks', :silence_stdout do Rake.application.rake_require 'tasks/gitlab/password' stub_username('foobar') - stub_password(Gitlab::Password.test_default) + stub_password('secretpassword') end describe ':reset' do context 'when all inputs are correct' do it 'updates the password properly' do run_rake_task('gitlab:password:reset', user_1.username) - expect(user_1.reload.valid_password?(Gitlab::Password.test_default)).to eq(true) + expect(user_1.reload.valid_password?('secretpassword')).to eq(true) end end @@ -55,7 +55,7 @@ RSpec.describe 'gitlab:password rake tasks', :silence_stdout do context 'when passwords do not match' do before do - stub_password(Gitlab::Password.test_default, "different" + Gitlab::Password.test_default) + stub_password('randompassword', 'differentpassword') end it 'aborts with an error' do diff --git a/yarn.lock b/yarn.lock index a0ef7910326..0987780f489 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11223,10 +11223,10 @@ svg-tags@^1.0.0: resolved "https://registry.yarnpkg.com/svg-tags/-/svg-tags-1.0.0.tgz#58f71cee3bd519b59d4b2a843b6c7de64ac04764" integrity sha1-WPcc7jvVGbWdSyqEO2x95krAR2Q= -swagger-ui-dist@^3.52.3: - version "3.52.3" - resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-3.52.3.tgz#a09b5cdccac69e3f5f1cbd258654a110119a7f0e" - integrity sha512-7QSY4milmYx5O8dbzU5tTftiaoZt+4JGxahTTBiLAnbTvhTyzum9rsjDIJjC+xeT8Tt1KfB38UuQQjmrh2THDQ== +swagger-ui-dist@4.8.0: + version "4.8.0" + resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-4.8.0.tgz#5f39a038a02ffbd5defb8e1921a9ac1620d779ae" + integrity sha512-jdcO4XcbwkAtrwvHp90Usjx3d4JZMjaiS02CxBFfuSxr6G8DBXPcK471+N6BcBkwZK7VTgpUBFAyyarsAvKYFQ== symbol-observable@^1.0.4: version "1.2.0" |
