diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-03-11 06:08:14 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-03-11 06:08:14 +0300 |
commit | b131b6f6804bbcd22a22ce4ffe6566a384843f26 (patch) | |
tree | 12556081b23ccc647a73faf3af2f9f38f84bbb29 | |
parent | ca443618b0decc6b2754bf560a9c7a319d3e9873 (diff) |
Add latest changes from gitlab-org/gitlab@master
80 files changed, 1583 insertions, 282 deletions
diff --git a/app/assets/javascripts/authentication/webauthn/util.js b/app/assets/javascripts/authentication/webauthn/util.js index eeda2bfaeaf..2a0740cf488 100644 --- a/app/assets/javascripts/authentication/webauthn/util.js +++ b/app/assets/javascripts/authentication/webauthn/util.js @@ -46,6 +46,17 @@ export const bufferToBase64 = (input) => { }; /** + * Return a URL-safe base64 string. + * + * RFC: https://datatracker.ietf.org/doc/html/rfc4648#section-5 + * @param {String} base64Str + * @returns {String} + */ +export const base64ToBase64Url = (base64Str) => { + return base64Str.replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); +}; + +/** * Returns a copy of the given object with the id property converted to buffer * * @param {Object} param diff --git a/app/assets/javascripts/jira_connect/subscriptions/components/app.vue b/app/assets/javascripts/jira_connect/subscriptions/components/app.vue index 905e242e977..afdb414e82c 100644 --- a/app/assets/javascripts/jira_connect/subscriptions/components/app.vue +++ b/app/assets/javascripts/jira_connect/subscriptions/components/app.vue @@ -3,6 +3,7 @@ import { GlAlert, GlLink, GlSprintf } from '@gitlab/ui'; import { isEmpty } from 'lodash'; import { mapState, mapMutations } from 'vuex'; import { retrieveAlert } from '~/jira_connect/subscriptions/utils'; +import { I18N_DEFAULT_SIGN_IN_ERROR_MESSAGE } from '../constants'; import { SET_ALERT } from '../store/mutation_types'; import SignInPage from '../pages/sign_in.vue'; import SubscriptionsPage from '../pages/subscriptions.vue'; @@ -28,6 +29,11 @@ export default { default: [], }, }, + data() { + return { + user: null, + }; + }, computed: { ...mapState(['alert']), shouldShowAlert() { @@ -37,7 +43,7 @@ export default { return !isEmpty(this.subscriptions); }, userSignedIn() { - return Boolean(!this.usersPath); + return Boolean(!this.usersPath || this.user); }, }, created() { @@ -51,6 +57,15 @@ export default { const { linkUrl, title, message, variant } = retrieveAlert() || {}; this.setAlert({ linkUrl, title, message, variant }); }, + onSignInOauth(user) { + this.user = user; + }, + onSignInError() { + this.setAlert({ + message: I18N_DEFAULT_SIGN_IN_ERROR_MESSAGE, + variant: 'danger', + }); + }, }, }; </script> @@ -78,11 +93,16 @@ export default { </template> </gl-alert> - <user-link :user-signed-in="userSignedIn" :has-subscriptions="hasSubscriptions" /> + <user-link :user-signed-in="userSignedIn" :has-subscriptions="hasSubscriptions" :user="user" /> <h2 class="gl-text-center gl-mb-7">{{ s__('JiraService|GitLab for Jira Configuration') }}</h2> <div class="gl-layout-w-limited gl-mx-auto gl-px-5 gl-mb-7"> - <sign-in-page v-if="!userSignedIn" :has-subscriptions="hasSubscriptions" /> + <sign-in-page + v-if="!userSignedIn" + :has-subscriptions="hasSubscriptions" + @sign-in-oauth="onSignInOauth" + @error="onSignInError" + /> <subscriptions-page v-else :has-subscriptions="hasSubscriptions" /> </div> </div> diff --git a/app/assets/javascripts/jira_connect/subscriptions/components/sign_in_button.vue b/app/assets/javascripts/jira_connect/subscriptions/components/sign_in_legacy_button.vue index 627abcdd4a0..ec718d5b3ca 100644 --- a/app/assets/javascripts/jira_connect/subscriptions/components/sign_in_button.vue +++ b/app/assets/javascripts/jira_connect/subscriptions/components/sign_in_legacy_button.vue @@ -1,7 +1,7 @@ <script> import { GlButton } from '@gitlab/ui'; import { getGitlabSignInURL } from '~/jira_connect/subscriptions/utils'; -import { s__ } from '~/locale'; +import { I18N_DEFAULT_SIGN_IN_BUTTON_TEXT } from '~/jira_connect/subscriptions/constants'; export default { components: { @@ -27,7 +27,7 @@ export default { }, }, i18n: { - defaultButtonText: s__('Integrations|Sign in to GitLab'), + defaultButtonText: I18N_DEFAULT_SIGN_IN_BUTTON_TEXT, }, }; </script> diff --git a/app/assets/javascripts/jira_connect/subscriptions/components/sign_in_oauth_button.vue b/app/assets/javascripts/jira_connect/subscriptions/components/sign_in_oauth_button.vue new file mode 100644 index 00000000000..d7ec909cb28 --- /dev/null +++ b/app/assets/javascripts/jira_connect/subscriptions/components/sign_in_oauth_button.vue @@ -0,0 +1,124 @@ +<script> +import { GlButton } from '@gitlab/ui'; +import axios from '~/lib/utils/axios_utils'; +import { + I18N_DEFAULT_SIGN_IN_BUTTON_TEXT, + OAUTH_WINDOW_OPTIONS, + PKCE_CODE_CHALLENGE_DIGEST_ALGORITHM, +} from '~/jira_connect/subscriptions/constants'; +import { setUrlParams } from '~/lib/utils/url_utility'; +import AccessorUtilities from '~/lib/utils/accessor'; + +import { createCodeVerifier, createCodeChallenge } from '../pkce'; + +export default { + components: { + GlButton, + }, + inject: ['oauthMetadata'], + data() { + return { + token: null, + loading: false, + codeVerifier: null, + canUseCrypto: AccessorUtilities.canUseCrypto(), + }; + }, + mounted() { + window.addEventListener('message', this.handleWindowMessage); + }, + beforeDestroy() { + window.removeEventListener('message', this.handleWindowMessage); + }, + methods: { + async startOAuthFlow() { + this.loading = true; + + // Generate state necessary for PKCE OAuth flow + this.codeVerifier = createCodeVerifier(); + const codeChallenge = await createCodeChallenge(this.codeVerifier); + + // Build the initial OAuth authorization URL + const { oauth_authorize_url: oauthAuthorizeURL } = this.oauthMetadata; + const oauthAuthorizeURLWithChallenge = setUrlParams( + { + code_challenge: codeChallenge, + code_challenge_method: PKCE_CODE_CHALLENGE_DIGEST_ALGORITHM.short, + }, + oauthAuthorizeURL, + ); + + window.open( + oauthAuthorizeURLWithChallenge, + this.$options.i18n.defaultButtonText, + OAUTH_WINDOW_OPTIONS, + ); + }, + async handleWindowMessage(event) { + if (window.origin !== event.origin) { + this.loading = false; + this.handleError(); + return; + } + + // Verify that OAuth state isn't altered. + const state = event.data?.state; + if (state !== this.oauthMetadata.state) { + this.loading = false; + this.handleError(); + return; + } + + // Request access token and load the authenticated user. + const code = event.data?.code; + try { + const accessToken = await this.getOAuthToken(code); + await this.loadUser(accessToken); + } catch (e) { + this.handleError(); + } finally { + this.loading = false; + } + }, + handleError() { + this.$emit('error'); + }, + async getOAuthToken(code) { + const { + oauth_token_payload: oauthTokenPayload, + oauth_token_url: oauthTokenURL, + } = this.oauthMetadata; + const { data } = await axios.post(oauthTokenURL, { + ...oauthTokenPayload, + code, + code_verifier: this.codeVerifier, + }); + + return data.access_token; + }, + async loadUser(accessToken) { + const { data } = await axios.get('/api/v4/user', { + headers: { Authorization: `Bearer ${accessToken}` }, + }); + + this.$emit('sign-in', data); + }, + }, + i18n: { + defaultButtonText: I18N_DEFAULT_SIGN_IN_BUTTON_TEXT, + }, +}; +</script> +<template> + <gl-button + category="primary" + variant="info" + :loading="loading" + :disabled="!canUseCrypto" + @click="startOAuthFlow" + > + <slot> + {{ $options.i18n.defaultButtonText }} + </slot> + </gl-button> +</template> diff --git a/app/assets/javascripts/jira_connect/subscriptions/components/user_link.vue b/app/assets/javascripts/jira_connect/subscriptions/components/user_link.vue index fad3d2616d8..5e2c83aff65 100644 --- a/app/assets/javascripts/jira_connect/subscriptions/components/user_link.vue +++ b/app/assets/javascripts/jira_connect/subscriptions/components/user_link.vue @@ -25,6 +25,11 @@ export default { type: Boolean, required: true, }, + user: { + type: Object, + required: false, + default: null, + }, }, data() { return { @@ -32,8 +37,19 @@ export default { }; }, computed: { + gitlabUserName() { + return gon.current_username ?? this.user?.username; + }, gitlabUserHandle() { - return `@${gon.current_username}`; + return this.gitlabUserName ? `@${this.gitlabUserName}` : undefined; + }, + gitlabUserLink() { + return this.gitlabUserPath ?? `${gon.relative_root_url}/${this.gitlabUserName}`; + }, + signedInText() { + return this.gitlabUserHandle + ? this.$options.i18n.signedInAsUserText + : this.$options.i18n.signedInText; }, }, async created() { @@ -42,14 +58,15 @@ export default { i18n: { signInText: __('Sign in to GitLab'), signedInAsUserText: __('Signed in to GitLab as %{user_link}'), + signedInText: __('Signed in to GitLab'), }, }; </script> <template> <div class="jira-connect-user gl-font-base"> - <gl-sprintf v-if="userSignedIn" :message="$options.i18n.signedInAsUserText"> + <gl-sprintf v-if="userSignedIn" :message="signedInText"> <template #user_link> - <gl-link data-testid="gitlab-user-link" :href="gitlabUserPath" target="_blank"> + <gl-link data-testid="gitlab-user-link" :href="gitlabUserLink" target="_blank"> {{ gitlabUserHandle }} </gl-link> </template> diff --git a/app/assets/javascripts/jira_connect/subscriptions/constants.js b/app/assets/javascripts/jira_connect/subscriptions/constants.js index 2a65b7bc1fa..d30ebdbb487 100644 --- a/app/assets/javascripts/jira_connect/subscriptions/constants.js +++ b/app/assets/javascripts/jira_connect/subscriptions/constants.js @@ -1,5 +1,26 @@ +import { s__ } from '~/locale'; + export const DEFAULT_GROUPS_PER_PAGE = 10; export const ALERT_LOCALSTORAGE_KEY = 'gitlab_alert'; export const MINIMUM_SEARCH_TERM_LENGTH = 3; export const ADD_NAMESPACE_MODAL_ID = 'add-namespace-modal'; + +export const I18N_DEFAULT_SIGN_IN_BUTTON_TEXT = s__('Integrations|Sign in to GitLab'); +export const I18N_DEFAULT_SIGN_IN_ERROR_MESSAGE = s__('Integrations|Failed to sign in to GitLab.'); + +const OAUTH_WINDOW_SIZE = 800; +export const OAUTH_WINDOW_OPTIONS = [ + 'resizable=yes', + 'scrollbars=yes', + 'status=yes', + `width=${OAUTH_WINDOW_SIZE}`, + `height=${OAUTH_WINDOW_SIZE}`, + `left=${window.screen.width / 2 - OAUTH_WINDOW_SIZE / 2}`, + `top=${window.screen.height / 2 - OAUTH_WINDOW_SIZE / 2}`, +].join(','); + +export const PKCE_CODE_CHALLENGE_DIGEST_ALGORITHM = { + long: 'SHA-256', + short: 'S256', +}; diff --git a/app/assets/javascripts/jira_connect/subscriptions/index.js b/app/assets/javascripts/jira_connect/subscriptions/index.js index cd1fc1d4455..320f0f8aa6c 100644 --- a/app/assets/javascripts/jira_connect/subscriptions/index.js +++ b/app/assets/javascripts/jira_connect/subscriptions/index.js @@ -21,7 +21,14 @@ export function initJiraConnect() { Vue.use(Translate); Vue.use(GlFeatureFlagsPlugin); - const { groupsPath, subscriptions, subscriptionsPath, usersPath, gitlabUserPath } = el.dataset; + const { + groupsPath, + subscriptions, + subscriptionsPath, + usersPath, + gitlabUserPath, + oauthMetadata, + } = el.dataset; sizeToParent(); return new Vue({ @@ -33,6 +40,7 @@ export function initJiraConnect() { subscriptionsPath, usersPath, gitlabUserPath, + oauthMetadata: oauthMetadata ? JSON.parse(oauthMetadata) : null, }, render(createElement) { return createElement(JiraConnectApp); diff --git a/app/assets/javascripts/jira_connect/subscriptions/pages/sign_in.vue b/app/assets/javascripts/jira_connect/subscriptions/pages/sign_in.vue index 2bce5afc72b..a24ee33b723 100644 --- a/app/assets/javascripts/jira_connect/subscriptions/pages/sign_in.vue +++ b/app/assets/javascripts/jira_connect/subscriptions/pages/sign_in.vue @@ -1,14 +1,17 @@ <script> import { s__ } from '~/locale'; + +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import SubscriptionsList from '../components/subscriptions_list.vue'; -import SignInButton from '../components/sign_in_button.vue'; export default { name: 'SignInPage', components: { SubscriptionsList, - SignInButton, + SignInLegacyButton: () => import('../components/sign_in_legacy_button.vue'), + SignInOauthButton: () => import('../components/sign_in_oauth_button.vue'), }, + mixins: [glFeatureFlagMixin()], inject: ['usersPath'], props: { hasSubscriptions: { @@ -16,25 +19,47 @@ export default { required: true, }, }, + computed: { + useSignInOauthButton() { + return this.glFeatures.jiraConnectOauth; + }, + }, i18n: { - signinButtonTextWithSubscriptions: s__('Integrations|Sign in to add namespaces'), + signInButtonTextWithSubscriptions: s__('Integrations|Sign in to add namespaces'), signInText: s__('JiraService|Sign in to GitLab.com to get started.'), }, + methods: { + onSignInError() { + this.$emit('error'); + }, + }, }; </script> <template> <div v-if="hasSubscriptions"> <div class="gl-display-flex gl-justify-content-end"> - <sign-in-button :users-path="usersPath"> - {{ $options.i18n.signinButtonTextWithSubscriptions }} - </sign-in-button> + <sign-in-oauth-button + v-if="useSignInOauthButton" + @sign-in="$emit('sign-in-oauth', $event)" + @error="onSignInError" + > + {{ $options.i18n.signInButtonTextWithSubscriptions }} + </sign-in-oauth-button> + <sign-in-legacy-button v-else :users-path="usersPath"> + {{ $options.i18n.signInButtonTextWithSubscriptions }} + </sign-in-legacy-button> </div> <subscriptions-list /> </div> <div v-else class="gl-text-center"> <p class="gl-mb-7">{{ $options.i18n.signInText }}</p> - <sign-in-button class="gl-mb-7" :users-path="usersPath" /> + <sign-in-oauth-button + v-if="useSignInOauthButton" + @sign-in="$emit('sign-in-oauth', $event)" + @error="onSignInError" + /> + <sign-in-legacy-button v-else class="gl-mb-7" :users-path="usersPath" /> </div> </template> diff --git a/app/assets/javascripts/jira_connect/subscriptions/pkce.js b/app/assets/javascripts/jira_connect/subscriptions/pkce.js new file mode 100644 index 00000000000..18ea5cae860 --- /dev/null +++ b/app/assets/javascripts/jira_connect/subscriptions/pkce.js @@ -0,0 +1,60 @@ +import { bufferToBase64, base64ToBase64Url } from '~/authentication/webauthn/util'; +import { PKCE_CODE_CHALLENGE_DIGEST_ALGORITHM } from './constants'; + +// PKCE codeverifier should have a maximum length of 128 characters. +// Using 96 bytes generates a string of 128 characters. +// RFC: https://datatracker.ietf.org/doc/html/rfc7636#section-4.1 +export const CODE_VERIFIER_BYTES = 96; + +/** + * Generate a cryptographically random string. + * @param {Number} lengthBytes + * @returns {String} a random string + */ +function getRandomString(lengthBytes) { + // generate random values and load them into byteArray. + const byteArray = new Uint8Array(lengthBytes); + window.crypto.getRandomValues(byteArray); + + // Convert array to string + const randomString = bufferToBase64(byteArray); + return randomString; +} + +/** + * Creates a code verifier to be used for OAuth PKCE authentication. + * The code verifier has 128 characters. + * + * RFC: https://datatracker.ietf.org/doc/html/rfc7636#section-4.1 + * @returns {String} code verifier + */ +export function createCodeVerifier() { + const verifier = getRandomString(CODE_VERIFIER_BYTES); + return base64ToBase64Url(verifier); +} + +/** + * Creates a code challenge for OAuth PKCE authentication. + * The code challenge is derived from the given [codeVerifier]. + * [codeVerifier] is tranformed in the following way (as per the RFC): + * code_challenge = BASE64URL-ENCODE(SHA256(ASCII(codeVerifier))) + * + * RFC: https://datatracker.ietf.org/doc/html/rfc7636#section-4.2 + * @param {String} codeVerifier + * @returns {String} code challenge + */ +export async function createCodeChallenge(codeVerifier) { + // Generate SHA-256 digest of the [codeVerifier] + const buffer = new TextEncoder().encode(codeVerifier); + const digestArrayBuffer = await window.crypto.subtle.digest( + PKCE_CODE_CHALLENGE_DIGEST_ALGORITHM.long, + buffer, + ); + + // Convert digest to a Base64URL-encoded string + const digestHash = bufferToBase64(digestArrayBuffer); + // Escape string to remove reserved charaters + const codeChallenge = base64ToBase64Url(digestHash); + + return codeChallenge; +} diff --git a/app/assets/javascripts/lib/utils/accessor.js b/app/assets/javascripts/lib/utils/accessor.js index d4a6d70c62c..f7cdc564538 100644 --- a/app/assets/javascripts/lib/utils/accessor.js +++ b/app/assets/javascripts/lib/utils/accessor.js @@ -50,8 +50,16 @@ function canUseLocalStorage() { return safe; } +/** + * Determines if `window.crypto` is available. + */ +function canUseCrypto() { + return window.crypto?.subtle !== undefined; +} + const AccessorUtilities = { canUseLocalStorage, + canUseCrypto, }; export default AccessorUtilities; diff --git a/app/assets/javascripts/pages/jira_connect/oauth_callbacks/index.js b/app/assets/javascripts/pages/jira_connect/oauth_callbacks/index.js new file mode 100644 index 00000000000..3fe238dcb35 --- /dev/null +++ b/app/assets/javascripts/pages/jira_connect/oauth_callbacks/index.js @@ -0,0 +1,28 @@ +function getOriginURL() { + const origin = new URL(window.opener.location); + origin.hash = ''; + origin.search = ''; + + return origin; +} + +function postMessageToJiraConnectApp(data) { + window.opener.postMessage(data, getOriginURL().toString()); +} + +function initOAuthCallbacks() { + const params = new URLSearchParams(window.location.search); + if (params.has('code') && params.has('state')) { + postMessageToJiraConnectApp({ + success: true, + code: params.get('code'), + state: params.get('state'), + }); + } else { + postMessageToJiraConnectApp({ success: false }); + } + + window.close(); +} + +initOAuthCallbacks(); diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/commit_message_dropdown.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/commit_message_dropdown.vue index 73d75352cb5..5baeb309f79 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/commit_message_dropdown.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/commit_message_dropdown.vue @@ -21,7 +21,9 @@ export default { <gl-dropdown right text="Use an existing commit message" - variant="link" + category="tertiary" + variant="confirm" + size="small" class="mr-commit-dropdown" > <gl-dropdown-item diff --git a/app/controllers/concerns/spammable_actions/captcha_check/rest_api_actions_support.rb b/app/controllers/concerns/spammable_actions/captcha_check/rest_api_actions_support.rb new file mode 100644 index 00000000000..2ebfa90e6da --- /dev/null +++ b/app/controllers/concerns/spammable_actions/captcha_check/rest_api_actions_support.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# This module should be included to support CAPTCHA check for REST API actions via Grape. +# +# If the request is directly handled by a controller action, then the corresponding module which +# supports HTML or JSON formats should be used instead. +module SpammableActions::CaptchaCheck::RestApiActionsSupport + extend ActiveSupport::Concern + include SpammableActions::CaptchaCheck::Common + include Spam::Concerns::HasSpamActionResponseFields + + private + + def with_captcha_check_rest_api(spammable:, &block) + # In the case of the REST API, the request is handled by Grape, so if there is a spam-related + # error, we don't render directly, instead we will pass the error message and other necessary + # fields to the Grape api error helper for it to handle. + captcha_render_lambda = -> do + fields = spam_action_response_fields(spammable) + + fields.delete :spam + # NOTE: "409 - Conflict" seems to be the most appropriate HTTP status code for a response + # which requires a CAPTCHA to be solved in order for the request to be resubmitted. + # https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.10 + status = 409 + + # NOTE: This nested 'error' key may not be consistent with all other API error responses, + # because they are not currently consistent across different API endpoints + # and models. Some (snippets) will nest errors in an errors key like this, + # while others (issues) will return the model's errors hash without an errors key, + # while still others just return a plain string error. + # See https://gitlab.com/groups/gitlab-org/-/epics/5527#revisit-inconsistent-shape-of-error-responses-in-rest-api + fields[:message] = { error: spammable.errors.full_messages.to_sentence } + render_structured_api_error!(fields, status) + end + + with_captcha_check_common(spammable: spammable, captcha_render_lambda: captcha_render_lambda, &block) + end +end diff --git a/app/controllers/jira_connect/oauth_callbacks_controller.rb b/app/controllers/jira_connect/oauth_callbacks_controller.rb new file mode 100644 index 00000000000..f603a563402 --- /dev/null +++ b/app/controllers/jira_connect/oauth_callbacks_controller.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# This controller's role is to serve as a landing page +# that users get redirected to after installing and authenticating +# The GitLab.com for Jira App (https://marketplace.atlassian.com/apps/1221011/gitlab-com-for-jira-cloud) +# +class JiraConnect::OauthCallbacksController < ApplicationController + feature_category :integrations + + def index; end +end diff --git a/app/controllers/jira_connect/subscriptions_controller.rb b/app/controllers/jira_connect/subscriptions_controller.rb index fcd95c7942c..ec6ba07a125 100644 --- a/app/controllers/jira_connect/subscriptions_controller.rb +++ b/app/controllers/jira_connect/subscriptions_controller.rb @@ -16,6 +16,10 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController p.style_src(*style_src_values) end + before_action do + push_frontend_feature_flag(:jira_connect_oauth, @user, default_enabled: :yaml) + end + before_action :allow_rendering_in_iframe, only: :index before_action :verify_qsh_claim!, only: :index before_action :authenticate_user!, only: :create diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index fb2fa547447..ba6c0380edf 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -178,7 +178,7 @@ module AuthHelper end def google_tag_manager_enabled? - return false unless Gitlab.dev_env_or_com? + return false unless Gitlab.com? if Feature.enabled?(:gtm_nonce, type: :ops) extra_config.has_key?('google_tag_manager_nonce_id') && diff --git a/app/helpers/jira_connect_helper.rb b/app/helpers/jira_connect_helper.rb index 9a0f0944fd1..67b85b26f9e 100644 --- a/app/helpers/jira_connect_helper.rb +++ b/app/helpers/jira_connect_helper.rb @@ -9,12 +9,38 @@ module JiraConnectHelper subscriptions: subscriptions.map { |s| serialize_subscription(s) }.to_json, subscriptions_path: jira_connect_subscriptions_path, users_path: current_user ? nil : jira_connect_users_path, # users_path is used to determine if user is signed in - gitlab_user_path: current_user ? user_path(current_user) : nil + gitlab_user_path: current_user ? user_path(current_user) : nil, + oauth_metadata: Feature.enabled?(:jira_connect_oauth, current_user) ? jira_connect_oauth_data.to_json : nil } end private + def jira_connect_oauth_data + oauth_authorize_url = oauth_authorization_url( + client_id: ENV['JIRA_CONNECT_OAUTH_CLIENT_ID'], + response_type: 'code', + scope: 'api', + redirect_uri: jira_connect_oauth_callbacks_url, + state: oauth_state + ) + + { + oauth_authorize_url: oauth_authorize_url, + oauth_token_url: oauth_token_url, + state: oauth_state, + oauth_token_payload: { + grant_type: :authorization_code, + client_id: ENV['JIRA_CONNECT_OAUTH_CLIENT_ID'], + redirect_uri: jira_connect_oauth_callbacks_url + } + } + end + + def oauth_state + @oauth_state ||= SecureRandom.hex(32) + end + def serialize_subscription(subscription) { group: { diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index e9466a9e97e..f0389000eb3 100644 --- a/app/helpers/sessions_helper.rb +++ b/app/helpers/sessions_helper.rb @@ -5,7 +5,7 @@ module SessionsHelper def recently_confirmed_com? strong_memoize(:recently_confirmed_com) do - ::Gitlab.dev_env_or_com? && + ::Gitlab.com? && !!flash[:notice]&.include?(t(:confirmed, scope: [:devise, :confirmations])) end end diff --git a/app/helpers/whats_new_helper.rb b/app/helpers/whats_new_helper.rb index ccccfcb930b..bbf56c51c6d 100644 --- a/app/helpers/whats_new_helper.rb +++ b/app/helpers/whats_new_helper.rb @@ -10,7 +10,7 @@ module WhatsNewHelper end def display_whats_new? - (Gitlab.dev_env_org_or_com? || user_signed_in?) && + (Gitlab.org_or_com? || user_signed_in?) && !Gitlab::CurrentSettings.current_application_settings.whats_new_variant_disabled? end diff --git a/app/models/integrations/base_chat_notification.rb b/app/models/integrations/base_chat_notification.rb index d0d54a92021..d5b6357cb66 100644 --- a/app/models/integrations/base_chat_notification.rb +++ b/app/models/integrations/base_chat_notification.rb @@ -241,7 +241,6 @@ module Integrations def notify_for_ref?(data) return true if data[:object_kind] == 'tag_push' - return true if data[:object_kind] == 'deployment' && !Feature.enabled?(:chat_notification_deployment_protected_branch_filter, project) ref = data[:ref] || data.dig(:object_attributes, :ref) return true if ref.blank? # No need to check protected branches when there is no ref diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 77897c5807f..f8e7a912896 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -67,7 +67,7 @@ class BasePolicy < DeclarativePolicy::Base rule { default }.enable :read_cross_project - condition(:is_gitlab_com, score: 0, scope: :global) { ::Gitlab.dev_env_or_com? } + condition(:is_gitlab_com, score: 0, scope: :global) { ::Gitlab.com? } end BasePolicy.prepend_mod_with('BasePolicy') diff --git a/app/services/spam/spam_params.rb b/app/services/spam/spam_params.rb index ccc17a42f01..81db6b390b2 100644 --- a/app/services/spam/spam_params.rb +++ b/app/services/spam/spam_params.rb @@ -25,6 +25,7 @@ module Spam # then the spam check may fail, or the SpamLog or UserAgentDetail may have missing fields. class SpamParams def self.new_from_request(request:) + self.normalize_grape_request_headers(request: request) self.new( captcha_response: request.headers['X-GitLab-Captcha-Response'], spam_log_id: request.headers['X-GitLab-Spam-Log-Id'], @@ -52,5 +53,14 @@ module Spam other.user_agent == user_agent && other.referer == referer end + + def self.normalize_grape_request_headers(request:) + # If needed, make a normalized copy of Grape headers with the case of 'GitLab' (with an + # uppercase 'L') instead of 'Gitlab' (with a lowercase 'l'), because Grape header helper keys + # are "coerced into a capitalized kebab case". See https://github.com/ruby-grape/grape#request + %w[X-Gitlab-Captcha-Response X-Gitlab-Spam-Log-Id].each do |header| + request.headers[header.gsub('Gitlab', 'GitLab')] = request.headers[header] if request.headers.key?(header) + end + end end end diff --git a/app/views/devise/shared/_email_opted_in.html.haml b/app/views/devise/shared/_email_opted_in.html.haml index 3817f9f651d..898b8f31f1d 100644 --- a/app/views/devise/shared/_email_opted_in.html.haml +++ b/app/views/devise/shared/_email_opted_in.html.haml @@ -1,4 +1,4 @@ -- return unless Gitlab.dev_env_or_com? +- return unless Gitlab.com? .gl-mb-3.js-email-opt-in.hidden .gl-font-weight-bold.gl-mb-3 diff --git a/app/views/devise/shared/_terms_of_service_notice.html.haml b/app/views/devise/shared/_terms_of_service_notice.html.haml index 75d567a03fd..1c6dc1f2d5d 100644 --- a/app/views/devise/shared/_terms_of_service_notice.html.haml +++ b/app/views/devise/shared/_terms_of_service_notice.html.haml @@ -1,7 +1,7 @@ - return unless Gitlab::CurrentSettings.current_application_settings.enforce_terms? %p.gl-text-gray-500.gl-mt-5.gl-mb-0 - - if Gitlab.dev_env_or_com? + - if Gitlab.com? = html_escape(s_("SignUp|By clicking %{button_text}, I agree that I have read and accepted the GitLab %{link_start}Terms of Use and Privacy Policy%{link_end}")) % { button_text: button_text, link_start: "<a href='#{terms_path}' target='_blank' rel='noreferrer noopener'>".html_safe, link_end: '</a>'.html_safe } - else diff --git a/app/views/jira_connect/oauth_callbacks/index.html.haml b/app/views/jira_connect/oauth_callbacks/index.html.haml new file mode 100644 index 00000000000..d35834bf05d --- /dev/null +++ b/app/views/jira_connect/oauth_callbacks/index.html.haml @@ -0,0 +1 @@ +%p= s_('Integrations|You can close this window.') diff --git a/config/feature_flags/development/chat_notification_deployment_protected_branch_filter.yml b/config/feature_flags/development/jira_connect_oauth.yml index 0b81a06c593..2f090d56bd1 100644 --- a/config/feature_flags/development/chat_notification_deployment_protected_branch_filter.yml +++ b/config/feature_flags/development/jira_connect_oauth.yml @@ -1,8 +1,8 @@ --- -name: chat_notification_deployment_protected_branch_filter -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74423 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/349131 -milestone: '14.7' +name: jira_connect_oauth +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81126 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/355048 +milestone: '14.9' type: development group: group::integrations default_enabled: false diff --git a/config/initializers/gitlab_experiment.rb b/config/initializers/gitlab_experiment.rb index fdb21d90c28..9a6ef325c9e 100644 --- a/config/initializers/gitlab_experiment.rb +++ b/config/initializers/gitlab_experiment.rb @@ -49,7 +49,7 @@ Gitlab::Experiment.configure do |config| # valid_domains = %w[about.gitlab.com docs.gitlab.com gitlab.com gdk.test localhost] config.redirect_url_validator = lambda do |url| - Gitlab.dev_env_or_com? && (url = URI.parse(url)) && valid_domains.include?(url.host) + Gitlab.com? && (url = URI.parse(url)) && valid_domains.include?(url.host) rescue URI::InvalidURIError false end diff --git a/config/routes/jira_connect.rb b/config/routes/jira_connect.rb index 1e871d52c80..344f0114364 100644 --- a/config/routes/jira_connect.rb +++ b/config/routes/jira_connect.rb @@ -20,4 +20,6 @@ namespace :jira_connect do put :update end end + + resources :oauth_callbacks, only: [:index] end diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index b142f6385f7..37c64bbd42d 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -24,7 +24,7 @@ class AddNewTrailPlans < ActiveRecord::Migration[6.0] end def up - return unless Gitlab.dev_env_or_com? + return unless Gitlab.com? ultimate_trial = Plan.create!(name: 'ultimate_trial', title: 'Ultimate Trial') premium_trial = Plan.create!(name: 'premium_trial', title: 'Premium Trial') @@ -34,7 +34,7 @@ class AddNewTrailPlans < ActiveRecord::Migration[6.0] end def down - return unless Gitlab.dev_env_or_com? + return unless Gitlab.com? Plan.where(name: %w(ultimate_trial premium_trial)).delete_all end diff --git a/db/post_migrate/20210805192450_update_trial_plans_ci_daily_pipeline_schedule_triggers.rb b/db/post_migrate/20210805192450_update_trial_plans_ci_daily_pipeline_schedule_triggers.rb index 9552058dd73..f27f61729a3 100644 --- a/db/post_migrate/20210805192450_update_trial_plans_ci_daily_pipeline_schedule_triggers.rb +++ b/db/post_migrate/20210805192450_update_trial_plans_ci_daily_pipeline_schedule_triggers.rb @@ -29,7 +29,7 @@ class UpdateTrialPlansCiDailyPipelineScheduleTriggers < ActiveRecord::Migration[ end def up - return unless Gitlab.dev_env_or_com? + return unless Gitlab.com? if plan_limits_present? create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', PREMIUM_TRIAL, EVERY_5_MINUTES) @@ -38,7 +38,7 @@ class UpdateTrialPlansCiDailyPipelineScheduleTriggers < ActiveRecord::Migration[ end def down - return unless Gitlab.dev_env_or_com? + return unless Gitlab.com? if plan_limits_present? create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', PREMIUM_TRIAL, 0) diff --git a/db/post_migrate/20211029102822_add_open_source_plan.rb b/db/post_migrate/20211029102822_add_open_source_plan.rb index 00266640f03..bb65637ffca 100644 --- a/db/post_migrate/20211029102822_add_open_source_plan.rb +++ b/db/post_migrate/20211029102822_add_open_source_plan.rb @@ -24,7 +24,7 @@ class AddOpenSourcePlan < Gitlab::Database::Migration[1.0] end def up - return unless Gitlab.dev_env_or_com? + return unless Gitlab.com? opensource = Plan.create!(name: 'opensource', title: 'Open Source Program') @@ -32,7 +32,7 @@ class AddOpenSourcePlan < Gitlab::Database::Migration[1.0] end def down - return unless Gitlab.dev_env_or_com? + return unless Gitlab.com? Plan.where(name: 'opensource').delete_all end diff --git a/doc/api/deploy_tokens.md b/doc/api/deploy_tokens.md index 77c2fe5e162..ace7e55f882 100644 --- a/doc/api/deploy_tokens.md +++ b/doc/api/deploy_tokens.md @@ -94,6 +94,46 @@ Example response: ] ``` +### Get a project deploy token + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82467) in GitLab 14.9. + +Get a single project's deploy token by ID. + +```plaintext +GET /projects/:id/deploy_tokens/:token_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| ---------- | -------------- | ---------------------- | ----------- | +| `id` | integer/string | **{check-circle}** Yes | ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | +| `token_id` | integer | **{check-circle}** Yes | ID of the deploy token | + +Example request: + +```shell +curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/deploy_tokens/1" +``` + +Example response: + +```json +{ + "id": 1, + "name": "MyToken", + "username": "gitlab+deploy-token-1", + "expires_at": "2020-02-14T00:00:00.000Z", + "revoked": false, + "expired": false, + "scopes": [ + "read_repository", + "read_registry" + ] +} +``` + ### Create a project deploy token > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/21811) in GitLab 12.9. @@ -108,7 +148,7 @@ Parameters: | Attribute | Type | Required | Description | | ------------ | ---------------- | ---------------------- | ----------- | -| `id` | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | +| `id` | integer/string | **{check-circle}** Yes | ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | | `name` | string | **{check-circle}** Yes | New deploy token's name | | `expires_at` | datetime | **{dotted-circle}** No | Expiration date for the deploy token. Does not expire if no value is provided. Expected in ISO 8601 format (`2019-03-15T08:00:00Z`) | | `username` | string | **{dotted-circle}** No | Username for deploy token. Default is `gitlab+deploy-token-{n}` | @@ -153,8 +193,8 @@ Parameters: | Attribute | Type | Required | Description | | ---------- | -------------- | ---------------------- | ----------- | -| `id` | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | -| `token_id` | integer | **{check-circle}** Yes | The ID of the deploy token | +| `id` | integer/string | **{check-circle}** Yes | ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | +| `token_id` | integer | **{check-circle}** Yes | ID of the deploy token | Example request: @@ -210,6 +250,46 @@ Example response: ] ``` +### Get a group deploy token + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82467) in GitLab 14.9. + +Get a single group's deploy token by ID. + +```plaintext +GET /groups/:id/deploy_tokens/:token_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| ----------- | -------------- | ---------------------- | ----------- | +| `id` | integer/string | **{check-circle}** Yes | ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) owned by the authenticated user | +| `token_id` | integer | **{check-circle}** Yes | ID of the deploy token | + +Example request: + +```shell +curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/1/deploy_tokens/1" +``` + +Example response: + +```json +{ + "id": 1, + "name": "MyToken", + "username": "gitlab+deploy-token-1", + "expires_at": "2020-02-14T00:00:00.000Z", + "revoked": false, + "expired": false, + "scopes": [ + "read_repository", + "read_registry" + ] +} +``` + ### Create a group deploy token > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/21811) in GitLab 12.9. @@ -224,7 +304,7 @@ Parameters: | Attribute | Type | Required | Description | | ------------ | ---- | --------- | ----------- | -| `id` | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) owned by the authenticated user | +| `id` | integer/string | **{check-circle}** Yes | ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) owned by the authenticated user | | `name` | string | **{check-circle}** Yes | New deploy token's name | | `expires_at` | datetime | **{dotted-circle}** No | Expiration date for the deploy token. Does not expire if no value is provided. Expected in ISO 8601 format (`2019-03-15T08:00:00Z`) | | `username` | string | **{dotted-circle}** No | Username for deploy token. Default is `gitlab+deploy-token-{n}` | @@ -269,8 +349,8 @@ Parameters: | Attribute | Type | Required | Description | | ----------- | -------------- | ---------------------- | ----------- | -| `id` | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | -| `token_id` | integer | **{check-circle}** Yes | The ID of the deploy token | +| `id` | integer/string | **{check-circle}** Yes | ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) owned by the authenticated user | +| `token_id` | integer | **{check-circle}** Yes | ID of the deploy token | Example request: diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md index d0e65b8c4eb..457b083c217 100644 --- a/doc/api/graphql/index.md +++ b/doc/api/graphql/index.md @@ -187,55 +187,74 @@ NOTE: The complexity limits may be revised in future, and additionally, the complexity of a query may be altered. -## Spam - -GraphQL mutations can be detected as spam. If this happens, a -[GraphQL top-level error](https://spec.graphql.org/June2018/#sec-Errors) is raised. For example: - -```json -{ - "errors": [ - { - "message": "Request denied. Spam detected", - "locations": [ { "line": 6, "column": 7 } ], - "path": [ "updateSnippet" ], - "extensions": { - "spam": true +## Resolve mutations detected as spam + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/327360) in GitLab 13.11. + +GraphQL mutations can be detected as spam. If a mutation is detected as spam and: + +- A CAPTCHA service is not configured, a + [GraphQL top-level error](https://spec.graphql.org/June2018/#sec-Errors) is raised. For example: + + ```json + { + "errors": [ + { + "message": "Request denied. Spam detected", + "locations": [ { "line": 6, "column": 7 } ], + "path": [ "updateSnippet" ], + "extensions": { + "spam": true + } + } + ], + "data": { + "updateSnippet": { + "snippet": null } } - ], - "data": { - "updateSnippet": { - "snippet": null + } + ``` + +- A CAPTCHA service is configured, you receive a response with: + - `needsCaptchaResponse` set to `true`. + - The `spamLogId` and `captchaSiteKey` fields set. + + For example: + + ```json + { + "errors": [ + { + "message": "Request denied. Solve CAPTCHA challenge and retry", + "locations": [ { "line": 6, "column": 7 } ], + "path": [ "updateSnippet" ], + "extensions": { + "needsCaptchaResponse": true, + "captchaSiteKey": "6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI", + "spamLogId": 67 + } + } + ], + "data": { + "updateSnippet": { + "snippet": null, + } } } -} -``` - -If a mutation is detected as potential spam and a CAPTCHA service is configured: + ``` - Use the `captchaSiteKey` to obtain a CAPTCHA response value using the appropriate CAPTCHA API. Only [Google reCAPTCHA v2](https://developers.google.com/recaptcha/docs/display) is supported. - Resubmit the request with the `X-GitLab-Captcha-Response` and `X-GitLab-Spam-Log-Id` headers set. - -```json -{ - "errors": [ - { - "message": "Request denied. Solve CAPTCHA challenge and retry", - "locations": [ { "line": 6, "column": 7 } ], - "path": [ "updateSnippet" ], - "extensions": { - "needsCaptchaResponse": true, - "captchaSiteKey": "6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI", - "spamLogId": 67 - } - } - ], - "data": { - "updateSnippet": { - "snippet": null, - } - } -} + +NOTE: +The GitLab GraphiQL implementation doesn't permit passing of headers, so we must write +this as a cURL query. `--data-binary` is used to properly handle escaped double quotes +in the JSON-embedded query. + +```shell +export CAPTCHA_RESPONSE="<CAPTCHA response obtained from CAPTCHA service>" +export SPAM_LOG_ID="<spam_log_id obtained from initial REST response>" +curl --header "Authorization: Bearer $PRIVATE_TOKEN" --header "Content-Type: application/json" --header "X-GitLab-Captcha-Response: $CAPTCHA_RESPONSE" --header "X-GitLab-Spam-Log-Id: $SPAM_LOG_ID" --request POST --data-binary '{"query": "mutation {createSnippet(input: {title: \"Title\" visibilityLevel: public blobActions: [ { action: create filePath: \"BlobPath\" content: \"BlobContent\" } ] }) { snippet { id title } errors }}"}' "https://gitlab.example.com/api/graphql" ``` diff --git a/doc/api/index.md b/doc/api/index.md index 589bc0416a1..6cdf64a01af 100644 --- a/doc/api/index.md +++ b/doc/api/index.md @@ -767,3 +767,35 @@ some API endpoints also support `text/plain`. In [GitLab 13.10 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/250342), API endpoints do not support `text/plain` by default, unless it's explicitly documented. + +## Resolve requests detected as spam + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/352913) in GitLab 14.9. + +REST API requests can be detected as spam. If a request is detected as spam and: + +- A CAPTCHA service is not configured, an error response is returned. For example: + + ```json + {"message":{"error":"Your snippet has been recognized as spam and has been discarded."}} + ``` + +- A CAPTCHA service is configured, you receive a response with: + - `needs_captcha_response` set to `true`. + - The `spam_log_id` and `captcha_site_key` fields set. + + For example: + + ```json + {"needs_captcha_response":true,"spam_log_id":42,"captcha_site_key":"6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI","message":{"error":"Your snippet has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."}} + ``` + +- Use the `captcha_site_key` to obtain a CAPTCHA response value using the appropriate CAPTCHA API. + Only [Google reCAPTCHA v2](https://developers.google.com/recaptcha/docs/display) is supported. +- Resubmit the request with the `X-GitLab-Captcha-Response` and `X-GitLab-Spam-Log-Id` headers set. + +```shell +export CAPTCHA_RESPONSE="<CAPTCHA response obtained from CAPTCHA service>" +export SPAM_LOG_ID="<spam_log_id obtained from initial REST response>" +curl --request POST --header "PRIVATE-TOKEN: $PRIVATE_TOKEN" --header "X-GitLab-Captcha-Response: $CAPTCHA_RESPONSE" --header "X-GitLab-Spam-Log-Id: $SPAM_LOG_ID" "https://gitlab.example.com/api/v4/snippets?title=Title&file_name=FileName&content=Content&visibility=public" +``` diff --git a/doc/development/integrations/jira_connect.md b/doc/development/integrations/jira_connect.md index ceaa052008d..da1ab6dc2c8 100644 --- a/doc/development/integrations/jira_connect.md +++ b/doc/development/integrations/jira_connect.md @@ -74,3 +74,45 @@ If you use Gitpod and you get an error about Jira not being able to access the d 1. When the GDK is running, select **Ports** in the bottom-right corner. 1. On the left sidebar, select the port the GDK is listening to (typically `3000`). 1. If the port is marked as private, select the lock icon to make it public. + +## Test the GitLab OAuth authentication flow + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81126) in GitLab 14.9 [with a flag](../../administration/feature_flags.md) named `jira_connect_oauth`. Disabled by default. + +GitLab for Jira users can authenticate with GitLab using GitLab OAuth. + +FLAG: +By default this feature is not available. To make it available, +ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `jira_connect_oauth`. +The feature is not ready for production use. + +The following steps describe setting up an environment to test the GitLab OAuth flow: + +1. Start a Gitpod session and open the rails console. + + ```shell + bundle exec rails console + ``` + +1. Enable the feature flag. + + ```shell + Feature.enable(:jira_connect_oauth) + ``` + +1. On your GitLab instance, go to **Admin > Applications**. +1. Create a new application with the following settings: + - Name: `Jira Connect` + - Redirect URI: `YOUR_GITPOD_INSTANCE/-/jira_connect/oauth_callbacks` + - Scopes: `api` + - Trusted: **No** + - Confidential: **No** +1. Copy the Application ID. +1. Go to [gitpod.io/variables](https://gitpod.io/variables). +1. Create a new variable named `JIRA_CONNECT_OAUTH_CLIENT_ID`, with a scope of `*/*`, and paste the Application ID as the value. + +If you already have an active Gitpod instance, use the following command in the Gitpod terminal to set the environment variable: + +```shell +eval $(gp env -e JIRA_CONNECT_OAUTH_CLIENT_ID=$YOUR_APPLICATION_ID) +``` diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index 57ba68f15ae..056a31c276d 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -315,7 +315,6 @@ To disable analyzer rules: 1. In one or more `ruleset.identifier` sub sections, list the rules that you want disabled. Every `ruleset.identifier` section has: - a `type` field, to name the predefined rule identifier that the targeted analyzer uses. - - a `value` field, to name the rule to be disabled. ##### Example: Disable predefined rules of SAST analyzers @@ -345,6 +344,9 @@ and `sobelow` by matching the `type` and `value` of identifiers: value = "sql_injection" ``` +Those vulnerabilities containing the provided type and value are now disabled, meaning +they won't be displayed in Merge Request nor the Vulnerability Report. + #### Override predefined analyzer rules To override analyzer rules: @@ -365,30 +367,40 @@ To override analyzer rules: ##### Example: Override predefined rules of SAST analyzers -In the following example, rules from `eslint` -and `gosec` are matched by the `type` and `value` of identifiers and -then overridden: +Before adding a ruleset, we verify which vulnerability will be overwritten by viewing the [`gl-sast-report.json`](#reports-json-format): + +```json +"identifiers": [ + { + "type": "gosec_rule_id", + "name": "Gosec Rule ID G307", + "value": "G307" + }, + { + "type": "CWE", + "name": "CWE-703", + "value": "703", + "url": "https://cwe.mitre.org/data/definitions/703.html" + } + ] +``` + +In the following example, rules from `gosec` are matched by the `type` +and `value` of identifiers and then overridden: ```toml -[eslint] - [[eslint.ruleset]] - [eslint.ruleset.identifier] - type = "eslint_rule_id" - value = "security/detect-object-injection" - [eslint.ruleset.override] - description = "OVERRIDDEN description" - message = "OVERRIDDEN message" - name = "OVERRIDDEN name" - severity = "Critical" [gosec] [[gosec.ruleset]] [gosec.ruleset.identifier] type = "CWE" - value = "CWE-79" + value = "703" [gosec.ruleset.override] severity = "Critical" ``` +If a vulnerability is found with a type `CWE` with a value of `703` then +the vulnerability severity is overwritten to `Critical`. + #### Synthesize a custom configuration To create a custom configuration, you can use passthrough chains. diff --git a/lib/api/deploy_tokens.rb b/lib/api/deploy_tokens.rb index e9beeb18d62..074c307e881 100644 --- a/lib/api/deploy_tokens.rb +++ b/lib/api/deploy_tokens.rb @@ -93,6 +93,21 @@ module API end end + desc 'Get a project deploy token' do + detail 'This feature was introduced in GitLab 14.9' + success Entities::DeployToken + end + params do + requires :token_id, type: Integer, desc: 'The deploy token ID' + end + get ':id/deploy_tokens/:token_id' do + authorize!(:read_deploy_token, user_project) + + deploy_token = user_project.deploy_tokens.find(params[:token_id]) + + present deploy_token, with: Entities::DeployToken + end + desc 'Delete a project deploy token' do detail 'This feature was introduced in GitLab 12.9' end @@ -159,6 +174,21 @@ module API end end + desc 'Get a group deploy token' do + detail 'This feature was introduced in GitLab 14.9' + success Entities::DeployToken + end + params do + requires :token_id, type: Integer, desc: 'The deploy token ID' + end + get ':id/deploy_tokens/:token_id' do + authorize!(:read_deploy_token, user_group) + + deploy_token = user_group.deploy_tokens.find(params[:token_id]) + + present deploy_token, with: Entities::DeployToken + end + desc 'Delete a group deploy token' do detail 'This feature was introduced in GitLab 12.9' end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 8612192558b..de9d42bdce7 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -474,17 +474,22 @@ module API model.errors.messages end - def render_spam_error! - render_api_error!({ error: 'Spam detected' }, 400) + def render_api_error!(message, status) + render_structured_api_error!({ 'message' => message }, status) end - def render_api_error!(message, status) + def render_structured_api_error!(hash, status) + # Use this method instead of `render_api_error!` when you have additional top-level + # hash entries in addition to 'message' which need to be passed to `#error!` + set_status_code_in_env(status) + error!(hash, status, header) + end + + def set_status_code_in_env(status) # grape-logging doesn't pass the status code, so this is a # workaround for getting that information in the loggers: # https://github.com/aserafin/grape_logging/issues/71 env[API_RESPONSE_STATUS_CODE] = Rack::Utils.status_code(status) - - error!({ 'message' => message }, status, header) end def handle_api_exception(exception) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index b5a559516f4..e9bb9fe7a97 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -4,6 +4,7 @@ module API class Issues < ::API::Base include PaginationParams helpers Helpers::IssuesHelpers + helpers SpammableActions::CaptchaCheck::RestApiActionsSupport before { authenticate_non_get! } @@ -275,14 +276,12 @@ module API params: issue_params, spam_params: spam_params).execute - if issue.spam? - render_api_error!({ error: 'Spam detected' }, 400) - end - if issue.valid? present issue, with: Entities::Issue, current_user: current_user, project: user_project else - render_validation_error!(issue) + with_captcha_check_rest_api(spammable: issue) do + render_validation_error!(issue) + end end rescue ::ActiveRecord::RecordNotUnique render_api_error!('Duplicated issue', 409) @@ -320,12 +319,12 @@ module API params: update_params, spam_params: spam_params).execute(issue) - render_spam_error! if issue.spam? - if issue.valid? present issue, with: Entities::Issue, current_user: current_user, project: user_project else - render_validation_error!(issue) + with_captcha_check_rest_api(spammable: issue) do + render_validation_error!(issue) + end end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index fdbfdf1f7a9..a80e45637dc 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -13,6 +13,7 @@ module API end resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do helpers Helpers::SnippetsHelpers + helpers SpammableActions::CaptchaCheck::RestApiActionsSupport helpers do def check_snippets_enabled forbidden! unless user_project.feature_available?(:snippets, current_user) @@ -82,9 +83,9 @@ module API if service_response.success? present snippet, with: Entities::ProjectSnippet, current_user: current_user else - render_spam_error! if snippet.spam? - - render_api_error!({ error: service_response.message }, service_response.http_status) + with_captcha_check_rest_api(spammable: snippet) do + render_api_error!({ error: service_response.message }, service_response.http_status) + end end end @@ -124,9 +125,9 @@ module API if service_response.success? present snippet, with: Entities::ProjectSnippet, current_user: current_user else - render_spam_error! if snippet.spam? - - render_api_error!({ error: service_response.message }, service_response.http_status) + with_captcha_check_rest_api(spammable: snippet) do + render_api_error!({ error: service_response.message }, service_response.http_status) + end end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index c4b17a62b59..9a3c68bc854 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -9,6 +9,7 @@ module API resource :snippets do helpers Helpers::SnippetsHelpers + helpers SpammableActions::CaptchaCheck::RestApiActionsSupport helpers do def snippets_for_current_user SnippetsFinder.new(current_user, author: current_user).execute @@ -91,9 +92,9 @@ module API if service_response.success? present snippet, with: Entities::PersonalSnippet, current_user: current_user else - render_spam_error! if snippet.spam? - - render_api_error!({ error: service_response.message }, service_response.http_status) + with_captcha_check_rest_api(spammable: snippet) do + render_api_error!({ error: service_response.message }, service_response.http_status) + end end end @@ -135,9 +136,9 @@ module API if service_response.success? present snippet, with: Entities::PersonalSnippet, current_user: current_user else - render_spam_error! if snippet.spam? - - render_api_error!({ error: service_response.message }, service_response.http_status) + with_captcha_check_rest_api(spammable: snippet) do + render_api_error!({ error: service_response.message }, service_response.http_status) + end end end diff --git a/lib/gitlab.rb b/lib/gitlab.rb index d03a5add8e8..d33120575a2 100644 --- a/lib/gitlab.rb +++ b/lib/gitlab.rb @@ -88,12 +88,8 @@ module Gitlab Gitlab::Saas.subdomain_regex === Gitlab.config.gitlab.url end - def self.dev_env_org_or_com? - dev_env_or_com? || org? - end - - def self.dev_env_or_com? - com? + def self.org_or_com? + org? || com? end def self.dev_or_test_env? diff --git a/lib/gitlab/experiment/rollout/feature.rb b/lib/gitlab/experiment/rollout/feature.rb index c24f2a23862..70c363877b1 100644 --- a/lib/gitlab/experiment/rollout/feature.rb +++ b/lib/gitlab/experiment/rollout/feature.rb @@ -13,7 +13,7 @@ module Gitlab # no inclusions, etc.) def enabled? return false unless feature_flag_defined? - return false unless Gitlab.dev_env_or_com? + return false unless Gitlab.com? return false unless ::Feature.enabled?(:gitlab_experiment, type: :ops, default_enabled: :yaml) feature_flag_instance.state != :off diff --git a/lib/gitlab/experimentation/controller_concern.rb b/lib/gitlab/experimentation/controller_concern.rb index 303d952381f..a68e2db4dac 100644 --- a/lib/gitlab/experimentation/controller_concern.rb +++ b/lib/gitlab/experimentation/controller_concern.rb @@ -20,7 +20,7 @@ module Gitlab end def set_experimentation_subject_id_cookie - if Gitlab.dev_env_or_com? + if Gitlab.com? return if cookies[:experimentation_subject_id].present? cookies.permanent.signed[:experimentation_subject_id] = { diff --git a/lib/gitlab/experimentation/experiment.rb b/lib/gitlab/experimentation/experiment.rb index 8ba95520638..b13f55e7969 100644 --- a/lib/gitlab/experimentation/experiment.rb +++ b/lib/gitlab/experimentation/experiment.rb @@ -18,7 +18,7 @@ module Gitlab # Temporary change, we will change `experiment_percentage` in future to `Feature.enabled? Feature.enabled?(feature_flag_name, type: :experiment, default_enabled: :yaml) - ::Gitlab.dev_env_or_com? && experiment_percentage > 0 + ::Gitlab.com? && experiment_percentage > 0 end def enabled_for_index?(index) diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index c76e006bae4..a3c53869789 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -40,7 +40,6 @@ module Gitlab gon.ee = Gitlab.ee? gon.jh = Gitlab.jh? gon.dot_com = Gitlab.com? - gon.dev_env_or_com = Gitlab.dev_env_or_com? if current_user gon.current_user_id = current_user.id diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e93715c2105..4d5893bf2f1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19843,6 +19843,9 @@ msgstr "" msgid "Integrations|Failed to load namespaces. Please try again." msgstr "" +msgid "Integrations|Failed to sign in to GitLab." +msgstr "" + msgid "Integrations|Failed to unlink namespace. Please try again." msgstr "" @@ -19957,6 +19960,9 @@ msgstr "" msgid "Integrations|Use default settings" msgstr "" +msgid "Integrations|You can close this window." +msgstr "" + msgid "Integrations|You can now close this window and return to the GitLab for Jira application." msgstr "" @@ -34205,6 +34211,9 @@ msgstr "" msgid "Signed in" msgstr "" +msgid "Signed in to GitLab" +msgstr "" + msgid "Signed in to GitLab as %{user_link}" msgstr "" diff --git a/qa/qa/specs/features/api/4_verify/remove_runner_spec.rb b/qa/qa/specs/features/api/4_verify/remove_runner_spec.rb index e69074e17e2..0d10783735b 100644 --- a/qa/qa/specs/features/api/4_verify/remove_runner_spec.rb +++ b/qa/qa/specs/features/api/4_verify/remove_runner_spec.rb @@ -20,7 +20,7 @@ module QA end # Removing a runner via the UI is covered by `spec/features/runners_spec.rb`` - it 'removes the runner' do + it 'removes the runner', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/355302', type: :investigating } do expect(runner.project.runners.size).to eq(1) expect(runner.project.runners.first[:description]).to eq(executor) diff --git a/qa/qa/specs/features/browser_ui/3_create/web_ide/web_terminal_spec.rb b/qa/qa/specs/features/browser_ui/3_create/web_ide/web_terminal_spec.rb index 022731faade..09459057992 100644 --- a/qa/qa/specs/features/browser_ui/3_create/web_ide/web_terminal_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/web_ide/web_terminal_spec.rb @@ -4,6 +4,8 @@ module QA RSpec.describe( 'Create', :runner, + # TODO: remove limitation to only run on main when the bug is fixed + only: { pipeline: :main }, quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338179', type: :bug diff --git a/spec/controllers/concerns/spammable_actions/captcha_check/rest_api_actions_support_spec.rb b/spec/controllers/concerns/spammable_actions/captcha_check/rest_api_actions_support_spec.rb new file mode 100644 index 00000000000..07c564b555e --- /dev/null +++ b/spec/controllers/concerns/spammable_actions/captcha_check/rest_api_actions_support_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SpammableActions::CaptchaCheck::RestApiActionsSupport do + include Rack::Test::Methods + + subject do + Class.new(Grape::API) do + helpers API::Helpers + helpers SpammableActions::CaptchaCheck::RestApiActionsSupport + + get ':id' do + # NOTE: This was the only way that seemed to work to inject the mock spammable into the + # Grape rack app instance. If there's a better way, improvements are welcome. + spammable = Object.fake_spammable_factory + with_captcha_check_rest_api(spammable: spammable) do + render_api_error!(spammable.errors, 400) + end + end + end + end + + def app + subject + end + + before do + allow(Gitlab::Recaptcha).to receive(:load_configurations!) { true } + end + + describe '#with_captcha_check_json_format' do + let(:spammable) { instance_double(Snippet) } + + before do + expect(spammable).to receive(:render_recaptcha?).at_least(:once) { render_recaptcha } + allow(Object).to receive(:fake_spammable_factory) { spammable } + end + + context 'when spammable.render_recaptcha? is true' do + let(:render_recaptcha) { true } + let(:spam_log) { instance_double(SpamLog, id: 1) } + let(:spammable) { instance_double(Snippet, spam?: true, render_recaptcha?: render_recaptcha, spam_log: spam_log) } + let(:recaptcha_site_key) { 'abc123' } + let(:err_msg) { 'You gotta solve the CAPTCHA' } + let(:spam_action_response_fields) do + { + spam: true, + needs_captcha_response: render_recaptcha, + spam_log_id: 1, + captcha_site_key: recaptcha_site_key + } + end + + it 'renders json containing spam_action_response_fields' do + allow(spammable).to receive_message_chain('errors.full_messages.to_sentence') { err_msg } + allow(Gitlab::CurrentSettings).to receive(:recaptcha_site_key) { recaptcha_site_key } + response = get '/test' + expected_response = { + 'needs_captcha_response' => render_recaptcha, + 'spam_log_id' => 1, + 'captcha_site_key' => recaptcha_site_key, + 'message' => { 'error' => err_msg } + } + expect(Gitlab::Json.parse(response.body)).to eq(expected_response) + expect(response.status).to eq(409) + end + end + + context 'when spammable.render_recaptcha? is false' do + let(:render_recaptcha) { false } + let(:errors) { { 'base' => "It's definitely spam" } } + + it 'yields to block' do + allow(spammable).to receive(:errors) { errors } + + response = get 'test' + expected_response = { + 'message' => errors + } + expect(Gitlab::Json.parse(response.body)).to eq(expected_response) + expect(response.status).to eq(400) + end + end + end +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 73c83ed0ed4..9d3711d8a96 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1018,6 +1018,7 @@ RSpec.describe Projects::IssuesController do end it 'returns 200 status' do + update_verified_issue expect(response).to have_gitlab_http_status(:ok) end diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index b99c28627f2..15b45099a06 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -243,12 +243,12 @@ RSpec.describe ApplicationExperiment, :experiment do with_them do it "returns the url or nil if invalid" do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Gitlab).to receive(:com?).and_return(true) expect(application_experiment.process_redirect_url(url)).to eq(processed_url) end it "considers all urls invalid when not on dev or com" do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(false) + allow(Gitlab).to receive(:com?).and_return(false) expect(application_experiment.process_redirect_url(url)).to be_nil end end diff --git a/spec/fixtures/api/schemas/public_api/v4/deploy_token.json b/spec/fixtures/api/schemas/public_api/v4/deploy_token.json index c4d3f944aea..102ab95a4ee 100644 --- a/spec/fixtures/api/schemas/public_api/v4/deploy_token.json +++ b/spec/fixtures/api/schemas/public_api/v4/deploy_token.json @@ -5,7 +5,9 @@ "name", "username", "expires_at", - "scopes" + "scopes", + "revoked", + "expired" ], "properties": { "id": { @@ -26,6 +28,12 @@ }, "token": { "type": "string" + }, + "revoked": { + "type": "boolean" + }, + "expired": { + "type": "boolean" } } }
\ No newline at end of file diff --git a/spec/frontend/authentication/webauthn/util_spec.js b/spec/frontend/authentication/webauthn/util_spec.js index c9b8bfd8679..bc44b47d0ba 100644 --- a/spec/frontend/authentication/webauthn/util_spec.js +++ b/spec/frontend/authentication/webauthn/util_spec.js @@ -1,4 +1,4 @@ -import { base64ToBuffer, bufferToBase64 } from '~/authentication/webauthn/util'; +import { base64ToBuffer, bufferToBase64, base64ToBase64Url } from '~/authentication/webauthn/util'; const encodedString = 'SGVsbG8gd29ybGQh'; const stringBytes = [72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 33]; @@ -16,4 +16,19 @@ describe('Webauthn utils', () => { const buffer = base64ToBuffer(encodedString); expect(bufferToBase64(buffer)).toBe(encodedString); }); + + describe('base64ToBase64Url', () => { + it.each` + argument | expectedResult + ${'asd+'} | ${'asd-'} + ${'asd/'} | ${'asd_'} + ${'asd='} | ${'asd'} + ${'+asd'} | ${'-asd'} + ${'/asd'} | ${'_asd'} + ${'=asd'} | ${'=asd'} + ${'a+bc/def=ghigjk=='} | ${'a-bc_def=ghigjk'} + `('returns $expectedResult when argument is $argument', ({ argument, expectedResult }) => { + expect(base64ToBase64Url(argument)).toBe(expectedResult); + }); + }); }); diff --git a/spec/frontend/jira_connect/subscriptions/components/app_spec.js b/spec/frontend/jira_connect/subscriptions/components/app_spec.js index aa0f1440b20..6b3ca7ffd65 100644 --- a/spec/frontend/jira_connect/subscriptions/components/app_spec.js +++ b/spec/frontend/jira_connect/subscriptions/components/app_spec.js @@ -8,6 +8,7 @@ import SubscriptionsPage from '~/jira_connect/subscriptions/pages/subscriptions. import UserLink from '~/jira_connect/subscriptions/components/user_link.vue'; import createStore from '~/jira_connect/subscriptions/store'; import { SET_ALERT } from '~/jira_connect/subscriptions/store/mutation_types'; +import { I18N_DEFAULT_SIGN_IN_ERROR_MESSAGE } from '~/jira_connect/subscriptions/constants'; import { __ } from '~/locale'; import { mockSubscription } from '../mock_data'; @@ -24,6 +25,7 @@ describe('JiraConnectApp', () => { const findAlertLink = () => findAlert().findComponent(GlLink); const findSignInPage = () => wrapper.findComponent(SignInPage); const findSubscriptionsPage = () => wrapper.findComponent(SubscriptionsPage); + const findUserLink = () => wrapper.findComponent(UserLink); const createComponent = ({ provide, mountFn = shallowMountExtended } = {}) => { store = createStore(); @@ -78,10 +80,11 @@ describe('JiraConnectApp', () => { }, }); - const userLink = wrapper.findComponent(UserLink); + const userLink = findUserLink(); expect(userLink.exists()).toBe(true); expect(userLink.props()).toEqual({ hasSubscriptions: false, + user: null, userSignedIn: false, }); }); @@ -153,4 +156,55 @@ describe('JiraConnectApp', () => { }); }); }); + + describe('when user signed out', () => { + describe('when sign in page emits `sign-in-oauth` event', () => { + const mockUser = { name: 'test' }; + beforeEach(async () => { + createComponent({ + provide: { + usersPath: '/mock', + subscriptions: [], + }, + }); + findSignInPage().vm.$emit('sign-in-oauth', mockUser); + + await nextTick(); + }); + + it('hides sign in page and renders subscriptions page', () => { + expect(findSignInPage().exists()).toBe(false); + expect(findSubscriptionsPage().exists()).toBe(true); + }); + + it('sets correct UserLink props', () => { + expect(findUserLink().props()).toMatchObject({ + user: mockUser, + userSignedIn: true, + }); + }); + }); + + describe('when sign in page emits `error` event', () => { + beforeEach(async () => { + createComponent({ + provide: { + usersPath: '/mock', + subscriptions: [], + }, + }); + findSignInPage().vm.$emit('error'); + + await nextTick(); + }); + + it('displays alert', () => { + const alert = findAlert(); + + expect(alert.exists()).toBe(true); + expect(alert.html()).toContain(I18N_DEFAULT_SIGN_IN_ERROR_MESSAGE); + expect(alert.props('variant')).toBe('danger'); + }); + }); + }); }); diff --git a/spec/frontend/jira_connect/subscriptions/components/sign_in_button_spec.js b/spec/frontend/jira_connect/subscriptions/components/sign_in_legacy_button_spec.js index 94dcf9decec..4ebfaed261e 100644 --- a/spec/frontend/jira_connect/subscriptions/components/sign_in_button_spec.js +++ b/spec/frontend/jira_connect/subscriptions/components/sign_in_legacy_button_spec.js @@ -1,18 +1,18 @@ import { GlButton } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import { getGitlabSignInURL } from '~/jira_connect/subscriptions/utils'; -import SignInButton from '~/jira_connect/subscriptions/components/sign_in_button.vue'; +import SignInLegacyButton from '~/jira_connect/subscriptions/components/sign_in_legacy_button.vue'; import waitForPromises from 'helpers/wait_for_promises'; const MOCK_USERS_PATH = '/user'; jest.mock('~/jira_connect/subscriptions/utils'); -describe('SignInButton', () => { +describe('SignInLegacyButton', () => { let wrapper; const createComponent = ({ slots } = {}) => { - wrapper = shallowMount(SignInButton, { + wrapper = shallowMount(SignInLegacyButton, { propsData: { usersPath: MOCK_USERS_PATH, }, @@ -30,7 +30,7 @@ describe('SignInButton', () => { createComponent(); expect(findButton().exists()).toBe(true); - expect(findButton().text()).toBe(SignInButton.i18n.defaultButtonText); + expect(findButton().text()).toBe(SignInLegacyButton.i18n.defaultButtonText); }); describe.each` diff --git a/spec/frontend/jira_connect/subscriptions/components/sign_in_oauth_button_spec.js b/spec/frontend/jira_connect/subscriptions/components/sign_in_oauth_button_spec.js new file mode 100644 index 00000000000..18274cd4362 --- /dev/null +++ b/spec/frontend/jira_connect/subscriptions/components/sign_in_oauth_button_spec.js @@ -0,0 +1,204 @@ +import { GlButton } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import MockAdapter from 'axios-mock-adapter'; +import { nextTick } from 'vue'; +import SignInOauthButton from '~/jira_connect/subscriptions/components/sign_in_oauth_button.vue'; +import { + I18N_DEFAULT_SIGN_IN_BUTTON_TEXT, + OAUTH_WINDOW_OPTIONS, +} from '~/jira_connect/subscriptions/constants'; +import axios from '~/lib/utils/axios_utils'; +import waitForPromises from 'helpers/wait_for_promises'; +import httpStatus from '~/lib/utils/http_status'; +import AccessorUtilities from '~/lib/utils/accessor'; + +jest.mock('~/lib/utils/accessor'); +jest.mock('~/jira_connect/subscriptions/utils'); +jest.mock('~/jira_connect/subscriptions/pkce', () => ({ + createCodeVerifier: jest.fn().mockReturnValue('mock-verifier'), + createCodeChallenge: jest.fn().mockResolvedValue('mock-challenge'), +})); + +const mockOauthMetadata = { + oauth_authorize_url: 'https://gitlab.com/mockOauth', + oauth_token_url: 'https://gitlab.com/mockOauthToken', + state: 'good-state', +}; + +describe('SignInOauthButton', () => { + let wrapper; + let mockAxios; + + const createComponent = ({ slots } = {}) => { + wrapper = shallowMount(SignInOauthButton, { + slots, + provide: { + oauthMetadata: mockOauthMetadata, + }, + }); + }; + + beforeEach(() => { + mockAxios = new MockAdapter(axios); + }); + + afterEach(() => { + wrapper.destroy(); + mockAxios.restore(); + }); + + const findButton = () => wrapper.findComponent(GlButton); + + it('displays a button', () => { + createComponent(); + + expect(findButton().exists()).toBe(true); + expect(findButton().text()).toBe(I18N_DEFAULT_SIGN_IN_BUTTON_TEXT); + }); + + it.each` + scenario | cryptoAvailable + ${'when crypto API is available'} | ${true} + ${'when crypto API is unavailable'} | ${false} + `('$scenario when canUseCrypto returns $cryptoAvailable', ({ cryptoAvailable }) => { + AccessorUtilities.canUseCrypto = jest.fn().mockReturnValue(cryptoAvailable); + createComponent(); + + expect(findButton().props('disabled')).toBe(!cryptoAvailable); + }); + + describe('on click', () => { + beforeEach(async () => { + jest.spyOn(window, 'open').mockReturnValue(); + createComponent(); + + findButton().vm.$emit('click'); + + await nextTick(); + }); + + it('sets `loading` prop of button to `true`', () => { + expect(findButton().props('loading')).toBe(true); + }); + + it('calls `window.open` with correct arguments', () => { + expect(window.open).toHaveBeenCalledWith( + `${mockOauthMetadata.oauth_authorize_url}?code_challenge=mock-challenge&code_challenge_method=S256`, + I18N_DEFAULT_SIGN_IN_BUTTON_TEXT, + OAUTH_WINDOW_OPTIONS, + ); + }); + + it('sets the `codeVerifier` internal state', () => { + expect(wrapper.vm.codeVerifier).toBe('mock-verifier'); + }); + + describe('on window message event', () => { + describe('when window message properties are corrupted', () => { + describe.each` + origin | state | messageOrigin | messageState + ${window.origin} | ${mockOauthMetadata.state} | ${'bad-origin'} | ${mockOauthMetadata.state} + ${window.origin} | ${mockOauthMetadata.state} | ${window.origin} | ${'bad-state'} + `( + 'when message is [state=$messageState, origin=$messageOrigin]', + ({ messageOrigin, messageState }) => { + beforeEach(async () => { + const mockEvent = { + origin: messageOrigin, + data: { + state: messageState, + code: '1234', + }, + }; + window.dispatchEvent(new MessageEvent('message', mockEvent)); + await waitForPromises(); + }); + + it('emits `error` event', () => { + expect(wrapper.emitted('error')).toBeTruthy(); + }); + + it('does not emit `sign-in` event', () => { + expect(wrapper.emitted('sign-in')).toBeFalsy(); + }); + + it('sets `loading` prop of button to `false`', () => { + expect(findButton().props('loading')).toBe(false); + }); + }, + ); + }); + + describe('when window message properties are valid', () => { + const mockAccessToken = '5678'; + const mockUser = { name: 'test user' }; + const mockEvent = { + origin: window.origin, + data: { + state: mockOauthMetadata.state, + code: '1234', + }, + }; + + describe('when API requests succeed', () => { + beforeEach(async () => { + jest.spyOn(axios, 'post'); + jest.spyOn(axios, 'get'); + mockAxios + .onPost(mockOauthMetadata.oauth_token_url) + .replyOnce(httpStatus.OK, { access_token: mockAccessToken }); + mockAxios.onGet('/api/v4/user').replyOnce(httpStatus.OK, mockUser); + + window.dispatchEvent(new MessageEvent('message', mockEvent)); + + await waitForPromises(); + }); + + it('executes POST request to Oauth token endpoint', () => { + expect(axios.post).toHaveBeenCalledWith(mockOauthMetadata.oauth_token_url, { + code: '1234', + code_verifier: 'mock-verifier', + }); + }); + + it('executes GET request to fetch user data', () => { + expect(axios.get).toHaveBeenCalledWith('/api/v4/user', { + headers: { Authorization: `Bearer ${mockAccessToken}` }, + }); + }); + + it('emits `sign-in` event with user data', () => { + expect(wrapper.emitted('sign-in')[0]).toEqual([mockUser]); + }); + }); + + describe('when API requests fail', () => { + beforeEach(async () => { + jest.spyOn(axios, 'post'); + jest.spyOn(axios, 'get'); + mockAxios + .onPost(mockOauthMetadata.oauth_token_url) + .replyOnce(httpStatus.INTERNAL_SERVER_ERROR, { access_token: mockAccessToken }); + mockAxios.onGet('/api/v4/user').replyOnce(httpStatus.INTERNAL_SERVER_ERROR, mockUser); + + window.dispatchEvent(new MessageEvent('message', mockEvent)); + + await waitForPromises(); + }); + + it('emits `error` event', () => { + expect(wrapper.emitted('error')).toBeTruthy(); + }); + + it('does not emit `sign-in` event', () => { + expect(wrapper.emitted('sign-in')).toBeFalsy(); + }); + + it('sets `loading` prop of button to `false`', () => { + expect(findButton().props('loading')).toBe(false); + }); + }); + }); + }); + }); +}); diff --git a/spec/frontend/jira_connect/subscriptions/components/user_link_spec.js b/spec/frontend/jira_connect/subscriptions/components/user_link_spec.js index b98a36269a3..2f5e47d1ae4 100644 --- a/spec/frontend/jira_connect/subscriptions/components/user_link_spec.js +++ b/spec/frontend/jira_connect/subscriptions/components/user_link_spec.js @@ -7,7 +7,7 @@ jest.mock('~/jira_connect/subscriptions/utils', () => ({ getGitlabSignInURL: jest.fn().mockImplementation((path) => Promise.resolve(path)), })); -describe('SubscriptionsList', () => { +describe('UserLink', () => { let wrapper; const createComponent = (propsData = {}, { provide } = {}) => { @@ -68,24 +68,35 @@ describe('SubscriptionsList', () => { }); describe('gitlab user link', () => { - window.gon = { current_username: 'root' }; + describe.each` + current_username | gitlabUserPath | user | expectedUserHandle | expectedUserLink + ${'root'} | ${'/root'} | ${{ username: 'test-user' }} | ${'@root'} | ${'/root'} + ${'root'} | ${'/root'} | ${undefined} | ${'@root'} | ${'/root'} + ${undefined} | ${undefined} | ${{ username: 'test-user' }} | ${'@test-user'} | ${'/test-user'} + `( + 'when current_username=$current_username, gitlabUserPath=$gitlabUserPath and user=$user', + ({ current_username, gitlabUserPath, user, expectedUserHandle, expectedUserLink }) => { + beforeEach(() => { + window.gon = { current_username, relative_root_url: '' }; - beforeEach(() => { - createComponent( - { - userSignedIn: true, - hasSubscriptions: true, - }, - { provide: { gitlabUserPath: '/root' } }, - ); - }); + createComponent( + { + userSignedIn: true, + hasSubscriptions: true, + user, + }, + { provide: { gitlabUserPath } }, + ); + }); - it('renders with correct href', () => { - expect(findGitlabUserLink().attributes('href')).toBe('/root'); - }); + it(`sets href to ${expectedUserLink}`, () => { + expect(findGitlabUserLink().attributes('href')).toBe(expectedUserLink); + }); - it('contains GitLab user handle', () => { - expect(findGitlabUserLink().text()).toBe('@root'); - }); + it(`renders ${expectedUserHandle} as text`, () => { + expect(findGitlabUserLink().text()).toBe(expectedUserHandle); + }); + }, + ); }); }); diff --git a/spec/frontend/jira_connect/subscriptions/pages/sign_in_spec.js b/spec/frontend/jira_connect/subscriptions/pages/sign_in_spec.js index 4e3297506f1..175896c4ab0 100644 --- a/spec/frontend/jira_connect/subscriptions/pages/sign_in_spec.js +++ b/spec/frontend/jira_connect/subscriptions/pages/sign_in_spec.js @@ -1,26 +1,44 @@ -import { mount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import SignInPage from '~/jira_connect/subscriptions/pages/sign_in.vue'; -import SignInButton from '~/jira_connect/subscriptions/components/sign_in_button.vue'; +import SignInLegacyButton from '~/jira_connect/subscriptions/components/sign_in_legacy_button.vue'; +import SignInOauthButton from '~/jira_connect/subscriptions/components/sign_in_oauth_button.vue'; import SubscriptionsList from '~/jira_connect/subscriptions/components/subscriptions_list.vue'; import createStore from '~/jira_connect/subscriptions/store'; +import { I18N_DEFAULT_SIGN_IN_BUTTON_TEXT } from '../../../../../app/assets/javascripts/jira_connect/subscriptions/constants'; jest.mock('~/jira_connect/subscriptions/utils'); +const mockUsersPath = '/test'; +const defaultProvide = { + oauthMetadata: {}, + usersPath: mockUsersPath, +}; + describe('SignInPage', () => { let wrapper; let store; - const findSignInButton = () => wrapper.findComponent(SignInButton); + const findSignInLegacyButton = () => wrapper.findComponent(SignInLegacyButton); + const findSignInOauthButton = () => wrapper.findComponent(SignInOauthButton); const findSubscriptionsList = () => wrapper.findComponent(SubscriptionsList); - const createComponent = ({ provide, props } = {}) => { + const createComponent = ({ props, jiraConnectOauthEnabled } = {}) => { store = createStore(); - wrapper = mount(SignInPage, { + wrapper = shallowMount(SignInPage, { store, - provide, + provide: { + ...defaultProvide, + glFeatures: { + jiraConnectOauth: jiraConnectOauthEnabled, + }, + }, propsData: props, + stubs: { + SignInLegacyButton, + SignInOauthButton, + }, }); }; @@ -29,33 +47,74 @@ describe('SignInPage', () => { }); describe('template', () => { - const mockUsersPath = '/test'; describe.each` - scenario | expectSubscriptionsList | signInButtonText - ${'with subscriptions'} | ${true} | ${SignInPage.i18n.signinButtonTextWithSubscriptions} - ${'without subscriptions'} | ${false} | ${SignInButton.i18n.defaultButtonText} - `('$scenario', ({ expectSubscriptionsList, signInButtonText }) => { - beforeEach(() => { - createComponent({ - provide: { - usersPath: mockUsersPath, - }, - props: { - hasSubscriptions: expectSubscriptionsList, - }, + scenario | hasSubscriptions | signInButtonText + ${'with subscriptions'} | ${true} | ${SignInPage.i18n.signInButtonTextWithSubscriptions} + ${'without subscriptions'} | ${false} | ${I18N_DEFAULT_SIGN_IN_BUTTON_TEXT} + `('$scenario', ({ hasSubscriptions, signInButtonText }) => { + describe('when `jiraConnectOauthEnabled` feature flag is disabled', () => { + beforeEach(() => { + createComponent({ + jiraConnectOauthEnabled: false, + props: { + hasSubscriptions, + }, + }); }); - }); - it(`renders sign in button with text ${signInButtonText}`, () => { - expect(findSignInButton().text()).toMatchInterpolatedText(signInButtonText); + it('renders legacy sign in button', () => { + const button = findSignInLegacyButton(); + expect(button.props('usersPath')).toBe(mockUsersPath); + expect(button.text()).toMatchInterpolatedText(signInButtonText); + }); }); - it('renders sign in button with `usersPath` prop', () => { - expect(findSignInButton().props('usersPath')).toBe(mockUsersPath); + describe('when `jiraConnectOauthEnabled` feature flag is enabled', () => { + beforeEach(() => { + createComponent({ + jiraConnectOauthEnabled: true, + props: { + hasSubscriptions, + }, + }); + }); + + describe('oauth sign in button', () => { + it('renders oauth sign in button', () => { + const button = findSignInOauthButton(); + expect(button.text()).toMatchInterpolatedText(signInButtonText); + }); + + describe('when button emits `sign-in` event', () => { + it('emits `sign-in-oauth` event', () => { + const button = findSignInOauthButton(); + + const mockUser = { name: 'test' }; + button.vm.$emit('sign-in', mockUser); + + expect(wrapper.emitted('sign-in-oauth')[0]).toEqual([mockUser]); + }); + }); + + describe('when button emits `error` event', () => { + it('emits `error` event', () => { + const button = findSignInOauthButton(); + button.vm.$emit('error'); + + expect(wrapper.emitted('error')).toBeTruthy(); + }); + }); + }); }); - it(`${expectSubscriptionsList ? 'renders' : 'does not render'} subscriptions list`, () => { - expect(findSubscriptionsList().exists()).toBe(expectSubscriptionsList); + it(`${hasSubscriptions ? 'renders' : 'does not render'} subscriptions list`, () => { + createComponent({ + props: { + hasSubscriptions, + }, + }); + + expect(findSubscriptionsList().exists()).toBe(hasSubscriptions); }); }); }); diff --git a/spec/frontend/jira_connect/subscriptions/pkce_spec.js b/spec/frontend/jira_connect/subscriptions/pkce_spec.js new file mode 100644 index 00000000000..4ee88059b7a --- /dev/null +++ b/spec/frontend/jira_connect/subscriptions/pkce_spec.js @@ -0,0 +1,48 @@ +import crypto from 'crypto'; +import { TextEncoder, TextDecoder } from 'util'; + +import { createCodeVerifier, createCodeChallenge } from '~/jira_connect/subscriptions/pkce'; + +global.TextEncoder = TextEncoder; +global.TextDecoder = TextDecoder; + +describe('pkce', () => { + beforeAll(() => { + Object.defineProperty(global.self, 'crypto', { + value: { + getRandomValues: (arr) => crypto.randomBytes(arr.length), + subtle: { + digest: jest.fn().mockResolvedValue(new ArrayBuffer(1)), + }, + }, + }); + }); + + describe('createCodeVerifier', () => { + it('calls `window.crypto.getRandomValues`', () => { + window.crypto.getRandomValues = jest.fn(); + createCodeVerifier(); + + expect(window.crypto.getRandomValues).toHaveBeenCalled(); + }); + + it(`returns a string with 128 characters`, () => { + const codeVerifier = createCodeVerifier(); + expect(codeVerifier).toHaveLength(128); + }); + }); + + describe('createCodeChallenge', () => { + it('calls `window.crypto.subtle.digest` with correct arguments', async () => { + await createCodeChallenge('1234'); + + expect(window.crypto.subtle.digest).toHaveBeenCalledWith('SHA-256', expect.anything()); + }); + + it('returns base64 URL-encoded string', async () => { + const codeChallenge = await createCodeChallenge('1234'); + + expect(codeChallenge).toBe('AA'); + }); + }); +}); diff --git a/spec/helpers/jira_connect_helper_spec.rb b/spec/helpers/jira_connect_helper_spec.rb index 0f78185dc7d..1c1b2a22b7c 100644 --- a/spec/helpers/jira_connect_helper_spec.rb +++ b/spec/helpers/jira_connect_helper_spec.rb @@ -7,6 +7,11 @@ RSpec.describe JiraConnectHelper do let_it_be(:subscription) { create(:jira_connect_subscription) } let(:user) { create(:user) } + let(:client_id) { '123' } + + before do + stub_env('JIRA_CONNECT_OAUTH_CLIENT_ID', client_id) + end subject { helper.jira_connect_app_data([subscription]) } @@ -29,6 +34,47 @@ RSpec.describe JiraConnectHelper do expect(subject[:users_path]).to eq(jira_connect_users_path) end + context 'with oauth_metadata' do + let(:oauth_metadata) { helper.jira_connect_app_data([subscription])[:oauth_metadata] } + + subject(:parsed_oauth_metadata) { Gitlab::Json.parse(oauth_metadata).deep_symbolize_keys } + + it 'assigns oauth_metadata' do + expect(parsed_oauth_metadata).to include( + oauth_authorize_url: start_with('http://test.host/oauth/authorize?'), + oauth_token_url: 'http://test.host/oauth/token', + state: %r/[a-z0-9.]{32}/, + oauth_token_payload: hash_including( + grant_type: 'authorization_code', + client_id: client_id, + redirect_uri: 'http://test.host/-/jira_connect/oauth_callbacks' + ) + ) + end + + it 'includes oauth_authorize_url with all params' do + params = Rack::Utils.parse_nested_query(URI.parse(parsed_oauth_metadata[:oauth_authorize_url]).query) + + expect(params).to include( + 'client_id' => client_id, + 'response_type' => 'code', + 'scope' => 'api', + 'redirect_uri' => 'http://test.host/-/jira_connect/oauth_callbacks', + 'state' => parsed_oauth_metadata[:state] + ) + end + + context 'jira_connect_oauth feature is disabled' do + before do + stub_feature_flags(jira_connect_oauth: false) + end + + it 'does not assign oauth_metadata' do + expect(oauth_metadata).to be_nil + end + end + end + it 'passes group as "skip_groups" param' do skip_groups_param = CGI.escape('skip_groups[]') diff --git a/spec/helpers/sessions_helper_spec.rb b/spec/helpers/sessions_helper_spec.rb index 816e43669bd..fd3d7100ba1 100644 --- a/spec/helpers/sessions_helper_spec.rb +++ b/spec/helpers/sessions_helper_spec.rb @@ -8,7 +8,7 @@ RSpec.describe SessionsHelper do context 'when on .com' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Gitlab).to receive(:com?).and_return(true) end it 'when flash notice is empty it is false' do @@ -29,7 +29,7 @@ RSpec.describe SessionsHelper do context 'when not on .com' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(false) + allow(Gitlab).to receive(:com?).and_return(false) end it 'when flash notice is devise confirmed message it is false' do diff --git a/spec/helpers/whats_new_helper_spec.rb b/spec/helpers/whats_new_helper_spec.rb index 9ae7ef38736..011152b2d6a 100644 --- a/spec/helpers/whats_new_helper_spec.rb +++ b/spec/helpers/whats_new_helper_spec.rb @@ -39,14 +39,14 @@ RSpec.describe WhatsNewHelper do subject { helper.display_whats_new? } it 'returns true when gitlab.com' do - allow(Gitlab).to receive(:dev_env_org_or_com?).and_return(true) + allow(Gitlab).to receive(:org_or_com?).and_return(true) expect(subject).to be true end context 'when self-managed' do before do - allow(Gitlab).to receive(:dev_env_org_or_com?).and_return(false) + allow(Gitlab).to receive(:org_or_com?).and_return(false) end it 'returns true if user is signed in' do @@ -71,7 +71,7 @@ RSpec.describe WhatsNewHelper do with_them do it 'returns correct result depending on variant' do - allow(Gitlab).to receive(:dev_env_org_or_com?).and_return(true) + allow(Gitlab).to receive(:org_or_com?).and_return(true) Gitlab::CurrentSettings.update!(whats_new_variant: ApplicationSetting.whats_new_variants[variant]) expect(subject).to eq(result) diff --git a/spec/lib/gitlab/experiment/rollout/feature_spec.rb b/spec/lib/gitlab/experiment/rollout/feature_spec.rb index 21520a39565..82603e6fe0f 100644 --- a/spec/lib/gitlab/experiment/rollout/feature_spec.rb +++ b/spec/lib/gitlab/experiment/rollout/feature_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Gitlab::Experiment::Rollout::Feature, :experiment do before do stub_feature_flags(gitlab_experiment: true) allow(subject).to receive(:feature_flag_defined?).and_return(true) - allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Gitlab).to receive(:com?).and_return(true) allow(subject).to receive(:feature_flag_instance).and_return(double(state: :on)) end @@ -26,7 +26,7 @@ RSpec.describe Gitlab::Experiment::Rollout::Feature, :experiment do end it "isn't enabled if we're not in dev or dotcom environments" do - expect(Gitlab).to receive(:dev_env_or_com?).and_return(false) + expect(Gitlab).to receive(:com?).and_return(false) expect(subject).not_to be_enabled end diff --git a/spec/lib/gitlab/experimentation/controller_concern_spec.rb b/spec/lib/gitlab/experimentation/controller_concern_spec.rb index 8a96771eeb8..435a0d56301 100644 --- a/spec/lib/gitlab/experimentation/controller_concern_spec.rb +++ b/spec/lib/gitlab/experimentation/controller_concern_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do } ) - allow(Gitlab).to receive(:dev_env_or_com?).and_return(is_gitlab_com) + allow(Gitlab).to receive(:com?).and_return(is_gitlab_com) Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage) end diff --git a/spec/lib/gitlab/experimentation/experiment_spec.rb b/spec/lib/gitlab/experimentation/experiment_spec.rb index d9bf85460b3..a5cc69b9538 100644 --- a/spec/lib/gitlab/experimentation/experiment_spec.rb +++ b/spec/lib/gitlab/experimentation/experiment_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Gitlab::Experimentation::Experiment do describe '#active?' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(on_gitlab_com) + allow(Gitlab).to receive(:com?).and_return(on_gitlab_com) end subject { experiment.active? } diff --git a/spec/lib/gitlab_spec.rb b/spec/lib/gitlab_spec.rb index 1cc4e69b3d9..c44bb64a5c0 100644 --- a/spec/lib/gitlab_spec.rb +++ b/spec/lib/gitlab_spec.rb @@ -80,36 +80,42 @@ RSpec.describe Gitlab do end describe '.com?' do - it "is true when on #{Gitlab::Saas.com_url}" do - stub_config_setting(url: Gitlab::Saas.com_url) + context 'when not simulating SaaS' do + before do + stub_env('GITLAB_SIMULATE_SAAS', '0') + end - expect(described_class.com?).to eq true - end + it "is true when on #{Gitlab::Saas.com_url}" do + stub_config_setting(url: Gitlab::Saas.com_url) - it "is true when on #{Gitlab::Saas.staging_com_url}" do - stub_config_setting(url: Gitlab::Saas.staging_com_url) + expect(described_class.com?).to eq true + end - expect(described_class.com?).to eq true - end + it "is true when on #{Gitlab::Saas.staging_com_url}" do + stub_config_setting(url: Gitlab::Saas.staging_com_url) - it 'is true when on other gitlab subdomain' do - url_with_subdomain = Gitlab::Saas.com_url.gsub('https://', 'https://example.') - stub_config_setting(url: url_with_subdomain) + expect(described_class.com?).to eq true + end - expect(described_class.com?).to eq true - end + it 'is true when on other gitlab subdomain' do + url_with_subdomain = Gitlab::Saas.com_url.gsub('https://', 'https://example.') + stub_config_setting(url: url_with_subdomain) - it 'is true when on other gitlab subdomain with hyphen' do - url_with_subdomain = Gitlab::Saas.com_url.gsub('https://', 'https://test-example.') - stub_config_setting(url: url_with_subdomain) + expect(described_class.com?).to eq true + end - expect(described_class.com?).to eq true - end + it 'is true when on other gitlab subdomain with hyphen' do + url_with_subdomain = Gitlab::Saas.com_url.gsub('https://', 'https://test-example.') + stub_config_setting(url: url_with_subdomain) - it 'is false when not on GitLab.com' do - stub_config_setting(url: 'http://example.com') + expect(described_class.com?).to eq true + end - expect(described_class.com?).to eq false + it 'is false when not on GitLab.com' do + stub_config_setting(url: 'http://example.com') + + expect(described_class.com?).to eq false + end end it 'is true when GITLAB_SIMULATE_SAAS is true and in development' do @@ -210,31 +216,23 @@ RSpec.describe Gitlab do end end - describe '.dev_env_org_or_com?' do + describe '.org_or_com?' do it 'is true when on .com' do allow(described_class).to receive_messages(com?: true, org?: false) - expect(described_class.dev_env_org_or_com?).to eq true + expect(described_class.org_or_com?).to eq true end it 'is true when org' do allow(described_class).to receive_messages(com?: false, org?: true) - expect(described_class.dev_env_org_or_com?).to eq true + expect(described_class.org_or_com?).to eq true end it 'is false when not dev, org or com' do allow(described_class).to receive_messages(com?: false, org?: false) - expect(described_class.dev_env_org_or_com?).to eq false - end - end - - describe '.dev_env_or_com?' do - it 'is correctly calling com?' do - expect(described_class).to receive(:com?).and_call_original - - expect(described_class.dev_env_or_com?).to eq false + expect(described_class.org_or_com?).to eq false end end diff --git a/spec/migrations/20210805192450_update_trial_plans_ci_daily_pipeline_schedule_triggers_spec.rb b/spec/migrations/20210805192450_update_trial_plans_ci_daily_pipeline_schedule_triggers_spec.rb index 0b2f76baf1a..b1885b96adb 100644 --- a/spec/migrations/20210805192450_update_trial_plans_ci_daily_pipeline_schedule_triggers_spec.rb +++ b/spec/migrations/20210805192450_update_trial_plans_ci_daily_pipeline_schedule_triggers_spec.rb @@ -16,7 +16,7 @@ RSpec.describe UpdateTrialPlansCiDailyPipelineScheduleTriggers, :migration do context 'when the environment is dev or com' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Gitlab).to receive(:com?).and_return(true) end it 'sets the trial plan limits for ci_daily_pipeline_schedule_triggers' do @@ -57,7 +57,7 @@ RSpec.describe UpdateTrialPlansCiDailyPipelineScheduleTriggers, :migration do context 'when the environment is anything other than dev or com' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(false) + allow(Gitlab).to receive(:com?).and_return(false) end it 'does not update the plan limits' do @@ -75,7 +75,7 @@ RSpec.describe UpdateTrialPlansCiDailyPipelineScheduleTriggers, :migration do context 'when the environment is dev or com' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Gitlab).to receive(:com?).and_return(true) end it 'sets the trial plan limits ci_daily_pipeline_schedule_triggers to zero' do @@ -116,7 +116,7 @@ RSpec.describe UpdateTrialPlansCiDailyPipelineScheduleTriggers, :migration do context 'when the environment is anything other than dev or com' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(false) + allow(Gitlab).to receive(:com?).and_return(false) end it 'does not change the ultimate trial plan limits' do diff --git a/spec/migrations/add_new_trail_plans_spec.rb b/spec/migrations/add_new_trail_plans_spec.rb index 8ba6da11ad1..c1b488e8c3c 100644 --- a/spec/migrations/add_new_trail_plans_spec.rb +++ b/spec/migrations/add_new_trail_plans_spec.rb @@ -7,7 +7,7 @@ require_migration! RSpec.describe AddNewTrailPlans, :migration do describe '#up' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return true + allow(Gitlab).to receive(:com?).and_return true end it 'creates 2 entries within the plans table' do @@ -40,7 +40,7 @@ RSpec.describe AddNewTrailPlans, :migration do context 'when the instance is not SaaS' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return false + allow(Gitlab).to receive(:com?).and_return false end it 'does not create plans and plan limits and returns' do @@ -58,7 +58,7 @@ RSpec.describe AddNewTrailPlans, :migration do context 'when the instance is SaaS' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return true + allow(Gitlab).to receive(:com?).and_return true end it 'removes the newly added ultimate and premium trial entries' do @@ -77,7 +77,7 @@ RSpec.describe AddNewTrailPlans, :migration do context 'when the instance is not SaaS' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return false + allow(Gitlab).to receive(:com?).and_return false table(:plans).create!(id: 1, name: 'ultimate_trial', title: 'Ultimate Trial') table(:plans).create!(id: 2, name: 'premium_trial', title: 'Premium Trial') table(:plan_limits).create!(id: 1, plan_id: 1) diff --git a/spec/migrations/add_open_source_plan_spec.rb b/spec/migrations/add_open_source_plan_spec.rb index 04b26662f82..6e1cd544141 100644 --- a/spec/migrations/add_open_source_plan_spec.rb +++ b/spec/migrations/add_open_source_plan_spec.rb @@ -7,7 +7,7 @@ require_migration! RSpec.describe AddOpenSourcePlan, :migration do describe '#up' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return true + allow(Gitlab).to receive(:com?).and_return true end it 'creates 1 entry within the plans table' do @@ -35,7 +35,7 @@ RSpec.describe AddOpenSourcePlan, :migration do context 'when the instance is not SaaS' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return false + allow(Gitlab).to receive(:com?).and_return false end it 'does not create plans and plan limits and returns' do @@ -52,7 +52,7 @@ RSpec.describe AddOpenSourcePlan, :migration do context 'when the instance is SaaS' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return true + allow(Gitlab).to receive(:com?).and_return true end it 'removes the newly added opensource entry' do @@ -70,7 +70,7 @@ RSpec.describe AddOpenSourcePlan, :migration do context 'when the instance is not SaaS' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return false + allow(Gitlab).to receive(:com?).and_return false table(:plans).create!(id: 1, name: 'opensource', title: 'Open Source Program') table(:plan_limits).create!(id: 1, plan_id: 1) end diff --git a/spec/requests/api/deploy_tokens_spec.rb b/spec/requests/api/deploy_tokens_spec.rb index 38380fa4460..b5f8da1f327 100644 --- a/spec/requests/api/deploy_tokens_spec.rb +++ b/spec/requests/api/deploy_tokens_spec.rb @@ -130,6 +130,55 @@ RSpec.describe API::DeployTokens do end end + describe 'GET /projects/:id/deploy_tokens/:token_id' do + subject do + get api("/projects/#{project.id}/deploy_tokens/#{deploy_token.id}", user) + response + end + + context 'when unauthenticated' do + let(:user) { nil } + + it { is_expected.to have_gitlab_http_status(:not_found) } + end + + context 'when authenticated as non-admin user' do + before do + project.add_developer(user) + end + + it { is_expected.to have_gitlab_http_status(:forbidden) } + end + + context 'when authenticated as maintainer' do + before do + project.add_maintainer(user) + end + + it { is_expected.to have_gitlab_http_status(:ok) } + + it 'returns specific deploy token for the project' do + subject + + expect(response).to match_response_schema('public_api/v4/deploy_token') + end + + context 'invalid request' do + it 'returns not found with invalid project id' do + get api("/projects/bad_id/deploy_tokens/#{deploy_token.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns not found with invalid token id' do + get api("/projects/#{project.id}/deploy_tokens/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + describe 'GET /groups/:id/deploy_tokens' do subject do get api("/groups/#{group.id}/deploy_tokens", user) @@ -188,6 +237,55 @@ RSpec.describe API::DeployTokens do end end + describe 'GET /groups/:id/deploy_tokens/:token_id' do + subject do + get api("/groups/#{group.id}/deploy_tokens/#{group_deploy_token.id}", user) + response + end + + context 'when unauthenticated' do + let(:user) { nil } + + it { is_expected.to have_gitlab_http_status(:forbidden) } + end + + context 'when authenticated as non-admin user' do + before do + group.add_developer(user) + end + + it { is_expected.to have_gitlab_http_status(:forbidden) } + end + + context 'when authenticated as maintainer' do + before do + group.add_maintainer(user) + end + + it { is_expected.to have_gitlab_http_status(:ok) } + + it 'returns specific deploy token for the group' do + subject + + expect(response).to match_response_schema('public_api/v4/deploy_token') + end + + context 'invalid request' do + it 'returns not found with invalid group id' do + get api("/groups/bad_id/deploy_tokens/#{group_deploy_token.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns not found with invalid token id' do + get api("/groups/#{group.id}/deploy_tokens/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + describe 'DELETE /projects/:id/deploy_tokens/:token_id' do subject do delete api("/projects/#{project.id}/deploy_tokens/#{deploy_token.id}", user) @@ -232,10 +330,10 @@ RSpec.describe API::DeployTokens do it 'returns bad_request with invalid token id' do expect(::Projects::DeployTokens::DestroyService).to receive(:new) - .with(project, user, token_id: 999) + .with(project, user, token_id: non_existing_record_id) .and_raise(ActiveRecord::RecordNotFound) - delete api("/projects/#{project.id}/deploy_tokens/999", user) + delete api("/projects/#{project.id}/deploy_tokens/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -395,10 +493,10 @@ RSpec.describe API::DeployTokens do it 'returns not found with invalid deploy token id' do expect(::Groups::DeployTokens::DestroyService).to receive(:new) - .with(group, user, token_id: 999) + .with(group, user, token_id: non_existing_record_id) .and_raise(ActiveRecord::RecordNotFound) - delete api("/groups/#{group.id}/deploy_tokens/999", user) + delete api("/groups/#{group.id}/deploy_tokens/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb index 82692366589..7c8994ad9ba 100644 --- a/spec/requests/api/issues/post_projects_issues_spec.rb +++ b/spec/requests/api/issues/post_projects_issues_spec.rb @@ -447,7 +447,7 @@ RSpec.describe API::Issues do post_issue expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq({ 'error' => 'Spam detected' }) + expect(json_response['message']['base']).to match_array([/issue has been recognized as spam/]) end it 'creates a new spam log entry' do diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index 925b74c5545..f173d80af24 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -217,7 +217,7 @@ RSpec.describe API::Issues do update_issue expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to include('message' => { 'error' => 'Spam detected' }) + expect(json_response['message']['base']).to match_array([/issue has been recognized as spam/]) end it 'creates a new spam log entry' do diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 512cbf7c321..72519ed1683 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -276,7 +276,7 @@ RSpec.describe API::ProjectSnippets do it 'rejects the snippet' do expect { subject }.not_to change { Snippet.count } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq({ "error" => "Spam detected" }) + expect(json_response['message']['error']).to match(/snippet has been recognized as spam/) end it 'creates a spam log' do @@ -344,7 +344,7 @@ RSpec.describe API::ProjectSnippets do .not_to change { snippet.reload.title } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq({ "error" => "Spam detected" }) + expect(json_response['message']['error']).to match(/snippet has been recognized as spam/) end it 'creates a spam log' do diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index dd5e6ac8a5e..13160519996 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -325,7 +325,7 @@ RSpec.describe API::Snippets, factory_default: :keep do expect { subject }.not_to change { Snippet.count } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq({ "error" => "Spam detected" }) + expect(json_response['message']['error']).to match(/snippet has been recognized as spam/) end it 'creates a spam log' do @@ -392,7 +392,7 @@ RSpec.describe API::Snippets, factory_default: :keep do .not_to change { snippet.reload.title } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq({ "error" => "Spam detected" }) + expect(json_response['message']['error']).to match(/snippet has been recognized as spam/) end it 'creates a spam log' do diff --git a/spec/requests/jira_connect/oauth_callbacks_controller_spec.rb b/spec/requests/jira_connect/oauth_callbacks_controller_spec.rb new file mode 100644 index 00000000000..1e4628e5d59 --- /dev/null +++ b/spec/requests/jira_connect/oauth_callbacks_controller_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::OauthCallbacksController do + describe 'GET /-/jira_connect/oauth_callbacks' do + context 'when logged in' do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'renders a page prompting the user to close the window' do + get '/-/jira_connect/oauth_callbacks' + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to include('You can close this window.') + end + end + end +end diff --git a/spec/services/spam/spam_params_spec.rb b/spec/services/spam/spam_params_spec.rb index e7e8b468adb..7e74641c0fa 100644 --- a/spec/services/spam/spam_params_spec.rb +++ b/spec/services/spam/spam_params_spec.rb @@ -3,18 +3,25 @@ require 'spec_helper' RSpec.describe Spam::SpamParams do + shared_examples 'constructs from a request' do + it 'constructs from a request' do + expected = ::Spam::SpamParams.new( + captcha_response: captcha_response, + spam_log_id: spam_log_id, + ip_address: ip_address, + user_agent: user_agent, + referer: referer + ) + expect(described_class.new_from_request(request: request)).to eq(expected) + end + end + describe '.new_from_request' do let(:captcha_response) { 'abc123' } let(:spam_log_id) { 42 } let(:ip_address) { '0.0.0.0' } let(:user_agent) { 'Lynx' } let(:referer) { 'http://localhost' } - let(:headers) do - { - 'X-GitLab-Captcha-Response' => captcha_response, - 'X-GitLab-Spam-Log-Id' => spam_log_id - } - end let(:env) do { @@ -24,17 +31,28 @@ RSpec.describe Spam::SpamParams do } end - let(:request) {double(:request, headers: headers, env: env)} + let(:request) { double(:request, headers: headers, env: env) } - it 'constructs from a request' do - expected = ::Spam::SpamParams.new( - captcha_response: captcha_response, - spam_log_id: spam_log_id, - ip_address: ip_address, - user_agent: user_agent, - referer: referer - ) - expect(described_class.new_from_request(request: request)).to eq(expected) + context 'with a normal Rails request' do + let(:headers) do + { + 'X-GitLab-Captcha-Response' => captcha_response, + 'X-GitLab-Spam-Log-Id' => spam_log_id + } + end + + it_behaves_like 'constructs from a request' + end + + context 'with a grape request' do + let(:headers) do + { + 'X-Gitlab-Captcha-Response' => captcha_response, + 'X-Gitlab-Spam-Log-Id' => spam_log_id + } + end + + it_behaves_like 'constructs from a request' end end end diff --git a/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb b/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb index 2a976fb7421..d6415e98289 100644 --- a/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb @@ -692,16 +692,6 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name context 'notification enabled for all branches' do it_behaves_like "triggered #{integration_name} integration", event_type: "pipeline", branches_to_be_notified: "all" end - - context 'when chat_notification_deployment_protected_branch_filter is disabled' do - before do - stub_feature_flags(chat_notification_deployment_protected_branch_filter: false) - end - - context 'notification enabled only for default branch' do - it_behaves_like "triggered #{integration_name} integration", event_type: "pipeline", branches_to_be_notified: "default" - end - end end end end diff --git a/spec/views/devise/sessions/new.html.haml_spec.rb b/spec/views/devise/sessions/new.html.haml_spec.rb index 0109d05abe4..e8232a2c067 100644 --- a/spec/views/devise/sessions/new.html.haml_spec.rb +++ b/spec/views/devise/sessions/new.html.haml_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'devise/sessions/new' do before do stub_devise disable_captcha - allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Gitlab).to receive(:com?).and_return(true) end it 'when flash is anything it renders marketing text' do diff --git a/spec/views/devise/shared/_signup_box.html.haml_spec.rb b/spec/views/devise/shared/_signup_box.html.haml_spec.rb index 37dbfd39f2d..1f0cd213f7b 100644 --- a/spec/views/devise/shared/_signup_box.html.haml_spec.rb +++ b/spec/views/devise/shared/_signup_box.html.haml_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'devise/shared/_signup_box' do context 'when on .com' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Gitlab).to receive(:com?).and_return(true) end it 'shows expected GitLab text' do @@ -39,7 +39,7 @@ RSpec.describe 'devise/shared/_signup_box' do context 'when not on .com' do before do - allow(Gitlab).to receive(:dev_env_or_com?).and_return(false) + allow(Gitlab).to receive(:com?).and_return(false) end it 'shows expected text without GitLab' do @@ -53,7 +53,7 @@ RSpec.describe 'devise/shared/_signup_box' do context 'when terms are not enforced' do before do allow(Gitlab::CurrentSettings.current_application_settings).to receive(:enforce_terms?).and_return(false) - allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Gitlab).to receive(:com?).and_return(true) end it 'shows expected text with placeholders' do |