diff options
160 files changed, 3556 insertions, 731 deletions
diff --git a/.gitlab/ci/docs.gitlab-ci.yml b/.gitlab/ci/docs.gitlab-ci.yml index b871ba33974..e35f491db79 100644 --- a/.gitlab/ci/docs.gitlab-ci.yml +++ b/.gitlab/ci/docs.gitlab-ci.yml @@ -45,6 +45,7 @@ docs lint: image: "registry.gitlab.com/gitlab-org/gitlab-docs/lint:vale-2.3.3-markdownlint-0.23.2" stage: test needs: [] + allow_failure: true script: - scripts/lint-doc.sh # Prepare docs for build diff --git a/.haml-lint_todo.yml b/.haml-lint_todo.yml index 4163c7bacd1..83aba188d2b 100644 --- a/.haml-lint_todo.yml +++ b/.haml-lint_todo.yml @@ -63,6 +63,8 @@ linters: - "app/views/admin/users/new.html.haml" - "app/views/admin/users/projects.html.haml" - "app/views/admin/users/show.html.haml" + - 'app/views/authentication/_authenticate.html.haml' + - 'app/views/authentication/_register.html.haml' - "app/views/clusters/clusters/_cluster.html.haml" - "app/views/clusters/clusters/new.html.haml" - "app/views/dashboard/milestones/index.html.haml" @@ -311,8 +313,6 @@ linters: - "app/views/shared/web_hooks/_form.html.haml" - "app/views/shared/web_hooks/_hook.html.haml" - "app/views/shared/wikis/_pages_wiki_page.html.haml" - - "app/views/u2f/_authenticate.html.haml" - - "app/views/u2f/_register.html.haml" - "app/views/users/_deletion_guidance.html.haml" - "ee/app/views/admin/_namespace_plan_info.html.haml" - "ee/app/views/admin/application_settings/_templates.html.haml" @@ -512,3 +512,5 @@ gem 'json_schemer', '~> 0.2.12' gem 'oj', '~> 3.10.6' gem 'multi_json', '~> 1.14.1' gem 'yajl-ruby', '~> 1.4.1', require: 'yajl' + +gem 'webauthn', '~> 2.3' diff --git a/Gemfile.lock b/Gemfile.lock index b6daa9abaa7..fcd5be4afe6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -73,6 +73,7 @@ GEM public_suffix (>= 2.0.2, < 5.0) aes_key_wrap (1.0.1) akismet (3.0.0) + android_key_attestation (0.3.0) apollo_upload_server (2.0.2) graphql (>= 1.8) rails (>= 4.2) @@ -93,6 +94,7 @@ GEM encryptor (~> 3.0.0) attr_required (1.0.1) awesome_print (1.8.0) + awrence (1.1.1) aws-eventstream (1.1.0) aws-partitions (1.345.0) aws-sdk-cloudformation (1.41.0) @@ -167,6 +169,7 @@ GEM activemodel (>= 4.0.0) activesupport (>= 4.0.0) mime-types (>= 1.16) + cbor (0.5.9.6) character_set (1.4.0) charlock_holmes (0.7.6) childprocess (3.0.0) @@ -189,6 +192,9 @@ GEM contracts (0.11.0) cork (0.3.0) colored2 (~> 3.1) + cose (1.0.0) + cbor (~> 0.5.9) + openssl-signature_algorithm (~> 0.4.0) countries (3.0.0) i18n_data (~> 0.8.0) sixarm_ruby_unaccent (~> 1.1) @@ -802,6 +808,8 @@ GEM validate_email validate_url webfinger (>= 1.0.1) + openssl (2.2.0) + openssl-signature_algorithm (0.4.0) opentracing (0.5.0) optimist (3.0.1) org-ruby (0.9.12) @@ -1026,6 +1034,8 @@ GEM rubyzip (2.0.0) rugged (0.28.4.1) safe_yaml (1.0.4) + safety_net_attestation (0.4.0) + jwt (~> 2.0) sanitize (5.2.1) crass (~> 1.0.2) nokogiri (>= 1.8.0) @@ -1050,6 +1060,7 @@ GEM scss_lint (0.56.0) rake (>= 0.9, < 13) sass (~> 3.5.3) + securecompare (1.0.0) seed-fu (2.3.7) activerecord (>= 3.1) activesupport (>= 3.1) @@ -1135,6 +1146,9 @@ GEM parslet (~> 1.8.0) toml-rb (1.0.0) citrus (~> 3.0, > 3.0) + tpm-key_attestation (0.9.0) + bindata (~> 2.4) + openssl-signature_algorithm (~> 0.4.0) truncato (0.7.11) htmlentities (~> 4.3.1) nokogiri (>= 1.7.0, <= 2.0) @@ -1186,6 +1200,16 @@ GEM vmstat (2.3.0) warden (1.2.8) rack (>= 2.0.6) + webauthn (2.3.0) + android_key_attestation (~> 0.3.0) + awrence (~> 1.1) + bindata (~> 2.4) + cbor (~> 0.5.9) + cose (~> 1.0) + openssl (~> 2.0) + safety_net_attestation (~> 0.4.0) + securecompare (~> 1.0) + tpm-key_attestation (~> 0.9.0) webfinger (1.1.0) activesupport httpclient (>= 2.4) @@ -1472,6 +1496,7 @@ DEPENDENCIES validates_hostname (~> 1.0.10) version_sorter (~> 2.2.4) vmstat (~> 2.3.0) + webauthn (~> 2.3) webmock (~> 3.5.1) webpack-rails (~> 0.9.10) wikicloth (= 0.8.1) diff --git a/app/assets/javascripts/authentication/mount_2fa.js b/app/assets/javascripts/authentication/mount_2fa.js index 9917151ac81..dd5a42fa5fc 100644 --- a/app/assets/javascripts/authentication/mount_2fa.js +++ b/app/assets/javascripts/authentication/mount_2fa.js @@ -1,14 +1,23 @@ import $ from 'jquery'; import initU2F from './u2f'; +import initWebauthn from './webauthn'; import U2FRegister from './u2f/register'; +import WebAuthnRegister from './webauthn/register'; export const mount2faAuthentication = () => { - // Soon this will conditionally mount a webauthn app (see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26692) - initU2F(); + if (gon.webauthn) { + initWebauthn(); + } else { + initU2F(); + } }; export const mount2faRegistration = () => { - // Soon this will conditionally mount a webauthn app (see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26692) - const u2fRegister = new U2FRegister($('#js-register-u2f'), gon.u2f); - u2fRegister.start(); + if (gon.webauthn) { + const webauthnRegister = new WebAuthnRegister($('#js-register-token-2fa'), gon.webauthn); + webauthnRegister.start(); + } else { + const u2fRegister = new U2FRegister($('#js-register-token-2fa'), gon.u2f); + u2fRegister.start(); + } }; diff --git a/app/assets/javascripts/authentication/u2f/authenticate.js b/app/assets/javascripts/authentication/u2f/authenticate.js index 201cd5c2e61..f9b5ca3e5b4 100644 --- a/app/assets/javascripts/authentication/u2f/authenticate.js +++ b/app/assets/javascripts/authentication/u2f/authenticate.js @@ -40,7 +40,6 @@ export default class U2FAuthenticate { this.signRequests = u2fParams.sign_requests.map(request => omit(request, 'challenge')); this.templates = { - setup: '#js-authenticate-token-2fa-setup', inProgress: '#js-authenticate-token-2fa-in-progress', error: '#js-authenticate-token-2fa-error', authenticated: '#js-authenticate-token-2fa-authenticated', @@ -86,7 +85,7 @@ export default class U2FAuthenticate { renderError(error) { this.renderTemplate('error', { error_message: error.message(), - error_code: error.errorCode, + error_name: error.errorCode, }); return this.container.find('#js-token-2fa-try-again').on('click', this.renderInProgress); } diff --git a/app/assets/javascripts/authentication/u2f/register.js b/app/assets/javascripts/authentication/u2f/register.js index 52c0ce1fc04..9773a9185f8 100644 --- a/app/assets/javascripts/authentication/u2f/register.js +++ b/app/assets/javascripts/authentication/u2f/register.js @@ -1,5 +1,6 @@ import $ from 'jquery'; import { template as lodashTemplate } from 'lodash'; +import { __ } from '~/locale'; import importU2FLibrary from './util'; import U2FError from './error'; @@ -24,11 +25,10 @@ export default class U2FRegister { this.signRequests = u2fParams.sign_requests; this.templates = { - notSupported: '#js-register-u2f-not-supported', - setup: '#js-register-u2f-setup', - inProgress: '#js-register-u2f-in-progress', - error: '#js-register-u2f-error', - registered: '#js-register-u2f-registered', + message: '#js-register-2fa-message', + setup: '#js-register-token-2fa-setup', + error: '#js-register-token-2fa-error', + registered: '#js-register-token-2fa-registered', }; } @@ -65,18 +65,22 @@ export default class U2FRegister { renderSetup() { this.renderTemplate('setup'); - return this.container.find('#js-setup-u2f-device').on('click', this.renderInProgress); + return this.container.find('#js-setup-token-2fa-device').on('click', this.renderInProgress); } renderInProgress() { - this.renderTemplate('inProgress'); + this.renderTemplate('message', { + message: __( + 'Trying to communicate with your device. Plug it in (if needed) and press the button on the device now.', + ), + }); return this.register(); } renderError(error) { this.renderTemplate('error', { error_message: error.message(), - error_code: error.errorCode, + error_name: error.errorCode, }); return this.container.find('#js-token-2fa-try-again').on('click', this.renderSetup); } @@ -89,6 +93,10 @@ export default class U2FRegister { } renderNotSupported() { - return this.renderTemplate('notSupported'); + return this.renderTemplate('message', { + message: __( + "Your browser doesn't support U2F. Please use Google Chrome desktop (version 41 or newer).", + ), + }); } } diff --git a/app/assets/javascripts/authentication/webauthn/authenticate.js b/app/assets/javascripts/authentication/webauthn/authenticate.js new file mode 100644 index 00000000000..42c4c2b63bd --- /dev/null +++ b/app/assets/javascripts/authentication/webauthn/authenticate.js @@ -0,0 +1,69 @@ +import WebAuthnError from './error'; +import WebAuthnFlow from './flow'; +import { supported, convertGetParams, convertGetResponse } from './util'; + +// Authenticate WebAuthn devices for users to authenticate with. +// +// State Flow #1: setup -> in_progress -> authenticated -> POST to server +// State Flow #2: setup -> in_progress -> error -> setup +export default class WebAuthnAuthenticate { + constructor(container, form, webauthnParams, fallbackButton, fallbackUI) { + this.container = container; + this.webauthnParams = convertGetParams(JSON.parse(webauthnParams.options)); + this.renderInProgress = this.renderInProgress.bind(this); + + this.form = form; + this.fallbackButton = fallbackButton; + this.fallbackUI = fallbackUI; + if (this.fallbackButton) { + this.fallbackButton.addEventListener('click', this.switchToFallbackUI.bind(this)); + } + + this.flow = new WebAuthnFlow(container, { + inProgress: '#js-authenticate-token-2fa-in-progress', + error: '#js-authenticate-token-2fa-error', + authenticated: '#js-authenticate-token-2fa-authenticated', + }); + + this.container.on('click', '#js-token-2fa-try-again', this.renderInProgress); + } + + start() { + if (!supported()) { + this.switchToFallbackUI(); + } else { + this.renderInProgress(); + } + } + + authenticate() { + navigator.credentials + .get({ publicKey: this.webauthnParams }) + .then(resp => { + const convertedResponse = convertGetResponse(resp); + this.renderAuthenticated(JSON.stringify(convertedResponse)); + }) + .catch(err => { + this.flow.renderError(new WebAuthnError(err, 'authenticate')); + }); + } + + renderInProgress() { + this.flow.renderTemplate('inProgress'); + this.authenticate(); + } + + renderAuthenticated(deviceResponse) { + this.flow.renderTemplate('authenticated'); + const container = this.container[0]; + container.querySelector('#js-device-response').value = deviceResponse; + container.querySelector(this.form).submit(); + this.fallbackButton.classList.add('hidden'); + } + + switchToFallbackUI() { + this.fallbackButton.classList.add('hidden'); + this.container[0].classList.add('hidden'); + this.fallbackUI.classList.remove('hidden'); + } +} diff --git a/app/assets/javascripts/authentication/webauthn/error.js b/app/assets/javascripts/authentication/webauthn/error.js new file mode 100644 index 00000000000..a1a3f861c25 --- /dev/null +++ b/app/assets/javascripts/authentication/webauthn/error.js @@ -0,0 +1,28 @@ +import { __ } from '~/locale'; +import { isHTTPS, FLOW_AUTHENTICATE, FLOW_REGISTER } from './util'; + +export default class WebAuthnError { + constructor(error, flowType) { + this.error = error; + this.errorName = error.name || 'UnknownError'; + this.message = this.message.bind(this); + this.httpsDisabled = !isHTTPS(); + this.flowType = flowType; + } + + message() { + if (this.errorName === 'NotSupportedError') { + return __('Your device is not compatible with GitLab. Please try another device'); + } else if (this.errorName === 'InvalidStateError' && this.flowType === FLOW_AUTHENTICATE) { + return __('This device has not been registered with us.'); + } else if (this.errorName === 'InvalidStateError' && this.flowType === FLOW_REGISTER) { + return __('This device has already been registered with us.'); + } else if (this.errorName === 'SecurityError' && this.httpsDisabled) { + return __( + 'WebAuthn only works with HTTPS-enabled websites. Contact your administrator for more details.', + ); + } + + return __('There was a problem communicating with your device.'); + } +} diff --git a/app/assets/javascripts/authentication/webauthn/flow.js b/app/assets/javascripts/authentication/webauthn/flow.js new file mode 100644 index 00000000000..10a1debc876 --- /dev/null +++ b/app/assets/javascripts/authentication/webauthn/flow.js @@ -0,0 +1,24 @@ +import { template } from 'lodash'; + +/** + * Generic abstraction for WebAuthnFlows, especially for register / authenticate + */ +export default class WebAuthnFlow { + constructor(container, templates) { + this.container = container; + this.templates = templates; + } + + renderTemplate(name, params) { + const templateString = document.querySelector(this.templates[name]).innerHTML; + const compiledTemplate = template(templateString); + this.container.html(compiledTemplate(params)); + } + + renderError(error) { + this.renderTemplate('error', { + error_message: error.message(), + error_name: error.errorName, + }); + } +} diff --git a/app/assets/javascripts/authentication/webauthn/index.js b/app/assets/javascripts/authentication/webauthn/index.js new file mode 100644 index 00000000000..bbf694c7698 --- /dev/null +++ b/app/assets/javascripts/authentication/webauthn/index.js @@ -0,0 +1,13 @@ +import $ from 'jquery'; +import WebAuthnAuthenticate from './authenticate'; + +export default () => { + const webauthnAuthenticate = new WebAuthnAuthenticate( + $('#js-authenticate-token-2fa'), + '#js-login-token-2fa-form', + gon.webauthn, + document.querySelector('#js-login-2fa-device'), + document.querySelector('.js-2fa-form'), + ); + webauthnAuthenticate.start(); +}; diff --git a/app/assets/javascripts/authentication/webauthn/register.js b/app/assets/javascripts/authentication/webauthn/register.js new file mode 100644 index 00000000000..06e4ffd6f3e --- /dev/null +++ b/app/assets/javascripts/authentication/webauthn/register.js @@ -0,0 +1,78 @@ +import { __ } from '~/locale'; +import WebAuthnError from './error'; +import WebAuthnFlow from './flow'; +import { supported, isHTTPS, convertCreateParams, convertCreateResponse } from './util'; + +// Register WebAuthn devices for users to authenticate with. +// +// State Flow #1: setup -> in_progress -> registered -> POST to server +// State Flow #2: setup -> in_progress -> error -> setup +export default class WebAuthnRegister { + constructor(container, webauthnParams) { + this.container = container; + this.renderInProgress = this.renderInProgress.bind(this); + this.webauthnOptions = convertCreateParams(webauthnParams.options); + + this.flow = new WebAuthnFlow(container, { + message: '#js-register-2fa-message', + setup: '#js-register-token-2fa-setup', + error: '#js-register-token-2fa-error', + registered: '#js-register-token-2fa-registered', + }); + + this.container.on('click', '#js-token-2fa-try-again', this.renderInProgress); + } + + start() { + if (!supported()) { + // we show a special error message when the user visits the site + // using a non-ssl connection as this makes WebAuthn unavailable in + // any case, regardless of the used browser + this.renderNotSupported(!isHTTPS()); + } else { + this.renderSetup(); + } + } + + register() { + navigator.credentials + .create({ + publicKey: this.webauthnOptions, + }) + .then(cred => this.renderRegistered(JSON.stringify(convertCreateResponse(cred)))) + .catch(err => this.flow.renderError(new WebAuthnError(err, 'register'))); + } + + renderSetup() { + this.flow.renderTemplate('setup'); + this.container.find('#js-setup-token-2fa-device').on('click', this.renderInProgress); + } + + renderInProgress() { + this.flow.renderTemplate('message', { + message: __( + 'Trying to communicate with your device. Plug it in (if needed) and press the button on the device now.', + ), + }); + return this.register(); + } + + renderRegistered(deviceResponse) { + this.flow.renderTemplate('registered'); + // Prefer to do this instead of interpolating using Underscore templates + // because of JSON escaping issues. + this.container.find('#js-device-response').val(deviceResponse); + } + + renderNotSupported(noHttps) { + const message = noHttps + ? __( + 'WebAuthn only works with HTTPS-enabled websites. Contact your administrator for more details.', + ) + : __( + "Your browser doesn't support WebAuthn. Please use a supported browser, e.g. Chrome (67+) or Firefox (60+).", + ); + + this.flow.renderTemplate('message', { message }); + } +} diff --git a/app/assets/javascripts/authentication/webauthn/util.js b/app/assets/javascripts/authentication/webauthn/util.js new file mode 100644 index 00000000000..5f06c000afe --- /dev/null +++ b/app/assets/javascripts/authentication/webauthn/util.js @@ -0,0 +1,120 @@ +export function supported() { + return Boolean( + navigator.credentials && + navigator.credentials.create && + navigator.credentials.get && + window.PublicKeyCredential, + ); +} + +export function isHTTPS() { + return window.location.protocol.startsWith('https'); +} + +export const FLOW_AUTHENTICATE = 'authenticate'; +export const FLOW_REGISTER = 'register'; + +// adapted from https://stackoverflow.com/a/21797381/8204697 +function base64ToBuffer(base64) { + const binaryString = window.atob(base64); + const len = binaryString.length; + const bytes = new Uint8Array(len); + for (let i = 0; i < len; i += 1) { + bytes[i] = binaryString.charCodeAt(i); + } + return bytes.buffer; +} + +// adapted from https://stackoverflow.com/a/9458996/8204697 +function bufferToBase64(buffer) { + if (typeof buffer === 'string') { + return buffer; + } + + let binary = ''; + const bytes = new Uint8Array(buffer); + const len = bytes.byteLength; + for (let i = 0; i < len; i += 1) { + binary += String.fromCharCode(bytes[i]); + } + return window.btoa(binary); +} + +/** + * Returns a copy of the given object with the id property converted to buffer + * + * @param {Object} param + */ +function convertIdToBuffer({ id, ...rest }) { + return { + ...rest, + id: base64ToBuffer(id), + }; +} + +/** + * Returns a copy of the given array with all `id`s of the items converted to buffer + * + * @param {Array} items + */ +function convertIdsToBuffer(items) { + return items.map(convertIdToBuffer); +} + +/** + * Returns an object with keys of the given props, and values from the given object converted to base64 + * + * @param {String} obj + * @param {Array} props + */ +function convertPropertiesToBase64(obj, props) { + return props.reduce( + (acc, property) => Object.assign(acc, { [property]: bufferToBase64(obj[property]) }), + {}, + ); +} + +export function convertGetParams({ allowCredentials, challenge, ...rest }) { + return { + ...rest, + ...(allowCredentials ? { allowCredentials: convertIdsToBuffer(allowCredentials) } : {}), + challenge: base64ToBuffer(challenge), + }; +} + +export function convertGetResponse(webauthnResponse) { + return { + type: webauthnResponse.type, + id: webauthnResponse.id, + rawId: bufferToBase64(webauthnResponse.rawId), + response: convertPropertiesToBase64(webauthnResponse.response, [ + 'clientDataJSON', + 'authenticatorData', + 'signature', + 'userHandle', + ]), + clientExtensionResults: webauthnResponse.getClientExtensionResults(), + }; +} + +export function convertCreateParams({ challenge, user, excludeCredentials, ...rest }) { + return { + ...rest, + challenge: base64ToBuffer(challenge), + user: convertIdToBuffer(user), + ...(excludeCredentials ? { excludeCredentials: convertIdsToBuffer(excludeCredentials) } : {}), + }; +} + +export function convertCreateResponse(webauthnResponse) { + return { + type: webauthnResponse.type, + id: webauthnResponse.id, + rawId: bufferToBase64(webauthnResponse.rawId), + clientExtensionResults: webauthnResponse.getClientExtensionResults(), + response: convertPropertiesToBase64(webauthnResponse.response, [ + 'clientDataJSON', + 'attestationObject', + ]), + }; +} diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index e3e140ea35e..fded391cc84 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -2,7 +2,14 @@ /* eslint-disable vue/no-v-html */ import { escape } from 'lodash'; import { mapActions, mapGetters } from 'vuex'; -import { GlDeprecatedButton, GlTooltipDirective, GlLoadingIcon, GlIcon } from '@gitlab/ui'; +import { + GlDeprecatedButton, + GlTooltipDirective, + GlSafeHtmlDirective, + GlLoadingIcon, + GlIcon, + GlButton, +} from '@gitlab/ui'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import { truncateSha } from '~/lib/utils/text_utility'; @@ -21,9 +28,11 @@ export default { GlIcon, FileIcon, DiffStats, + GlButton, }, directives: { GlTooltip: GlTooltipDirective, + SafeHtml: GlSafeHtmlDirective, }, props: { discussionPath: { @@ -77,6 +86,21 @@ export default { return this.discussionPath; }, + submoduleDiffCompareLinkText() { + if (this.diffFile.submodule_compare) { + const truncatedOldSha = escape(truncateSha(this.diffFile.submodule_compare.old_sha)); + const truncatedNewSha = escape(truncateSha(this.diffFile.submodule_compare.new_sha)); + return sprintf( + s__('Compare %{oldCommitId}...%{newCommitId}'), + { + oldCommitId: `<span class="commit-sha">${truncatedOldSha}</span>`, + newCommitId: `<span class="commit-sha">${truncatedNewSha}</span>`, + }, + false, + ); + } + return null; + }, filePath() { if (this.diffFile.submodule) { return `${this.diffFile.file_path} @ ${truncateSha(this.diffFile.blob.id)}`; @@ -311,5 +335,18 @@ export default { </a> </div> </div> + + <div + v-if="diffFile.submodule_compare" + class="file-actions d-none d-sm-flex align-items-center flex-wrap" + > + <gl-button + v-gl-tooltip.hover + v-safe-html="submoduleDiffCompareLinkText" + class="submodule-compare" + :title="s__('Compare submodule commit revisions')" + :href="diffFile.submodule_compare.url" + /> + </div> </div> </template> diff --git a/app/assets/javascripts/packages/details/components/dependency_row.vue b/app/assets/javascripts/packages/details/components/dependency_row.vue index 367ed6ca22a..1a2202b23c8 100644 --- a/app/assets/javascripts/packages/details/components/dependency_row.vue +++ b/app/assets/javascripts/packages/details/components/dependency_row.vue @@ -26,7 +26,7 @@ export default { <div v-if="showVersion" - class="table-section section-50 gl-display-flex gl-justify-content-md-end" + class="table-section section-50 gl-display-flex gl-md-justify-content-end" data-testid="version-pattern" > <span class="gl-text-body">{{ dependency.version_pattern }}</span> diff --git a/app/assets/javascripts/pages/milestones/shared/components/delete_milestone_modal.vue b/app/assets/javascripts/pages/milestones/shared/components/delete_milestone_modal.vue index b35f0cbec2e..983062c79f1 100644 --- a/app/assets/javascripts/pages/milestones/shared/components/delete_milestone_modal.vue +++ b/app/assets/javascripts/pages/milestones/shared/components/delete_milestone_modal.vue @@ -1,5 +1,5 @@ <script> -/* eslint-disable vue/no-v-html */ +import { GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import axios from '~/lib/utils/axios_utils'; import { deprecatedCreateFlash as Flash } from '~/flash'; @@ -12,6 +12,9 @@ export default { components: { DeprecatedModal, }, + directives: { + SafeHtml, + }, props: { issueCount: { type: Number, @@ -125,7 +128,7 @@ Once deleted, it cannot be undone or recovered.`), @submit="onSubmit" > <template #body="props"> - <p v-html="props.text"></p> + <p v-safe-html="props.text"></p> </template> </deprecated-modal> </template> diff --git a/app/assets/javascripts/pipelines/pipeline_details_bundle.js b/app/assets/javascripts/pipelines/pipeline_details_bundle.js index c57be7c75b0..745f5b886a5 100644 --- a/app/assets/javascripts/pipelines/pipeline_details_bundle.js +++ b/app/assets/javascripts/pipelines/pipeline_details_bundle.js @@ -14,10 +14,20 @@ import createTestReportsStore from './stores/test_reports'; Vue.use(Translate); +const SELECTORS = { + PIPELINE_DETAILS: '.js-pipeline-details-vue', + PIPELINE_GRAPH: '#js-pipeline-graph-vue', + PIPELINE_HEADER: '#js-pipeline-header-vue', + PIPELINE_TESTS: '#js-pipeline-tests-detail', +}; + const createPipelinesDetailApp = mediator => { + if (!document.querySelector(SELECTORS.PIPELINE_GRAPH)) { + return; + } // eslint-disable-next-line no-new new Vue({ - el: '#js-pipeline-graph-vue', + el: SELECTORS.PIPELINE_GRAPH, components: { pipelineGraph, }, @@ -47,9 +57,12 @@ const createPipelinesDetailApp = mediator => { }; const createPipelineHeaderApp = mediator => { + if (!document.querySelector(SELECTORS.PIPELINE_HEADER)) { + return; + } // eslint-disable-next-line no-new new Vue({ - el: '#js-pipeline-header-vue', + el: SELECTORS.PIPELINE_HEADER, components: { pipelineHeader, }, @@ -93,9 +106,8 @@ const createPipelineHeaderApp = mediator => { }; const createTestDetails = () => { - const el = document.querySelector('#js-pipeline-tests-detail'); + const el = document.querySelector(SELECTORS.PIPELINE_TESTS); const { summaryEndpoint, suiteEndpoint } = el?.dataset || {}; - const testReportsStore = createTestReportsStore({ summaryEndpoint, suiteEndpoint, @@ -115,7 +127,7 @@ const createTestDetails = () => { }; export default () => { - const { dataset } = document.querySelector('.js-pipeline-details-vue'); + const { dataset } = document.querySelector(SELECTORS.PIPELINE_DETAILS); const mediator = new PipelinesMediator({ endpoint: dataset.endpoint }); mediator.fetchPipeline(); diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js index 3bd512c89bf..9d3f4eb01ed 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js @@ -17,7 +17,7 @@ export default function deviseState() { return stateKey.pipelineFailed; } else if (this.workInProgress) { return stateKey.workInProgress; - } else if (this.hasMergeableDiscussionsState) { + } else if (this.hasMergeableDiscussionsState && !this.autoMergeEnabled) { return stateKey.unresolvedDiscussions; } else if (this.isPipelineBlocked) { return stateKey.pipelineBlocked; diff --git a/app/assets/stylesheets/framework/tables.scss b/app/assets/stylesheets/framework/tables.scss index 312158bd3c9..59e83608d9d 100644 --- a/app/assets/stylesheets/framework/tables.scss +++ b/app/assets/stylesheets/framework/tables.scss @@ -193,6 +193,10 @@ table { } } + .detected { + width: 9%; + } + .status { width: 8%; } @@ -202,7 +206,7 @@ table { } .identifier { - width: 12%; + width: 16%; } .scanner { diff --git a/app/assets/stylesheets/page_bundles/todos.scss b/app/assets/stylesheets/page_bundles/todos.scss index 062994e7f51..26d9607d219 100644 --- a/app/assets/stylesheets/page_bundles/todos.scss +++ b/app/assets/stylesheets/page_bundles/todos.scss @@ -151,18 +151,6 @@ } } -@include media-breakpoint-down(sm) { - .todos-filters { - .dropdown-menu-toggle { - width: 130px; - } - - .dropdown-menu-toggle-sort { - width: auto; - } - } -} - @include media-breakpoint-down(lg) { .todos-filters { .filter-categories { @@ -206,6 +194,10 @@ .dropdown-menu-toggle { width: 100%; } + + .dropdown-menu-toggle-sort { + width: auto; + } } } diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 85836962b06..4dc1f2034f3 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -257,7 +257,8 @@ } } -table.u2f-registrations { +table.u2f-registrations, +.webauthn-registrations { th:not(:last-child), td:not(:last-child) { border-right: solid 1px transparent; diff --git a/app/assets/stylesheets/utilities.scss b/app/assets/stylesheets/utilities.scss index 6792a27f2cd..9c666331c4f 100644 --- a/app/assets/stylesheets/utilities.scss +++ b/app/assets/stylesheets/utilities.scss @@ -112,27 +112,10 @@ top: 66vh; } -// Remove when https://gitlab.com/gitlab-org/gitlab-ui/-/issues/871 -// gets fixed on GitLab UI -.gl-sm-w-auto\! { - @media (min-width: $breakpoint-sm) { - width: auto !important; - } -} - - .gl-shadow-x0-y0-b3-s1-blue-500 { box-shadow: inset 0 0 3px $gl-border-size-1 $blue-500; } -// remove when https://gitlab.com/gitlab-org/gitlab-ui/-/merge_requests/1692 is merged -.gl-border-t-transparent { - border-top-color: transparent; -} - -.gl-align-items-flex-end { - align-items: flex-end; -} .gl-sm-align-items-flex-end { @media (min-width: $breakpoint-sm) { @@ -152,15 +135,11 @@ } } -.gl-align-items-stretch { - align-items: stretch; -} - .gl-min-h-6 { min-height: $gl-spacing-scale-6; } -.gl-justify-content-md-end { +.gl-md-justify-content-end { @media (min-width: $breakpoint-md) { width: auto !important; } diff --git a/app/controllers/admin/concerns/authenticates_2fa_for_admin_mode.rb b/app/controllers/admin/concerns/authenticates_2fa_for_admin_mode.rb index 6014ed0dd13..03783cd75a3 100644 --- a/app/controllers/admin/concerns/authenticates_2fa_for_admin_mode.rb +++ b/app/controllers/admin/concerns/authenticates_2fa_for_admin_mode.rb @@ -11,7 +11,13 @@ module Authenticates2FAForAdminMode return handle_locked_user(user) unless user.can?(:log_in) session[:otp_user_id] = user.id - setup_u2f_authentication(user) + push_frontend_feature_flag(:webauthn) + + if user.two_factor_webauthn_enabled? + setup_webauthn_authentication(user) + else + setup_u2f_authentication(user) + end render 'admin/sessions/two_factor', layout: 'application' end @@ -24,7 +30,11 @@ module Authenticates2FAForAdminMode if user_params[:otp_attempt].present? && session[:otp_user_id] admin_mode_authenticate_with_two_factor_via_otp(user) elsif user_params[:device_response].present? && session[:otp_user_id] - admin_mode_authenticate_with_two_factor_via_u2f(user) + if user.two_factor_webauthn_enabled? + admin_mode_authenticate_with_two_factor_via_webauthn(user) + else + admin_mode_authenticate_with_two_factor_via_u2f(user) + end elsif user && user.valid_password?(user_params[:password]) admin_mode_prompt_for_two_factor(user) else @@ -52,18 +62,17 @@ module Authenticates2FAForAdminMode def admin_mode_authenticate_with_two_factor_via_u2f(user) if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge]) - # Remove any lingering user data from login - session.delete(:otp_user_id) - session.delete(:challenge) - - # The admin user has successfully passed 2fa, enable admin mode ignoring password - enable_admin_mode + admin_handle_two_factor_success else - user.increment_failed_attempts! - Gitlab::AppLogger.info("Failed Admin Mode Login: user=#{user.username} ip=#{request.remote_ip} method=U2F") - flash.now[:alert] = _('Authentication via U2F device failed.') + admin_handle_two_factor_failure(user, 'U2F') + end + end - admin_mode_prompt_for_two_factor(user) + def admin_mode_authenticate_with_two_factor_via_webauthn(user) + if Webauthn::AuthenticateService.new(user, user_params[:device_response], session[:challenge]).execute + admin_handle_two_factor_success + else + admin_handle_two_factor_failure(user, 'WebAuthn') end end @@ -81,4 +90,21 @@ module Authenticates2FAForAdminMode flash.now[:alert] = _('Invalid login or password') render :new end + + def admin_handle_two_factor_success + # Remove any lingering user data from login + session.delete(:otp_user_id) + session.delete(:challenge) + + # The admin user has successfully passed 2fa, enable admin mode ignoring password + enable_admin_mode + end + + def admin_handle_two_factor_failure(user, method) + user.increment_failed_attempts! + Gitlab::AppLogger.info("Failed Admin Mode Login: user=#{user.username} ip=#{request.remote_ip} method=#{method}") + flash.now[:alert] = _('Authentication via %{method} device failed.') % { method: method } + + admin_mode_prompt_for_two_factor(user) + end end diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index b93c98a4790..5bdabe2bfd1 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -23,8 +23,14 @@ module AuthenticatesWithTwoFactor session[:otp_user_id] = user.id session[:user_updated_at] = user.updated_at + push_frontend_feature_flag(:webauthn) + + if user.two_factor_webauthn_enabled? + setup_webauthn_authentication(user) + else + setup_u2f_authentication(user) + end - setup_u2f_authentication(user) render 'devise/sessions/two_factor' end @@ -46,7 +52,11 @@ module AuthenticatesWithTwoFactor if user_params[:otp_attempt].present? && session[:otp_user_id] authenticate_with_two_factor_via_otp(user) elsif user_params[:device_response].present? && session[:otp_user_id] - authenticate_with_two_factor_via_u2f(user) + if user.two_factor_webauthn_enabled? + authenticate_with_two_factor_via_webauthn(user) + else + authenticate_with_two_factor_via_u2f(user) + end elsif user && user.valid_password?(user_params[:password]) prompt_for_two_factor(user) end @@ -89,16 +99,17 @@ module AuthenticatesWithTwoFactor # Authenticate using the response from a U2F (universal 2nd factor) device def authenticate_with_two_factor_via_u2f(user) if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge]) - # Remove any lingering user data from login - clear_two_factor_attempt! + handle_two_factor_success(user) + else + handle_two_factor_failure(user, 'U2F') + end + end - remember_me(user) if user_params[:remember_me] == '1' - sign_in(user, message: :two_factor_authenticated, event: :authentication) + def authenticate_with_two_factor_via_webauthn(user) + if Webauthn::AuthenticateService.new(user, user_params[:device_response], session[:challenge]).execute + handle_two_factor_success(user) else - user.increment_failed_attempts! - Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=U2F") - flash.now[:alert] = _('Authentication via U2F device failed.') - prompt_for_two_factor(user) + handle_two_factor_failure(user, 'WebAuthn') end end @@ -116,8 +127,39 @@ module AuthenticatesWithTwoFactor sign_requests: sign_requests }) end end + + def setup_webauthn_authentication(user) + if user.webauthn_registrations.present? + + webauthn_registration_ids = user.webauthn_registrations.pluck(:credential_xid) + + get_options = WebAuthn::Credential.options_for_get(allow: webauthn_registration_ids, + user_verification: 'discouraged', + extensions: { appid: WebAuthn.configuration.origin }) + + session[:credentialRequestOptions] = get_options + session[:challenge] = get_options.challenge + gon.push(webauthn: { options: get_options.to_json }) + end + end + # rubocop: enable CodeReuse/ActiveRecord + def handle_two_factor_success(user) + # Remove any lingering user data from login + clear_two_factor_attempt! + + remember_me(user) if user_params[:remember_me] == '1' + sign_in(user, message: :two_factor_authenticated, event: :authentication) + end + + def handle_two_factor_failure(user, method) + user.increment_failed_attempts! + Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=#{method}") + flash.now[:alert] = _('Authentication via %{method} device failed.') % { method: method } + prompt_for_two_factor(user) + end + def handle_changed_user(user) clear_two_factor_attempt! diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index f600c34ca75..5de6d84fdd9 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -2,6 +2,9 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController skip_before_action :check_two_factor_requirement + before_action do + push_frontend_feature_flag(:webauthn) + end def show unless current_user.two_factor_enabled? @@ -33,7 +36,12 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController @qr_code = build_qr_code @account_string = account_string - setup_u2f_registration + + if Feature.enabled?(:webauthn) + setup_webauthn_registration + else + setup_u2f_registration + end end def create @@ -48,7 +56,13 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController else @error = _('Invalid pin code') @qr_code = build_qr_code - setup_u2f_registration + + if Feature.enabled?(:webauthn) + setup_webauthn_registration + else + setup_u2f_registration + end + render 'show' end end @@ -56,7 +70,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController # A U2F (universal 2nd factor) device's information is stored after successful # registration, which is then used while 2FA authentication is taking place. def create_u2f - @u2f_registration = U2fRegistration.register(current_user, u2f_app_id, u2f_registration_params, session[:challenges]) + @u2f_registration = U2fRegistration.register(current_user, u2f_app_id, device_registration_params, session[:challenges]) if @u2f_registration.persisted? session.delete(:challenges) @@ -68,6 +82,21 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController end end + def create_webauthn + @webauthn_registration = Webauthn::RegisterService.new(current_user, device_registration_params, session[:challenge]).execute + if @webauthn_registration.persisted? + session.delete(:challenge) + + redirect_to profile_two_factor_auth_path, notice: s_("Your WebAuthn device was registered!") + else + @qr_code = build_qr_code + + setup_webauthn_registration + + render :show + end + end + def codes Users::UpdateService.new(current_user, user: current_user).execute! do |user| @codes = user.generate_otp_backup_codes! @@ -112,11 +141,11 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController # Actual communication is performed using a Javascript API def setup_u2f_registration @u2f_registration ||= U2fRegistration.new - @u2f_registrations = current_user.u2f_registrations + @registrations = u2f_registrations u2f = U2F::U2F.new(u2f_app_id) registration_requests = u2f.registration_requests - sign_requests = u2f.authentication_requests(@u2f_registrations.map(&:key_handle)) + sign_requests = u2f.authentication_requests(current_user.u2f_registrations.map(&:key_handle)) session[:challenges] = registration_requests.map(&:challenge) gon.push(u2f: { challenges: session[:challenges], app_id: u2f_app_id, @@ -124,8 +153,53 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController sign_requests: sign_requests }) end - def u2f_registration_params - params.require(:u2f_registration).permit(:device_response, :name) + def device_registration_params + params.require(:device_registration).permit(:device_response, :name) + end + + def setup_webauthn_registration + @registrations = webauthn_registrations + @webauthn_registration ||= WebauthnRegistration.new + + unless current_user.webauthn_xid + current_user.user_detail.update!(webauthn_xid: WebAuthn.generate_user_id) + end + + options = webauthn_options + session[:challenge] = options.challenge + + gon.push(webauthn: { options: options, app_id: u2f_app_id }) + end + + # Adds delete path to u2f registrations + # to reduce logic in view template + def u2f_registrations + current_user.u2f_registrations.map do |u2f_registration| + { + name: u2f_registration.name, + created_at: u2f_registration.created_at, + delete_path: profile_u2f_registration_path(u2f_registration) + } + end + end + + def webauthn_registrations + current_user.webauthn_registrations.map do |webauthn_registration| + { + name: webauthn_registration.name, + created_at: webauthn_registration.created_at, + delete_path: profile_webauthn_registration_path(webauthn_registration) + } + end + end + + def webauthn_options + WebAuthn::Credential.options_for_create( + user: { id: current_user.webauthn_xid, name: current_user.username }, + exclude: current_user.webauthn_registrations.map { |c| c.credential_xid }, + authenticator_selection: { user_verification: 'discouraged' }, + rp: { name: 'GitLab' } + ) end def groups_notification(groups) @@ -133,6 +207,6 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController leave_group_links = groups.map { |group| view_context.link_to (s_("leave %{group_name}") % { group_name: group.full_name }), leave_group_members_path(group), remote: false, method: :delete}.to_sentence s_(%{The group settings for %{group_links} require you to enable Two-Factor Authentication for your account. You can %{leave_group_links}.}) - .html_safe % { group_links: group_links.html_safe, leave_group_links: leave_group_links.html_safe } + .html_safe % { group_links: group_links.html_safe, leave_group_links: leave_group_links.html_safe } end end diff --git a/app/controllers/profiles/webauthn_registrations_controller.rb b/app/controllers/profiles/webauthn_registrations_controller.rb new file mode 100644 index 00000000000..81b1dd6f710 --- /dev/null +++ b/app/controllers/profiles/webauthn_registrations_controller.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class Profiles::WebauthnRegistrationsController < Profiles::ApplicationController + def destroy + webauthn_registration = current_user.webauthn_registrations.find(params[:id]) + webauthn_registration.destroy + + redirect_to profile_two_factor_auth_path, status: :found, notice: _("Successfully deleted WebAuthn device.") + end +end diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index c9f46eb72c5..9a28088ccf6 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -6,6 +6,9 @@ class ProfilesController < Profiles::ApplicationController before_action :user before_action :authorize_change_username!, only: :update_username skip_before_action :require_email, only: [:show, :update] + before_action do + push_frontend_feature_flag(:webauthn) + end def show end diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 0bb4e0fb5ee..921da788ad2 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -43,6 +43,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont :discussion_locked, label_ids: [], assignee_ids: [], + reviewer_ids: [], update_task: [:index, :checked, :line_number, :line_source] ] end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 3986382a5f9..848625ff6b5 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -42,7 +42,7 @@ class ProjectsController < Projects::ApplicationController before_action only: [:edit] do push_frontend_feature_flag(:service_desk_custom_address, @project) - push_frontend_feature_flag(:approval_suggestions, @project) + push_frontend_feature_flag(:approval_suggestions, @project, default_enabled: true) end layout :determine_layout diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 5da2bafbcb3..318553b5e0a 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -29,6 +29,9 @@ class SessionsController < Devise::SessionsController before_action :save_failed_login, if: :action_new_and_failed_login? before_action :load_recaptcha before_action :set_invite_params, only: [:new] + before_action do + push_frontend_feature_flag(:webauthn) + end after_action :log_failed_login, if: :action_new_and_failed_login? after_action :verify_known_sign_in, only: [:create] @@ -293,7 +296,9 @@ class SessionsController < Devise::SessionsController def authentication_method if user_params[:otp_attempt] "two-factor" - elsif user_params[:device_response] + elsif user_params[:device_response] && Feature.enabled?(:webauthn) + "two-factor-via-webauthn-device" + elsif user_params[:device_response] && !Feature.enabled?(:webauthn) "two-factor-via-u2f-device" else "standard" diff --git a/app/graphql/types/issuable_severity_enum.rb b/app/graphql/types/issuable_severity_enum.rb new file mode 100644 index 00000000000..563965d991e --- /dev/null +++ b/app/graphql/types/issuable_severity_enum.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Types + class IssuableSeverityEnum < BaseEnum + graphql_name 'IssuableSeverity' + description 'Incident severity' + + ::IssuableSeverity.severities.keys.each do |severity| + value severity.upcase, value: severity, description: "#{severity.titleize} severity" + end + end +end diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb index e49cb3b3932..37af317996d 100644 --- a/app/graphql/types/issue_type.rb +++ b/app/graphql/types/issue_type.rb @@ -105,6 +105,9 @@ module Types Types::AlertManagement::AlertType, null: true, description: 'Alert associated to this issue' + + field :severity, Types::IssuableSeverityEnum, null: true, + description: 'Severity level of the incident' end end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 3b25de521d0..7c254e069f6 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -100,20 +100,43 @@ module DiffHelper end def submodule_link(blob, ref, repository = @repository) - project_url, tree_url = submodule_links(blob, ref, repository) - commit_id = if tree_url.nil? - Commit.truncate_sha(blob.id) - else - link_to Commit.truncate_sha(blob.id), tree_url - end + urls = submodule_links(blob, ref, repository) + + folder_name = truncate(blob.name, length: 40) + folder_name = link_to(folder_name, urls.web) if urls&.web + + commit_id = Commit.truncate_sha(blob.id) + commit_id = link_to(commit_id, urls.tree) if urls&.tree [ - content_tag(:span, link_to(truncate(blob.name, length: 40), project_url)), + content_tag(:span, folder_name), '@', content_tag(:span, commit_id, class: 'commit-sha') ].join(' ').html_safe end + def submodule_diff_compare_link(diff_file) + compare_url = submodule_links(diff_file.blob, diff_file.content_sha, diff_file.repository, diff_file)&.compare + + link = "" + + if compare_url + + link_text = [ + _('Compare'), + ' ', + content_tag(:span, Commit.truncate_sha(diff_file.old_blob.id), class: 'commit-sha'), + '...', + content_tag(:span, Commit.truncate_sha(diff_file.blob.id), class: 'commit-sha') + ].join('').html_safe + + tooltip = _('Compare submodule commit revisions') + link = content_tag(:span, link_to(link_text, compare_url, class: 'btn has-tooltip', title: tooltip), class: 'submodule-compare') + end + + link + end + def diff_file_blob_raw_url(diff_file, only_path: false) project_raw_url(@project, tree_join(diff_file.content_sha, diff_file.file_path), only_path: only_path) end diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb index 2e7146eaa43..959178c47d7 100644 --- a/app/helpers/submodule_helper.rb +++ b/app/helpers/submodule_helper.rb @@ -6,12 +6,12 @@ module SubmoduleHelper VALID_SUBMODULE_PROTOCOLS = %w[http https git ssh].freeze # links to files listing for submodule if submodule is a project on this server - def submodule_links(submodule_item, ref = nil, repository = @repository) - repository.submodule_links.for(submodule_item, ref) + def submodule_links(submodule_item, ref = nil, repository = @repository, diff_file = nil) + repository.submodule_links.for(submodule_item, ref, diff_file) end - def submodule_links_for_url(submodule_item_id, url, repository) - return [nil, nil] unless url + def submodule_links_for_url(submodule_item_id, url, repository, old_submodule_item_id = nil) + return [nil, nil, nil] unless url if url == '.' || url == './' url = File.join(Gitlab.config.gitlab.url, repository.project.full_path) @@ -34,21 +34,24 @@ module SubmoduleHelper project.sub!(/\.git\z/, '') if self_url?(url, namespace, project) - [url_helpers.namespace_project_path(namespace, project), - url_helpers.namespace_project_tree_path(namespace, project, submodule_item_id)] + [ + url_helpers.namespace_project_path(namespace, project), + url_helpers.namespace_project_tree_path(namespace, project, submodule_item_id), + (url_helpers.namespace_project_compare_path(namespace, project, to: submodule_item_id, from: old_submodule_item_id) if old_submodule_item_id) + ] elsif relative_self_url?(url) - relative_self_links(url, submodule_item_id, repository.project) + relative_self_links(url, submodule_item_id, old_submodule_item_id, repository.project) elsif gist_github_dot_com_url?(url) gist_github_com_tree_links(namespace, project, submodule_item_id) elsif github_dot_com_url?(url) - github_com_tree_links(namespace, project, submodule_item_id) + github_com_tree_links(namespace, project, submodule_item_id, old_submodule_item_id) elsif gitlab_dot_com_url?(url) - gitlab_com_tree_links(namespace, project, submodule_item_id) + gitlab_com_tree_links(namespace, project, submodule_item_id, old_submodule_item_id) else - [sanitize_submodule_url(url), nil] + [sanitize_submodule_url(url), nil, nil] end else - [sanitize_submodule_url(url), nil] + [sanitize_submodule_url(url), nil, nil] end end @@ -79,22 +82,30 @@ module SubmoduleHelper url.start_with?('../', './') end - def gitlab_com_tree_links(namespace, project, commit) + def gitlab_com_tree_links(namespace, project, commit, old_commit) base = ['https://gitlab.com/', namespace, '/', project].join('') - [base, [base, '/-/tree/', commit].join('')] + [ + base, + [base, '/-/tree/', commit].join(''), + ([base, '/-/compare/', old_commit, '...', commit].join('') if old_commit) + ] end def gist_github_com_tree_links(namespace, project, commit) base = ['https://gist.github.com/', namespace, '/', project].join('') - [base, [base, commit].join('/')] + [base, [base, commit].join('/'), nil] end - def github_com_tree_links(namespace, project, commit) + def github_com_tree_links(namespace, project, commit, old_commit) base = ['https://github.com/', namespace, '/', project].join('') - [base, [base, '/tree/', commit].join('')] + [ + base, + [base, '/tree/', commit].join(''), + ([base, '/compare/', old_commit, '...', commit].join('') if old_commit) + ] end - def relative_self_links(relative_path, commit, project) + def relative_self_links(relative_path, commit, old_commit, project) relative_path = relative_path.rstrip absolute_project_path = "/" + project.full_path @@ -107,7 +118,7 @@ module SubmoduleHelper target_namespace_path = File.dirname(submodule_project_path) if target_namespace_path == '/' || target_namespace_path.start_with?(absolute_project_path) - return [nil, nil] + return [nil, nil, nil] end target_namespace_path.sub!(%r{^/}, '') @@ -116,10 +127,11 @@ module SubmoduleHelper begin [ url_helpers.namespace_project_path(target_namespace_path, submodule_base), - url_helpers.namespace_project_tree_path(target_namespace_path, submodule_base, commit) + url_helpers.namespace_project_tree_path(target_namespace_path, submodule_base, commit), + (url_helpers.namespace_project_compare_path(target_namespace_path, submodule_base, to: commit, from: old_commit) if old_commit) ] rescue ActionController::UrlGenerationError - [nil, nil] + [nil, nil, nil] end end diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb index f44a5b804fa..79c448e5985 100644 --- a/app/models/ci/build_trace_chunk.rb +++ b/app/models/ci/build_trace_chunk.rb @@ -108,6 +108,14 @@ module Ci Ci::BuildTraceChunkFlushWorker.perform_async(id) end + def persisted? + !redis? + end + + def live? + redis? + end + private def get_data @@ -170,14 +178,6 @@ module Ci save! if changed? end - def persisted? - !redis? - end - - def live? - redis? - end - def full? size == CHUNK_SIZE end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ca2ad7672bc..34589002568 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -262,7 +262,7 @@ module Ci scope :internal, -> { where(source: internal_sources) } scope :no_child, -> { where.not(source: :parent_pipeline) } - scope :ci_sources, -> { where(config_source: Enums::Ci::Pipeline.ci_config_sources_values) } + scope :ci_sources, -> { where(source: Enums::Ci::Pipeline.ci_sources.values) } scope :for_user, -> (user) { where(user: user) } scope :for_sha, -> (sha) { where(sha: sha) } scope :for_source_sha, -> (source_sha) { where(source_sha: source_sha) } @@ -1033,7 +1033,11 @@ module Ci end def cacheable? - Enums::Ci::Pipeline.ci_config_sources.key?(config_source.to_sym) + !dangling? + end + + def dangling? + Enums::Ci::Pipeline.dangling_sources.key?(source.to_sym) end def source_ref_path diff --git a/app/models/concerns/enums/ci/pipeline.rb b/app/models/concerns/enums/ci/pipeline.rb index c1086fcc617..f1bc43a12d8 100644 --- a/app/models/concerns/enums/ci/pipeline.rb +++ b/app/models/concerns/enums/ci/pipeline.rb @@ -36,6 +36,23 @@ module Enums } end + # Dangling sources are those events that generate pipelines for which + # we don't want to directly affect the ref CI status. + # - when a webide pipeline fails it does not change the ref CI status to failed + # - when a child pipeline (from parent_pipeline source) fails it affects its + # parent pipeline. It's up to the parent to affect the ref CI status + # - when an ondemand_dast_scan pipeline runs it is for testing purpose and should + # not affect the ref CI status. + def self.dangling_sources + sources.slice(:webide, :parent_pipeline, :ondemand_dast_scan) + end + + # CI sources are those pipeline events that affect the CI status of the ref + # they run for. By definition it excludes dangling pipelines. + def self.ci_sources + sources.except(*dangling_sources.keys) + end + # Returns the `Hash` to use for creating the `config_sources` enum for # `Ci::Pipeline`. def self.config_sources @@ -50,24 +67,6 @@ module Enums parameter_source: 7 } end - - def self.ci_config_sources - config_sources.slice( - :unknown_source, - :repository_source, - :auto_devops_source, - :remote_source, - :external_project_source - ) - end - - def self.ci_config_sources_values - ci_config_sources.values - end - - def self.non_ci_config_source_values - config_sources.values - ci_config_sources.values - end end end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 78161a3b446..64bb22e886e 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -177,6 +177,10 @@ module Issuable assignees.count > 1 end + def allows_reviewers? + false + end + def supports_time_tracking? is_a?(TimeTrackable) && !incident? end @@ -185,6 +189,12 @@ module Issuable is_a?(Issue) && super end + def severity + return IssuableSeverity::DEFAULT unless incident? + + issuable_severity&.severity || IssuableSeverity::DEFAULT + end + private def description_max_length_for_new_records_is_valid diff --git a/app/models/issuable_severity.rb b/app/models/issuable_severity.rb index cd3e967946b..d68b3dc48ee 100644 --- a/app/models/issuable_severity.rb +++ b/app/models/issuable_severity.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class IssuableSeverity < ApplicationRecord + DEFAULT = 'unknown' + belongs_to :issue validates :issue, presence: true, uniqueness: true diff --git a/app/models/issue.rb b/app/models/issue.rb index 93fc3325b38..b9c8c3cf9fe 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -60,6 +60,7 @@ class Issue < ApplicationRecord end end + has_one :issuable_severity has_one :sentry_issue has_one :alert_management_alert, class_name: 'AlertManagement::Alert' has_and_belongs_to_many :self_managed_prometheus_alert_events, join_table: :issues_self_managed_prometheus_alert_events # rubocop: disable Rails/HasAndBelongsToMany diff --git a/app/models/lfs_objects_project.rb b/app/models/lfs_objects_project.rb index 674294f0916..e5632ff2842 100644 --- a/app/models/lfs_objects_project.rb +++ b/app/models/lfs_objects_project.rb @@ -19,6 +19,7 @@ class LfsObjectsProject < ApplicationRecord } scope :project_id_in, ->(ids) { where(project_id: ids) } + scope :lfs_object_in, -> (lfs_objects) { where(lfs_object: lfs_objects) } private diff --git a/app/models/members_preloader.rb b/app/models/members_preloader.rb index 6da8d5f3161..88db7f63bd9 100644 --- a/app/models/members_preloader.rb +++ b/app/models/members_preloader.rb @@ -12,6 +12,7 @@ class MembersPreloader ActiveRecord::Associations::Preloader.new.preload(members, :source) ActiveRecord::Associations::Preloader.new.preload(members.map(&:user), :status) ActiveRecord::Associations::Preloader.new.preload(members.map(&:user), :u2f_registrations) + ActiveRecord::Associations::Preloader.new.preload(members.map(&:user), :webauthn_registrations) end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 88062240fb3..6174fd139b3 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -955,8 +955,9 @@ class MergeRequest < ApplicationRecord self.class.wip_title(self.title) end - def mergeable?(skip_ci_check: false) - return false unless mergeable_state?(skip_ci_check: skip_ci_check) + def mergeable?(skip_ci_check: false, skip_discussions_check: false) + return false unless mergeable_state?(skip_ci_check: skip_ci_check, + skip_discussions_check: skip_discussions_check) check_mergeability @@ -1658,6 +1659,10 @@ class MergeRequest < ApplicationRecord end end + def allows_reviewers? + Feature.enabled?(:merge_request_reviewers, project) + end + private def with_rebase_lock diff --git a/app/models/project.rb b/app/models/project.rb index 8e781ffe55d..a40f3b35add 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -280,10 +280,9 @@ class Project < ApplicationRecord # The relation :all_pipelines is intended to be used when we want to get the # whole list of pipelines associated to the project has_many :all_pipelines, class_name: 'Ci::Pipeline', inverse_of: :project - # The relation :ci_pipelines is intended to be used when we want to get only - # those pipeline which are directly related to CI. There are - # other pipelines, like webide ones, that we won't retrieve - # if we use this relation. + # The relation :ci_pipelines includes all those that directly contribute to the + # latest status of a ref. This does not include dangling pipelines such as those + # from webide, child pipelines, etc. has_many :ci_pipelines, -> { ci_sources }, class_name: 'Ci::Pipeline', @@ -2709,9 +2708,11 @@ class Project < ApplicationRecord end def oids(objects, oids: []) - collection = oids.any? ? objects.where(oid: oids) : objects + objects = objects.where(oid: oids) if oids.any? - collection.pluck(:oid) + [].tap do |out| + objects.each_batch { |relation| out.concat(relation.pluck(:oid)) } + end end end diff --git a/app/models/user.rb b/app/models/user.rb index 061d958ea72..0b5c6b9a7ab 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -113,6 +113,7 @@ class User < ApplicationRecord has_many :personal_access_tokens, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :identities, dependent: :destroy, autosave: true # rubocop:disable Cop/ActiveRecordDependent has_many :u2f_registrations, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :webauthn_registrations has_many :chat_names, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_one :user_synced_attributes_metadata, autosave: true has_one :aws_role, class_name: 'Aws::Role' @@ -286,6 +287,7 @@ class User < ApplicationRecord delegate :path, to: :namespace, allow_nil: true, prefix: true delegate :job_title, :job_title=, to: :user_detail, allow_nil: true delegate :bio, :bio=, :bio_html, to: :user_detail, allow_nil: true + delegate :webauthn_xid, :webauthn_xid=, to: :user_detail, allow_nil: true accepts_nested_attributes_for :user_preference, update_only: true accepts_nested_attributes_for :user_detail, update_only: true @@ -434,14 +436,21 @@ class User < ApplicationRecord FROM u2f_registrations AS u2f WHERE u2f.user_id = users.id ) OR users.otp_required_for_login = ? + OR + EXISTS ( + SELECT * + FROM webauthn_registrations AS webauthn + WHERE webauthn.user_id = users.id + ) SQL where(with_u2f_registrations, true) end def self.without_two_factor - joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id") - .where("u2f.id IS NULL AND users.otp_required_for_login = ?", false) + joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id + LEFT OUTER JOIN webauthn_registrations AS webauthn ON webauthn.user_id = users.id") + .where("u2f.id IS NULL AND webauthn.id IS NULL AND users.otp_required_for_login = ?", false) end # @@ -754,11 +763,12 @@ class User < ApplicationRecord otp_backup_codes: nil ) self.u2f_registrations.destroy_all # rubocop: disable Cop/DestroyAll + self.webauthn_registrations.destroy_all # rubocop: disable Cop/DestroyAll end end def two_factor_enabled? - two_factor_otp_enabled? || two_factor_u2f_enabled? + two_factor_otp_enabled? || two_factor_webauthn_u2f_enabled? end def two_factor_otp_enabled? @@ -773,6 +783,16 @@ class User < ApplicationRecord end end + def two_factor_webauthn_u2f_enabled? + two_factor_u2f_enabled? || two_factor_webauthn_enabled? + end + + def two_factor_webauthn_enabled? + return false unless Feature.enabled?(:webauthn) + + (webauthn_registrations.loaded? && webauthn_registrations.any?) || (!webauthn_registrations.loaded? && webauthn_registrations.exists?) + end + def namespace_move_dir_allowed if namespace&.any_project_has_container_registry_tags? errors.add(:username, _('cannot be changed if a personal project has container registry tags.')) diff --git a/app/serializers/diff_file_base_entity.rb b/app/serializers/diff_file_base_entity.rb index 2af14f1eb82..9f27191c3c8 100644 --- a/app/serializers/diff_file_base_entity.rb +++ b/app/serializers/diff_file_base_entity.rb @@ -12,11 +12,23 @@ class DiffFileBaseEntity < Grape::Entity expose :submodule?, as: :submodule expose :submodule_link do |diff_file, options| - memoized_submodule_links(diff_file, options).first + memoized_submodule_links(diff_file, options)&.web end expose :submodule_tree_url do |diff_file| - memoized_submodule_links(diff_file, options).last + memoized_submodule_links(diff_file, options)&.tree + end + + expose :submodule_compare do |diff_file| + url = memoized_submodule_links(diff_file, options)&.compare + + next unless url + + { + url: url, + old_sha: diff_file.old_blob&.id, + new_sha: diff_file.blob&.id + } end expose :edit_path, if: -> (_, options) { options[:merge_request] } do |diff_file| @@ -96,11 +108,9 @@ class DiffFileBaseEntity < Grape::Entity def memoized_submodule_links(diff_file, options) strong_memoize(:submodule_links) do - if diff_file.submodule? - options[:submodule_links].for(diff_file.blob, diff_file.content_sha) - else - [] - end + next unless diff_file.submodule? + + options[:submodule_links].for(diff_file.blob, diff_file.content_sha, diff_file) end end diff --git a/app/serializers/merge_request_basic_entity.rb b/app/serializers/merge_request_basic_entity.rb index 82baf4a4a78..ef1177e9967 100644 --- a/app/serializers/merge_request_basic_entity.rb +++ b/app/serializers/merge_request_basic_entity.rb @@ -9,6 +9,7 @@ class MergeRequestBasicEntity < Grape::Entity expose :milestone, using: API::Entities::Milestone expose :labels, using: LabelEntity expose :assignees, using: API::Entities::UserBasic + expose :reviewers, if: -> (m) { m.allows_reviewers? }, using: API::Entities::UserBasic expose :task_status, :task_status_short expose :lock_version, :lock_version end diff --git a/app/serializers/merge_request_reviewer_entity.rb b/app/serializers/merge_request_reviewer_entity.rb new file mode 100644 index 00000000000..fefd116014f --- /dev/null +++ b/app/serializers/merge_request_reviewer_entity.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class MergeRequestReviewerEntity < ::API::Entities::UserBasic + expose :can_merge do |reviewer, options| + options[:merge_request]&.can_be_merged_by?(reviewer) + end +end + +MergeRequestReviewerEntity.prepend_if_ee('EE::MergeRequestReviewerEntity') diff --git a/app/serializers/merge_request_sidebar_extras_entity.rb b/app/serializers/merge_request_sidebar_extras_entity.rb index 7276509c363..9db8e52abef 100644 --- a/app/serializers/merge_request_sidebar_extras_entity.rb +++ b/app/serializers/merge_request_sidebar_extras_entity.rb @@ -4,4 +4,8 @@ class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity expose :assignees do |merge_request| MergeRequestAssigneeEntity.represent(merge_request.assignees, merge_request: merge_request) end + + expose :reviewers, if: -> (m) { m.allows_reviewers? } do |merge_request| + MergeRequestReviewerEntity.represent(merge_request.reviewers, merge_request: merge_request) + end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 65a73dadc2e..51c161a5e60 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -46,7 +46,7 @@ class IssuableBaseService < BaseService params[:assignee_ids] = params[:assignee_ids].first(1) end - assignee_ids = params[:assignee_ids].select { |assignee_id| assignee_can_read?(issuable, assignee_id) } + assignee_ids = params[:assignee_ids].select { |assignee_id| user_can_read?(issuable, assignee_id) } if params[:assignee_ids].map(&:to_s) == [IssuableFinder::Params::NONE] params[:assignee_ids] = [] @@ -57,15 +57,15 @@ class IssuableBaseService < BaseService end end - def assignee_can_read?(issuable, assignee_id) - new_assignee = User.find_by_id(assignee_id) + def user_can_read?(issuable, user_id) + user = User.find_by_id(user_id) - return false unless new_assignee + return false unless user ability_name = :"read_#{issuable.to_ability_name}" resource = issuable.persisted? ? issuable : project - can?(new_assignee, ability_name, resource) + can?(user, ability_name, resource) end def filter_milestone diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 3dc9a5cd227..abc3f99797d 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -97,6 +97,28 @@ module MergeRequests unless merge_request.can_allow_collaboration?(current_user) params.delete(:allow_collaboration) end + + filter_reviewer(merge_request) + end + + def filter_reviewer(merge_request) + return if params[:reviewer_ids].blank? + + unless can_admin_issuable?(merge_request) && merge_request.allows_reviewers? + params.delete(:reviewer_ids) + + return + end + + reviewer_ids = params[:reviewer_ids].select { |reviewer_id| user_can_read?(merge_request, reviewer_id) } + + if params[:reviewer_ids].map(&:to_s) == [IssuableFinder::Params::NONE] + params[:reviewer_ids] = [] + elsif reviewer_ids.any? + params[:reviewer_ids] = reviewer_ids + else + params.delete(:reviewer_ids) + end end def merge_request_metrics_service(merge_request) diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 961a7cb1ef6..437e87dadf7 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -10,13 +10,14 @@ module MergeRequests class MergeService < MergeRequests::MergeBaseService delegate :merge_jid, :state, to: :@merge_request - def execute(merge_request) + def execute(merge_request, options = {}) if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService) FfMergeService.new(project, current_user, params).execute(merge_request) return end @merge_request = merge_request + @options = options validate! @@ -55,7 +56,7 @@ module MergeRequests error = if @merge_request.should_be_rebased? 'Only fast-forward merge is allowed for your project. Please update your source branch' - elsif !@merge_request.mergeable? + elsif !@merge_request.mergeable?(skip_discussions_check: @options[:skip_discussions_check]) 'Merge request is not mergeable' elsif !@merge_request.squash && project.squash_always? 'This project requires squashing commits when merge requests are accepted.' diff --git a/app/services/webauthn/authenticate_service.rb b/app/services/webauthn/authenticate_service.rb new file mode 100644 index 00000000000..a4513c62c2d --- /dev/null +++ b/app/services/webauthn/authenticate_service.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module Webauthn + class AuthenticateService < BaseService + def initialize(user, device_response, challenge) + @user = user + @device_response = device_response + @challenge = challenge + end + + def execute + parsed_device_response = Gitlab::Json.parse(@device_response) + + # appid is set for legacy U2F devices, will be used in a future iteration + # rp_id = @app_id + # unless parsed_device_response['clientExtensionResults'] && parsed_device_response['clientExtensionResults']['appid'] + # rp_id = URI(@app_id).host + # end + + webauthn_credential = WebAuthn::Credential.from_get(parsed_device_response) + encoded_raw_id = Base64.strict_encode64(webauthn_credential.raw_id) + stored_webauthn_credential = @user.webauthn_registrations.find_by_credential_xid(encoded_raw_id) + + encoder = WebAuthn.configuration.encoder + + if stored_webauthn_credential && + validate_webauthn_credential(webauthn_credential) && + verify_webauthn_credential(webauthn_credential, stored_webauthn_credential, @challenge, encoder) + + stored_webauthn_credential.update!(counter: webauthn_credential.sign_count) + return true + end + + false + rescue JSON::ParserError, WebAuthn::SignCountVerificationError, WebAuthn::Error + false + end + + ## + # Validates that webauthn_credential is syntactically valid + # + # duplicated from WebAuthn::PublicKeyCredential#verify + # which can't be used here as we need to call WebAuthn::AuthenticatorAssertionResponse#verify instead + # (which is done in #verify_webauthn_credential) + def validate_webauthn_credential(webauthn_credential) + webauthn_credential.type == WebAuthn::TYPE_PUBLIC_KEY && + webauthn_credential.raw_id && webauthn_credential.id && + webauthn_credential.raw_id == WebAuthn.standard_encoder.decode(webauthn_credential.id) + end + + ## + # Verifies that webauthn_credential matches stored_credential with the given challenge + # + def verify_webauthn_credential(webauthn_credential, stored_credential, challenge, encoder) + webauthn_credential.response.verify( + encoder.decode(challenge), + public_key: encoder.decode(stored_credential.public_key), + sign_count: stored_credential.counter) + end + end +end diff --git a/app/services/webauthn/register_service.rb b/app/services/webauthn/register_service.rb new file mode 100644 index 00000000000..21be22027a8 --- /dev/null +++ b/app/services/webauthn/register_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Webauthn + class RegisterService < BaseService + def initialize(user, params, challenge) + @user = user + @params = params + @challenge = challenge + end + + def execute + registration = WebauthnRegistration.new + + begin + webauthn_credential = WebAuthn::Credential.from_create(Gitlab::Json.parse(@params[:device_response])) + webauthn_credential.verify(@challenge) + + registration.update( + credential_xid: Base64.strict_encode64(webauthn_credential.raw_id), + public_key: webauthn_credential.public_key, + counter: webauthn_credential.sign_count, + name: @params[:name], + user: @user + ) + rescue JSON::ParserError + registration.errors.add(:base, _('Your WebAuthn device did not send a valid JSON response.')) + rescue WebAuthn::Error => e + registration.errors.add(:base, e.message) + end + + registration + end + end +end diff --git a/app/views/admin/sessions/_two_factor_otp.html.haml b/app/views/admin/sessions/_two_factor_otp.html.haml index 9d4acbf1b99..8d5588de06e 100644 --- a/app/views/admin/sessions/_two_factor_otp.html.haml +++ b/app/views/admin/sessions/_two_factor_otp.html.haml @@ -1,4 +1,4 @@ -= form_tag(admin_session_path, { method: :post, class: "edit_user gl-show-field-errors js-2fa-form #{'hidden' if current_user.two_factor_u2f_enabled?}" }) do += form_tag(admin_session_path, { method: :post, class: "edit_user gl-show-field-errors js-2fa-form #{'hidden' if current_user.two_factor_webauthn_u2f_enabled?}" }) do .form-group = label_tag :user_otp_attempt, _('Two-Factor Authentication code') = text_field_tag 'user[otp_attempt]', nil, class: 'form-control', required: true, autofocus: true, autocomplete: 'off', title: _('This field is required.') diff --git a/app/views/admin/sessions/two_factor.html.haml b/app/views/admin/sessions/two_factor.html.haml index 746d57dbad1..531ab206157 100644 --- a/app/views/admin/sessions/two_factor.html.haml +++ b/app/views/admin/sessions/two_factor.html.haml @@ -11,5 +11,5 @@ .login-body - if current_user.two_factor_otp_enabled? = render 'admin/sessions/two_factor_otp' - - if current_user.two_factor_u2f_enabled? - = render 'u2f/authenticate', render_remember_me: false, target_path: admin_session_path + - if current_user.two_factor_webauthn_u2f_enabled? + = render 'authentication/authenticate', render_remember_me: false, target_path: admin_session_path diff --git a/app/views/u2f/_authenticate.html.haml b/app/views/authentication/_authenticate.html.haml index 6658d70df8d..17e855dbddd 100644 --- a/app/views/u2f/_authenticate.html.haml +++ b/app/views/authentication/_authenticate.html.haml @@ -6,7 +6,7 @@ %script#js-authenticate-token-2fa-error{ type: "text/template" } %div - %p <%= error_message %> (#{_("error code:")} <%= error_code %>) + %p <%= error_message %> (<%= error_name %>) %a.btn.btn-block.btn-warning#js-token-2fa-try-again= _("Try again?") %script#js-authenticate-token-2fa-authenticated{ type: "text/template" } diff --git a/app/views/authentication/_register.html.haml b/app/views/authentication/_register.html.haml new file mode 100644 index 00000000000..55ab6220744 --- /dev/null +++ b/app/views/authentication/_register.html.haml @@ -0,0 +1,37 @@ +#js-register-token-2fa + +-# haml-lint:disable InlineJavaScript +%script#js-register-2fa-message{ type: "text/template" } + %p <%= message %> + +%script#js-register-token-2fa-setup{ type: "text/template" } + - if current_user.two_factor_otp_enabled? + .row.gl-mb-3 + .col-md-5 + %button#js-setup-token-2fa-device.btn.btn-info= _("Set up new device") + .col-md-7 + %p= _("Your device needs to be set up. Plug it in (if needed) and click the button on the left.") + - else + .row.gl-mb-3 + .col-md-4 + %button#js-setup-token-2fa-device.btn.btn-info.btn-block{ disabled: true }= _("Set up new device") + .col-md-8 + %p= _("You need to register a two-factor authentication app before you can set up a device.") + +%script#js-register-token-2fa-error{ type: "text/template" } + %div + %p + %span <%= error_message %> (<%= error_name %>) + %a.btn.btn-warning#js-token-2fa-try-again= _("Try again?") + +%script#js-register-token-2fa-registered{ type: "text/template" } + .row.gl-mb-3 + .col-md-12 + %p= _("Your device was successfully set up! Give it a name and register it with the GitLab server.") + = form_tag(target_path, method: :post) do + .row.gl-mb-3 + .col-md-3 + = text_field_tag 'device_registration[name]', nil, class: 'form-control', placeholder: _("Pick a name") + .col-md-3 + = hidden_field_tag 'device_registration[device_response]', nil, class: 'form-control', required: true, id: "js-device-response" + = submit_tag _("Register device"), class: "btn btn-success" diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml index 115ebc94238..8e05488c091 100644 --- a/app/views/devise/sessions/two_factor.html.haml +++ b/app/views/devise/sessions/two_factor.html.haml @@ -3,7 +3,7 @@ .login-box .login-body - if @user.two_factor_otp_enabled? - = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post, html: { class: "edit_user gl-show-field-errors js-2fa-form #{'hidden' if @user.two_factor_u2f_enabled?}" }) do |f| + = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post, html: { class: "edit_user gl-show-field-errors js-2fa-form #{'hidden' if @user.two_factor_webauthn_u2f_enabled?}" }) do |f| - resource_params = params[resource_name].presence || params = f.hidden_field :remember_me, value: resource_params.fetch(:remember_me, 0) %div @@ -12,6 +12,5 @@ %p.form-text.text-muted.hint Enter the code from the two-factor app on your mobile device. If you've lost your device, you may enter one of your recovery codes. .prepend-top-20 = f.submit "Verify code", class: "btn btn-success", data: { qa_selector: 'verify_code_button' } - - - if @user.two_factor_u2f_enabled? - = render "u2f/authenticate", params: params, resource: resource, resource_name: resource_name, render_remember_me: true, target_path: new_user_session_path + - if @user.two_factor_webauthn_u2f_enabled? + = render "authentication/authenticate", params: params, resource: resource, resource_name: resource_name, render_remember_me: true, target_path: new_user_session_path diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml index bce43b16d27..8013d0bec8f 100644 --- a/app/views/profiles/two_factor_auths/show.html.haml +++ b/app/views/profiles/two_factor_auths/show.html.haml @@ -1,6 +1,7 @@ - page_title _('Two-Factor Authentication'), _('Account') - add_to_breadcrumbs(_('Two-Factor Authentication'), profile_account_path) - @content_class = "limit-container-width" unless fluid_layout +- webauthn_enabled = Feature.enabled?(:webauthn) .js-two-factor-auth{ 'data-two-factor-skippable' => "#{two_factor_skippable?}", 'data-two_factor_skip_url' => skip_profile_two_factor_auth_path } .row.gl-mt-3 @@ -18,7 +19,7 @@ %div = link_to _('Disable two-factor authentication'), profile_two_factor_auth_path, method: :delete, - data: { confirm: _('Are you sure? This will invalidate your registered applications and U2F devices.') }, + data: { confirm: webauthn_enabled ? _('Are you sure? This will invalidate your registered applications and U2F / WebAuthn devices.') : _('Are you sure? This will invalidate your registered applications and U2F devices.') }, class: 'btn btn-danger gl-mr-3' = form_tag codes_profile_two_factor_auth_path, {style: 'display: inline-block', method: :post} do |f| = submit_tag _('Regenerate recovery codes'), class: 'btn' @@ -58,22 +59,35 @@ .row.gl-mt-3 .col-lg-4 %h4.gl-mt-0 - = _('Register Universal Two-Factor (U2F) Device') + - if webauthn_enabled + = _('Register WebAuthn Device') + - else + = _('Register Universal Two-Factor (U2F) Device') %p = _('Use a hardware device to add the second factor of authentication.') %p - = _("As U2F devices are only supported by a few browsers, we require that you set up a two-factor authentication app before a U2F device. That way you'll always be able to log in - even when you're using an unsupported browser.") + - if webauthn_enabled + = _("As WebAuthn devices are only supported by a few browsers, we require that you set up a two-factor authentication app before a WebAuthn device. That way you'll always be able to log in - even when you're using an unsupported browser.") + - else + = _("As U2F devices are only supported by a few browsers, we require that you set up a two-factor authentication app before a U2F device. That way you'll always be able to log in - even when you're using an unsupported browser.") .col-lg-8 - - if @u2f_registration.errors.present? - = form_errors(@u2f_registration) - = render "u2f/register" + - registration = webauthn_enabled ? @webauthn_registration : @u2f_registration + - if registration.errors.present? + = form_errors(registration) + - if webauthn_enabled + = render "authentication/register", target_path: create_webauthn_profile_two_factor_auth_path + - else + = render "authentication/register", target_path: create_u2f_profile_two_factor_auth_path %hr %h5 - = _('U2F Devices (%{length})') % { length: @u2f_registrations.length } + - if webauthn_enabled + = _('WebAuthn Devices (%{length})') % { length: @registrations.length } + - else + = _('U2F Devices (%{length})') % { length: @registrations.length } - - if @u2f_registrations.present? + - if @registrations.present? .table-responsive %table.table.table-bordered.u2f-registrations %colgroup @@ -86,12 +100,15 @@ %th= s_('2FADevice|Registered On') %th %tbody - - @u2f_registrations.each do |registration| + - @registrations.each do |registration| %tr - %td= registration.name.presence || html_escape_once(_("<no name set>")).html_safe - %td= registration.created_at.to_date.to_s(:medium) - %td= link_to _('Delete'), profile_u2f_registration_path(registration), method: :delete, class: "btn btn-danger float-right", data: { confirm: _('Are you sure you want to delete this device? This action cannot be undone.') } + %td= registration[:name].presence || html_escape_once(_("<no name set>")).html_safe + %td= registration[:created_at].to_date.to_s(:medium) + %td= link_to _('Delete'), registration[:delete_path], method: :delete, class: "btn btn-danger float-right", data: { confirm: _('Are you sure you want to delete this device? This action cannot be undone.') } - else .settings-message.text-center - = _("You don't have any U2F devices registered yet.") + - if webauthn_enabled + = _("You don't have any WebAuthn devices registered yet.") + - else + = _("You don't have any U2F devices registered yet.") diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index bd023e0442c..187ebcb739c 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -9,6 +9,10 @@ .file-header-content = render "projects/diffs/file_header", diff_file: diff_file, url: "##{file_hash}" + - if diff_file.submodule? + .file-actions.d-none.d-sm-block + = submodule_diff_compare_link(diff_file) + - unless diff_file.submodule? - blob = diff_file.blob .file-actions.d-none.d-sm-block diff --git a/app/views/u2f/_register.html.haml b/app/views/u2f/_register.html.haml deleted file mode 100644 index a83b55379da..00000000000 --- a/app/views/u2f/_register.html.haml +++ /dev/null @@ -1,40 +0,0 @@ -#js-register-u2f - --# haml-lint:disable InlineJavaScript -%script#js-register-u2f-not-supported{ type: "text/template" } - %p= _("Your browser doesn't support U2F. Please use Google Chrome desktop (version 41 or newer).") - -%script#js-register-u2f-setup{ type: "text/template" } - - if current_user.two_factor_otp_enabled? - .row.gl-mb-3 - .col-md-4 - %button#js-setup-u2f-device.btn.btn-info.btn-block= _("Set up new U2F device") - .col-md-8 - %p= _("Your U2F device needs to be set up. Plug it in (if not already) and click the button on the left.") - - else - .row.gl-mb-3 - .col-md-4 - %button#js-setup-u2f-device.btn.btn-info.btn-block{ disabled: true }= _("Set up new U2F device") - .col-md-8 - %p= _("You need to register a two-factor authentication app before you can set up a U2F device.") - -%script#js-register-u2f-in-progress{ type: "text/template" } - %p= _("Trying to communicate with your device. Plug it in (if you haven't already) and press the button on the device now.") - -%script#js-register-u2f-error{ type: "text/template" } - %div - %p - %span <%= error_message %> (#{_("error code:")} <%= error_code %>) - %a.btn.btn-warning#js-token-2fa-try-again= _("Try again?") - -%script#js-register-u2f-registered{ type: "text/template" } - .row.gl-mb-3 - .col-md-12 - %p= _("Your device was successfully set up! Give it a name and register it with the GitLab server.") - = form_tag(create_u2f_profile_two_factor_auth_path, method: :post) do - .row.gl-mb-3 - .col-md-3 - = text_field_tag 'u2f_registration[name]', nil, class: 'form-control', placeholder: _("Pick a name") - .col-md-3 - = hidden_field_tag 'u2f_registration[device_response]', nil, class: 'form-control', required: true, id: "js-device-response" - = submit_tag _("Register U2F device"), class: "btn btn-success" diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 86f20275c3b..4f5c6882180 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -897,7 +897,7 @@ :urgency: :low :resource_boundary: :unknown :weight: 1 - :idempotent: + :idempotent: true :tags: [] - :name: pipeline_background:ci_daily_build_group_report_results :feature_category: :continuous_integration diff --git a/app/workers/ci/build_trace_chunk_flush_worker.rb b/app/workers/ci/build_trace_chunk_flush_worker.rb index 43a19f8a6d6..2908c7c2d0b 100644 --- a/app/workers/ci/build_trace_chunk_flush_worker.rb +++ b/app/workers/ci/build_trace_chunk_flush_worker.rb @@ -1,10 +1,12 @@ # frozen_string_literal: true module Ci - class BuildTraceChunkFlushWorker # rubocop:disable Scalability/IdempotentWorker + class BuildTraceChunkFlushWorker include ApplicationWorker include PipelineBackgroundQueue + idempotent! + # rubocop: disable CodeReuse/ActiveRecord def perform(chunk_id) ::Ci::BuildTraceChunk.find_by(id: chunk_id).try do |chunk| diff --git a/changelogs/unreleased/1449-add-created-at-index-on-audit-events.yml b/changelogs/unreleased/1449-add-created-at-index-on-audit-events.yml new file mode 100644 index 00000000000..c4dacb3cb30 --- /dev/null +++ b/changelogs/unreleased/1449-add-created-at-index-on-audit-events.yml @@ -0,0 +1,5 @@ +--- +title: Optimise index on audit events for CSV export +merge_request: 41266 +author: +type: added diff --git a/changelogs/unreleased/220916-new-comment-not-drop-merge-train.yml b/changelogs/unreleased/220916-new-comment-not-drop-merge-train.yml new file mode 100644 index 00000000000..707b5d541f1 --- /dev/null +++ b/changelogs/unreleased/220916-new-comment-not-drop-merge-train.yml @@ -0,0 +1,5 @@ +--- +title: Prevent MRs to be dropped from Merge Trains for open discussions +merge_request: 39957 +author: +type: changed diff --git a/changelogs/unreleased/22506-webauthn-step-1.yml b/changelogs/unreleased/22506-webauthn-step-1.yml new file mode 100644 index 00000000000..3be52ec3ede --- /dev/null +++ b/changelogs/unreleased/22506-webauthn-step-1.yml @@ -0,0 +1,5 @@ +--- +title: WebAuthn support (behind feature flag) +merge_request: 26692 +author: Jan Beckmann +type: added diff --git a/changelogs/unreleased/233652-replace-bootstrap-alerts-in-ee-app-views-groups-push_rules-edit-ht.yml b/changelogs/unreleased/233652-replace-bootstrap-alerts-in-ee-app-views-groups-push_rules-edit-ht.yml new file mode 100644 index 00000000000..2ce9223cb9d --- /dev/null +++ b/changelogs/unreleased/233652-replace-bootstrap-alerts-in-ee-app-views-groups-push_rules-edit-ht.yml @@ -0,0 +1,5 @@ +--- +title: Replace bootstrap alerts in ee/app/views/groups/push_rules/edit.html.haml +merge_request: 41069 +author: Jacopo Beschi @jacopo-beschi +type: changed diff --git a/changelogs/unreleased/238568-add-incident-severity-to-issue-graphql-query.yml b/changelogs/unreleased/238568-add-incident-severity-to-issue-graphql-query.yml new file mode 100644 index 00000000000..16dab226d09 --- /dev/null +++ b/changelogs/unreleased/238568-add-incident-severity-to-issue-graphql-query.yml @@ -0,0 +1,5 @@ +--- +title: Exposes Incident's severity via GraphQL +merge_request: 40945 +author: +type: added diff --git a/changelogs/unreleased/244848-fix-lfs-cleanup-data-loss-bug.yml b/changelogs/unreleased/244848-fix-lfs-cleanup-data-loss-bug.yml new file mode 100644 index 00000000000..cd1e378e558 --- /dev/null +++ b/changelogs/unreleased/244848-fix-lfs-cleanup-data-loss-bug.yml @@ -0,0 +1,5 @@ +--- +title: Correctly preserve LFS objects in design or wiki repositories +merge_request: 41352 +author: +type: fixed diff --git a/changelogs/unreleased/compare-link-git-submodule-update.yml b/changelogs/unreleased/compare-link-git-submodule-update.yml new file mode 100644 index 00000000000..8632c095c5f --- /dev/null +++ b/changelogs/unreleased/compare-link-git-submodule-update.yml @@ -0,0 +1,5 @@ +--- +title: Add link to compare changes intoduced by a git submodule update +merge_request: 37740 +author: Daniel Seemer @Phaiax +type: added diff --git a/changelogs/unreleased/feature-issue_241962.yml b/changelogs/unreleased/feature-issue_241962.yml new file mode 100644 index 00000000000..1f95ecdc432 --- /dev/null +++ b/changelogs/unreleased/feature-issue_241962.yml @@ -0,0 +1,5 @@ +--- +title: Replace v-html to v-safe-html directive +merge_request: 41305 +author: Kazuya Kojima +type: other diff --git a/config/feature_flags/development/merge_request_reviewers.yml b/config/feature_flags/development/merge_request_reviewers.yml new file mode 100644 index 00000000000..2180662b9df --- /dev/null +++ b/config/feature_flags/development/merge_request_reviewers.yml @@ -0,0 +1,7 @@ +--- +name: merge_request_reviewers +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40488 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/245190 +group: group::source code +type: development +default_enabled: false diff --git a/config/initializers/webauthn.rb b/config/initializers/webauthn.rb new file mode 100644 index 00000000000..8dc5dfd56ed --- /dev/null +++ b/config/initializers/webauthn.rb @@ -0,0 +1,35 @@ +WebAuthn.configure do |config| + # This value needs to match `window.location.origin` evaluated by + # the User Agent during registration and authentication ceremonies. + config.origin = Settings.gitlab['base_url'] + + # Relying Party name for display purposes + # config.rp_name = "Example Inc." + + # Optionally configure a client timeout hint, in milliseconds. + # This hint specifies how long the browser should wait for any + # interaction with the user. + # This hint may be overridden by the browser. + # https://www.w3.org/TR/webauthn/#dom-publickeycredentialcreationoptions-timeout + # config.credential_options_timeout = 120_000 + + # You can optionally specify a different Relying Party ID + # (https://www.w3.org/TR/webauthn/#relying-party-identifier) + # if it differs from the default one. + # + # In this case the default would be "auth.example.com", but you can set it to + # the suffix "example.com" + # + # config.rp_id = "example.com" + + # Configure preferred binary-to-text encoding scheme. This should match the encoding scheme + # used in your client-side (user agent) code before sending the credential to the server. + # Supported values: `:base64url` (default), `:base64` or `false` to disable all encoding. + # + config.encoding = :base64 + + # Possible values: "ES256", "ES384", "ES512", "PS256", "PS384", "PS512", "RS256", "RS384", "RS512", "RS1" + # Default: ["ES256", "PS256", "RS256"] + # + # config.algorithms << "ES384" +end diff --git a/config/routes/profile.rb b/config/routes/profile.rb index 6126a3b593b..3eda53318e3 100644 --- a/config/routes/profile.rb +++ b/config/routes/profile.rb @@ -63,9 +63,11 @@ resource :profile, only: [:show, :update] do post :create_u2f post :codes patch :skip + post :create_webauthn end end resources :u2f_registrations, only: [:destroy] + resources :webauthn_registrations, only: [:destroy] end end diff --git a/db/post_migrate/20200903064431_add_created_at_index_to_audit_events.rb b/db/post_migrate/20200903064431_add_created_at_index_to_audit_events.rb new file mode 100644 index 00000000000..94dc2cb7adf --- /dev/null +++ b/db/post_migrate/20200903064431_add_created_at_index_to_audit_events.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddCreatedAtIndexToAuditEvents < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + INDEX_NAME = 'idx_audit_events_on_entity_id_desc_author_id_created_at' + OLD_INDEX_NAME = 'index_audit_events_on_entity_id_entity_type_id_desc_author_id' + + def up + add_concurrent_index(:audit_events, [:entity_id, :entity_type, :id, :author_id, :created_at], order: { id: :desc }, name: INDEX_NAME) + remove_concurrent_index_by_name(:audit_events, OLD_INDEX_NAME) + end + + def down + add_concurrent_index(:audit_events, [:entity_id, :entity_type, :id, :author_id], order: { id: :desc }, name: OLD_INDEX_NAME) + remove_concurrent_index_by_name(:audit_events, INDEX_NAME) + end +end diff --git a/db/schema_migrations/20200903064431 b/db/schema_migrations/20200903064431 new file mode 100644 index 00000000000..a3351912182 --- /dev/null +++ b/db/schema_migrations/20200903064431 @@ -0,0 +1 @@ +5c065dc7905fd1292e270d2248810d71fa71d6b6996e9d60c463a7eb36042881
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0539f45bb2b..3ef216a5667 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19075,6 +19075,8 @@ CREATE UNIQUE INDEX epic_user_mentions_on_epic_id_and_note_id_index ON public.ep CREATE UNIQUE INDEX epic_user_mentions_on_epic_id_index ON public.epic_user_mentions USING btree (epic_id) WHERE (note_id IS NULL); +CREATE INDEX idx_audit_events_on_entity_id_desc_author_id_created_at ON public.audit_events USING btree (entity_id, entity_type, id DESC, author_id, created_at); + CREATE INDEX idx_ci_pipelines_artifacts_locked ON public.ci_pipelines USING btree (ci_ref_id, id) WHERE (locked = 1); CREATE INDEX idx_container_scanning_findings ON public.vulnerability_occurrences USING btree (id) WHERE (report_type = 2); @@ -19267,8 +19269,6 @@ CREATE INDEX index_approvers_on_user_id ON public.approvers USING btree (user_id CREATE UNIQUE INDEX index_atlassian_identities_on_extern_uid ON public.atlassian_identities USING btree (extern_uid); -CREATE INDEX index_audit_events_on_entity_id_entity_type_id_desc_author_id ON public.audit_events USING btree (entity_id, entity_type, id DESC, author_id); - CREATE INDEX index_award_emoji_on_awardable_type_and_awardable_id ON public.award_emoji USING btree (awardable_type, awardable_id); CREATE INDEX index_award_emoji_on_user_id_and_name ON public.award_emoji USING btree (user_id, name); diff --git a/doc/administration/geo/disaster_recovery/index.md b/doc/administration/geo/disaster_recovery/index.md index 2d837ebb369..e9b566b7849 100644 --- a/doc/administration/geo/disaster_recovery/index.md +++ b/doc/administration/geo/disaster_recovery/index.md @@ -15,7 +15,9 @@ See [Geo current limitations](../replication/index.md#current-limitations) for m CAUTION: **Warning:** Disaster recovery for multi-secondary configurations is in **Alpha**. -For the latest updates, check the multi-secondary [Disaster Recovery epic](https://gitlab.com/groups/gitlab-org/-/epics/65). +For the latest updates, check the [Disaster Recovery epic for complete maturity](https://gitlab.com/groups/gitlab-org/-/epics/590). +Multi-secondary configurations require the complete re-synchronization and re-configuration of all non-promoted secondaries and +will cause downtime. ## Promoting a **secondary** Geo node in single-secondary configurations diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index e933f8a15da..f6ff13edabd 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -588,8 +588,9 @@ database encryption. Proceed with caution. 1. On the **GitLab server**, make the following changes to `/etc/gitlab/gitlab.rb`: ```ruby - gitlab_pages['enable'] = false pages_external_url "http://<pages_server_URL>" + gitlab_pages['enable'] = false + gitlab_rails['pages_enabled']=false gitlab_rails['pages_path'] = "/mnt/pages" ``` diff --git a/doc/administration/postgresql/external.md b/doc/administration/postgresql/external.md index e2cfb95ec48..632b68fb014 100644 --- a/doc/administration/postgresql/external.md +++ b/doc/administration/postgresql/external.md @@ -11,8 +11,7 @@ If you use a cloud-managed service, or provide your own PostgreSQL instance: 1. Set up PostgreSQL according to the [database requirements document](../../install/requirements.md#database). -1. Set up a `gitlab` username with a password of your choice. The `gitlab` user - needs privileges to create the `gitlabhq_production` database. +1. Set up a `gitlab` user with a password of your choice, create the `gitlabhq_production` database, and make the user an owner of the database. You can see an example of this setup in the [installation from source documentation](../../install/installation.md#6-database). 1. If you are using a cloud-managed service, you may need to grant additional roles to your `gitlab` user: - Amazon RDS requires the [`rds_superuser`](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Appendix.PostgreSQL.CommonDBATasks.html#Appendix.PostgreSQL.CommonDBATasks.Roles) role. diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 0fb210cf112..b66277e71e1 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -5680,6 +5680,11 @@ type EpicIssue implements Noteable { relativePosition: Int """ + Severity level of the incident + """ + severity: IssuableSeverity + + """ State of the issue """ state: IssueState! @@ -6928,6 +6933,11 @@ type Group { severity: [VulnerabilitySeverity!] """ + List vulnerabilities by sort order + """ + sort: VulnerabilitySort = severity_desc + + """ Filter vulnerabilities by state """ state: [VulnerabilityState!] @@ -7264,6 +7274,36 @@ type InstanceSecurityDashboard { } """ +Incident severity +""" +enum IssuableSeverity { + """ + Critical severity + """ + CRITICAL + + """ + High severity + """ + HIGH + + """ + Low severity + """ + LOW + + """ + Medium severity + """ + MEDIUM + + """ + Unknown severity + """ + UNKNOWN +} + +""" State of a GitLab issue or merge request """ enum IssuableState { @@ -7510,6 +7550,11 @@ type Issue implements Noteable { relativePosition: Int """ + Severity level of the incident + """ + severity: IssuableSeverity + + """ State of the issue """ state: IssueState! @@ -12690,6 +12735,11 @@ type Project { severity: [VulnerabilitySeverity!] """ + List vulnerabilities by sort order + """ + sort: VulnerabilitySort = severity_desc + + """ Filter vulnerabilities by state """ state: [VulnerabilityState!] @@ -13452,6 +13502,11 @@ type Query { severity: [VulnerabilitySeverity!] """ + List vulnerabilities by sort order + """ + sort: VulnerabilitySort = severity_desc + + """ Filter vulnerabilities by state """ state: [VulnerabilityState!] @@ -18390,6 +18445,21 @@ enum VulnerabilitySeverity { } """ +Vulnerability sort values +""" +enum VulnerabilitySort { + """ + Severity in ascending order + """ + severity_asc + + """ + Severity in descending order + """ + severity_desc +} + +""" The state of the vulnerability. """ enum VulnerabilityState { diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 01a6b0307a2..1a64472d636 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -15845,6 +15845,20 @@ "deprecationReason": null }, { + "name": "severity", + "description": "Severity level of the incident", + "args": [ + + ], + "type": { + "kind": "ENUM", + "name": "IssuableSeverity", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { "name": "state", "description": "State of the issue", "args": [ @@ -19048,6 +19062,16 @@ "defaultValue": null }, { + "name": "sort", + "description": "List vulnerabilities by sort order", + "type": { + "kind": "ENUM", + "name": "VulnerabilitySort", + "ofType": null + }, + "defaultValue": "severity_desc" + }, + { "name": "after", "description": "Returns the elements in the list that come after the specified cursor.", "type": { @@ -20077,6 +20101,47 @@ }, { "kind": "ENUM", + "name": "IssuableSeverity", + "description": "Incident severity", + "fields": null, + "inputFields": null, + "interfaces": null, + "enumValues": [ + { + "name": "UNKNOWN", + "description": "Unknown severity", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "LOW", + "description": "Low severity", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "MEDIUM", + "description": "Medium severity", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "HIGH", + "description": "High severity", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "CRITICAL", + "description": "Critical severity", + "isDeprecated": false, + "deprecationReason": null + } + ], + "possibleTypes": null + }, + { + "kind": "ENUM", "name": "IssuableState", "description": "State of a GitLab issue or merge request", "fields": null, @@ -20728,6 +20793,20 @@ "deprecationReason": null }, { + "name": "severity", + "description": "Severity level of the incident", + "args": [ + + ], + "type": { + "kind": "ENUM", + "name": "IssuableSeverity", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { "name": "state", "description": "State of the issue", "args": [ @@ -37133,6 +37212,16 @@ "defaultValue": null }, { + "name": "sort", + "description": "List vulnerabilities by sort order", + "type": { + "kind": "ENUM", + "name": "VulnerabilitySort", + "ofType": null + }, + "defaultValue": "severity_desc" + }, + { "name": "after", "description": "Returns the elements in the list that come after the specified cursor.", "type": { @@ -39462,6 +39551,16 @@ "defaultValue": null }, { + "name": "sort", + "description": "List vulnerabilities by sort order", + "type": { + "kind": "ENUM", + "name": "VulnerabilitySort", + "ofType": null + }, + "defaultValue": "severity_desc" + }, + { "name": "after", "description": "Returns the elements in the list that come after the specified cursor.", "type": { @@ -54036,6 +54135,29 @@ }, { "kind": "ENUM", + "name": "VulnerabilitySort", + "description": "Vulnerability sort values", + "fields": null, + "inputFields": null, + "interfaces": null, + "enumValues": [ + { + "name": "severity_desc", + "description": "Severity in descending order", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "severity_asc", + "description": "Severity in ascending order", + "isDeprecated": false, + "deprecationReason": null + } + ], + "possibleTypes": null + }, + { + "kind": "ENUM", "name": "VulnerabilityState", "description": "The state of the vulnerability.", "fields": null, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 829f18b1b18..2262e1b6a3e 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -950,6 +950,7 @@ Relationship between an epic and an issue | `reference` | String! | Internal reference of the issue. Returned in shortened format by default | | `relationPath` | String | URI path of the epic-issue relation | | `relativePosition` | Int | Relative position of the issue (used for positioning in epic tree and issue boards) | +| `severity` | IssuableSeverity | Severity level of the incident | | `state` | IssueState! | State of the issue | | `statusPagePublishedIncident` | Boolean | Indicates whether an issue is published to the status page | | `subscribed` | Boolean! | Indicates the currently logged in user is subscribed to the issue | @@ -1123,6 +1124,7 @@ Represents a Group Membership | `milestone` | Milestone | Milestone of the issue | | `reference` | String! | Internal reference of the issue. Returned in shortened format by default | | `relativePosition` | Int | Relative position of the issue (used for positioning in epic tree and issue boards) | +| `severity` | IssuableSeverity | Severity level of the incident | | `state` | IssueState! | State of the issue | | `statusPagePublishedIncident` | Boolean | Indicates whether an issue is published to the status page | | `subscribed` | Boolean! | Indicates the currently logged in user is subscribed to the issue | diff --git a/doc/api/issues.md b/doc/api/issues.md index 2a0d66a8b3e..b1ab5c96fcb 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -680,164 +680,7 @@ Example response: } ``` -Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) will also see -the `weight` parameter: - -```json -{ - "project_id" : 4, - "description" : "Omnis vero earum sunt corporis dolor et placeat.", - "weight": null, - ... -} -``` - -Users on GitLab [Ultimate](https://about.gitlab.com/pricing/) will additionally see -the `epic` property: - -```javascript -{ - "project_id" : 4, - "description" : "Omnis vero earum sunt corporis dolor et placeat.", - "epic": { - "epic_iid" : 5, //deprecated, use `iid` of the `epic` attribute - "epic": { - "id" : 42, - "iid" : 5, - "title": "My epic epic", - "url" : "/groups/h5bp/-/epics/5", - "group_id": 8 - }, - // ... -} -``` - -**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API. - -**Note**: The `closed_by` attribute was [introduced in GitLab 10.6](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/17042). This value will only be present for issues which were closed after GitLab 10.6 and when the user account that closed the issue still exists. - -**Note**: The `epic_iid` attribute is deprecated and [will be removed in version 5](https://gitlab.com/gitlab-org/gitlab/-/issues/35157). -Please use `iid` of the `epic` attribute instead. - -## Single Issue - -Only for administrators. Get a single issue. - -The preferred way to do this is by using [personal access tokens](../user/profile/personal_access_tokens.md). - -```plaintext -GET /issues/:id -``` - -| Attribute | Type | Required | Description | -|-------------|---------|----------|--------------------------------------| -| `id` | integer | yes | The ID of the issue | - -```shell -curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/issues/41" -``` - -Example response: - -```json -{ - "id" : 1, - "milestone" : { - "due_date" : null, - "project_id" : 4, - "state" : "closed", - "description" : "Rerum est voluptatem provident consequuntur molestias similique ipsum dolor.", - "iid" : 3, - "id" : 11, - "title" : "v3.0", - "created_at" : "2016-01-04T15:31:39.788Z", - "updated_at" : "2016-01-04T15:31:39.788Z", - "closed_at" : "2016-01-05T15:31:46.176Z" - }, - "author" : { - "state" : "active", - "web_url" : "https://gitlab.example.com/root", - "avatar_url" : null, - "username" : "root", - "id" : 1, - "name" : "Administrator" - }, - "description" : "Omnis vero earum sunt corporis dolor et placeat.", - "state" : "closed", - "iid" : 1, - "assignees" : [{ - "avatar_url" : null, - "web_url" : "https://gitlab.example.com/lennie", - "state" : "active", - "username" : "lennie", - "id" : 9, - "name" : "Dr. Luella Kovacek" - }], - "assignee" : { - "avatar_url" : null, - "web_url" : "https://gitlab.example.com/lennie", - "state" : "active", - "username" : "lennie", - "id" : 9, - "name" : "Dr. Luella Kovacek" - }, - "labels" : [], - "upvotes": 4, - "downvotes": 0, - "merge_requests_count": 0, - "title" : "Ut commodi ullam eos dolores perferendis nihil sunt.", - "updated_at" : "2016-01-04T15:31:46.176Z", - "created_at" : "2016-01-04T15:31:46.176Z", - "closed_at" : null, - "closed_by" : null, - "subscribed": false, - "user_notes_count": 1, - "due_date": null, - "web_url": "http://example.com/my-group/my-project/issues/1", - "references": { - "short": "#1", - "relative": "#1", - "full": "my-group/my-project#1" - }, - "time_stats": { - "time_estimate": 0, - "total_time_spent": 0, - "human_time_estimate": null, - "human_total_time_spent": null - }, - "confidential": false, - "discussion_locked": false, - "_links": { - "self": "http://example.com/api/v4/projects/1/issues/2", - "notes": "http://example.com/api/v4/projects/1/issues/2/notes", - "award_emoji": "http://example.com/api/v4/projects/1/issues/2/award_emoji", - "project": "http://example.com/api/v4/projects/1" - }, - "task_completion_status":{ - "count":0, - "completed_count":0 - }, - "weight": null, - "has_tasks": false, - "_links": { - "self": "http://gitlab.dummy:3000/api/v4/projects/1/issues/1", - "notes": "http://gitlab.dummy:3000/api/v4/projects/1/issues/1/notes", - "award_emoji": "http://gitlab.dummy:3000/api/v4/projects/1/issues/1/award_emoji", - "project": "http://gitlab.dummy:3000/api/v4/projects/1" - }, - "references": { - "short": "#1", - "relative": "#1", - "full": "gitlab-org/gitlab-test#1" - }, - "subscribed": true, - "moved_to_id": null, - "epic_iid": null, - "epic": null -} -``` - -Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) will also see +Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) can also see the `weight` parameter: ```json @@ -849,7 +692,7 @@ the `weight` parameter: } ``` -Users on GitLab [Ultimate](https://about.gitlab.com/pricing/) will additionally see +Users on GitLab [Ultimate](https://about.gitlab.com/pricing/) can also see the `epic` property: ```javascript @@ -869,14 +712,20 @@ the `epic` property: } ``` -**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API. +NOTE: **Note:** +The `assignee` column is deprecated. We now show it as a single-sized array `assignees` to conform +to the GitLab EE API. -**Note**: The `closed_by` attribute was [introduced in GitLab 10.6](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/17042). This value will only be present for issues which were closed after GitLab 10.6 and when the user account that closed the issue still exists. +NOTE: **Note:** +The `closed_by` attribute was [introduced in GitLab 10.6](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/17042). +This value is only present for issues closed after GitLab 10.6 and if the user account +that closed the issue still exists. -**Note**: The `epic_iid` attribute is deprecated and [will be removed in version 5](https://gitlab.com/gitlab-org/gitlab/-/issues/35157). +NOTE: **Note:** +The `epic_iid` attribute is deprecated, and [will be removed in version 5](https://gitlab.com/gitlab-org/gitlab/-/issues/35157). Please use `iid` of the `epic` attribute instead. -## Single Project Issue +## Single project issue Get a single project issue. diff --git a/doc/development/telemetry/usage_ping.md b/doc/development/telemetry/usage_ping.md index 87eb3c5234a..81daedb0000 100644 --- a/doc/development/telemetry/usage_ping.md +++ b/doc/development/telemetry/usage_ping.md @@ -288,6 +288,29 @@ Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PF end ``` +1. Track event in API using `increment_unique_values(event_name, values)` helper method. + + In order to be able to track the event, Usage Ping must be enabled and the event feature `usage_data_<event_name>` must be enabled. + + Arguments: + + - `event_name`: event name. + - `values`: values counted, one value or array of values. + + Example usage: + + ```ruby + get ':id/registry/repositories' do + repositories = ContainerRepositoriesFinder.new( + user: current_user, subject: user_group + ).execute + + increment_unique_values('i_list_repositories', current_user.id) + + present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags], tags_count: params[:tags_count] + end + ``` + 1. Track event using base module `Gitlab::UsageDataCounters::HLLRedisCounter.track_event(entity_id, event_name)`. Arguments: diff --git a/doc/user/admin_area/analytics/dev_ops_report.md b/doc/user/admin_area/analytics/dev_ops_report.md index 8c21570937d..8ddd093893c 100644 --- a/doc/user/admin_area/analytics/dev_ops_report.md +++ b/doc/user/admin_area/analytics/dev_ops_report.md @@ -10,7 +10,9 @@ The DevOps Report gives you an overview of your entire instance's adoption of [Concurrent DevOps](https://about.gitlab.com/topics/concurrent-devops/) from planning to monitoring. -This displays the usage of these GitLab features over +## DevOps Score + +DevOps Score displays the usage of GitLab's major features on your instance over the last 30 days, averaged over the number of active users in that time period. It also provides a Lead score per feature, which is calculated based on GitLab's analysis of top-performing instances based on [usage ping data](../settings/usage_statistics.md#usage-ping-core-only) that GitLab has diff --git a/doc/user/application_security/img/unconfigured_security_approval_rules_and_enabled_jobs_v13_4.png b/doc/user/application_security/img/unconfigured_security_approval_rules_and_enabled_jobs_v13_4.png Binary files differnew file mode 100644 index 00000000000..f497b0fbc4e --- /dev/null +++ b/doc/user/application_security/img/unconfigured_security_approval_rules_and_enabled_jobs_v13_4.png diff --git a/doc/user/application_security/img/unconfigured_security_approval_rules_and_jobs_v13_4.png b/doc/user/application_security/img/unconfigured_security_approval_rules_and_jobs_v13_4.png Binary files differnew file mode 100644 index 00000000000..fc847b578f5 --- /dev/null +++ b/doc/user/application_security/img/unconfigured_security_approval_rules_and_jobs_v13_4.png diff --git a/doc/user/application_security/img/vulnerability-check_v13_0.png b/doc/user/application_security/img/vulnerability-check_v13_0.png Binary files differdeleted file mode 100644 index 9f0bd0f759b..00000000000 --- a/doc/user/application_security/img/vulnerability-check_v13_0.png +++ /dev/null diff --git a/doc/user/application_security/img/vulnerability-check_v13_4.png b/doc/user/application_security/img/vulnerability-check_v13_4.png Binary files differnew file mode 100644 index 00000000000..e0b53059b45 --- /dev/null +++ b/doc/user/application_security/img/vulnerability-check_v13_4.png diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md index da348cfc9c4..c3477fdcbb6 100644 --- a/doc/user/application_security/index.md +++ b/doc/user/application_security/index.md @@ -309,15 +309,29 @@ rating. ### Enabling Security Approvals within a project -To enable Security Approvals, a [project approval rule](../project/merge_requests/merge_request_approvals.md#adding--editing-a-default-approval-rule) -must be created with the case-sensitive name `Vulnerability-Check`. This approval group must be set -with the number of approvals required greater than zero. You must have Maintainer or Owner [permissions](../permissions.md#project-members-permissions) to manage approval rules. +To enable the `Vulnerability-Check` or `License-Check` Security Approvals, a [project approval rule](../project/merge_requests/merge_request_approvals.md#adding--editing-a-default-approval-rule) +must be created. A [security scanner job](#security-scanning-tools) must be enabled for +`Vulnerability-Check`, and a [license scanning](../compliance/license_compliance/index.md#configuration) +job must be enabled for `License-Check`. When the proper jobs aren't configured, the following +appears: + +![Unconfigured Approval Rules](img/unconfigured_security_approval_rules_and_jobs_v13_4.png) + +If at least one security scanner is enabled, you will be able to enable the `Vulnerability-Check` approval rule. If a license scanning job is enabled, you will be able to enable the `License-Check` rule. + +![Unconfigured Approval Rules with valid pipeline jobs](img/unconfigured_security_approval_rules_and_enabled_jobs_v13_4.png) + +For this approval group, you must set the number of approvals required to greater than zero. You +must have Maintainer or Owner [permissions](../permissions.md#project-members-permissions) +to manage approval rules. + +Follow these steps to enable `Vulnerability-Check`: 1. Navigate to your project's **Settings > General** and expand **Merge request approvals**. -1. Click **Add approval rule**, or **Edit**. - - Add or change the **Rule name** to `Vulnerability-Check` (case sensitive). +1. Click **Enable**, or **Edit**. +1. Add or change the **Rule name** to `Vulnerability-Check` (case sensitive). -![Vulnerability Check Approver Rule](img/vulnerability-check_v13_0.png) +![Vulnerability Check Approver Rule](img/vulnerability-check_v13_4.png) Once this group is added to your project, the approval rule is enabled for all merge requests. @@ -334,32 +348,14 @@ An approval is optional when the security report: - Contains no new vulnerabilities when compared to the target branch. - Contains only new vulnerabilities of `low` or `medium` severity. -## Enabling License Approvals within a project +### Enabling License Approvals within a project > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13067) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.3. -`License-Check` is an approval rule you can enable to allow an individual or group to approve a -merge request that contains a `denied` license. - -You can enable `License-Check` one of two ways: - -- Create a [project approval rule](../project/merge_requests/merge_request_approvals.md#multiple-approval-rules-premium) - with the case-sensitive name `License-Check`. -- Create an approval group in the [project policies section for License Compliance](../compliance/license_compliance/index.md#policies). - You must set this approval group's number of approvals required to greater than zero. Once you - enable this group in your project, the approval rule is enabled for all merge requests. - -Any code changes cause the approvals required to reset. - -An approval is required when a license report: - -- Contains a dependency that includes a software license that is `denied`. -- Is not generated during pipeline execution. - -An approval is optional when a license report: - -- Contains no software license violations. -- Contains only new licenses that are `allowed` or unknown. +`License-Check` is a [security approval rule](#enabling-security-approvals-within-a-project) +you can enable to allow an individual or group to approve a merge request that contains a `denied` +license. For instructions on enabling this rule, see +[Enabling license approvals within a project](../compliance/license_compliance/index.md#enabling-license-approvals-within-a-project). ## Working in an offline environment diff --git a/doc/user/compliance/license_compliance/img/license-check_v13_4.png b/doc/user/compliance/license_compliance/img/license-check_v13_4.png Binary files differnew file mode 100644 index 00000000000..d3658cbaa18 --- /dev/null +++ b/doc/user/compliance/license_compliance/img/license-check_v13_4.png diff --git a/doc/user/compliance/license_compliance/index.md b/doc/user/compliance/license_compliance/index.md index 1756c7ae9f3..3bf6f310751 100644 --- a/doc/user/compliance/license_compliance/index.md +++ b/doc/user/compliance/license_compliance/index.md @@ -724,17 +724,21 @@ Developers of the project can view the policies configured in a project. ![View Policies](img/policies_v13_0.png) -### Enabling License Approvals within a project +## Enabling License Approvals within a project > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13067) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.3. -`License-Check` is an approval rule you can enable to allow an approver, individual, or group to -approve a merge request that contains a `denied` license. +`License-Check` is a [security approval](../../application_security/index.md#enabling-security-approvals-within-a-project) rule you can enable to allow an individual or group to approve a +merge request that contains a `denied` license. You can enable `License-Check` one of two ways: -- Create a [project approval rule](../../project/merge_requests/merge_request_approvals.md#multiple-approval-rules-premium) - with the case-sensitive name `License-Check`. +1. Navigate to your project's **Settings > General** and expand **Merge request approvals**. +1. Click **Enable** or **Edit**. +1. Add or change the **Rule name** to `License-Check` (case sensitive). + +![License Check Approver Rule](img/license-check_v13_4.png) + - Create an approval group in the [project policies section for License Compliance](#policies). You must set this approval group's number of approvals required to greater than zero. Once you enable this group in your project, the approval rule is enabled for all merge requests. diff --git a/haml_lint/linter/documentation_links.rb b/haml_lint/linter/documentation_links.rb index f8e0eec5cdc..5bee083e9ec 100644 --- a/haml_lint/linter/documentation_links.rb +++ b/haml_lint/linter/documentation_links.rb @@ -92,7 +92,8 @@ module HamlLint File.open(path_to_file).any? do |line| result = line.match(MARKDOWN_HEADER) - string_to_anchor(result[:header]) == anchor if result + # TODO:Do an exact match for anchors (Follow-up https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39850) + anchor.start_with?(string_to_anchor(result[:header])) if result end end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index c4a3385efd0..b811e11a75c 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -537,6 +537,20 @@ module API ) end + # @param event_name [String] the event name + # @param values [Array|String] the values counted + def increment_unique_values(event_name, values) + return unless values.present? + + feature_name = "usage_data_#{event_name}" + return unless Feature.enabled?(feature_name) + return unless Gitlab::CurrentSettings.usage_ping_enabled? + + Gitlab::UsageDataCounters::HLLRedisCounter.track_event(values, event_name) + rescue => error + Gitlab::AppLogger.warn("Redis tracking event failed for event: #{event_name}, message: #{error.message}") + end + def with_api_params(&block) yield({ api: true, request: request }) end diff --git a/lib/api/users.rb b/lib/api/users.rb index b630ca057db..73bb43b88fc 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -116,6 +116,7 @@ module API entity = current_user&.admin? ? Entities::UserWithAdmin : Entities::UserBasic users = users.preload(:identities, :u2f_registrations) if entity == Entities::UserWithAdmin + users = users.preload(:identities, :webauthn_registrations) if entity == Entities::UserWithAdmin users, options = with_custom_attributes(users, { with: entity, current_user: current_user }) users = users.preload(:user_detail) diff --git a/lib/gitlab/cleanup/orphan_lfs_file_references.rb b/lib/gitlab/cleanup/orphan_lfs_file_references.rb index 3df243e319e..6710b0bba66 100644 --- a/lib/gitlab/cleanup/orphan_lfs_file_references.rb +++ b/lib/gitlab/cleanup/orphan_lfs_file_references.rb @@ -25,7 +25,7 @@ module Gitlab private def remove_orphan_references - invalid_references = project.lfs_objects_projects.where(lfs_object: orphan_objects) # rubocop:disable CodeReuse/ActiveRecord + invalid_references = project.lfs_objects_projects.lfs_object_in(orphan_objects) if dry_run log_info("Found invalid references: #{invalid_references.count}") @@ -41,26 +41,22 @@ module Gitlab end end - def lfs_oids_from_repository - project.repository.gitaly_blob_client.get_all_lfs_pointers.map(&:lfs_oid) - end - - def orphan_oids - lfs_oids_from_database - lfs_oids_from_repository - end + def orphan_objects + # Get these first so racing with a git push can't remove any LFS objects + oids = project.lfs_objects_oids - def lfs_oids_from_database - oids = [] + repos = [ + project.repository, + project.design_repository, + project.wiki.repository + ].select(&:exists?) - project.lfs_objects.each_batch do |relation| - oids += relation.pluck(:oid) # rubocop:disable CodeReuse/ActiveRecord + repos.flat_map do |repo| + oids -= repo.gitaly_blob_client.get_all_lfs_pointers.map(&:lfs_oid) end - oids - end - - def orphan_objects - LfsObject.where(oid: orphan_oids) # rubocop:disable CodeReuse/ActiveRecord + # The remaining OIDs are not used by any repository, so are orphans + LfsObject.for_oids(oids) end def log_info(msg) diff --git a/lib/gitlab/graphql/representation/submodule_tree_entry.rb b/lib/gitlab/graphql/representation/submodule_tree_entry.rb index 8d17cb9eecc..aa5e74cc837 100644 --- a/lib/gitlab/graphql/representation/submodule_tree_entry.rb +++ b/lib/gitlab/graphql/representation/submodule_tree_entry.rb @@ -24,11 +24,11 @@ module Gitlab end def web_url - @submodule_links.first + @submodule_links&.web end def tree_url - @submodule_links.last + @submodule_links&.tree end end end diff --git a/lib/gitlab/submodule_links.rb b/lib/gitlab/submodule_links.rb index b0ee0877f30..38b10c5892d 100644 --- a/lib/gitlab/submodule_links.rb +++ b/lib/gitlab/submodule_links.rb @@ -4,14 +4,18 @@ module Gitlab class SubmoduleLinks include Gitlab::Utils::StrongMemoize + Urls = Struct.new(:web, :tree, :compare) + def initialize(repository) @repository = repository @cache_store = {} end - def for(submodule, sha) + def for(submodule, sha, diff_file = nil) submodule_url = submodule_url_for(sha, submodule.path) - SubmoduleHelper.submodule_links_for_url(submodule.id, submodule_url, repository) + old_submodule_id = old_submodule_id(submodule_url, diff_file) + urls = SubmoduleHelper.submodule_links_for_url(submodule.id, submodule_url, repository, old_submodule_id) + Urls.new(*urls) if urls.any? end private @@ -29,5 +33,15 @@ module Gitlab urls = submodule_urls_for(sha) urls && urls[path] end + + def old_submodule_id(submodule_url, diff_file) + return unless diff_file&.old_blob && diff_file&.old_content_sha + + # if the submodule url has changed from old_sha to sha, a compare link does not make sense + # + old_submodule_url = submodule_url_for(diff_file.old_content_sha, diff_file.old_blob.path) + + diff_file.old_blob.id if old_submodule_url == submodule_url + end end end diff --git a/lib/gitlab/utils/markdown.rb b/lib/gitlab/utils/markdown.rb index 82c4a0e3b23..1f833f69642 100644 --- a/lib/gitlab/utils/markdown.rb +++ b/lib/gitlab/utils/markdown.rb @@ -4,11 +4,13 @@ module Gitlab module Utils module Markdown PUNCTUATION_REGEXP = /[^\p{Word}\- ]/u.freeze + PRODUCT_SUFFIX = /\s*\((core|starter|premium|ultimate)(\s+only)?\)/.freeze def string_to_anchor(string) string .strip .downcase + .gsub(PRODUCT_SUFFIX, '') .gsub(PUNCTUATION_REGEXP, '') # remove punctuation .tr(' ', '-') # replace spaces with dash .squeeze('-') # replace multiple dashes with one diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 895b41a4ca7..cece15ac5c1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3325,6 +3325,9 @@ msgstr "" msgid "Are you sure? The device will be signed out of GitLab and all remember me tokens revoked." msgstr "" +msgid "Are you sure? This will invalidate your registered applications and U2F / WebAuthn devices." +msgstr "" + msgid "Are you sure? This will invalidate your registered applications and U2F devices." msgstr "" @@ -3346,6 +3349,9 @@ msgstr "" msgid "As U2F devices are only supported by a few browsers, we require that you set up a two-factor authentication app before a U2F device. That way you'll always be able to log in - even when you're using an unsupported browser." msgstr "" +msgid "As WebAuthn devices are only supported by a few browsers, we require that you set up a two-factor authentication app before a WebAuthn device. That way you'll always be able to log in - even when you're using an unsupported browser." +msgstr "" + msgid "As we continue to build more features for SAST, we'd love your feedback on the SAST configuration feature in %{linkStart}this issue%{linkEnd}." msgstr "" @@ -3575,7 +3581,7 @@ msgstr "" msgid "Authentication method updated" msgstr "" -msgid "Authentication via U2F device failed." +msgid "Authentication via %{method} device failed." msgstr "" msgid "Author" @@ -6346,6 +6352,9 @@ msgstr "" msgid "Compare" msgstr "" +msgid "Compare %{oldCommitId}...%{newCommitId}" +msgstr "" + msgid "Compare Git revisions" msgstr "" @@ -6361,6 +6370,9 @@ msgstr "" msgid "Compare changes with the merge request target branch" msgstr "" +msgid "Compare submodule commit revisions" +msgstr "" + msgid "Compare with previous version" msgstr "" @@ -20489,10 +20501,13 @@ msgstr "" msgid "Register Two-Factor Authenticator" msgstr "" -msgid "Register U2F device" +msgid "Register Universal Two-Factor (U2F) Device" +msgstr "" + +msgid "Register WebAuthn Device" msgstr "" -msgid "Register Universal Two-Factor (U2F) Device" +msgid "Register device" msgstr "" msgid "Register for GitLab" @@ -22661,7 +22676,7 @@ msgstr "" msgid "Set up assertions/attributes/claims (email, first_name, last_name) and NameID according to %{docsLinkStart}the documentation %{icon}%{docsLinkEnd}" msgstr "" -msgid "Set up new U2F device" +msgid "Set up new device" msgstr "" msgid "Set up new password" @@ -24042,6 +24057,9 @@ msgstr "" msgid "Successfully deleted U2F device." msgstr "" +msgid "Successfully deleted WebAuthn device." +msgstr "" + msgid "Successfully removed email." msgstr "" @@ -24835,7 +24853,7 @@ msgstr "" msgid "The remote repository is being updated..." msgstr "" -msgid "The repository can be commited to, and issues, comments and other entities can be created." +msgid "The repository can be committed to, and issues, comments and other entities can be created." msgstr "" msgid "The repository for this project does not exist." @@ -26414,6 +26432,9 @@ msgstr "" msgid "Try using a different search term to find the file you are looking for." msgstr "" +msgid "Trying to communicate with your device. Plug it in (if needed) and press the button on the device now." +msgstr "" + msgid "Trying to communicate with your device. Plug it in (if you haven't already) and press the button on the device now." msgstr "" @@ -27744,6 +27765,9 @@ msgstr "" msgid "Vulnerability|Description" msgstr "" +msgid "Vulnerability|Detected" +msgstr "" + msgid "Vulnerability|Evidence" msgstr "" @@ -27861,6 +27885,12 @@ msgstr "" msgid "Web terminal" msgstr "" +msgid "WebAuthn Devices (%{length})" +msgstr "" + +msgid "WebAuthn only works with HTTPS-enabled websites. Contact your administrator for more details." +msgstr "" + msgid "WebIDE|Merge request" msgstr "" @@ -28550,6 +28580,9 @@ msgstr "" msgid "You don't have any U2F devices registered yet." msgstr "" +msgid "You don't have any WebAuthn devices registered yet." +msgstr "" + msgid "You don't have any active chat names." msgstr "" @@ -28676,7 +28709,7 @@ msgstr "" msgid "You need to be logged in." msgstr "" -msgid "You need to register a two-factor authentication app before you can set up a U2F device." +msgid "You need to register a two-factor authentication app before you can set up a device." msgstr "" msgid "You need to set terms to be enforced" @@ -28859,10 +28892,13 @@ msgstr "" msgid "Your U2F device did not send a valid JSON response." msgstr "" -msgid "Your U2F device needs to be set up. Plug it in (if not already) and click the button on the left." +msgid "Your U2F device was registered!" msgstr "" -msgid "Your U2F device was registered!" +msgid "Your WebAuthn device did not send a valid JSON response." +msgstr "" + +msgid "Your WebAuthn device was registered!" msgstr "" msgid "Your access request to the %{source_type} has been withdrawn." @@ -28886,6 +28922,9 @@ msgstr "" msgid "Your browser doesn't support U2F. Please use Google Chrome desktop (version 41 or newer)." msgstr "" +msgid "Your browser doesn't support WebAuthn. Please use a supported browser, e.g. Chrome (67+) or Firefox (60+)." +msgstr "" + msgid "Your changes can be committed to %{branch_name} because a merge request is open." msgstr "" @@ -28922,6 +28961,12 @@ msgstr "" msgid "Your deployment services will be broken, you will need to manually fix the services after renaming." msgstr "" +msgid "Your device is not compatible with GitLab. Please try another device" +msgstr "" + +msgid "Your device needs to be set up. Plug it in (if needed) and click the button on the left." +msgstr "" + msgid "Your device was successfully set up! Give it a name and register it with the GitLab server." msgstr "" @@ -29473,9 +29518,6 @@ msgstr "" msgid "error" msgstr "" -msgid "error code:" -msgstr "" - msgid "estimateCommand|%{slash_command} will update the estimated time with the latest command." msgstr "" diff --git a/spec/controllers/admin/sessions_controller_spec.rb b/spec/controllers/admin/sessions_controller_spec.rb index 82366cc6952..35982e57034 100644 --- a/spec/controllers/admin/sessions_controller_spec.rb +++ b/spec/controllers/admin/sessions_controller_spec.rb @@ -220,10 +220,8 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do end end - context 'when using two-factor authentication via U2F' do - let(:user) { create(:admin, :two_factor_via_u2f) } - - def authenticate_2fa_u2f(user_params) + shared_examples 'when using two-factor authentication via hardware device' do + def authenticate_2fa(user_params) post(:create, params: { user: user_params }, session: { otp_user_id: user.id }) end @@ -239,14 +237,18 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do end it 'can login with valid auth' do + # we can stub both without an differentiation between webauthn / u2f + # as these not interfere with each other und this saves us passing aroud + # parameters allow(U2fRegistration).to receive(:authenticate).and_return(true) + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(true) expect(controller.current_user_mode.admin_mode?).to be(false) controller.store_location_for(:redirect, admin_root_path) controller.current_user_mode.request_admin_mode! - authenticate_2fa_u2f(login: user.username, device_response: '{}') + authenticate_2fa(login: user.username, device_response: '{}') expect(response).to redirect_to admin_root_path expect(controller.current_user_mode.admin_mode?).to be(true) @@ -254,16 +256,33 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do it 'cannot login with invalid auth' do allow(U2fRegistration).to receive(:authenticate).and_return(false) + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(false) expect(controller.current_user_mode.admin_mode?).to be(false) controller.current_user_mode.request_admin_mode! - authenticate_2fa_u2f(login: user.username, device_response: '{}') + authenticate_2fa(login: user.username, device_response: '{}') expect(response).to render_template('admin/sessions/two_factor') expect(controller.current_user_mode.admin_mode?).to be(false) end end + + context 'when using two-factor authentication via U2F' do + it_behaves_like 'when using two-factor authentication via hardware device' do + let(:user) { create(:admin, :two_factor_via_u2f) } + + before do + stub_feature_flags(webauthn: false) + end + end + end + + context 'when using two-factor authentication via WebAuthn' do + it_behaves_like 'when using two-factor authentication via hardware device' do + let(:user) { create(:admin, :two_factor_via_webauthn) } + end + end end end diff --git a/spec/controllers/profiles/webauthn_registrations_controller_spec.rb b/spec/controllers/profiles/webauthn_registrations_controller_spec.rb new file mode 100644 index 00000000000..0c475039963 --- /dev/null +++ b/spec/controllers/profiles/webauthn_registrations_controller_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Profiles::WebauthnRegistrationsController do + let(:user) { create(:user, :two_factor_via_webauthn) } + + before do + sign_in(user) + end + + describe '#destroy' do + it 'deletes the given webauthn registration' do + registration_to_delete = user.webauthn_registrations.first + + expect { delete :destroy, params: { id: registration_to_delete.id } }.to change { user.webauthn_registrations.count }.by(-1) + expect(response).to be_redirect + end + end +end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index acec1cf22fd..2c3815b36af 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -416,6 +416,10 @@ RSpec.describe SessionsController do post(:create, params: { user: user_params }, session: { otp_user_id: user.id }) end + before do + stub_feature_flags(webauthn: false) + end + context 'remember_me field' do it 'sets a remember_user_token cookie when enabled' do allow(U2fRegistration).to receive(:authenticate).and_return(true) diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 7e121b10632..d4b2ac5f056 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -81,6 +81,14 @@ FactoryBot.define do end end + trait :two_factor_via_webauthn do + transient { registrations_count { 5 } } + + after(:create) do |user, evaluator| + create_list(:webauthn_registration, evaluator.registrations_count, user: user) + end + end + trait :readme do project_view { :readme } end diff --git a/spec/factories/webauthn_registrations.rb b/spec/factories/webauthn_registrations.rb new file mode 100644 index 00000000000..ac803885244 --- /dev/null +++ b/spec/factories/webauthn_registrations.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :webauthn_registration do + credential_xid { SecureRandom.base64(88) } + public_key { SecureRandom.base64(103) } + name { FFaker::BaconIpsum.characters(10) } + counter { 1 } + user + end +end diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb index 8dbedc0a7ee..5762a54a717 100644 --- a/spec/features/u2f_spec.rb +++ b/spec/features/u2f_spec.rb @@ -3,22 +3,14 @@ require 'spec_helper' RSpec.describe 'Using U2F (Universal 2nd Factor) Devices for Authentication', :js do - def manage_two_factor_authentication - click_on 'Manage two-factor authentication' - expect(page).to have_content("Set up new U2F device") - wait_for_requests - end + include Spec::Support::Helpers::Features::TwoFactorHelpers - def register_u2f_device(u2f_device = nil, name: 'My device') - u2f_device ||= FakeU2fDevice.new(page, name) - u2f_device.respond_to_u2f_registration - click_on 'Set up new U2F device' - expect(page).to have_content('Your device was successfully set up') - fill_in "Pick a name", with: name - click_on 'Register U2F device' - u2f_device + before do + stub_feature_flags(webauthn: false) end + it_behaves_like 'hardware device for 2fa', 'U2F' + describe "registration" do let(:user) { create(:user) } @@ -27,31 +19,7 @@ RSpec.describe 'Using U2F (Universal 2nd Factor) Devices for Authentication', :j user.update_attribute(:otp_required_for_login, true) end - describe 'when 2FA via OTP is disabled' do - before do - user.update_attribute(:otp_required_for_login, false) - end - - it 'does not allow registering a new device' do - visit profile_account_path - click_on 'Enable two-factor authentication' - - expect(page).to have_button('Set up new U2F device', disabled: true) - end - end - describe 'when 2FA via OTP is enabled' do - it 'allows registering a new device with a name' do - visit profile_account_path - manage_two_factor_authentication - expect(page).to have_content("You've already enabled two-factor authentication using one time password authenticators") - - u2f_device = register_u2f_device - - expect(page).to have_content(u2f_device.name) - expect(page).to have_content('Your U2F device was registered') - end - it 'allows registering more than one device' do visit profile_account_path @@ -68,21 +36,6 @@ RSpec.describe 'Using U2F (Universal 2nd Factor) Devices for Authentication', :j expect(page).to have_content(second_device.name) expect(U2fRegistration.count).to eq(2) end - - it 'allows deleting a device' do - visit profile_account_path - manage_two_factor_authentication - expect(page).to have_content("You've already enabled two-factor authentication using one time password authenticators") - - first_u2f_device = register_u2f_device - second_u2f_device = register_u2f_device(name: 'My other device') - - accept_confirm { click_on "Delete", match: :first } - - expect(page).to have_content('Successfully deleted') - expect(page.body).not_to match(first_u2f_device.name) - expect(page).to have_content(second_u2f_device.name) - end end it 'allows the same device to be registered for multiple users' do @@ -111,9 +64,9 @@ RSpec.describe 'Using U2F (Universal 2nd Factor) Devices for Authentication', :j # Have the "u2f device" respond with bad data page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };") - click_on 'Set up new U2F device' + click_on 'Set up new device' expect(page).to have_content('Your device was successfully set up') - click_on 'Register U2F device' + click_on 'Register device' expect(U2fRegistration.count).to eq(0) expect(page).to have_content("The form contains the following error") @@ -126,9 +79,9 @@ RSpec.describe 'Using U2F (Universal 2nd Factor) Devices for Authentication', :j # Failed registration page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };") - click_on 'Set up new U2F device' + click_on 'Set up new device' expect(page).to have_content('Your device was successfully set up') - click_on 'Register U2F device' + click_on 'Register device' expect(page).to have_content("The form contains the following error") # Successful registration @@ -228,12 +181,12 @@ RSpec.describe 'Using U2F (Universal 2nd Factor) Devices for Authentication', :j user = gitlab_sign_in(:user) user.update_attribute(:otp_required_for_login, true) visit profile_two_factor_auth_path - expect(page).to have_content("Your U2F device needs to be set up.") + expect(page).to have_content("Your device needs to be set up.") first_device = register_u2f_device # Register second device visit profile_two_factor_auth_path - expect(page).to have_content("Your U2F device needs to be set up.") + expect(page).to have_content("Your device needs to be set up.") second_device = register_u2f_device(name: 'My other device') gitlab_sign_out @@ -249,50 +202,4 @@ RSpec.describe 'Using U2F (Universal 2nd Factor) Devices for Authentication', :j end end end - - describe 'fallback code authentication' do - let(:user) { create(:user) } - - def assert_fallback_ui(page) - expect(page).to have_button('Verify code') - expect(page).to have_css('#user_otp_attempt') - expect(page).not_to have_link('Sign in via 2FA code') - expect(page).not_to have_css('#js-authenticate-token-2fa') - end - - before do - # Register and logout - gitlab_sign_in(user) - user.update_attribute(:otp_required_for_login, true) - visit profile_account_path - end - - describe 'when no u2f device is registered' do - before do - gitlab_sign_out - gitlab_sign_in(user) - end - - it 'shows the fallback otp code UI' do - assert_fallback_ui(page) - end - end - - describe 'when a u2f device is registered' do - before do - manage_two_factor_authentication - @u2f_device = register_u2f_device - gitlab_sign_out - gitlab_sign_in(user) - end - - it 'provides a button that shows the fallback otp code UI' do - expect(page).to have_link('Sign in via 2FA code') - - click_link('Sign in via 2FA code') - - assert_fallback_ui(page) - end - end - end end diff --git a/spec/features/webauthn_spec.rb b/spec/features/webauthn_spec.rb new file mode 100644 index 00000000000..2ffb6bb3477 --- /dev/null +++ b/spec/features/webauthn_spec.rb @@ -0,0 +1,234 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Using WebAuthn Devices for Authentication', :js do + include Spec::Support::Helpers::Features::TwoFactorHelpers + let(:app_id) { "http://#{Capybara.current_session.server.host}:#{Capybara.current_session.server.port}" } + + before do + WebAuthn.configuration.origin = app_id + end + + it_behaves_like 'hardware device for 2fa', 'WebAuthn' + + describe 'registration' do + let(:user) { create(:user) } + + before do + gitlab_sign_in(user) + user.update_attribute(:otp_required_for_login, true) + end + + describe 'when 2FA via OTP is enabled' do + it 'allows registering more than one device' do + visit profile_account_path + + # First device + manage_two_factor_authentication + first_device = register_webauthn_device + expect(page).to have_content('Your WebAuthn device was registered') + + # Second device + second_device = register_webauthn_device(name: 'My other device') + expect(page).to have_content('Your WebAuthn device was registered') + + expect(page).to have_content(first_device.name) + expect(page).to have_content(second_device.name) + expect(WebauthnRegistration.count).to eq(2) + end + end + + it 'allows the same device to be registered for multiple users' do + # First user + visit profile_account_path + manage_two_factor_authentication + webauthn_device = register_webauthn_device + expect(page).to have_content('Your WebAuthn device was registered') + gitlab_sign_out + + # Second user + user = gitlab_sign_in(:user) + user.update_attribute(:otp_required_for_login, true) + visit profile_account_path + manage_two_factor_authentication + register_webauthn_device(webauthn_device, name: 'My other device') + expect(page).to have_content('Your WebAuthn device was registered') + + expect(WebauthnRegistration.count).to eq(2) + end + + context 'when there are form errors' do + let(:mock_register_js) do + <<~JS + const mockResponse = { + type: 'public-key', + id: '', + rawId: '', + response: { + clientDataJSON: '', + attestationObject: '', + }, + getClientExtensionResults: () => {}, + }; + navigator.credentials.create = function(_) {return Promise.resolve(mockResponse);} + JS + end + + it "doesn't register the device if there are errors" do + visit profile_account_path + manage_two_factor_authentication + + # Have the "webauthn device" respond with bad data + page.execute_script(mock_register_js) + click_on 'Set up new device' + expect(page).to have_content('Your device was successfully set up') + click_on 'Register device' + + expect(WebauthnRegistration.count).to eq(0) + expect(page).to have_content('The form contains the following error') + expect(page).to have_content('did not send a valid JSON response') + end + + it 'allows retrying registration' do + visit profile_account_path + manage_two_factor_authentication + + # Failed registration + page.execute_script(mock_register_js) + click_on 'Set up new device' + expect(page).to have_content('Your device was successfully set up') + click_on 'Register device' + expect(page).to have_content('The form contains the following error') + + # Successful registration + register_webauthn_device + + expect(page).to have_content('Your WebAuthn device was registered') + expect(WebauthnRegistration.count).to eq(1) + end + end + end + + describe 'authentication' do + let(:otp_required_for_login) { true } + let(:user) { create(:user, webauthn_xid: WebAuthn.generate_user_id, otp_required_for_login: otp_required_for_login) } + + describe 'when there is only an U2F device' do + let!(:u2f_device) do + fake_device = U2F::FakeU2F.new(app_id) # "Client" + u2f = U2F::U2F.new(app_id) # "Server" + + challenges = u2f.registration_requests.map(&:challenge) + device_response = fake_device.register_response(challenges[0]) + device_registration_params = { device_response: device_response, + name: 'My device' } + + U2fRegistration.register(user, app_id, device_registration_params, challenges) + FakeU2fDevice.new(page, 'My device', fake_device) + end + + it 'falls back to U2F' do + gitlab_sign_in(user) + + u2f_device.respond_to_u2f_authentication + + expect(page).to have_css('.sign-out-link', visible: false) + end + end + + describe 'when there is a WebAuthn device' do + let!(:webauthn_device) do + add_webauthn_device(app_id, user) + end + + describe 'when 2FA via OTP is disabled' do + let(:otp_required_for_login) { false } + + it 'allows logging in with the WebAuthn device' do + gitlab_sign_in(user) + + webauthn_device.respond_to_webauthn_authentication + + expect(page).to have_css('.sign-out-link', visible: false) + end + end + + describe 'when 2FA via OTP is enabled' do + it 'allows logging in with the WebAuthn device' do + gitlab_sign_in(user) + + webauthn_device.respond_to_webauthn_authentication + + expect(page).to have_css('.sign-out-link', visible: false) + end + end + + describe 'when a given WebAuthn device has already been registered by another user' do + describe 'but not the current user' do + let(:other_user) { create(:user, webauthn_xid: WebAuthn.generate_user_id, otp_required_for_login: otp_required_for_login) } + + it 'does not allow logging in with that particular device' do + # Register other user with a different WebAuthn device + other_device = add_webauthn_device(app_id, other_user) + + # Try authenticating user with the old WebAuthn device + gitlab_sign_in(user) + other_device.respond_to_webauthn_authentication + expect(page).to have_content('Authentication via WebAuthn device failed') + end + end + + describe "and also the current user" do + # TODO Uncomment once WebAuthn::FakeClient supports passing credential options + # (especially allow_credentials, as this is needed to specify which credential the + # fake client should use. Currently, the first credential is always used). + # There is an issue open for this: https://github.com/cedarcode/webauthn-ruby/issues/259 + it "allows logging in with that particular device" do + pending("support for passing credential options in FakeClient") + # Register current user with the same WebAuthn device + current_user = gitlab_sign_in(:user) + visit profile_account_path + manage_two_factor_authentication + register_webauthn_device(webauthn_device) + gitlab_sign_out + + # Try authenticating user with the same WebAuthn device + gitlab_sign_in(current_user) + webauthn_device.respond_to_webauthn_authentication + + expect(page).to have_css('.sign-out-link', visible: false) + end + end + end + + describe 'when a given WebAuthn device has not been registered' do + it 'does not allow logging in with that particular device' do + unregistered_device = FakeWebauthnDevice.new(page, 'My device') + gitlab_sign_in(user) + unregistered_device.respond_to_webauthn_authentication + + expect(page).to have_content('Authentication via WebAuthn device failed') + end + end + + describe 'when more than one device has been registered by the same user' do + it 'allows logging in with either device' do + first_device = add_webauthn_device(app_id, user) + second_device = add_webauthn_device(app_id, user) + + # Authenticate as both devices + [first_device, second_device].each do |device| + gitlab_sign_in(user) + # register_webauthn_device(device) + device.respond_to_webauthn_authentication + + expect(page).to have_css('.sign-out-link', visible: false) + + gitlab_sign_out + end + end + end + end + end +end diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json index ac0a0314455..3c19528d71b 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_basic.json +++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json @@ -15,6 +15,13 @@ "$ref": "../public_api/v4/user/basic.json" } }, + "reviewers": { + "type": ["array"], + "items": { + "type": "object", + "$ref": "../public_api/v4/user/basic.json" + } + }, "milestone": { "type": [ "object", "null" ] }, diff --git a/spec/fixtures/api/schemas/entities/merge_request_sidebar_extras.json b/spec/fixtures/api/schemas/entities/merge_request_sidebar_extras.json index 11076ec73de..c54ae551d6a 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_sidebar_extras.json +++ b/spec/fixtures/api/schemas/entities/merge_request_sidebar_extras.json @@ -17,6 +17,10 @@ "assignees": { "type": "array", "items": { "$ref": "../public_api/v4/user/basic.json" } + }, + "reviewers": { + "type": "array", + "items": { "$ref": "../public_api/v4/user/basic.json" } } }, "additionalProperties": false diff --git a/spec/frontend/authentication/u2f/authenticate_spec.js b/spec/frontend/authentication/u2f/authenticate_spec.js index 8abef2ae1b2..7a87b420195 100644 --- a/spec/frontend/authentication/u2f/authenticate_spec.js +++ b/spec/frontend/authentication/u2f/authenticate_spec.js @@ -76,7 +76,7 @@ describe('U2FAuthenticate', () => { describe('errors', () => { it('displays an error message', () => { - const setupButton = container.find('#js-login-u2f-device'); + const setupButton = container.find('#js-login-2fa-device'); setupButton.trigger('click'); u2fDevice.respondToAuthenticateRequest({ errorCode: 'error!', @@ -87,14 +87,14 @@ describe('U2FAuthenticate', () => { }); it('allows retrying authentication after an error', () => { - let setupButton = container.find('#js-login-u2f-device'); + let setupButton = container.find('#js-login-2fa-device'); setupButton.trigger('click'); u2fDevice.respondToAuthenticateRequest({ errorCode: 'error!', }); const retryButton = container.find('#js-token-2fa-try-again'); retryButton.trigger('click'); - setupButton = container.find('#js-login-u2f-device'); + setupButton = container.find('#js-login-2fa-device'); setupButton.trigger('click'); u2fDevice.respondToAuthenticateRequest({ deviceData: 'this is data from the device', diff --git a/spec/frontend/authentication/u2f/register_spec.js b/spec/frontend/authentication/u2f/register_spec.js index 3c2ecdbba66..e89ef773be6 100644 --- a/spec/frontend/authentication/u2f/register_spec.js +++ b/spec/frontend/authentication/u2f/register_spec.js @@ -13,8 +13,8 @@ describe('U2FRegister', () => { beforeEach(done => { loadFixtures('u2f/register.html'); u2fDevice = new MockU2FDevice(); - container = $('#js-register-u2f'); - component = new U2FRegister(container, $('#js-register-u2f-templates'), {}, 'token'); + container = $('#js-register-token-2fa'); + component = new U2FRegister(container, {}); component .start() .then(done) @@ -22,9 +22,9 @@ describe('U2FRegister', () => { }); it('allows registering a U2F device', () => { - const setupButton = container.find('#js-setup-u2f-device'); + const setupButton = container.find('#js-setup-token-2fa-device'); - expect(setupButton.text()).toBe('Set up new U2F device'); + expect(setupButton.text()).toBe('Set up new device'); setupButton.trigger('click'); const inProgressMessage = container.children('p'); @@ -41,7 +41,7 @@ describe('U2FRegister', () => { describe('errors', () => { it("doesn't allow the same device to be registered twice (for the same user", () => { - const setupButton = container.find('#js-setup-u2f-device'); + const setupButton = container.find('#js-setup-token-2fa-device'); setupButton.trigger('click'); u2fDevice.respondToRegisterRequest({ errorCode: 4, @@ -52,7 +52,7 @@ describe('U2FRegister', () => { }); it('displays an error message for other errors', () => { - const setupButton = container.find('#js-setup-u2f-device'); + const setupButton = container.find('#js-setup-token-2fa-device'); setupButton.trigger('click'); u2fDevice.respondToRegisterRequest({ errorCode: 'error!', @@ -63,14 +63,14 @@ describe('U2FRegister', () => { }); it('allows retrying registration after an error', () => { - let setupButton = container.find('#js-setup-u2f-device'); + let setupButton = container.find('#js-setup-token-2fa-device'); setupButton.trigger('click'); u2fDevice.respondToRegisterRequest({ errorCode: 'error!', }); - const retryButton = container.find('#U2FTryAgain'); + const retryButton = container.find('#js-token-2fa-try-again'); retryButton.trigger('click'); - setupButton = container.find('#js-setup-u2f-device'); + setupButton = container.find('#js-setup-token-2fa-device'); setupButton.trigger('click'); u2fDevice.respondToRegisterRequest({ deviceData: 'this is data from the device', diff --git a/spec/frontend/authentication/webauthn/authenticate_spec.js b/spec/frontend/authentication/webauthn/authenticate_spec.js new file mode 100644 index 00000000000..0a82adfd0ee --- /dev/null +++ b/spec/frontend/authentication/webauthn/authenticate_spec.js @@ -0,0 +1,132 @@ +import $ from 'jquery'; +import waitForPromises from 'helpers/wait_for_promises'; +import WebAuthnAuthenticate from '~/authentication/webauthn/authenticate'; +import MockWebAuthnDevice from './mock_webauthn_device'; +import { useMockNavigatorCredentials } from './util'; + +const mockResponse = { + type: 'public-key', + id: '', + rawId: '', + response: { clientDataJSON: '', authenticatorData: '', signature: '', userHandle: '' }, + getClientExtensionResults: () => {}, +}; + +describe('WebAuthnAuthenticate', () => { + preloadFixtures('webauthn/authenticate.html'); + useMockNavigatorCredentials(); + + let fallbackElement; + let webAuthnDevice; + let container; + let component; + let submitSpy; + + const findDeviceResponseInput = () => container[0].querySelector('#js-device-response'); + const findDeviceResponseInputValue = () => findDeviceResponseInput().value; + const findMessage = () => container[0].querySelector('p'); + const findRetryButton = () => container[0].querySelector('#js-token-2fa-try-again'); + const expectAuthenticated = () => { + expect(container.text()).toMatchInterpolatedText( + 'We heard back from your device. You have been authenticated.', + ); + expect(findDeviceResponseInputValue()).toBe(JSON.stringify(mockResponse)); + expect(submitSpy).toHaveBeenCalled(); + }; + + beforeEach(() => { + loadFixtures('webauthn/authenticate.html'); + fallbackElement = document.createElement('div'); + fallbackElement.classList.add('js-2fa-form'); + webAuthnDevice = new MockWebAuthnDevice(); + container = $('#js-authenticate-token-2fa'); + component = new WebAuthnAuthenticate( + container, + '#js-login-token-2fa-form', + { + options: + // we need some valid base64 for base64ToBuffer + // so we use "YQ==" = base64("a") + JSON.stringify({ + challenge: 'YQ==', + timeout: 120000, + allowCredentials: [ + { type: 'public-key', id: 'YQ==' }, + { type: 'public-key', id: 'YQ==' }, + ], + userVerification: 'discouraged', + }), + }, + document.querySelector('#js-login-2fa-device'), + fallbackElement, + ); + submitSpy = jest.spyOn(HTMLFormElement.prototype, 'submit'); + }); + + describe('with webauthn unavailable', () => { + let oldGetCredentials; + + beforeEach(() => { + oldGetCredentials = window.navigator.credentials.get; + window.navigator.credentials.get = null; + }); + + afterEach(() => { + window.navigator.credentials.get = oldGetCredentials; + }); + + it('falls back to normal 2fa', () => { + component.start(); + + expect(container.html()).toBe(''); + expect(container[0]).toHaveClass('hidden'); + expect(fallbackElement).not.toHaveClass('hidden'); + }); + }); + + describe('with webauthn available', () => { + beforeEach(() => { + component.start(); + }); + + it('shows in progress', () => { + const inProgressMessage = container.find('p'); + + expect(inProgressMessage.text()).toMatchInterpolatedText( + "Trying to communicate with your device. Plug it in (if you haven't already) and press the button on the device now.", + ); + }); + + it('allows authenticating via a WebAuthn device', () => { + webAuthnDevice.respondToAuthenticateRequest(mockResponse); + + return waitForPromises().then(() => { + expectAuthenticated(); + }); + }); + + describe('errors', () => { + beforeEach(() => { + webAuthnDevice.rejectAuthenticateRequest(new DOMException()); + + return waitForPromises(); + }); + + it('displays an error message', () => { + expect(submitSpy).not.toHaveBeenCalled(); + expect(findMessage().textContent).toMatchInterpolatedText( + 'There was a problem communicating with your device. (Error)', + ); + }); + + it('allows retrying authentication after an error', () => { + findRetryButton().click(); + webAuthnDevice.respondToAuthenticateRequest(mockResponse); + + return waitForPromises().then(() => { + expectAuthenticated(); + }); + }); + }); + }); +}); diff --git a/spec/frontend/authentication/webauthn/error_spec.js b/spec/frontend/authentication/webauthn/error_spec.js new file mode 100644 index 00000000000..26f1ca5e27d --- /dev/null +++ b/spec/frontend/authentication/webauthn/error_spec.js @@ -0,0 +1,50 @@ +import WebAuthnError from '~/authentication/webauthn/error'; + +describe('WebAuthnError', () => { + it.each([ + [ + 'NotSupportedError', + 'Your device is not compatible with GitLab. Please try another device', + 'authenticate', + ], + ['InvalidStateError', 'This device has not been registered with us.', 'authenticate'], + ['InvalidStateError', 'This device has already been registered with us.', 'register'], + ['UnknownError', 'There was a problem communicating with your device.', 'register'], + ])('exception %s will have message %s, flow type: %s', (exception, expectedMessage, flowType) => { + expect(new WebAuthnError(new DOMException('', exception), flowType).message()).toEqual( + expectedMessage, + ); + }); + + describe('SecurityError', () => { + const { location } = window; + + beforeEach(() => { + delete window.location; + window.location = {}; + }); + + afterEach(() => { + window.location = location; + }); + + it('returns a descriptive error if https is disabled', () => { + window.location.protocol = 'http:'; + + const expectedMessage = + 'WebAuthn only works with HTTPS-enabled websites. Contact your administrator for more details.'; + expect( + new WebAuthnError(new DOMException('', 'SecurityError'), 'authenticate').message(), + ).toEqual(expectedMessage); + }); + + it('returns a generic error if https is enabled', () => { + window.location.protocol = 'https:'; + + const expectedMessage = 'There was a problem communicating with your device.'; + expect( + new WebAuthnError(new DOMException('', 'SecurityError'), 'authenticate').message(), + ).toEqual(expectedMessage); + }); + }); +}); diff --git a/spec/frontend/authentication/webauthn/mock_webauthn_device.js b/spec/frontend/authentication/webauthn/mock_webauthn_device.js new file mode 100644 index 00000000000..39df94df46b --- /dev/null +++ b/spec/frontend/authentication/webauthn/mock_webauthn_device.js @@ -0,0 +1,35 @@ +/* eslint-disable no-unused-expressions */ + +export default class MockWebAuthnDevice { + constructor() { + this.respondToAuthenticateRequest = this.respondToAuthenticateRequest.bind(this); + this.respondToRegisterRequest = this.respondToRegisterRequest.bind(this); + window.navigator.credentials || (window.navigator.credentials = {}); + window.navigator.credentials.create = () => + new Promise((resolve, reject) => { + this.registerCallback = resolve; + this.registerRejectCallback = reject; + }); + window.navigator.credentials.get = () => + new Promise((resolve, reject) => { + this.authenticateCallback = resolve; + this.authenticateRejectCallback = reject; + }); + } + + respondToRegisterRequest(params) { + return this.registerCallback(params); + } + + respondToAuthenticateRequest(params) { + return this.authenticateCallback(params); + } + + rejectRegisterRequest(params) { + return this.registerRejectCallback(params); + } + + rejectAuthenticateRequest(params) { + return this.authenticateRejectCallback(params); + } +} diff --git a/spec/frontend/authentication/webauthn/register_spec.js b/spec/frontend/authentication/webauthn/register_spec.js new file mode 100644 index 00000000000..1de952d176d --- /dev/null +++ b/spec/frontend/authentication/webauthn/register_spec.js @@ -0,0 +1,131 @@ +import $ from 'jquery'; +import waitForPromises from 'helpers/wait_for_promises'; +import WebAuthnRegister from '~/authentication/webauthn/register'; +import MockWebAuthnDevice from './mock_webauthn_device'; +import { useMockNavigatorCredentials } from './util'; + +describe('WebAuthnRegister', () => { + preloadFixtures('webauthn/register.html'); + useMockNavigatorCredentials(); + + const mockResponse = { + type: 'public-key', + id: '', + rawId: '', + response: { + clientDataJSON: '', + attestationObject: '', + }, + getClientExtensionResults: () => {}, + }; + let webAuthnDevice; + let container; + let component; + + beforeEach(() => { + loadFixtures('webauthn/register.html'); + webAuthnDevice = new MockWebAuthnDevice(); + container = $('#js-register-token-2fa'); + component = new WebAuthnRegister(container, { + options: { + rp: '', + user: { + id: '', + name: '', + displayName: '', + }, + challenge: '', + pubKeyCredParams: '', + }, + }); + component.start(); + }); + + const findSetupButton = () => container.find('#js-setup-token-2fa-device'); + const findMessage = () => container.find('p'); + const findDeviceResponse = () => container.find('#js-device-response'); + const findRetryButton = () => container.find('#js-token-2fa-try-again'); + + it('shows setup button', () => { + expect(findSetupButton().text()).toBe('Set up new device'); + }); + + describe('when unsupported', () => { + const { location, PublicKeyCredential } = window; + + beforeEach(() => { + delete window.location; + delete window.credentials; + window.location = {}; + window.PublicKeyCredential = undefined; + }); + + afterEach(() => { + window.location = location; + window.PublicKeyCredential = PublicKeyCredential; + }); + + it.each` + httpsEnabled | expectedText + ${false} | ${'WebAuthn only works with HTTPS-enabled websites'} + ${true} | ${'Please use a supported browser, e.g. Chrome (67+) or Firefox'} + `('when https is $httpsEnabled', ({ httpsEnabled, expectedText }) => { + window.location.protocol = httpsEnabled ? 'https:' : 'http:'; + component.start(); + + expect(findMessage().text()).toContain(expectedText); + }); + }); + + describe('when setup', () => { + beforeEach(() => { + findSetupButton().trigger('click'); + }); + + it('shows in progress message', () => { + expect(findMessage().text()).toContain('Trying to communicate with your device'); + }); + + it('registers device', () => { + webAuthnDevice.respondToRegisterRequest(mockResponse); + + return waitForPromises().then(() => { + expect(findMessage().text()).toContain('Your device was successfully set up!'); + expect(findDeviceResponse().val()).toBe(JSON.stringify(mockResponse)); + }); + }); + + it.each` + errorName | expectedText + ${'NotSupportedError'} | ${'Your device is not compatible with GitLab'} + ${'NotAllowedError'} | ${'There was a problem communicating with your device'} + `('when fails with $errorName', ({ errorName, expectedText }) => { + webAuthnDevice.rejectRegisterRequest(new DOMException('', errorName)); + + return waitForPromises().then(() => { + expect(findMessage().text()).toContain(expectedText); + expect(findRetryButton().length).toBe(1); + }); + }); + + it('can retry', () => { + webAuthnDevice.respondToRegisterRequest({ + errorCode: 'error!', + }); + + return waitForPromises() + .then(() => { + findRetryButton().click(); + + expect(findMessage().text()).toContain('Trying to communicate with your device'); + + webAuthnDevice.respondToRegisterRequest(mockResponse); + return waitForPromises(); + }) + .then(() => { + expect(findMessage().text()).toContain('Your device was successfully set up!'); + expect(findDeviceResponse().val()).toBe(JSON.stringify(mockResponse)); + }); + }); + }); +}); diff --git a/spec/frontend/authentication/webauthn/util.js b/spec/frontend/authentication/webauthn/util.js new file mode 100644 index 00000000000..d8f5a67ee1f --- /dev/null +++ b/spec/frontend/authentication/webauthn/util.js @@ -0,0 +1,19 @@ +export function useMockNavigatorCredentials() { + let oldNavigatorCredentials; + let oldPublicKeyCredential; + + beforeEach(() => { + oldNavigatorCredentials = navigator.credentials; + oldPublicKeyCredential = window.PublicKeyCredential; + navigator.credentials = { + get: jest.fn(), + create: jest.fn(), + }; + window.PublicKeyCredential = function MockPublicKeyCredential() {}; + }); + + afterEach(() => { + navigator.credentials = oldNavigatorCredentials; + window.PublicKeyCredential = oldPublicKeyCredential; + }); +} diff --git a/spec/frontend/fixtures/u2f.rb b/spec/frontend/fixtures/u2f.rb index be3874d7c42..a6a8ba7318b 100644 --- a/spec/frontend/fixtures/u2f.rb +++ b/spec/frontend/fixtures/u2f.rb @@ -11,6 +11,10 @@ RSpec.context 'U2F' do clean_frontend_fixtures('u2f/') end + before do + stub_feature_flags(webauthn: false) + end + describe SessionsController, '(JavaScript fixtures)', type: :controller do include DeviseHelpers diff --git a/spec/frontend/fixtures/webauthn.rb b/spec/frontend/fixtures/webauthn.rb new file mode 100644 index 00000000000..b195fee76f0 --- /dev/null +++ b/spec/frontend/fixtures/webauthn.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.context 'WebAuthn' do + include JavaScriptFixturesHelpers + + let(:user) { create(:user, :two_factor_via_webauthn, otp_secret: 'otpsecret:coolkids') } + + before(:all) do + clean_frontend_fixtures('webauthn/') + end + + describe SessionsController, '(JavaScript fixtures)', type: :controller do + include DeviseHelpers + + render_views + + before do + set_devise_mapping(context: @request) + end + + it 'webauthn/authenticate.html' do + allow(controller).to receive(:find_user).and_return(user) + post :create, params: { user: { login: user.username, password: user.password } } + + expect(response).to be_successful + end + end + + describe Profiles::TwoFactorAuthsController, '(JavaScript fixtures)', type: :controller do + render_views + + before do + sign_in(user) + allow_next_instance_of(Profiles::TwoFactorAuthsController) do |instance| + allow(instance).to receive(:build_qr_code).and_return('qrcode:blackandwhitesquares') + end + end + + it 'webauthn/register.html' do + get :show + + expect(response).to be_successful + end + end +end diff --git a/spec/frontend/packages/details/components/__snapshots__/dependency_row_spec.js.snap b/spec/frontend/packages/details/components/__snapshots__/dependency_row_spec.js.snap index df2844eaa3c..39469bf4fd0 100644 --- a/spec/frontend/packages/details/components/__snapshots__/dependency_row_spec.js.snap +++ b/spec/frontend/packages/details/components/__snapshots__/dependency_row_spec.js.snap @@ -21,7 +21,7 @@ exports[`DependencyRow renders full dependency 1`] = ` </div> <div - class="table-section section-50 gl-display-flex gl-justify-content-md-end" + class="table-section section-50 gl-display-flex gl-md-justify-content-end" data-testid="version-pattern" > <span diff --git a/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js b/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js index 128e0f39c41..631d4647b17 100644 --- a/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js +++ b/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js @@ -30,6 +30,7 @@ describe('getStateKey', () => { expect(bound()).toEqual('notAllowedToMerge'); context.autoMergeEnabled = true; + context.hasMergeableDiscussionsState = true; expect(bound()).toEqual('autoMergeEnabled'); @@ -44,6 +45,7 @@ describe('getStateKey', () => { expect(bound()).toEqual('pipelineBlocked'); context.hasMergeableDiscussionsState = true; + context.autoMergeEnabled = false; expect(bound()).toEqual('unresolvedDiscussions'); diff --git a/spec/graphql/resolvers/project_pipeline_resolver_spec.rb b/spec/graphql/resolvers/project_pipeline_resolver_spec.rb index ef13f048e19..a6a86c49373 100644 --- a/spec/graphql/resolvers/project_pipeline_resolver_spec.rb +++ b/spec/graphql/resolvers/project_pipeline_resolver_spec.rb @@ -34,12 +34,10 @@ RSpec.describe Resolvers::ProjectPipelineResolver do expect { resolve_pipeline(project, {}) }.to raise_error(ArgumentError) end - context 'when the pipeline is not a ci_config_source' do + context 'when the pipeline is a dangling pipeline' do let(:pipeline) do - config_source_value = ::Enums::Ci::Pipeline.non_ci_config_source_values.first - config_source = ::Enums::Ci::Pipeline.config_sources.key(config_source_value) - - create(:ci_pipeline, config_source: config_source, project: project) + dangling_source = ::Enums::Ci::Pipeline.dangling_sources.each_value.first + create(:ci_pipeline, source: dangling_source, project: project) end it 'resolves pipeline for the passed iid' do diff --git a/spec/graphql/types/issuable_severity_enum_spec.rb b/spec/graphql/types/issuable_severity_enum_spec.rb new file mode 100644 index 00000000000..3f8bd76fa62 --- /dev/null +++ b/spec/graphql/types/issuable_severity_enum_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Types::IssuableSeverityEnum do + specify { expect(described_class.graphql_name).to eq('IssuableSeverity') } + + it 'exposes all the existing issuable severity values' do + expect(described_class.values.keys).to contain_exactly( + *%w[UNKNOWN LOW MEDIUM HIGH CRITICAL] + ) + end +end diff --git a/spec/graphql/types/issue_type_spec.rb b/spec/graphql/types/issue_type_spec.rb index 282d6b76023..db2a1751be0 100644 --- a/spec/graphql/types/issue_type_spec.rb +++ b/spec/graphql/types/issue_type_spec.rb @@ -15,7 +15,7 @@ RSpec.describe GitlabSchema.types['Issue'] do fields = %i[id iid title description state reference author assignees participants labels milestone due_date confidential discussion_locked upvotes downvotes user_notes_count web_path web_url relative_position subscribed time_estimate total_time_spent closed_at created_at updated_at task_completion_status - designs design_collection alert_management_alert] + designs design_collection alert_management_alert severity] fields.each do |field_name| expect(described_class).to have_graphql_field(field_name) diff --git a/spec/haml_lint/linter/documentation_links_spec.rb b/spec/haml_lint/linter/documentation_links_spec.rb index 68de8317b82..aa4c5dd7c39 100644 --- a/spec/haml_lint/linter/documentation_links_spec.rb +++ b/spec/haml_lint/linter/documentation_links_spec.rb @@ -20,6 +20,13 @@ RSpec.describe HamlLint::Linter::DocumentationLinks do it { is_expected.not_to report_lint } end + # TODO: Remove me after https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39715 is merged + context 'when link_to points to the existing file with partially matching anchor' do + let(:haml) { "= link_to 'Description', help_page_path('README.md', anchor: 'overview-premium'), target: '_blank'" } + + it { is_expected.not_to report_lint } + end + context 'when link_to points to the existing file path without .md extension' do let(:haml) { "= link_to 'Description', help_page_path('README')" } diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb index 426bca2ced2..a419b6b9c84 100644 --- a/spec/helpers/submodule_helper_spec.rb +++ b/spec/helpers/submodule_helper_spec.rb @@ -24,7 +24,11 @@ RSpec.describe SubmoduleHelper do allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(22) # set this just to be sure allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix)) stub_url([config.ssh_user, '@', config.host, ':gitlab-org/gitlab-foss.git'].join('')) - expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')]) + aggregate_failures do + expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss')) + expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')) + expect(subject.compare).to be_nil + end end it 'detects ssh on standard port without a username' do @@ -32,14 +36,22 @@ RSpec.describe SubmoduleHelper do allow(Gitlab.config.gitlab_shell).to receive(:ssh_user).and_return('') allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix)) stub_url([config.host, ':gitlab-org/gitlab-foss.git'].join('')) - expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')]) + aggregate_failures do + expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss')) + expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')) + expect(subject.compare).to be_nil + end end it 'detects ssh on non-standard port' do allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(2222) allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix)) stub_url(['ssh://', config.ssh_user, '@', config.host, ':2222/gitlab-org/gitlab-foss.git'].join('')) - expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')]) + aggregate_failures do + expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss')) + expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')) + expect(subject.compare).to be_nil + end end it 'detects ssh on non-standard port without a username' do @@ -47,21 +59,33 @@ RSpec.describe SubmoduleHelper do allow(Gitlab.config.gitlab_shell).to receive(:ssh_user).and_return('') allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix)) stub_url(['ssh://', config.host, ':2222/gitlab-org/gitlab-foss.git'].join('')) - expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')]) + aggregate_failures do + expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss')) + expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')) + expect(subject.compare).to be_nil + end end it 'detects http on standard port' do allow(Gitlab.config.gitlab).to receive(:port).and_return(80) allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) stub_url(['http://', config.host, '/gitlab-org/gitlab-foss.git'].join('')) - expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')]) + aggregate_failures do + expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss')) + expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')) + expect(subject.compare).to be_nil + end end it 'detects http on non-standard port' do allow(Gitlab.config.gitlab).to receive(:port).and_return(3000) allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) stub_url(['http://', config.host, ':3000/gitlab-org/gitlab-foss.git'].join('')) - expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')]) + aggregate_failures do + expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss')) + expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')) + expect(subject.compare).to be_nil + end end it 'works with relative_url_root' do @@ -69,7 +93,11 @@ RSpec.describe SubmoduleHelper do allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root') allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) stub_url(['http://', config.host, '/gitlab/root/gitlab-org/gitlab-foss.git'].join('')) - expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')]) + aggregate_failures do + expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss')) + expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')) + expect(subject.compare).to be_nil + end end it 'works with subgroups' do @@ -77,61 +105,105 @@ RSpec.describe SubmoduleHelper do allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root') allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) stub_url(['http://', config.host, '/gitlab/root/gitlab-org/sub/gitlab-foss.git'].join('')) - expect(subject).to eq([namespace_project_path('gitlab-org/sub', 'gitlab-foss'), namespace_project_tree_path('gitlab-org/sub', 'gitlab-foss', 'hash')]) + aggregate_failures do + expect(subject.web).to eq(namespace_project_path('gitlab-org/sub', 'gitlab-foss')) + expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org/sub', 'gitlab-foss', 'hash')) + expect(subject.compare).to be_nil + end end end context 'submodule on gist.github.com' do it 'detects ssh' do stub_url('git@gist.github.com:gitlab-org/gitlab-foss.git') - is_expected.to eq(['https://gist.github.com/gitlab-org/gitlab-foss', 'https://gist.github.com/gitlab-org/gitlab-foss/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gist.github.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gist.github.com/gitlab-org/gitlab-foss/hash') + expect(subject.compare).to be_nil + end end it 'detects http' do stub_url('http://gist.github.com/gitlab-org/gitlab-foss.git') - is_expected.to eq(['https://gist.github.com/gitlab-org/gitlab-foss', 'https://gist.github.com/gitlab-org/gitlab-foss/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gist.github.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gist.github.com/gitlab-org/gitlab-foss/hash') + expect(subject.compare).to be_nil + end end it 'detects https' do stub_url('https://gist.github.com/gitlab-org/gitlab-foss.git') - is_expected.to eq(['https://gist.github.com/gitlab-org/gitlab-foss', 'https://gist.github.com/gitlab-org/gitlab-foss/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gist.github.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gist.github.com/gitlab-org/gitlab-foss/hash') + expect(subject.compare).to be_nil + end end it 'handles urls with no .git on the end' do stub_url('http://gist.github.com/gitlab-org/gitlab-foss') - is_expected.to eq(['https://gist.github.com/gitlab-org/gitlab-foss', 'https://gist.github.com/gitlab-org/gitlab-foss/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gist.github.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gist.github.com/gitlab-org/gitlab-foss/hash') + expect(subject.compare).to be_nil + end end it 'returns original with non-standard url' do stub_url('http://gist.github.com/another/gitlab-org/gitlab-foss.git') - is_expected.to eq([repo.submodule_url_for, nil]) + aggregate_failures do + expect(subject.web).to eq(repo.submodule_url_for) + expect(subject.tree).to be_nil + expect(subject.compare).to be_nil + end end end context 'submodule on github.com' do it 'detects ssh' do stub_url('git@github.com:gitlab-org/gitlab-foss.git') - expect(subject).to eq(['https://github.com/gitlab-org/gitlab-foss', 'https://github.com/gitlab-org/gitlab-foss/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://github.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://github.com/gitlab-org/gitlab-foss/tree/hash') + expect(subject.compare).to be_nil + end end it 'detects http' do stub_url('http://github.com/gitlab-org/gitlab-foss.git') - expect(subject).to eq(['https://github.com/gitlab-org/gitlab-foss', 'https://github.com/gitlab-org/gitlab-foss/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://github.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://github.com/gitlab-org/gitlab-foss/tree/hash') + expect(subject.compare).to be_nil + end end it 'detects https' do stub_url('https://github.com/gitlab-org/gitlab-foss.git') - expect(subject).to eq(['https://github.com/gitlab-org/gitlab-foss', 'https://github.com/gitlab-org/gitlab-foss/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://github.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://github.com/gitlab-org/gitlab-foss/tree/hash') + expect(subject.compare).to be_nil + end end it 'handles urls with no .git on the end' do stub_url('http://github.com/gitlab-org/gitlab-foss') - expect(subject).to eq(['https://github.com/gitlab-org/gitlab-foss', 'https://github.com/gitlab-org/gitlab-foss/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://github.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://github.com/gitlab-org/gitlab-foss/tree/hash') + expect(subject.compare).to be_nil + end end it 'returns original with non-standard url' do stub_url('http://github.com/another/gitlab-org/gitlab-foss.git') - expect(subject).to eq([repo.submodule_url_for, nil]) + aggregate_failures do + expect(subject.web).to eq(repo.submodule_url_for) + expect(subject.tree).to be_nil + expect(subject.compare).to be_nil + end end end @@ -143,39 +215,67 @@ RSpec.describe SubmoduleHelper do allow(repo).to receive(:project).and_return(project) stub_url('./') - expect(subject).to eq(["/master-project/#{project.path}", "/master-project/#{project.path}/-/tree/hash"]) + aggregate_failures do + expect(subject.web).to eq("/master-project/#{project.path}") + expect(subject.tree).to eq("/master-project/#{project.path}/-/tree/hash") + expect(subject.compare).to be_nil + end end end context 'submodule on gitlab.com' do it 'detects ssh' do stub_url('git@gitlab.com:gitlab-org/gitlab-foss.git') - expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to be_nil + end end it 'detects http' do stub_url('http://gitlab.com/gitlab-org/gitlab-foss.git') - expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to be_nil + end end it 'detects https' do stub_url('https://gitlab.com/gitlab-org/gitlab-foss.git') - expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to be_nil + end end it 'handles urls with no .git on the end' do stub_url('http://gitlab.com/gitlab-org/gitlab-foss') - expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to be_nil + end end it 'handles urls with trailing whitespace' do stub_url('http://gitlab.com/gitlab-org/gitlab-foss.git ') - expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to be_nil + end end it 'returns original with non-standard url' do stub_url('http://gitlab.com/another/gitlab-org/gitlab-foss.git') - expect(subject).to eq([repo.submodule_url_for, nil]) + aggregate_failures do + expect(subject.web).to eq(repo.submodule_url_for) + expect(subject.tree).to be_nil + expect(subject.compare).to be_nil + end end end @@ -183,25 +283,29 @@ RSpec.describe SubmoduleHelper do it 'sanitizes unsupported protocols' do stub_url('javascript:alert("XSS");') - expect(subject).to eq([nil, nil]) + expect(subject).to be_nil end it 'sanitizes unsupported protocols disguised as a repository URL' do stub_url('javascript:alert("XSS");foo/bar.git') - expect(subject).to eq([nil, nil]) + expect(subject).to be_nil end it 'sanitizes invalid URL with extended ASCII' do stub_url('é') - expect(subject).to eq([nil, nil]) + expect(subject).to be_nil end it 'returns original' do stub_url('http://mygitserver.com/gitlab-org/gitlab-foss') - expect(subject).to eq([repo.submodule_url_for, nil]) + aggregate_failures do + expect(subject.web).to eq(repo.submodule_url_for) + expect(subject.tree).to be_nil + expect(subject.compare).to be_nil + end end end @@ -214,7 +318,11 @@ RSpec.describe SubmoduleHelper do stub_url(relative_path) result = subject - expect(result).to eq([expected_path, "#{expected_path}/-/tree/#{submodule_item.id}"]) + aggregate_failures do + expect(result.web).to eq(expected_path) + expect(result.tree).to eq("#{expected_path}/-/tree/#{submodule_item.id}") + expect(result.compare).to be_nil + end end it 'handles project under same group' do @@ -235,7 +343,7 @@ RSpec.describe SubmoduleHelper do result = subject - expect(result).to eq([nil, nil]) + expect(result).to be_nil end end @@ -245,7 +353,7 @@ RSpec.describe SubmoduleHelper do result = subject - expect(result).to eq([nil, nil]) + expect(result).to be_nil end end @@ -289,7 +397,7 @@ RSpec.describe SubmoduleHelper do end it 'returns no links' do - expect(subject).to eq([nil, nil]) + expect(subject).to be_nil end end end diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index d0fe9163c6e..ccfde95b00c 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -189,6 +189,62 @@ RSpec.describe API::Helpers do end end + describe '#increment_unique_values' do + let(:value) { '9f302fea-f828-4ca9-aef4-e10bd723c0b3' } + let(:event_name) { 'my_event' } + let(:unknown_event) { 'unknown' } + let(:feature) { "usage_data_#{event_name}" } + + context 'with feature enabled' do + before do + stub_feature_flags(feature => true) + end + + it 'tracks redis hll event' do + stub_application_setting(usage_ping_enabled: true) + + expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(value, event_name) + + subject.increment_unique_values(event_name, value) + end + + it 'does not track event usage ping is not enabled' do + stub_application_setting(usage_ping_enabled: false) + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + subject.increment_unique_values(event_name, value) + end + + it 'logs an exception for unknown event' do + stub_application_setting(usage_ping_enabled: true) + + expect(Gitlab::AppLogger).to receive(:warn).with("Redis tracking event failed for event: #{unknown_event}, message: Unknown event #{unknown_event}") + + subject.increment_unique_values(unknown_event, value) + end + + it 'does not track event for nil values' do + stub_application_setting(usage_ping_enabled: true) + + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + subject.increment_unique_values(unknown_event, nil) + end + end + + context 'with feature disabled' do + before do + stub_feature_flags(feature => false) + end + + it 'does not track event' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + subject.increment_unique_values(event_name, value) + end + end + end + describe '#order_options_with_tie_breaker' do subject { Class.new.include(described_class).new.order_options_with_tie_breaker } diff --git a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb index 2d17855707f..6e90f4457fa 100644 --- a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb +++ b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb @@ -65,6 +65,11 @@ RSpec.describe Banzai::Filter::TableOfContentsFilter do expect(doc.css('h1 a').first.attr('href')).to eq '#title-with-spaces' end + it 'removes a product suffix' do + doc = filter(header(1, "Title with spaces (ULTIMATE)")) + expect(doc.css('h1 a').first.attr('href')).to eq '#title-with-spaces' + end + it 'appends a unique number to duplicates' do doc = filter(header(1, 'One') + header(2, 'One')) diff --git a/spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb b/spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb index 47b2cf5dc4a..507440064bc 100644 --- a/spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb +++ b/spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb @@ -9,6 +9,8 @@ RSpec.describe Gitlab::Cleanup::OrphanLfsFileReferences do let!(:invalid_reference) { create(:lfs_objects_project, project: project, lfs_object: lfs_object) } + subject(:service) { described_class.new(project, logger: null_logger, dry_run: dry_run) } + before do allow(null_logger).to receive(:info) @@ -21,25 +23,66 @@ RSpec.describe Gitlab::Cleanup::OrphanLfsFileReferences do end context 'dry run' do + let(:dry_run) { true } + it 'prints messages and does not delete references' do expect(null_logger).to receive(:info).with("[DRY RUN] Looking for orphan LFS files for project #{project.name_with_namespace}") expect(null_logger).to receive(:info).with("[DRY RUN] Found invalid references: 1") - expect { described_class.new(project, logger: null_logger).run! } - .not_to change { project.lfs_objects.count } + expect { service.run! }.not_to change { project.lfs_objects.count } end end context 'regular run' do + let(:dry_run) { false } + it 'prints messages and deletes invalid reference' do expect(null_logger).to receive(:info).with("Looking for orphan LFS files for project #{project.name_with_namespace}") expect(null_logger).to receive(:info).with("Removed invalid references: 1") expect(ProjectCacheWorker).to receive(:perform_async).with(project.id, [], [:lfs_objects_size]) - expect { described_class.new(project, logger: null_logger, dry_run: false).run! } - .to change { project.lfs_objects.count }.from(2).to(1) + expect { service.run! }.to change { project.lfs_objects.count }.from(2).to(1) expect(LfsObjectsProject.exists?(invalid_reference.id)).to be_falsey end + + context 'LFS object is in design repository' do + before do + expect(project.design_repository).to receive(:exists?).and_return(true) + + stub_lfs_pointers(project.design_repository, lfs_object.oid) + end + + it 'is not removed' do + expect { service.run! }.not_to change { project.lfs_objects.count } + end + end + + context 'LFS object is in wiki repository' do + before do + expect(project.wiki.repository).to receive(:exists?).and_return(true) + + stub_lfs_pointers(project.wiki.repository, lfs_object.oid) + end + + it 'is not removed' do + expect { service.run! }.not_to change { project.lfs_objects.count } + end + end + end + + context 'LFS for project snippets' do + let(:snippet) { create(:project_snippet) } + + it 'is disabled' do + # Support project snippets here before enabling LFS for them + expect(snippet.repository.lfs_enabled?).to be_falsy + end + end + + def stub_lfs_pointers(repo, *oids) + expect(repo.gitaly_blob_client) + .to receive(:get_all_lfs_pointers) + .and_return(oids.map { |oid| OpenStruct.new(lfs_oid: oid) }) end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 69a86225219..d0ffda61cc1 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -15,6 +15,7 @@ issues: - resource_iteration_events - sent_notifications - sentry_issue +- issuable_severity - label_links - labels - last_edited_by @@ -649,6 +650,8 @@ zoom_meetings: - issue sentry_issue: - issue +issuable_severity: +- issue design_versions: *version epic: - subscriptions diff --git a/spec/lib/gitlab/submodule_links_spec.rb b/spec/lib/gitlab/submodule_links_spec.rb index c69326e12be..e2bbda81780 100644 --- a/spec/lib/gitlab/submodule_links_spec.rb +++ b/spec/lib/gitlab/submodule_links_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Gitlab::SubmoduleLinks do end it 'returns no links' do - expect(subject).to eq([nil, nil]) + expect(subject).to be_nil end end @@ -28,17 +28,28 @@ RSpec.describe Gitlab::SubmoduleLinks do end it 'returns no links' do - expect(subject).to eq([nil, nil]) + expect(subject).to be_nil end end context 'when the submodule is known' do before do - stub_urls({ 'gitlab-foss' => 'git@gitlab.com:gitlab-org/gitlab-foss.git' }) + gitlab_foss = 'git@gitlab.com:gitlab-org/gitlab-foss.git' + + stub_urls({ + 'ref' => { 'gitlab-foss' => gitlab_foss }, + 'other_ref' => { 'gitlab-foss' => gitlab_foss }, + 'signed-commits' => { 'gitlab-foss' => gitlab_foss }, + 'special_ref' => { 'gitlab-foss' => 'git@OTHER.com:gitlab-org/gitlab-foss.git' } + }) end it 'returns links and caches the by ref' do - expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to be_nil + end cache_store = links.instance_variable_get("@cache_store") @@ -49,13 +60,46 @@ RSpec.describe Gitlab::SubmoduleLinks do let(:ref) { 'signed-commits' } it 'returns links' do - expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash']) + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to be_nil + end + end + end + + context 'and the diff information is available' do + let(:old_ref) { 'other_ref' } + let(:diff_file) { double(old_blob: double(id: 'old-hash', path: 'gitlab-foss'), old_content_sha: old_ref) } + + subject { links.for(submodule_item, ref, diff_file) } + + it 'the returned links include the compare link' do + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/compare/old-hash...hash') + end + end + + context 'but the submodule url has changed' do + let(:old_ref) { 'special_ref' } + + it 'the returned links do not include the compare link' do + aggregate_failures do + expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss') + expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash') + expect(subject.compare).to be_nil + end + end end end end end - def stub_urls(urls) - allow(repo).to receive(:submodule_urls_for).and_return(urls) + def stub_urls(urls_by_ref) + allow(repo).to receive(:submodule_urls_for) do |ref| + urls_by_ref[ref] if urls_by_ref + end end end diff --git a/spec/lib/gitlab/utils/markdown_spec.rb b/spec/lib/gitlab/utils/markdown_spec.rb index 001ff5bc487..0ac594ca830 100644 --- a/spec/lib/gitlab/utils/markdown_spec.rb +++ b/spec/lib/gitlab/utils/markdown_spec.rb @@ -52,6 +52,22 @@ RSpec.describe Gitlab::Utils::Markdown do end end + context 'when string has a product suffix' do + let(:string) { 'My Header (ULTIMATE)' } + + it 'ignores a product suffix' do + is_expected.to eq 'my-header' + end + + context 'with only modifier' do + let(:string) { 'My Header (STARTER ONLY)' } + + it 'ignores a product suffix' do + is_expected.to eq 'my-header' + end + end + end + context 'when string is empty' do let(:string) { '' } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 297afd4d3ad..35f912f4875 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -221,6 +221,26 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '.ci_sources' do + subject { described_class.ci_sources } + + let!(:push_pipeline) { create(:ci_pipeline, source: :push) } + let!(:web_pipeline) { create(:ci_pipeline, source: :web) } + let!(:api_pipeline) { create(:ci_pipeline, source: :api) } + let!(:webide_pipeline) { create(:ci_pipeline, source: :webide) } + let!(:child_pipeline) { create(:ci_pipeline, source: :parent_pipeline) } + + it 'contains pipelines having CI only sources' do + expect(subject).to contain_exactly(push_pipeline, web_pipeline, api_pipeline) + end + + it 'filters on expected sources' do + expect(::Enums::Ci::Pipeline.ci_sources.keys).to contain_exactly( + *%i[unknown push web trigger schedule api external pipeline chat + merge_request_event external_pull_request_event]) + end + end + describe '.outside_pipeline_family' do subject(:outside_pipeline_family) { described_class.outside_pipeline_family(upstream_pipeline) } @@ -1239,14 +1259,42 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'pipeline caching' do - before do - pipeline.config_source = 'repository_source' + context 'if pipeline is cacheable' do + before do + pipeline.source = 'push' + end + + it 'performs ExpirePipelinesCacheWorker' do + expect(ExpirePipelineCacheWorker).to receive(:perform_async).with(pipeline.id) + + pipeline.cancel + end end - it 'performs ExpirePipelinesCacheWorker' do - expect(ExpirePipelineCacheWorker).to receive(:perform_async).with(pipeline.id) + context 'if pipeline is not cacheable' do + before do + pipeline.source = 'webide' + end - pipeline.cancel + it 'deos not perform ExpirePipelinesCacheWorker' do + expect(ExpirePipelineCacheWorker).not_to receive(:perform_async) + + pipeline.cancel + end + end + end + + describe '#dangling?' do + it 'returns true if pipeline comes from any dangling sources' do + pipeline.source = Enums::Ci::Pipeline.dangling_sources.each_key.first + + expect(pipeline).to be_dangling + end + + it 'returns true if pipeline comes from any CI sources' do + pipeline.source = Enums::Ci::Pipeline.ci_sources.each_key.first + + expect(pipeline).not_to be_dangling end end @@ -1959,12 +2007,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '.last_finished_for_ref_id' do let(:branch) { project.default_branch } let(:ref) { project.ci_refs.take } - let(:config_source) { Enums::Ci::Pipeline.config_sources[:parameter_source] } + let(:dangling_source) { Enums::Ci::Pipeline.sources[:ondemand_dast_scan] } let!(:pipeline1) { create(:ci_pipeline, :success, project: project, ref: branch) } let!(:pipeline2) { create(:ci_pipeline, :success, project: project, ref: branch) } let!(:pipeline3) { create(:ci_pipeline, :failed, project: project, ref: branch) } let!(:pipeline4) { create(:ci_pipeline, :success, project: project, ref: branch) } - let!(:pipeline5) { create(:ci_pipeline, :success, project: project, ref: branch, config_source: config_source) } + let!(:pipeline5) { create(:ci_pipeline, :success, project: project, ref: branch, source: dangling_source) } it 'returns the expected pipeline' do result = described_class.last_finished_for_ref_id(ref.id) diff --git a/spec/models/ci/ref_spec.rb b/spec/models/ci/ref_spec.rb index c9069d82e07..cb62646532c 100644 --- a/spec/models/ci/ref_spec.rb +++ b/spec/models/ci/ref_spec.rb @@ -107,8 +107,8 @@ RSpec.describe Ci::Ref do describe '#last_finished_pipeline_id' do let(:pipeline_status) { :running } - let(:config_source) { Enums::Ci::Pipeline.config_sources[:repository_source] } - let(:pipeline) { create(:ci_pipeline, pipeline_status, config_source: config_source) } + let(:pipeline_source) { Enums::Ci::Pipeline.sources[:push] } + let(:pipeline) { create(:ci_pipeline, pipeline_status, source: pipeline_source) } let(:ci_ref) { pipeline.ci_ref } context 'when there are no finished pipelines' do @@ -124,8 +124,8 @@ RSpec.describe Ci::Ref do expect(ci_ref.last_finished_pipeline_id).to eq(pipeline.id) end - context 'when the pipeline is not a ci_source' do - let(:config_source) { Enums::Ci::Pipeline.config_sources[:parameter_source] } + context 'when the pipeline a dangling pipeline' do + let(:pipeline_source) { Enums::Ci::Pipeline.sources[:ondemand_dast_scan] } it 'returns nil' do expect(ci_ref.last_finished_pipeline_id).to be_nil diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 8d2eb3b5e2a..b00395cd926 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -854,4 +854,41 @@ RSpec.describe Issuable do it { is_expected.to eq(incident) } end end + + describe '#severity' do + subject { issuable.severity } + + context 'when issuable is not an incident' do + using RSpec::Parameterized::TableSyntax + + where(:issuable_type, :severity) do + :issue | 'unknown' + :merge_request | 'unknown' + end + + with_them do + let(:issuable) { build_stubbed(issuable_type) } + + it { is_expected.to eq(severity) } + end + end + + context 'when issuable type is an incident' do + let!(:issuable) { build_stubbed(:incident) } + + context 'when incident does not have issuable_severity' do + it 'returns default serverity' do + is_expected.to eq(IssuableSeverity::DEFAULT) + end + end + + context 'when incident has issuable_severity' do + let!(:issuable_severity) { build_stubbed(:issuable_severity, issue: issuable, severity: 'critical') } + + it 'returns issuable serverity' do + is_expected.to eq('critical') + end + end + end + end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 6ea94b4cd1a..1c7b11257ce 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -44,10 +44,14 @@ RSpec.describe Deployment do describe 'modules' do it_behaves_like 'AtomicInternalId' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:deployable) { create(:ci_build, project: project) } + let_it_be(:environment) { create(:environment, project: project) } + let(:internal_id_attribute) { :iid } - let(:instance) { build(:deployment) } + let(:instance) { build(:deployment, deployable: deployable, environment: environment) } let(:scope) { :project } - let(:scope_attrs) { { project: instance.project } } + let(:scope_attrs) { { project: project } } let(:usage) { :deployments } end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index f869b586fbe..f1902d19993 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1212,4 +1212,14 @@ RSpec.describe Issue do end end end + + describe '#allows_reviewers?' do + it 'returns false as issues do not support reviewers feature' do + stub_feature_flags(merge_request_reviewers: true) + + issue = build_stubbed(:issue) + + expect(issue.allows_reviewers?).to be(false) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c9389ad7f43..3563bbb2867 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -881,8 +881,10 @@ RSpec.describe MergeRequest, factory_default: :keep do end describe '#diff_size' do + let_it_be(:project) { create(:project, :repository) } + let(:merge_request) do - build(:merge_request, source_branch: 'expand-collapse-files', target_branch: 'master') + build(:merge_request, source_project: project, source_branch: 'expand-collapse-files', target_branch: 'master') end context 'when there are MR diffs' do @@ -1512,7 +1514,9 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'new merge request' do - subject { build(:merge_request) } + let_it_be(:project) { create(:project, :repository) } + + subject { build(:merge_request, source_project: project) } context 'compare commits' do before do @@ -2116,11 +2120,13 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when merge request is not persisted' do + let_it_be(:project) { create(:project, :repository) } + context 'when compare commits are set in the service' do let(:commit) { spy('commit') } subject do - build(:merge_request, compare_commits: [commit, commit]) + build(:merge_request, source_project: project, compare_commits: [commit, commit]) end it 'returns commits from compare commits temporary data' do @@ -2129,7 +2135,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when compare commits are not set in the service' do - subject { build(:merge_request) } + subject { build(:merge_request, source_project: project) } it 'returns array with diff head sha element only' do expect(subject.all_commit_shas).to eq [subject.diff_head_sha] @@ -2467,6 +2473,57 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(subject.mergeable?).to be_truthy end + + context 'with skip_ci_check option' do + using RSpec::Parameterized::TableSyntax + + before do + allow(subject).to receive_messages(check_mergeability: nil, + can_be_merged?: true, + broken?: false) + end + + where(:mergeable_ci_state, :skip_ci_check, :expected_mergeable) do + false | false | false + false | true | true + true | false | true + true | true | true + end + + with_them do + it 'overrides mergeable_ci_state?' do + allow(subject).to receive(:mergeable_ci_state?) { mergeable_ci_state } + + expect(subject.mergeable?(skip_ci_check: skip_ci_check)).to eq(expected_mergeable) + end + end + end + + context 'with skip_discussions_check option' do + using RSpec::Parameterized::TableSyntax + + before do + allow(subject).to receive_messages(mergeable_ci_state?: true, + check_mergeability: nil, + can_be_merged?: true, + broken?: false) + end + + where(:mergeable_discussions_state, :skip_discussions_check, :expected_mergeable) do + false | false | false + false | true | true + true | false | true + true | true | true + end + + with_them do + it 'overrides mergeable_discussions_state?' do + allow(subject).to receive(:mergeable_discussions_state?) { mergeable_discussions_state } + + expect(subject.mergeable?(skip_discussions_check: skip_discussions_check)).to eq(expected_mergeable) + end + end + end end describe '#check_mergeability' do @@ -2570,6 +2627,10 @@ RSpec.describe MergeRequest, factory_default: :keep do it 'returns false' do expect(subject.mergeable_state?).to be_falsey end + + it 'returns true when skipping ci check' do + expect(subject.mergeable_state?(skip_ci_check: true)).to be(true) + end end context 'when #mergeable_discussions_state? is false' do @@ -2637,9 +2698,9 @@ RSpec.describe MergeRequest, factory_default: :keep do let(:pipeline) { create(:ci_empty_pipeline) } context 'when it is only allowed to merge when build is green' do - let_it_be(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } + let_it_be(:project) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true) } - subject { build(:merge_request, target_project: project) } + subject { build(:merge_request, source_project: project) } context 'and a failed pipeline is associated' do before do @@ -2678,9 +2739,9 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when it is only allowed to merge when build is green or skipped' do - let_it_be(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true, allow_merge_on_skipped_pipeline: true) } + let_it_be(:project) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true, allow_merge_on_skipped_pipeline: true) } - subject { build(:merge_request, target_project: project) } + subject { build(:merge_request, source_project: project) } context 'and a failed pipeline is associated' do before do @@ -2719,9 +2780,9 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when merges are not restricted to green builds' do - let_it_be(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: false) } + let_it_be(:project) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: false) } - subject { build(:merge_request, target_project: project) } + subject { build(:merge_request, source_project: project) } context 'and a failed pipeline is associated' do before do @@ -4145,4 +4206,22 @@ RSpec.describe MergeRequest, factory_default: :keep do .with_arguments(allow_nil: true) end end + + describe '#allows_reviewers?' do + it 'returns false without merge_request_reviewers feature' do + stub_feature_flags(merge_request_reviewers: false) + + merge_request = build_stubbed(:merge_request) + + expect(merge_request.allows_reviewers?).to be(false) + end + + it 'returns true with merge_request_reviewers feature' do + stub_feature_flags(merge_request_reviewers: true) + + merge_request = build_stubbed(:merge_request) + + expect(merge_request.allows_reviewers?).to be(true) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8ed0672af25..efa2353259c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -522,9 +522,10 @@ RSpec.describe Project do before do create(:ci_pipeline, project: project, ref: 'master', source: :web) create(:ci_pipeline, project: project, ref: 'master', source: :external) + create(:ci_pipeline, project: project, ref: 'master', source: :webide) end - it 'has ci pipelines' do + it 'excludes dangling pipelines such as :webide' do expect(project.ci_pipelines.size).to eq(2) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3733e47334a..a0ec61c4117 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -712,30 +712,40 @@ RSpec.describe User do expect(users_with_two_factor).not_to include(user_without_2fa.id) end - it "returns users with 2fa enabled via U2F" do - user_with_2fa = create(:user, :two_factor_via_u2f) - user_without_2fa = create(:user) - users_with_two_factor = described_class.with_two_factor.pluck(:id) + shared_examples "returns the right users" do |trait| + it "returns users with 2fa enabled via hardware token" do + user_with_2fa = create(:user, trait) + user_without_2fa = create(:user) + users_with_two_factor = described_class.with_two_factor.pluck(:id) - expect(users_with_two_factor).to include(user_with_2fa.id) - expect(users_with_two_factor).not_to include(user_without_2fa.id) - end + expect(users_with_two_factor).to include(user_with_2fa.id) + expect(users_with_two_factor).not_to include(user_without_2fa.id) + end - it "returns users with 2fa enabled via OTP and U2F" do - user_with_2fa = create(:user, :two_factor_via_otp, :two_factor_via_u2f) - user_without_2fa = create(:user) - users_with_two_factor = described_class.with_two_factor.pluck(:id) + it "returns users with 2fa enabled via OTP and hardware token" do + user_with_2fa = create(:user, :two_factor_via_otp, trait) + user_without_2fa = create(:user) + users_with_two_factor = described_class.with_two_factor.pluck(:id) - expect(users_with_two_factor).to eq([user_with_2fa.id]) - expect(users_with_two_factor).not_to include(user_without_2fa.id) + expect(users_with_two_factor).to eq([user_with_2fa.id]) + expect(users_with_two_factor).not_to include(user_without_2fa.id) + end + + it 'works with ORDER BY' do + user_with_2fa = create(:user, :two_factor_via_otp, trait) + + expect(described_class + .with_two_factor + .reorder_by_name).to eq([user_with_2fa]) + end end - it 'works with ORDER BY' do - user_with_2fa = create(:user, :two_factor_via_otp, :two_factor_via_u2f) + describe "and U2F" do + it_behaves_like "returns the right users", :two_factor_via_u2f + end - expect(described_class - .with_two_factor - .reorder_by_name).to eq([user_with_2fa]) + describe "and WebAuthn" do + it_behaves_like "returns the right users", :two_factor_via_webauthn end end @@ -749,22 +759,44 @@ RSpec.describe User do expect(users_without_two_factor).not_to include(user_with_2fa.id) end - it "excludes users with 2fa enabled via U2F" do - user_with_2fa = create(:user, :two_factor_via_u2f) - user_without_2fa = create(:user) - users_without_two_factor = described_class.without_two_factor.pluck(:id) + describe "and u2f" do + it "excludes users with 2fa enabled via U2F" do + user_with_2fa = create(:user, :two_factor_via_u2f) + user_without_2fa = create(:user) + users_without_two_factor = described_class.without_two_factor.pluck(:id) - expect(users_without_two_factor).to include(user_without_2fa.id) - expect(users_without_two_factor).not_to include(user_with_2fa.id) + expect(users_without_two_factor).to include(user_without_2fa.id) + expect(users_without_two_factor).not_to include(user_with_2fa.id) + end + + it "excludes users with 2fa enabled via OTP and U2F" do + user_with_2fa = create(:user, :two_factor_via_otp, :two_factor_via_u2f) + user_without_2fa = create(:user) + users_without_two_factor = described_class.without_two_factor.pluck(:id) + + expect(users_without_two_factor).to include(user_without_2fa.id) + expect(users_without_two_factor).not_to include(user_with_2fa.id) + end end - it "excludes users with 2fa enabled via OTP and U2F" do - user_with_2fa = create(:user, :two_factor_via_otp, :two_factor_via_u2f) - user_without_2fa = create(:user) - users_without_two_factor = described_class.without_two_factor.pluck(:id) + describe "and webauthn" do + it "excludes users with 2fa enabled via WebAuthn" do + user_with_2fa = create(:user, :two_factor_via_webauthn) + user_without_2fa = create(:user) + users_without_two_factor = described_class.without_two_factor.pluck(:id) - expect(users_without_two_factor).to include(user_without_2fa.id) - expect(users_without_two_factor).not_to include(user_with_2fa.id) + expect(users_without_two_factor).to include(user_without_2fa.id) + expect(users_without_two_factor).not_to include(user_with_2fa.id) + end + + it "excludes users with 2fa enabled via OTP and WebAuthn" do + user_with_2fa = create(:user, :two_factor_via_otp, :two_factor_via_webauthn) + user_without_2fa = create(:user) + users_without_two_factor = described_class.without_two_factor.pluck(:id) + + expect(users_without_two_factor).to include(user_without_2fa.id) + expect(users_without_two_factor).not_to include(user_with_2fa.id) + end end end diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 20602f24078..3b611b2bb9f 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -476,17 +476,18 @@ RSpec.describe API::Ci::Pipelines do end end - context 'when config source is not ci' do - let(:non_ci_config_source) { Enums::Ci::Pipeline.non_ci_config_source_values.first } - let(:pipeline_not_ci) do - create(:ci_pipeline, config_source: non_ci_config_source, project: project) + context 'when pipeline is a dangling pipeline' do + let(:dangling_source) { Enums::Ci::Pipeline.dangling_sources.each_value.first } + + let(:dangling_pipeline) do + create(:ci_pipeline, source: dangling_source, project: project) end it 'returns the specified pipeline' do - get api("/projects/#{project.id}/pipelines/#{pipeline_not_ci.id}", user) + get api("/projects/#{project.id}/pipelines/#{dangling_pipeline.id}", user) expect(response).to have_gitlab_http_status(:ok) - expect(json_response['sha']).to eq(pipeline_not_ci.sha) + expect(json_response['sha']).to eq(dangling_pipeline.sha) end end end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index f90e04bbd2e..b63a155a729 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -239,13 +239,13 @@ RSpec.describe API::Jobs do end end - context 'when config source not ci' do - let(:non_ci_config_source) { Enums::Ci::Pipeline.non_ci_config_source_values.first } + context 'when jobs belong to a dangling pipeline' do + let(:dangling_source) { Enums::Ci::Pipeline.dangling_sources.each_value.first } let(:pipeline) do - create(:ci_pipeline, config_source: non_ci_config_source, project: project) + create(:ci_pipeline, source: dangling_source, project: project) end - it 'returns the specified pipeline' do + it 'returns pipeline jobs' do expect(response).to have_gitlab_http_status(:ok) expect(json_response[0]['pipeline']['sha']).to eq(pipeline.sha.to_s) end diff --git a/spec/serializers/diff_file_base_entity_spec.rb b/spec/serializers/diff_file_base_entity_spec.rb index bf69a50a072..94c39e11790 100644 --- a/spec/serializers/diff_file_base_entity_spec.rb +++ b/spec/serializers/diff_file_base_entity_spec.rb @@ -7,21 +7,50 @@ RSpec.describe DiffFileBaseEntity do let(:repository) { project.repository } let(:entity) { described_class.new(diff_file, options).as_json } - context 'diff for a changed submodule' do - let(:commit_sha_with_changed_submodule) do - "cfe32cf61b73a0d5e9f13e774abde7ff789b1660" - end - - let(:commit) { project.commit(commit_sha_with_changed_submodule) } + context 'submodule information for a' do + let(:commit_sha) { "" } + let(:commit) { project.commit(commit_sha) } let(:options) { { request: {}, submodule_links: Gitlab::SubmoduleLinks.new(repository) } } let(:diff_file) { commit.diffs.diff_files.to_a.last } - it do - expect(entity[:submodule]).to eq(true) - expect(entity[:submodule_link]).to eq("https://github.com/randx/six") - expect(entity[:submodule_tree_url]).to eq( - "https://github.com/randx/six/tree/409f37c4f05865e4fb208c771485f211a22c4c2d" - ) + context 'newly added submodule' do + let(:commit_sha) { "cfe32cf61b73a0d5e9f13e774abde7ff789b1660" } + + it 'says it is a submodule and contains links' do + expect(entity[:submodule]).to eq(true) + expect(entity[:submodule_link]).to eq("https://github.com/randx/six") + expect(entity[:submodule_tree_url]).to eq( + "https://github.com/randx/six/tree/409f37c4f05865e4fb208c771485f211a22c4c2d" + ) + end + + it 'has no compare url because the submodule was newly added' do + expect(entity[:submodule_compare]).to eq(nil) + end + end + + context 'changed submodule' do + # Test repo does not contain a commit that changes a submodule, so we have create such a commit here + let(:commit_sha) { repository.update_submodule(project.members[0].user, "six", "c6bc3aa2ee76cadaf0cbd325067c55450997632e", message: "Go one commit back", branch: "master") } + + it 'contains a link to compare the changes' do + expect(entity[:submodule_compare]).to eq( + { + url: "https://github.com/randx/six/compare/409f37c4f05865e4fb208c771485f211a22c4c2d...c6bc3aa2ee76cadaf0cbd325067c55450997632e", + old_sha: "409f37c4f05865e4fb208c771485f211a22c4c2d", + new_sha: "c6bc3aa2ee76cadaf0cbd325067c55450997632e" + } + ) + end + end + + context 'normal file (no submodule)' do + let(:commit_sha) { '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' } + + it 'sets submodule to false' do + expect(entity[:submodule]).to eq(false) + expect(entity[:submodule_compare]).to eq(nil) + end end end diff --git a/spec/serializers/merge_request_basic_entity_spec.rb b/spec/serializers/merge_request_basic_entity_spec.rb index 1cddd87e917..4a8bcd72d9c 100644 --- a/spec/serializers/merge_request_basic_entity_spec.rb +++ b/spec/serializers/merge_request_basic_entity_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe MergeRequestBasicEntity do - let(:resource) { build(:merge_request) } + let(:resource) { build(:merge_request, params) } + let(:params) { {} } subject do described_class.new(resource).as_json @@ -14,4 +15,28 @@ RSpec.describe MergeRequestBasicEntity do expect(subject[:merge_status]).to eq 'checking' end + + describe '#reviewers' do + let(:params) { { reviewers: [reviewer] } } + let(:reviewer) { build(:user) } + + context 'when merge_request_reviewers feature is disabled' do + it 'does not contain assignees attributes' do + stub_feature_flags(merge_request_reviewers: false) + + expect(subject[:reviewers]).to be_nil + end + end + + context 'when merge_request_reviewers feature is enabled' do + it 'contains reviewers attributes' do + stub_feature_flags(merge_request_reviewers: true) + + expect(subject[:reviewers].count).to be 1 + expect(subject[:reviewers].first.keys).to include( + :id, :name, :username, :state, :avatar_url, :web_url + ) + end + end + end end diff --git a/spec/serializers/merge_request_sidebar_extras_entity_spec.rb b/spec/serializers/merge_request_sidebar_extras_entity_spec.rb new file mode 100644 index 00000000000..74e9956c8a0 --- /dev/null +++ b/spec/serializers/merge_request_sidebar_extras_entity_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequestSidebarExtrasEntity do + let_it_be(:assignee) { build(:user) } + let_it_be(:reviewer) { build(:user) } + let_it_be(:user) { build(:user) } + let_it_be(:project) { create :project, :repository } + + let(:params) do + { + source_project: project, + target_project: project, + assignees: [assignee], + reviewers: [reviewer] + } + end + + let(:resource) do + build(:merge_request, params) + end + + let(:request) { double('request', current_user: user, project: project) } + + let(:entity) { described_class.new(resource, request: request).as_json } + + describe '#assignees' do + it 'contains assignees attributes' do + expect(entity[:assignees].count).to be 1 + expect(entity[:assignees].first.keys).to include( + :id, :name, :username, :state, :avatar_url, :web_url, :can_merge + ) + end + end + + describe '#reviewers' do + context 'when merge_request_reviewers feature is disabled' do + it 'does not contain reviewers attributes' do + stub_feature_flags(merge_request_reviewers: false) + + expect(entity[:reviewers]).to be_nil + end + end + + context 'when merge_request_reviewers feature is enabled' do + it 'contains reviewers attributes' do + stub_feature_flags(merge_request_reviewers: true) + + expect(entity[:reviewers].count).to be 1 + expect(entity[:reviewers].first.keys).to include( + :id, :name, :username, :state, :avatar_url, :web_url, :can_merge + ) + end + end + end +end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 5ccefe3d129..941e1f20999 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -339,6 +339,10 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end end end + + it_behaves_like 'reviewer_ids filter' do + let(:execute) { service.execute } + end end it_behaves_like 'issuable record that supports quick actions' do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 57733c7772d..2b0adbd7b5d 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -435,6 +435,43 @@ RSpec.describe MergeRequests::MergeService do end end end + + context 'when not mergeable' do + let!(:error_message) { 'Merge request is not mergeable' } + + context 'with failing CI' do + before do + allow(merge_request).to receive(:mergeable_ci_state?) { false } + end + + it 'logs and saves error' do + service.execute(merge_request) + + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) + end + end + + context 'with unresolved discussions' do + before do + allow(merge_request).to receive(:mergeable_discussions_state?) { false } + end + + it 'logs and saves error' do + service.execute(merge_request) + + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) + end + + context 'when passing `skip_discussions_check: true` as `options` parameter' do + it 'merges the merge request' do + service.execute(merge_request, skip_discussions_check: true) + + expect(merge_request).to be_valid + expect(merge_request).to be_merged + end + end + end + end end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 5f5a94da400..097115af4ef 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -161,6 +161,29 @@ RSpec.describe MergeRequests::UpdateService, :mailer do expect(@merge_request.merge_params["force_remove_source_branch"]).to eq("1") end end + + it_behaves_like 'reviewer_ids filter' do + let(:opts) { {} } + let(:execute) { update_merge_request(opts) } + end + + context 'with an existing reviewer' do + let(:merge_request) do + create(:merge_request, :simple, source_project: project, reviewer_ids: [user2.id]) + end + + context 'when merge_request_reviewer feature is enabled' do + before do + stub_feature_flags(merge_request_reviewer: true) + end + + let(:opts) { { reviewer_ids: [IssuableFinder::Params::NONE] } } + + it 'removes reviewers' do + expect(update_merge_request(opts).reviewers).to eq [] + end + end + end end context 'after_save callback to store_mentions' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 87d6f99e8aa..498566c5d60 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -8,9 +8,9 @@ RSpec.describe NotificationService, :mailer do include NotificationHelpers let_it_be(:project, reload: true) { create(:project, :public) } + let_it_be_with_refind(:assignee) { create(:user) } let(:notification) { described_class.new } - let(:assignee) { create(:user) } around(:example, :deliver_mails_inline) do |example| # This is a temporary `around` hook until all the examples check the @@ -1585,20 +1585,21 @@ RSpec.describe NotificationService, :mailer do describe 'Merge Requests', :deliver_mails_inline do let(:another_project) { create(:project, :public, namespace: group) } let(:assignees) { Array.wrap(assignee) } - let(:author) { create(:user) } let(:merge_request) { create :merge_request, author: author, source_project: project, assignees: assignees, description: 'cc @participant' } + let_it_be_with_reload(:author) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :public, :repository, namespace: group) } before_all do build_team(project) add_users(project) + + project.add_maintainer(author) + project.add_maintainer(assignee) end before do - project.add_maintainer(author) - assignees.each { |assignee| project.add_maintainer(assignee) } add_user_subscriptions(merge_request) update_custom_notification(:new_merge_request, @u_guest_custom, resource: project) update_custom_notification(:new_merge_request, @u_custom_global) diff --git a/spec/services/webauthn/authenticate_service_spec.rb b/spec/services/webauthn/authenticate_service_spec.rb new file mode 100644 index 00000000000..61f64f24f5e --- /dev/null +++ b/spec/services/webauthn/authenticate_service_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'webauthn/fake_client' + +RSpec.describe Webauthn::AuthenticateService do + let(:client) { WebAuthn::FakeClient.new(origin) } + let(:user) { create(:user) } + let(:challenge) { Base64.strict_encode64(SecureRandom.random_bytes(32)) } + + let(:origin) { 'http://localhost' } + + before do + create_result = client.create(challenge: challenge) # rubocop:disable Rails/SaveBang + + webauthn_credential = WebAuthn::Credential.from_create(create_result) + + registration = WebauthnRegistration.new(credential_xid: Base64.strict_encode64(webauthn_credential.raw_id), + public_key: webauthn_credential.public_key, + counter: 0, + name: 'name', + user_id: user.id) + registration.save! + end + + describe '#execute' do + it 'returns true if the response is valid and a matching stored credential is present' do + get_result = client.get(challenge: challenge) + + get_result['clientExtensionResults'] = {} + service = Webauthn::AuthenticateService.new(user, get_result.to_json, challenge) + + expect(service.execute).to be_truthy + end + + it 'returns false if the response is valid but no matching stored credential is present' do + other_client = WebAuthn::FakeClient.new(origin) + other_client.create(challenge: challenge) # rubocop:disable Rails/SaveBang + + get_result = other_client.get(challenge: challenge) + + get_result['clientExtensionResults'] = {} + service = Webauthn::AuthenticateService.new(user, get_result.to_json, challenge) + + expect(service.execute).to be_falsey + end + end +end diff --git a/spec/services/webauthn/register_service_spec.rb b/spec/services/webauthn/register_service_spec.rb new file mode 100644 index 00000000000..bb9fa2080d2 --- /dev/null +++ b/spec/services/webauthn/register_service_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'webauthn/fake_client' + +RSpec.describe Webauthn::RegisterService do + let(:client) { WebAuthn::FakeClient.new(origin) } + let(:user) { create(:user) } + let(:challenge) { Base64.strict_encode64(SecureRandom.random_bytes(32)) } + + let(:origin) { 'http://localhost' } + + describe '#execute' do + it 'returns a registration if challenge matches' do + create_result = client.create(challenge: challenge) # rubocop:disable Rails/SaveBang + webauthn_credential = WebAuthn::Credential.from_create(create_result) + + params = { device_response: create_result.to_json, name: 'abc' } + service = Webauthn::RegisterService.new(user, params, challenge) + + registration = service.execute + expect(registration.credential_xid).to eq(Base64.strict_encode64(webauthn_credential.raw_id)) + expect(registration.errors.size).to eq(0) + end + + it 'returns an error if challenge does not match' do + create_result = client.create(challenge: Base64.strict_encode64(SecureRandom.random_bytes(16))) # rubocop:disable Rails/SaveBang + + params = { device_response: create_result.to_json, name: 'abc' } + service = Webauthn::RegisterService.new(user, params, challenge) + + registration = service.execute + expect(registration.errors.size).to eq(1) + end + end +end diff --git a/spec/support/helpers/fake_u2f_device.rb b/spec/support/helpers/fake_u2f_device.rb index f765b277175..2ed1222ebd3 100644 --- a/spec/support/helpers/fake_u2f_device.rb +++ b/spec/support/helpers/fake_u2f_device.rb @@ -3,9 +3,10 @@ class FakeU2fDevice attr_reader :name - def initialize(page, name) + def initialize(page, name, device = nil) @page = page @name = name + @u2f_device = device end def respond_to_u2f_registration diff --git a/spec/support/helpers/fake_webauthn_device.rb b/spec/support/helpers/fake_webauthn_device.rb new file mode 100644 index 00000000000..d2c2f7d6bf3 --- /dev/null +++ b/spec/support/helpers/fake_webauthn_device.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true +require 'webauthn/fake_client' + +class FakeWebauthnDevice + attr_reader :name + + def initialize(page, name, device = nil) + @page = page + @name = name + @webauthn_device = device + end + + def respond_to_webauthn_registration + app_id = @page.evaluate_script('gon.webauthn.app_id') + challenge = @page.evaluate_script('gon.webauthn.options.challenge') + + json_response = webauthn_device(app_id).create(challenge: challenge).to_json # rubocop:disable Rails/SaveBang + @page.execute_script <<~JS + var result = #{json_response}; + result.getClientExtensionResults = () => ({}); + navigator.credentials.create = function(_) { + return Promise.resolve(result); + }; + JS + end + + def respond_to_webauthn_authentication + app_id = @page.evaluate_script('JSON.parse(gon.webauthn.options).extensions.appid') + challenge = @page.evaluate_script('JSON.parse(gon.webauthn.options).challenge') + + begin + json_response = webauthn_device(app_id).get(challenge: challenge).to_json + + rescue RuntimeError + # A runtime error is raised from fake webauthn if no credentials have been registered yet. + # To be able to test non registered devices, credentials are created ad-hoc + webauthn_device(app_id).create # rubocop:disable Rails/SaveBang + json_response = webauthn_device(app_id).get(challenge: challenge).to_json + end + + @page.execute_script <<~JS + var result = #{json_response}; + result.getClientExtensionResults = () => ({}); + navigator.credentials.get = function(_) { + return Promise.resolve(result); + }; + JS + @page.click_link('Try again?', href: false) + end + + def fake_webauthn_authentication + @page.execute_script <<~JS + const mockResponse = { + type: 'public-key', + id: '', + rawId: '', + response: { clientDataJSON: '', authenticatorData: '', signature: '', userHandle: '' }, + getClientExtensionResults: () => {}, + }; + window.gl.resolveWebauthn(mockResponse); + JS + end + + def add_credential(app_id, credential_id, credential_key) + credentials = { URI.parse(app_id).host => { credential_id => { credential_key: credential_key, sign_count: 0 } } } + webauthn_device(app_id).send(:authenticator).instance_variable_set(:@credentials, credentials) + end + + private + + def webauthn_device(app_id) + @webauthn_device ||= WebAuthn::FakeClient.new(app_id) + end +end diff --git a/spec/support/helpers/features/two_factor_helpers.rb b/spec/support/helpers/features/two_factor_helpers.rb new file mode 100644 index 00000000000..08a7665201f --- /dev/null +++ b/spec/support/helpers/features/two_factor_helpers.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true +# These helpers allow you to manage and register +# U2F and WebAuthn devices +# +# Usage: +# describe "..." do +# include Spec::Support::Helpers::Features::TwoFactorHelpers +# ... +# +# manage_two_factor_authentication +# +module Spec + module Support + module Helpers + module Features + module TwoFactorHelpers + def manage_two_factor_authentication + click_on 'Manage two-factor authentication' + expect(page).to have_content("Set up new device") + wait_for_requests + end + + def register_u2f_device(u2f_device = nil, name: 'My device') + u2f_device ||= FakeU2fDevice.new(page, name) + u2f_device.respond_to_u2f_registration + click_on 'Set up new device' + expect(page).to have_content('Your device was successfully set up') + fill_in "Pick a name", with: name + click_on 'Register device' + u2f_device + end + + # Registers webauthn device via UI + def register_webauthn_device(webauthn_device = nil, name: 'My device') + webauthn_device ||= FakeWebauthnDevice.new(page, name) + webauthn_device.respond_to_webauthn_registration + click_on 'Set up new device' + expect(page).to have_content('Your device was successfully set up') + fill_in 'Pick a name', with: name + click_on 'Register device' + webauthn_device + end + + # Adds webauthn device directly via database + def add_webauthn_device(app_id, user, fake_device = nil, name: 'My device') + fake_device ||= WebAuthn::FakeClient.new(app_id) + + options_for_create = WebAuthn::Credential.options_for_create( + user: { id: user.webauthn_xid, name: user.username }, + authenticator_selection: { user_verification: 'discouraged' }, + rp: { name: 'GitLab' } + ) + challenge = options_for_create.challenge + + device_response = fake_device.create(challenge: challenge).to_json # rubocop:disable Rails/SaveBang + device_registration_params = { device_response: device_response, + name: name } + + Webauthn::RegisterService.new( + user, device_registration_params, challenge).execute + FakeWebauthnDevice.new(page, name, fake_device) + end + + def assert_fallback_ui(page) + expect(page).to have_button('Verify code') + expect(page).to have_css('#user_otp_attempt') + expect(page).not_to have_link('Sign in via 2FA code') + expect(page).not_to have_css("#js-authenticate-token-2fa") + end + end + end + end + end +end diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index 1118cfcf7ac..e21d4497cda 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -111,6 +111,11 @@ module LoginHelpers FakeU2fDevice.new(page, nil).fake_u2f_authentication end + def fake_successful_webauthn_authentication + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(true) + FakeWebauthnDevice.new(page, nil).fake_webauthn_authentication + end + def mock_auth_hash_with_saml_xml(provider, uid, email, saml_response) response_object = { document: saml_xml(saml_response) } mock_auth_hash(provider, uid, email, response_object: response_object) diff --git a/spec/support/shared_examples/features/2fa_shared_examples.rb b/spec/support/shared_examples/features/2fa_shared_examples.rb new file mode 100644 index 00000000000..ddc03e178ba --- /dev/null +++ b/spec/support/shared_examples/features/2fa_shared_examples.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'hardware device for 2fa' do |device_type| + include Spec::Support::Helpers::Features::TwoFactorHelpers + + def register_device(device_type, **kwargs) + case device_type.downcase + when "u2f" + register_u2f_device(**kwargs) + when "webauthn" + register_webauthn_device(**kwargs) + else + raise "Unknown device type #{device_type}" + end + end + + describe "registration" do + let(:user) { create(:user) } + + before do + gitlab_sign_in(user) + user.update_attribute(:otp_required_for_login, true) + end + + describe 'when 2FA via OTP is disabled' do + before do + user.update_attribute(:otp_required_for_login, false) + end + + it 'does not allow registering a new device' do + visit profile_account_path + click_on 'Enable two-factor authentication' + + expect(page).to have_button("Set up new device", disabled: true) + end + end + + describe 'when 2FA via OTP is enabled' do + it 'allows registering a new device with a name' do + visit profile_account_path + manage_two_factor_authentication + expect(page).to have_content("You've already enabled two-factor authentication using one time password authenticators") + + device = register_device(device_type) + + expect(page).to have_content(device.name) + expect(page).to have_content("Your #{device_type} device was registered") + end + + it 'allows deleting a device' do + visit profile_account_path + manage_two_factor_authentication + expect(page).to have_content("You've already enabled two-factor authentication using one time password authenticators") + + first_device = register_device(device_type) + second_device = register_device(device_type, name: 'My other device') + + expect(page).to have_content(first_device.name) + expect(page).to have_content(second_device.name) + + accept_confirm { click_on 'Delete', match: :first } + + expect(page).to have_content('Successfully deleted') + expect(page.body).not_to have_content(first_device.name) + expect(page.body).to have_content(second_device.name) + end + end + end + + describe 'fallback code authentication' do + let(:user) { create(:user) } + + before do + # Register and logout + gitlab_sign_in(user) + user.update_attribute(:otp_required_for_login, true) + visit profile_account_path + end + + describe 'when no device is registered' do + before do + gitlab_sign_out + gitlab_sign_in(user) + end + + it 'shows the fallback otp code UI' do + assert_fallback_ui(page) + end + end + + describe 'when a device is registered' do + before do + manage_two_factor_authentication + register_device(device_type) + gitlab_sign_out + gitlab_sign_in(user) + end + + it 'provides a button that shows the fallback otp code UI' do + expect(page).to have_link('Sign in via 2FA code') + + click_link('Sign in via 2FA code') + + assert_fallback_ui(page) + end + end + end +end diff --git a/spec/support/shared_examples/services/merge_request_shared_examples.rb b/spec/support/shared_examples/services/merge_request_shared_examples.rb new file mode 100644 index 00000000000..a7032640217 --- /dev/null +++ b/spec/support/shared_examples/services/merge_request_shared_examples.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'reviewer_ids filter' do + context 'filter_reviewer' do + let(:opts) { super().merge(reviewer_ids_param) } + + context 'without reviewer_ids' do + let(:reviewer_ids_param) { {} } + + it 'contains no reviewer_ids' do + expect(execute.reviewers).to eq [] + end + end + + context 'with reviewer_ids' do + let(:reviewer_ids_param) { { reviewer_ids: [reviewer1.id, reviewer2.id, reviewer3.id] } } + + let(:reviewer1) { create(:user) } + let(:reviewer2) { create(:user) } + let(:reviewer3) { create(:user) } + + context 'when the current user can admin the merge_request' do + context 'when merge_request_reviewer feature is enabled' do + before do + stub_feature_flags(merge_request_reviewer: true) + end + + context 'with reviewers who can read the merge_request' do + before do + project.add_developer(reviewer1) + project.add_developer(reviewer2) + end + + it 'contains reviewers who can read the merge_request' do + expect(execute.reviewers).to contain_exactly(reviewer1, reviewer2) + end + end + end + + context 'when merge_request_reviewer feature is disabled' do + before do + stub_feature_flags(merge_request_reviewer: false) + end + + it 'contains no reviewers' do + expect(execute.reviewers).to eq [] + end + end + end + + context 'when the current_user cannot admin the merge_request' do + before do + project.add_developer(user) + end + + it 'contains no reviewers' do + expect(execute.reviewers).to eq [] + end + end + end + end +end diff --git a/spec/views/admin/sessions/two_factor.html.haml_spec.rb b/spec/views/admin/sessions/two_factor.html.haml_spec.rb index 9c5ff9925c1..c7e0edbcd58 100644 --- a/spec/views/admin/sessions/two_factor.html.haml_spec.rb +++ b/spec/views/admin/sessions/two_factor.html.haml_spec.rb @@ -32,6 +32,10 @@ RSpec.describe 'admin/sessions/two_factor.html.haml' do context 'user has u2f active' do let(:user) { create(:admin, :two_factor_via_u2f) } + before do + stub_feature_flags(webauthn: false) + end + it 'shows enter u2f form' do render diff --git a/spec/workers/ci/build_trace_chunk_flush_worker_spec.rb b/spec/workers/ci/build_trace_chunk_flush_worker_spec.rb new file mode 100644 index 00000000000..352ad6d4cf6 --- /dev/null +++ b/spec/workers/ci/build_trace_chunk_flush_worker_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::BuildTraceChunkFlushWorker do + let(:data) { 'x' * Ci::BuildTraceChunk::CHUNK_SIZE } + + let(:chunk) do + create(:ci_build_trace_chunk, :redis_with_data, initial_data: data) + end + + it 'migrates chunk to a permanent store' do + expect(chunk).to be_live + + described_class.new.perform(chunk.id) + + expect(chunk.reload).to be_persisted + end + + describe '#perform' do + it_behaves_like 'an idempotent worker' do + let(:job_args) { [chunk.id] } + + it 'migrates build trace chunk to a safe store' do + subject + + expect(chunk.reload).to be_persisted + end + end + end +end |