diff options
59 files changed, 559 insertions, 408 deletions
diff --git a/.rubocop_todo/layout/space_inside_parens.yml b/.rubocop_todo/layout/space_inside_parens.yml index 599f041dc91..b22a58fe459 100644 --- a/.rubocop_todo/layout/space_inside_parens.yml +++ b/.rubocop_todo/layout/space_inside_parens.yml @@ -214,27 +214,6 @@ Layout/SpaceInsideParens: - 'spec/migrations/20211130165043_backfill_sequence_column_for_sprints_table_spec.rb' - 'spec/migrations/backfill_issues_upvotes_count_spec.rb' - 'spec/migrations/schedule_copy_ci_builds_columns_to_security_scans2_spec.rb' - - 'spec/models/ci/build_spec.rb' - - 'spec/models/ci/build_trace_spec.rb' - - 'spec/models/ci/pipeline_spec.rb' - - 'spec/models/ci/trigger_request_spec.rb' - - 'spec/models/clusters/applications/prometheus_spec.rb' - - 'spec/models/deploy_token_spec.rb' - - 'spec/models/environment_spec.rb' - - 'spec/models/environment_status_spec.rb' - - 'spec/models/experiment_spec.rb' - - 'spec/models/exported_protected_branch_spec.rb' - - 'spec/models/group_spec.rb' - - 'spec/models/integrations/jira_spec.rb' - - 'spec/models/member_spec.rb' - - 'spec/models/metrics/dashboard/annotation_spec.rb' - - 'spec/models/namespace_setting_spec.rb' - - 'spec/models/namespace_spec.rb' - - 'spec/models/network/graph_spec.rb' - - 'spec/models/packages/package_spec.rb' - - 'spec/models/project_spec.rb' - - 'spec/models/repository_spec.rb' - - 'spec/models/users/calloutable_spec.rb' - 'spec/policies/clusters/agent_policy_spec.rb' - 'spec/presenters/ci/build_presenter_spec.rb' - 'spec/presenters/packages/conan/package_presenter_spec.rb' diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_popover.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_popover.vue deleted file mode 100644 index 605da5d9352..00000000000 --- a/app/assets/javascripts/ci_variable_list/components/ci_variable_popover.vue +++ /dev/null @@ -1,58 +0,0 @@ -<script> -import { GlPopover, GlButton, GlTooltipDirective } from '@gitlab/ui'; - -export default { - maxTextLength: 95, - components: { - GlPopover, - GlButton, - }, - directives: { - GlTooltip: GlTooltipDirective, - }, - props: { - target: { - type: String, - required: true, - }, - value: { - type: String, - required: true, - }, - tooltipText: { - type: String, - required: true, - }, - }, - computed: { - displayValue() { - if (this.value.length > this.$options.maxTextLength) { - return `${this.value.substring(0, this.$options.maxTextLength)}...`; - } - return this.value; - }, - }, -}; -</script> - -<template> - <div id="popover-container"> - <gl-popover :target="target" placement="top" container="popover-container"> - <div - class="gl-display-flex gl-justify-content-space-between gl-align-items-center gl-word-break-all" - > - <div class="ci-popover-value gl-pr-3"> - {{ displayValue }} - </div> - <gl-button - v-gl-tooltip - category="tertiary" - icon="copy-to-clipboard" - :title="tooltipText" - :data-clipboard-text="value" - :aria-label="__('Copy to clipboard')" - /> - </div> - </gl-popover> - </div> -</template> diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue index 959ef6864fb..3cdcb68e919 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue @@ -1,71 +1,49 @@ <script> -import { - GlButton, - GlIcon, - GlLoadingIcon, - GlModalDirective, - GlTable, - GlTooltipDirective, -} from '@gitlab/ui'; +import { GlButton, GlLoadingIcon, GlModalDirective, GlTable, GlTooltipDirective } from '@gitlab/ui'; import { s__, __ } from '~/locale'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue'; import { ADD_CI_VARIABLE_MODAL_ID, variableText } from '../constants'; import { convertEnvironmentScope } from '../utils'; -import CiVariablePopover from './ci_variable_popover.vue'; export default { modalId: ADD_CI_VARIABLE_MODAL_ID, - trueIcon: 'mobile-issue-close', - falseIcon: 'close', - iconSize: 16, fields: [ { key: 'variableType', label: s__('CiVariables|Type'), - customStyle: { width: '70px' }, + thClass: 'gl-w-10p', }, { key: 'key', label: s__('CiVariables|Key'), tdClass: 'text-plain', sortable: true, - customStyle: { width: '40%' }, }, { key: 'value', label: s__('CiVariables|Value'), - customStyle: { width: '40%' }, + thClass: 'gl-w-15p', }, { - key: 'protected', - label: s__('CiVariables|Protected'), - customStyle: { width: '100px' }, - }, - { - key: 'masked', - label: s__('CiVariables|Masked'), - customStyle: { width: '100px' }, + key: 'options', + label: s__('CiVariables|Options'), + thClass: 'gl-w-10p', }, { key: 'environmentScope', label: s__('CiVariables|Environments'), - customStyle: { width: '20%' }, }, { key: 'actions', label: '', tdClass: 'text-right', - customStyle: { width: '35px' }, + thClass: 'gl-w-5p', }, ], components: { - CiVariablePopover, GlButton, - GlIcon, GlLoadingIcon, GlTable, - TooltipOnTruncate, }, directives: { GlModalDirective, @@ -97,6 +75,13 @@ export default { fields() { return this.$options.fields; }, + variablesWithOptions() { + return this.variables?.map((item, index) => ({ + ...item, + options: this.getOptions(item), + index, + })); + }, }, methods: { convertEnvironmentScopeValue(env) { @@ -108,8 +93,18 @@ export default { toggleHiddenState() { this.areValuesHidden = !this.areValuesHidden; }, - setSelectedVariable(variable = null) { - this.$emit('set-selected-variable', variable); + setSelectedVariable(index = -1) { + this.$emit('set-selected-variable', this.variables[index] ?? null); + }, + getOptions(item) { + const options = []; + if (item.protected) { + options.push(s__('CiVariables|Protected')); + } + if (item.masked) { + options.push(s__('CiVariables|Masked')); + } + return options.join(', '); }, }, }; @@ -121,7 +116,7 @@ export default { <gl-table v-else :fields="fields" - :items="variables" + :items="variablesWithOptions" tbody-tr-class="js-ci-variable-row" data-qa-selector="ci_variable_table_content" sort-by="key" @@ -137,23 +132,22 @@ export default { <col v-for="field in scope.fields" :key="field.key" :style="field.customStyle" /> </template> <template #cell(variableType)="{ item }"> - <div class="gl-pt-2"> - {{ generateTypeText(item) }} - </div> + {{ generateTypeText(item) }} </template> <template #cell(key)="{ item }"> - <div class="gl-display-flex gl-align-items-center"> - <tooltip-on-truncate :title="item.key" truncate-target="child"> - <span - :id="`ci-variable-key-${item.id}`" - class="gl-display-inline-block gl-max-w-full gl-text-truncate" - >{{ item.key }}</span - > - </tooltip-on-truncate> + <div + class="gl-display-flex gl-align-items-flex-start gl-justify-content-end gl-lg-justify-content-start gl-mr-n3" + > + <span + :id="`ci-variable-key-${item.id}`" + class="gl-display-inline-block gl-max-w-full gl-word-break-word" + >{{ item.key }}</span + > <gl-button v-gl-tooltip category="tertiary" icon="copy-to-clipboard" + class="gl-my-n3 gl-ml-2" :title="__('Copy key')" :data-clipboard-text="item.key" :aria-label="__('Copy to clipboard')" @@ -161,8 +155,10 @@ export default { </div> </template> <template #cell(value)="{ item }"> - <div class="gl-display-flex gl-align-items-center"> - <span v-if="areValuesHidden" data-testid="hiddenValue">*********************</span> + <div + class="gl-display-flex gl-align-items-flex-start gl-justify-content-end gl-lg-justify-content-start gl-mr-n3" + > + <span v-if="areValuesHidden" data-testid="hiddenValue">*****</span> <span v-else :id="`ci-variable-value-${item.id}`" @@ -174,31 +170,33 @@ export default { v-gl-tooltip category="tertiary" icon="copy-to-clipboard" + class="gl-my-n3 gl-ml-2" :title="__('Copy value')" :data-clipboard-text="item.value" :aria-label="__('Copy to clipboard')" /> </div> </template> - <template #cell(protected)="{ item }"> - <gl-icon v-if="item.protected" :size="$options.iconSize" :name="$options.trueIcon" /> - <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" /> - </template> - <template #cell(masked)="{ item }"> - <gl-icon v-if="item.masked" :size="$options.iconSize" :name="$options.trueIcon" /> - <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" /> + <template #cell(options)="{ item }"> + <span>{{ item.options }}</span> </template> <template #cell(environmentScope)="{ item }"> - <div class="gl-display-flex"> + <div + class="gl-display-flex gl-align-items-flex-start gl-justify-content-end gl-lg-justify-content-start gl-mr-n3" + > <span :id="`ci-variable-env-${item.id}`" - class="gl-display-inline-block gl-max-w-full gl-text-truncate" + class="gl-display-inline-block gl-max-w-full gl-word-break-word" >{{ convertEnvironmentScopeValue(item.environmentScope) }}</span > - <ci-variable-popover - :target="`ci-variable-env-${item.id}`" - :value="convertEnvironmentScopeValue(item.environmentScope)" - :tooltip-text="__('Copy environment')" + <gl-button + v-gl-tooltip + category="tertiary" + icon="copy-to-clipboard" + class="gl-my-n3 gl-ml-2" + :title="__('Copy environment')" + :data-clipboard-text="convertEnvironmentScopeValue(item.environmentScope)" + :aria-label="__('Copy to clipboard')" /> </div> </template> @@ -208,7 +206,7 @@ export default { icon="pencil" :aria-label="__('Edit')" data-qa-selector="edit_ci_variable_button" - @click="setSelectedVariable(item)" + @click="setSelectedVariable(item.index)" /> </template> <template #empty> diff --git a/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_table.vue b/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_table.vue index f078234829a..f3a84e22316 100644 --- a/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_table.vue +++ b/app/assets/javascripts/ci_variable_list/components/legacy_ci_variable_table.vue @@ -1,63 +1,48 @@ <script> -import { GlTable, GlButton, GlModalDirective, GlIcon, GlTooltipDirective } from '@gitlab/ui'; +import { GlTable, GlButton, GlModalDirective, GlTooltipDirective } from '@gitlab/ui'; import { mapState, mapActions } from 'vuex'; import { s__, __ } from '~/locale'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue'; import { ADD_CI_VARIABLE_MODAL_ID } from '../constants'; -import CiVariablePopover from './ci_variable_popover.vue'; export default { modalId: ADD_CI_VARIABLE_MODAL_ID, - trueIcon: 'mobile-issue-close', - falseIcon: 'close', - iconSize: 16, fields: [ { key: 'variable_type', label: s__('CiVariables|Type'), - customStyle: { width: '70px' }, + thClass: 'gl-w-10p', }, { key: 'key', label: s__('CiVariables|Key'), tdClass: 'text-plain', sortable: true, - customStyle: { width: '40%' }, }, { key: 'value', label: s__('CiVariables|Value'), - customStyle: { width: '40%' }, + thClass: 'gl-w-15p', }, { - key: 'protected', - label: s__('CiVariables|Protected'), - customStyle: { width: '100px' }, - }, - { - key: 'masked', - label: s__('CiVariables|Masked'), - customStyle: { width: '100px' }, + key: 'options', + label: s__('CiVariables|Options'), + thClass: 'gl-w-10p', }, { key: 'environment_scope', label: s__('CiVariables|Environments'), - customStyle: { width: '20%' }, }, { key: 'actions', label: '', tdClass: 'text-right', - customStyle: { width: '35px' }, + thClass: 'gl-w-5p', }, ], components: { - CiVariablePopover, GlButton, - GlIcon, GlTable, - TooltipOnTruncate, }, directives: { GlModalDirective, @@ -75,12 +60,32 @@ export default { fields() { return this.$options.fields; }, + variablesWithOptions() { + return this.variables?.map((item, index) => ({ + ...item, + options: this.getOptions(item), + index, + })); + }, }, mounted() { this.fetchVariables(); }, methods: { ...mapActions(['fetchVariables', 'toggleValues', 'editVariable']), + getOptions(item) { + const options = []; + if (item.protected) { + options.push(s__('CiVariables|Protected')); + } + if (item.masked) { + options.push(s__('CiVariables|Masked')); + } + return options.join(', '); + }, + editVariableClicked(index = -1) { + this.editVariable(this.variables[index] ?? null); + }, }, }; </script> @@ -89,7 +94,7 @@ export default { <div class="ci-variable-table" data-testid="ci-variable-table"> <gl-table :fields="fields" - :items="variables" + :items="variablesWithOptions" tbody-tr-class="js-ci-variable-row" data-qa-selector="ci_variable_table_content" sort-by="key" @@ -105,18 +110,19 @@ export default { <col v-for="field in scope.fields" :key="field.key" :style="field.customStyle" /> </template> <template #cell(key)="{ item }"> - <div class="gl-display-flex gl-align-items-center"> - <tooltip-on-truncate :title="item.key" truncate-target="child"> - <span - :id="`ci-variable-key-${item.id}`" - class="gl-display-inline-block gl-max-w-full gl-text-truncate" - >{{ item.key }}</span - > - </tooltip-on-truncate> + <div + class="gl-display-flex gl-align-items-flex-start gl-justify-content-end gl-lg-justify-content-start gl-mr-n3" + > + <span + :id="`ci-variable-key-${item.id}`" + class="gl-display-inline-block gl-max-w-full gl-word-break-word" + >{{ item.key }}</span + > <gl-button v-gl-tooltip category="tertiary" icon="copy-to-clipboard" + class="gl-my-n3 gl-ml-2" :title="__('Copy key')" :data-clipboard-text="item.key" :aria-label="__('Copy to clipboard')" @@ -124,8 +130,10 @@ export default { </div> </template> <template #cell(value)="{ item }"> - <div class="gl-display-flex gl-align-items-center"> - <span v-if="valuesHidden">*********************</span> + <div + class="gl-display-flex gl-align-items-flex-start gl-justify-content-end gl-lg-justify-content-start gl-mr-n3" + > + <span v-if="valuesHidden">*****</span> <span v-else :id="`ci-variable-value-${item.id}`" @@ -136,31 +144,33 @@ export default { v-gl-tooltip category="tertiary" icon="copy-to-clipboard" + class="gl-my-n3 gl-ml-2" :title="__('Copy value')" :data-clipboard-text="item.value" :aria-label="__('Copy to clipboard')" /> </div> </template> - <template #cell(protected)="{ item }"> - <gl-icon v-if="item.protected" :size="$options.iconSize" :name="$options.trueIcon" /> - <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" /> - </template> - <template #cell(masked)="{ item }"> - <gl-icon v-if="item.masked" :size="$options.iconSize" :name="$options.trueIcon" /> - <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" /> + <template #cell(options)="{ item }"> + <span>{{ item.options }}</span> </template> <template #cell(environment_scope)="{ item }"> - <div class="gl-display-flex"> + <div + class="gl-display-flex gl-align-items-flex-start gl-justify-content-end gl-lg-justify-content-start gl-mr-n3" + > <span :id="`ci-variable-env-${item.id}`" - class="gl-display-inline-block gl-max-w-full gl-text-truncate" + class="gl-display-inline-block gl-max-w-full gl-word-break-word" >{{ item.environment_scope }}</span > - <ci-variable-popover - :target="`ci-variable-env-${item.id}`" - :value="item.environment_scope" - :tooltip-text="__('Copy environment')" + <gl-button + v-gl-tooltip + category="tertiary" + icon="copy-to-clipboard" + class="gl-my-n3 gl-ml-2" + :title="__('Copy environment')" + :data-clipboard-text="item.environment_scope" + :aria-label="__('Copy to clipboard')" /> </div> </template> @@ -170,7 +180,7 @@ export default { icon="pencil" :aria-label="__('Edit')" data-qa-selector="edit_ci_variable_button" - @click="editVariable(item)" + @click="editVariableClicked(item.index)" /> </template> <template #empty> diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index ead5d7c9026..1216353be36 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -2,6 +2,7 @@ class PasswordsController < Devise::PasswordsController include GitlabRecaptcha + include Gitlab::Tracking::Helpers::WeakPasswordErrorEvent skip_before_action :require_no_authentication, only: [:edit, :update] @@ -41,6 +42,8 @@ class PasswordsController < Devise::PasswordsController resource.password_automatically_set = false resource.password_expires_at = nil resource.save(validate: false) if resource.changed? + else + track_weak_password_error(@user, self.class.name, 'create') end end end diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb index 09f91d036d0..738c41207d5 100644 --- a/app/controllers/profiles/passwords_controller.rb +++ b/app/controllers/profiles/passwords_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Profiles::PasswordsController < Profiles::ApplicationController + include Gitlab::Tracking::Helpers::WeakPasswordErrorEvent + skip_before_action :check_password_expiration, only: [:new, :create] skip_before_action :check_two_factor_requirement, only: [:new, :create] @@ -27,6 +29,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController redirect_to root_path, notice: _('Password successfully changed') else + track_weak_password_error(@user, self.class.name, 'create') render :new end end @@ -48,6 +51,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController flash[:notice] = _('Password was successfully updated. Please sign in again.') redirect_to new_user_session_path else + track_weak_password_error(@user, self.class.name, 'update') @user.reset render 'edit' end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index aa35dcbf486..83422c6d8cd 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -9,6 +9,7 @@ class RegistrationsController < Devise::RegistrationsController include BizibleCSP include GoogleAnalyticsCSP include RegistrationsTracking + include Gitlab::Tracking::Helpers::WeakPasswordErrorEvent layout 'devise' @@ -41,6 +42,7 @@ class RegistrationsController < Devise::RegistrationsController persist_accepted_terms_if_required(new_user) set_role_required(new_user) send_custom_confirmation_instructions(new_user, token) + track_weak_password_error(new_user, self.class.name, 'create') if pending_approval? NotificationService.new.new_instance_access_request(new_user) diff --git a/app/models/integrations/buildkite.rb b/app/models/integrations/buildkite.rb index 2f2fba314d5..5c08eac8557 100644 --- a/app/models/integrations/buildkite.rb +++ b/app/models/integrations/buildkite.rb @@ -6,7 +6,6 @@ module Integrations class Buildkite < BaseCi include HasWebHook include ReactivelyCached - extend Gitlab::Utils::Override ENDPOINT = "https://buildkite.com" diff --git a/app/models/integrations/datadog.rb b/app/models/integrations/datadog.rb index d4867600853..eecf6b3a60a 100644 --- a/app/models/integrations/datadog.rb +++ b/app/models/integrations/datadog.rb @@ -3,7 +3,6 @@ module Integrations class Datadog < Integration include HasWebHook - extend Gitlab::Utils::Override DEFAULT_DOMAIN = 'datadoghq.com' URL_TEMPLATE = 'https://webhook-intake.%{datadog_domain}/api/v2/webhook' diff --git a/app/models/integrations/drone_ci.rb b/app/models/integrations/drone_ci.rb index d1a64aa96d4..781acf65c47 100644 --- a/app/models/integrations/drone_ci.rb +++ b/app/models/integrations/drone_ci.rb @@ -6,7 +6,6 @@ module Integrations include PushDataValidations include ReactivelyCached prepend EnableSslVerification - extend Gitlab::Utils::Override DRONE_SAAS_HOSTNAME = 'cloud.drone.io' diff --git a/app/models/integrations/jenkins.rb b/app/models/integrations/jenkins.rb index 9755f077193..635c22cf7b0 100644 --- a/app/models/integrations/jenkins.rb +++ b/app/models/integrations/jenkins.rb @@ -5,7 +5,6 @@ module Integrations include HasWebHook prepend EnableSslVerification - extend Gitlab::Utils::Override field :jenkins_url, title: -> { s_('ProjectService|Jenkins server URL') }, diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index 3ca514ab1fd..25644dcf83b 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -3,7 +3,6 @@ # Accessible as Project#external_issue_tracker module Integrations class Jira < BaseIssueTracker - extend ::Gitlab::Utils::Override include Gitlab::Routing include ApplicationHelper include ActionView::Helpers::AssetUrlHelper diff --git a/app/models/integrations/mattermost.rb b/app/models/integrations/mattermost.rb index dae11b99bc5..60f3b10b80c 100644 --- a/app/models/integrations/mattermost.rb +++ b/app/models/integrations/mattermost.rb @@ -3,7 +3,6 @@ module Integrations class Mattermost < BaseChatNotification include SlackMattermostNotifier - extend ::Gitlab::Utils::Override def title s_('Mattermost notifications') diff --git a/app/models/integrations/packagist.rb b/app/models/integrations/packagist.rb index 7177c82a167..64dfef1a4ef 100644 --- a/app/models/integrations/packagist.rb +++ b/app/models/integrations/packagist.rb @@ -3,7 +3,6 @@ module Integrations class Packagist < Integration include HasWebHook - extend Gitlab::Utils::Override field :username, title: -> { s_('Username') }, diff --git a/app/models/integrations/slack.rb b/app/models/integrations/slack.rb index c254ea379bb..834013e18f1 100644 --- a/app/models/integrations/slack.rb +++ b/app/models/integrations/slack.rb @@ -3,7 +3,6 @@ module Integrations class Slack < BaseChatNotification include SlackMattermostNotifier - extend ::Gitlab::Utils::Override SUPPORTED_EVENTS_FOR_USAGE_LOG = %w[ push issue confidential_issue merge_request note confidential_note diff --git a/config/events/1666038724_Gitlab__Tracking__Helpers__WeakPasswordErrorEvent_track_weak_password_error.yml b/config/events/1666038724_Gitlab__Tracking__Helpers__WeakPasswordErrorEvent_track_weak_password_error.yml new file mode 100644 index 00000000000..d19db52074b --- /dev/null +++ b/config/events/1666038724_Gitlab__Tracking__Helpers__WeakPasswordErrorEvent_track_weak_password_error.yml @@ -0,0 +1,29 @@ +--- +description: User record is invalid due to weak password +category: Gitlab::Tracking::Helpers::WeakPasswordErrorEvent +action: track_weak_password_error +label_description: +property_description: +value_description: +extra_properties: + controller: + type: string + description: The controller which triggered the weak password check + method: + type: string + description: The method or controller action which triggered the weak password check +identifiers: +product_section: dev +product_stage: manage +product_group: group::authentication and authorization +product_category: authentication_and_authorization +milestone: "15.6" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/100237 +distributions: +- ce +- ee +tiers: +- free +- premium +- ultimate + diff --git a/doc/user/group/saml_sso/img/member_enterprise_badge_v14_0.png b/doc/user/group/saml_sso/img/member_enterprise_badge_v14_0.png Binary files differdeleted file mode 100644 index f9534b14a51..00000000000 --- a/doc/user/group/saml_sso/img/member_enterprise_badge_v14_0.png +++ /dev/null diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index fa6f378e811..85ace117caf 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -306,9 +306,11 @@ To migrate users to a new email domain, users must: ## User access and management -> SAML user provisioning [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/268142) in GitLab 13.7. +> - SAML user provisioning [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/268142) in GitLab 13.7. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325712) in GitLab 14.0, GitLab users created by [SAML SSO](index.md#user-access-and-management) or SCIM provisioning are displayed with an **Enterprise** badge in the **Members** view. -Once Group SSO is configured and enabled, users can access the GitLab.com group through the identity provider's dashboard. If [SCIM](scim_setup.md) is configured, see the [user access and linking setup section on the SCIM page](scim_setup.md#user-access-and-linking-setup). +After group SSO is configured and enabled, users can access the GitLab.com group through the identity provider's dashboard. +If [SCIM](scim_setup.md) is configured, see [user access](scim_setup.md#user-access) on the SCIM page. When a user tries to sign in with Group SSO, GitLab attempts to find or create a user based on the following: @@ -422,7 +424,7 @@ To rescind a user's access to the group when only SAML SSO is configured, either 1. The GitLab.com group. - Use Group Sync at the top-level of your group to [automatically remove the user](group_sync.md#automatic-member-removal). -To rescind a user's access to the group when also using SCIM, refer to [Blocking access](scim_setup.md#blocking-access). +To rescind a user's access to the group when also using SCIM, refer to [Remove access](scim_setup.md#remove-access). ### Unlinking accounts diff --git a/doc/user/group/saml_sso/scim_setup.md b/doc/user/group/saml_sso/scim_setup.md index 55990336a50..18af39f4271 100644 --- a/doc/user/group/saml_sso/scim_setup.md +++ b/doc/user/group/saml_sso/scim_setup.md @@ -168,13 +168,16 @@ Prerequisites: OneLogin provides a **GitLab (SaaS)** app in their catalog, which includes a SCIM integration. Contact OneLogin if you encounter issues. -## User access and linking setup +## User access -During the synchronization process, all of your users get GitLab accounts, welcoming them -to their respective groups, with an invitation email. When implementing SCIM provisioning, -you may want to warn your security-conscious employees about this email. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325712) in GitLab 14.0, GitLab users created by [SAML SSO](index.md#user-access-and-management) or SCIM provisioning are displayed with an **Enterprise** badge in the **Members** view. -The following diagram is a general outline on what happens when you add users to your SCIM app: +During the synchronization process, all new users: + +- Receive GitLab accounts. +- Are welcomed to their groups with an invitation email. You may want to warn your employees to expect this email. + +The following diagram describes what happens when you add users to your SCIM app: ```mermaid graph TD @@ -186,29 +189,38 @@ graph TD During provisioning: - Both primary and secondary emails are considered when checking whether a GitLab user account exists. -- Duplicate usernames are also handled, by adding suffix `1` upon user creation. For example, - due to already existing `test_user` username, `test_user1` is used. +- Duplicate usernames are handled by adding suffix `1` when creating the user. For example, if `test_user` already + exists, `test_user1` is used. If `test_user1` already exists, GitLab increments the suffix until an unused username + is found. -If [Group SAML](index.md) has been configured and you have an existing GitLab.com account, you can link your SCIM and SAML identities: +On subsequent visits, new and existing users can access groups either: -1. Update the [primary email](../../profile/index.md#change-your-primary-email) address in your GitLab.com user account to match the - user profile email address in your identity provider. -1. [Link your SAML identity](index.md#linking-saml-to-your-existing-gitlabcom-account). +- Through the identity provider's dashboard. +- By visiting links directly. -We recommend users do this prior to turning on sync, because while synchronization is active, there may be provisioning errors for existing users. +For role information, see the [Group SAML](index.md#user-access-and-management) page. -New users and existing users on subsequent visits can access the group through the identity provider's dashboard or by visiting links directly. +### Link SCIM and SAML identities -[In GitLab 14.0 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/325712), GitLab users created by [SAML SSO](index.md#user-access-and-management) or SCIM provisioning display with an **Enterprise** badge in the **Members** view. +If [group SAML](index.md) is configured and you have an existing GitLab.com account, users can link their SCIM and SAML +identities. Users should do this before synchronization is turned on because there can be provisioning errors for +existing users when synchronization is active. + +To link your SCIM and SAML identities: + +1. Update the [primary email](../../profile/index.md#change-your-primary-email) address in your GitLab.com user account + to match the user profile email address in your identity provider. +1. [Link your SAML identity](index.md#linking-saml-to-your-existing-gitlabcom-account). - +### Remove access -For role information, see the [Group SAML page](index.md#user-access-and-management) +Remove or deactivate a user on the identity provider to remove their access to: -### Blocking access +- The top-level group. +- All subgroups and projects. -To rescind access to the top-level group, all subgroups, and projects, remove or deactivate the user -on the identity provider. After the identity provider performs a sync, based on its configured schedule, the user's membership is revoked and they lose access. +After the identity provider performs a sync based on its configured schedule, the user's membership is revoked and they +lose access. NOTE: Deprovisioning does not delete the GitLab user account. diff --git a/doc/user/group/saml_sso/troubleshooting.md b/doc/user/group/saml_sso/troubleshooting.md index bde5ed1762a..7dafd2c5075 100644 --- a/doc/user/group/saml_sso/troubleshooting.md +++ b/doc/user/group/saml_sso/troubleshooting.md @@ -19,9 +19,10 @@ SAML responses are base64 encoded, so we recommend the following browser plugins - [SAML-tracer](https://addons.mozilla.org/en-US/firefox/addon/saml-tracer/) for Firefox. - [SAML Message Decoder](https://chrome.google.com/webstore/detail/saml-message-decoder/mpabchoaimgbdbbjjieoaeiibojelbhm?hl=en) for Chrome. -Specific attention should be paid to: +Pay specific attention to: -- The NameID, which we use to identify which user is signing in. If the user has previously signed in, this [must match the value we have stored](#verifying-nameid). +- The `NameID`, which we use to identify which user is signing in. If the user has previously signed in, this + [must match the value we have stored](#verify-nameid). - The presence of a `X509Certificate`, which we require to verify the response signature. - The `SubjectConfirmation` and `Conditions`, which can cause errors if misconfigured. @@ -32,7 +33,7 @@ using an identity provider. To generate a SAML Response: -1. Install one of the browser debugging tools previously mentioned. +1. Install one of the [browser debugging tools](#saml-debugging-tools). 1. Open a new browser tab. 1. Open the SAML tracer console: - Chrome: On a context menu on the page, select **Inspect**, then select the **SAML** tab in the opened developer @@ -43,16 +44,17 @@ To generate a SAML Response: [example SAML response](index.md#example-saml-response). 1. Within the SAML tracer, select the **Export** icon to save the response in JSON format. -## GitLab SAML Testing Environments +## Testing GitLab SAML -To troubleshoot, [a complete GitLab with SAML testing environment using Docker compose](https://gitlab.com/gitlab-com/support/toolbox/replication/tree/master/compose_files) -is available. +You can use one of the following to troubleshoot SAML: -If you only require a SAML provider for testing, a [quick start guide to start a Docker container](../../../administration/troubleshooting/test_environments.md#saml) with a plug and play SAML 2.0 Identity Provider (identity provider) is available. +- A [complete GitLab with SAML testing environment using Docker compose](https://gitlab.com/gitlab-com/support/toolbox/replication/tree/master/compose_files). +- A [quick start guide to start a Docker container](../../../administration/troubleshooting/test_environments.md#saml) + with a plug and play SAML 2.0 identity provider if you only require a SAML provider. +- A local environment by + [enabling SAML for groups on a self-managed instance](../../../integration/saml.md#configuring-group-saml-on-a-self-managed-gitlab-instance). -You can test the SaaS feature locally by [enabling SAML for groups on a self-managed instance](../../../integration/saml.md#configuring-group-saml-on-a-self-managed-gitlab-instance). - -## Verifying configuration +## Verify configuration For convenience, we've included some [example resources](../../../user/group/saml_sso/example_saml_config.md) used by our Support Team. While they may help you verify the SAML app configuration, they are not guaranteed to reflect the current state of third-party products. @@ -82,7 +84,7 @@ in case the customer has [configured SAML Group Sync](group_sync.md): - `json.class`: `GroupSamlGroupSyncWorker` - `json.args`: `<user ID> or <group ID>` -In the relevant log entry, the: +In the relevant log entry, the: - `json.args` are in the form `<userID>, <group ID>, [group link ID 1, group link ID 2, ..., group link ID N]`. @@ -148,13 +150,13 @@ If you do not wish to use that GitLab user with the SAML login, you can [unlink ### Message: "SAML authentication failed: User has already been taken" -The user that you're signed in with already has SAML linked to a different identity, or the NameID value has changed. +The user that you're signed in with already has SAML linked to a different identity, or the `NameID` value has changed. Here are possible causes and solutions: | Cause | Solution | | ---------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | You've tried to link multiple SAML identities to the same user, for a given identity provider. | Change the identity that you sign in with. To do so, [unlink the previous SAML identity](index.md#unlinking-accounts) from this GitLab account before attempting to sign in again. | -| The NameID changes every time the user requests SSO identification | [Check the NameID](#verifying-nameid) is not set with `Transient` format, or the NameID is not changing on subsequent requests.| +| The `NameID` changes every time the user requests SSO identification | [Check the `NameID`](#verify-nameid) is not set with `Transient` format, or the `NameID` is not changing on subsequent requests.| ### Message: "SAML authentication failed: Email has already been taken" @@ -171,9 +173,9 @@ User accounts are created in one of the following ways: ### Message: "SAML authentication failed: Extern UID has already been taken, User has already been taken" -Getting both of these errors at the same time suggests the NameID capitalization provided by the identity provider didn't exactly match the previous value for that user. +Getting both of these errors at the same time suggests the `NameID` capitalization provided by the identity provider didn't exactly match the previous value for that user. -This can be prevented by configuring the NameID to return a consistent value. Fixing this for an individual user involves changing the identifier for the user. For GitLab.com, the user needs to [unlink their SAML from the GitLab account](index.md#unlinking-accounts). +This can be prevented by configuring the `NameID` to return a consistent value. Fixing this for an individual user involves changing the identifier for the user. For GitLab.com, the user needs to [unlink their SAML from the GitLab account](index.md#unlinking-accounts). ### Message: "Request to link SAML account must be authorized" @@ -196,15 +198,15 @@ to [reset their password](https://gitlab.com/users/password/new) if both: ## Other user sign in issues -### Verifying NameID +### Verify `NameID` -In troubleshooting, any authenticated user can use the API to verify the NameID GitLab already has linked to the user by visiting [`https://gitlab.com/api/v4/user`](https://gitlab.com/api/v4/user) and checking the `extern_uid` under identities. +In troubleshooting, any authenticated user can use the API to verify the `NameID` GitLab already has linked to their user by visiting [`https://gitlab.com/api/v4/user`](https://gitlab.com/api/v4/user) and checking the `extern_uid` under identities. For self-managed, administrators can use the [users API](../../../api/users.md) to see the same information. When using SAML for groups, group members of a role with the appropriate permissions can make use of the [members API](../../../api/members.md) to view group SAML identity information for members of the group. -This can then be compared to the NameID being sent by the identity provider by decoding the message with a [SAML debugging tool](#saml-debugging-tools). We require that these match to identify users. +This can then be compared to the `NameID` being sent by the identity provider by decoding the message with a [SAML debugging tool](#saml-debugging-tools). We require that these match to identify users. ### Stuck in a login "loop" diff --git a/doc/user/group/saml_sso/troubleshooting_scim.md b/doc/user/group/saml_sso/troubleshooting_scim.md index 6f8aed4b386..22562c51e9e 100644 --- a/doc/user/group/saml_sso/troubleshooting_scim.md +++ b/doc/user/group/saml_sso/troubleshooting_scim.md @@ -8,93 +8,120 @@ info: To determine the technical writer assigned to the Stage/Group associated w This section contains possible solutions for problems you might encounter. -## How come I can't add a user after I removed them? +## User cannot be added after they are removed -As outlined in the [Blocking access section](scim_setup.md#blocking-access), when you remove a user, they are removed from the group. However, their account is not deleted. +When you remove a user, they are removed from the group but their account is not deleted +(see [remove access](scim_setup.md#remove-access)). When the user is added back to the SCIM app, GitLab cannot create a new user because the user already exists. -Solution: Have a user sign in directly to GitLab, then [manually link](scim_setup.md#user-access-and-linking-setup) their account. +To solve this problem: -## How do I diagnose why a user is unable to sign in +1. Have the user sign in directly to GitLab. +1. [Manually link](scim_setup.md#link-scim-and-saml-identities) their account. -Ensure that the user has been added to the SCIM app. +## User cannot sign in -If you receive "User is not linked to a SAML account", then most likely the user already exists in GitLab. Have the user follow the [User access and linking setup](scim_setup.md#user-access-and-linking-setup) instructions. +The following are possible solutions for problems where users cannot sign in: -The **Identity** (`extern_uid`) value stored by GitLab is updated by SCIM whenever `id` or `externalId` changes. Users cannot sign in unless the GitLab Identity (`extern_uid`) value matches the `NameId` sent by SAML. +- Ensure that the user was added to the SCIM app. +- If you receive the `User is not linked to a SAML account` error, the user probably already exists in GitLab. Have the + user follow the [Link SCIM and SAML identities](scim_setup.md#link-scim-and-saml-identities) instructions. +- The **Identity** (`extern_uid`) value stored by GitLab is updated by SCIM whenever `id` or `externalId` changes. Users + cannot sign in unless the GitLab Identity (`extern_uid`) value matches the `NameId` sent by SAML. This value is also + used by SCIM to match users on the `id`, and is updated by SCIM whenever the `id` or `externalId` values change. +- The SCIM `id` and SCIM `externalId` must be configured to the same value as the SAML `NameId`. You can trace SAML responses + using [debugging tools](troubleshooting.md#saml-debugging-tools), and check any errors against the + [SAML troubleshooting](troubleshooting.md) information. -This value is also used by SCIM to match users on the `id`, and is updated by SCIM whenever the `id` or `externalId` values change. +## Unsure if user's SAML `NameId` matches the SCIM `externalId` -It is important that this SCIM `id` and SCIM `externalId` are configured to the same value as the SAML `NameId`. SAML responses can be traced using [debugging tools](troubleshooting.md#saml-debugging-tools), and any errors can be checked against our [SAML troubleshooting docs](troubleshooting.md). +To check if a user's SAML `NameId` matches their SCIM `externalId`: -## How do I verify user's SAML NameId matches the SCIM externalId +- Administrators can use the Admin Area to [list SCIM identities for a user](../../admin_area/index.md#user-identities). +- Group owners can see the list of users and the identifier stored for each user in the group SAML SSO Settings page. +- You can use the [SCIM API](../../../api/scim.md) to manually retrieve the `external_uid` GitLab has stored for users and compare the value for each user from the [SAML API](../../../api/saml.md) . +- Have the user use a [SAML Tracer](troubleshooting.md#saml-debugging-tools) and compare the `external_uid` to + the value returned as the SAML `NameId`. -Administrators can use the Admin Area to [list SCIM identities for a user](../../admin_area/index.md#user-identities). +## Mismatched SCIM `extern_uid` and SAML `NameId` -Group owners can see the list of users and the `externalId` stored for each user in the group SAML SSO Settings page. +Whether the value was changed or you need to map to a different field, the following must map to the same field: -A possible alternative is to use the [SCIM API](../../../api/scim.md) to manually retrieve the `externalId` we have stored for users, also called the `external_uid` or `NameId`. +- `id` +- `externalId` +- `NameId` -To see how the `external_uid` compares to the value returned as the SAML NameId, you can have the user use a [SAML Tracer](troubleshooting.md#saml-debugging-tools). - -## Update or fix mismatched SCIM externalId and SAML NameId - -Whether the value was changed or you need to map to a different field, ensure `id`, `externalId`, and `NameId` all map to the same field. - -If the GitLab `externalId` doesn't match the SAML NameId, it needs to be updated in order for the user to sign in. Ideally your identity provider is configured to do such an update, but in some cases it may be unable to do so, such as when looking up a user fails due to an ID change. +If the GitLab `extern_uid` doesn't match the SAML `NameId`, it must be updated for the user to sign in. Your identity +provider should be configured to do this update. In some cases the identity provider cannot do the update, for example +when a user lookup fails because of an ID change. Be cautious if you revise the fields used by your SCIM identity provider, typically `id` and `externalId`. -We use these IDs to look up users. If the identity provider does not know the current values for these fields, +GitLab uses these IDs to look up users. If the identity provider does not know the current values for these fields, that provider may create duplicate users. -If the `externalId` for a user is not correct, and also doesn't match the SAML NameID, -you can address the problem in the following ways: +If the `extern_uid` for a user is not correct, and also doesn't match the SAML `NameID`, either: + +- Have users unlink and relink themselves, based on the + [SAML authentication failed: User has already been taken](troubleshooting.md#message-saml-authentication-failed-user-has-already-been-taken) + section. +- Unlink all users simultaneously by removing all users from the SCIM app while provisioning is turned on. +- Use the [SCIM API](../../../api/scim.md) to manually correct the `extern_uid` stored for users to match the SAML + `NameId`. To look up a user, you must know the desired value that matches the `NameId` as well as the current + `extern_uid`. -- You can have users unlink and relink themselves, based on the ["SAML authentication failed: User has already been taken"](troubleshooting.md#message-saml-authentication-failed-user-has-already-been-taken) section. -- You can unlink all users simultaneously, by removing all users from the SAML app while provisioning is turned on. -- Use the [SCIM API](../../../api/scim.md) to manually correct the `externalId` stored for users to match the SAML `NameId`. - To look up a user, you need to know the desired value that matches the `NameId` as well as the current `externalId`. +You must not: -It is important not to update these to incorrect values, since this causes users to be unable to sign in. It is also important not to assign a value to the wrong user, as this causes users to get signed into the wrong account. +- Update these to incorrect values because this causes users to be unable to sign in. +- Assign a value to the wrong user because this causes users to be signed in to the wrong account. -## I need to change my SCIM app +## Change SCIM app -Individual users can follow the instructions in the ["SAML authentication failed: User has already been taken"](index.md#change-the-saml-app) section. +When the SCIM app changes: -Alternatively, users can be removed from the SCIM app which de-links all removed users. Sync can then be turned on for the new SCIM app to [link existing users](scim_setup.md#user-access-and-linking-setup). +- Users can follow the instructions in the [Change the SAML app](index.md#change-the-saml-app) section. +- Administrators of the identity provider can: + 1. Remove users from the SCIM app, which unlinks all removed users. + 1. Turn on sync for the new SCIM app to [link existing users](scim_setup.md#link-scim-and-saml-identities). -## The SCIM app is throwing `"User has already been taken","status":409` error message +## SCIM app returns `"User has already been taken","status":409` error Changing the SAML or SCIM configuration or provider can cause the following problems: -| Problem | Solution | -| ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| SAML and SCIM identity mismatch. | First [verify that the user's SAML NameId matches the SCIM externalId](#how-do-i-verify-users-saml-nameid-matches-the-scim-externalid) and then [update or fix the mismatched SCIM externalId and SAML NameId](#update-or-fix-mismatched-scim-externalid-and-saml-nameid). | -| SCIM identity mismatch between GitLab and the identity provider SCIM app. | You can confirm whether you're hitting the error because of your SCIM identity mismatch between your SCIM app and GitLab.com by using the [SCIM API](../../../api/scim.md) which shows up in the `id` key and compares it with the user `externalId` in the SCIM app. You can use the same [SCIM API](../../../api/scim.md) to update the SCIM `id` for the user on GitLab.com. | +- SAML and SCIM identity mismatch. To solve this problem: + 1. [Verify that the user's SAML `NameId` matches the SCIM `extern_uid`](#unsure-if-users-saml-nameid-matches-the-scim-externalid). + 1. [Update or fix the mismatched SCIM `extern_uid` and SAML `NameId`](#mismatched-scim-extern_uid-and-saml-nameid). +- SCIM identity mismatch between GitLab and the identity provider SCIM app. To solve this problem: + 1. Use the [SCIM API](../../../api/scim.md), which displays the user's `extern_uid` stored in GitLab and compares it with the user `externalId` in + the SCIM app. + 1. Use the same SCIM API to update the SCIM `extern_uid` for the user on GitLab.com. ## Search Rails logs for SCIM requests GitLab.com administrators can search for SCIM requests in the `api_json.log` using the `pubsub-rails-inf-gprd-*` index in -[Kibana](https://about.gitlab.com/handbook/support/workflows/kibana.html#using-kibana). Use the following filters based on the internal -[SCIM API](../../../development/internal_api/index.md#scim-api): +[Kibana](https://about.gitlab.com/handbook/support/workflows/kibana.html#using-kibana). Use the following filters based +on the internal [SCIM API](../../../development/internal_api/index.md#scim-api): - `json.path`: `/scim/v2/groups/<group-path>` - `json.params.value`: `<externalId>` -In a relevant log entry, the `json.params.value` shows the values of SCIM parameters GitLab receives. These values can be used to verify if SCIM parameters configured in an -identity provider's SCIM app are communicated to GitLab as intended. For example, we can use these values as a definitive source on why an account was provisioned with a certain -set of details. This information can help where an account was SCIM provisioned with details that appear to be incongruent with what might have been configured within an identity -provider's SCIM app. +In a relevant log entry, the `json.params.value` shows the values of SCIM parameters GitLab receives. Use these values +to verify if SCIM parameters configured in an identity provider's SCIM app are communicated to GitLab as intended. -## Azure +For example, use these values as a definitive source on why an account was provisioned with a certain set of +details. This information can help where an account was SCIM provisioned with details that do not match +the SCIM app configuration. -### How do I verify my SCIM configuration is correct? +## Azure Active Directory -Review the following: +The following troubleshooting information is specifically for SCIM provisioned through Azure Active Directory. -- Ensure that the SCIM value for `id` matches the SAML value for `NameId`. -- Ensure that the SCIM value for `externalId` matches the SAML value for `NameId`. +### Verify my SCIM configuration is correct + +Ensure that: + +- The matching precedence for `externalId` is 1. +- The SCIM value for `externalId` matches the SAML value for `NameId`. Review the following SCIM parameters for sensible values: @@ -102,28 +129,37 @@ Review the following SCIM parameters for sensible values: - `displayName` - `emails[type eq "work"].value` -### Testing Azure connection: invalid credentials +### `invalid credentials` error when testing connection + +When testing the connection, you may encounter an error: + +```plaintext +You appear to have entered invalid credentials. Please confirm +you are using the correct information for an administrative account +``` -When testing the connection, you may encounter an error: **You appear to have entered invalid credentials. Please confirm you are using the correct information for an administrative account**. If `Tenant URL` and `secret token` are correct, check whether your group path contains characters that may be considered invalid JSON primitives (such as `.`). Removing such characters from the group path typically resolves the error. +If `Tenant URL` and `secret token` are correct, check whether your group path contains characters that may be considered +invalid JSON primitives (such as `.`). Removing or URL encoding these characters in the group path typically resolves the error. -### (Field) can't be blank sync error +### `(Field) can't be blank` sync error -When checking the Audit Events for the Provisioning, you can sometimes see the -error `Namespace can't be blank, Name can't be blank, and User can't be blank.` +When checking the Audit Events for the provisioning, you sometimes see a `Namespace can't be blank, Name can't be blank, +and User can't be blank.` error. -This is likely caused because not all required fields (such as first name and last name) are present for all users being mapped. +This error can occur because not all required fields (such as first name and last name) are present for all users +being mapped. As a workaround, try an alternate mapping: -1. Follow the Azure mapping instructions from above. +1. Follow the [Azure mapping instructions](scim_setup.md#configure-attribute-mappings). 1. Delete the `name.formatted` target attribute entry. 1. Change the `displayName` source attribute to have `name.formatted` target attribute. -### Failed to match an entry in the source and target systems Group 'Group-Name' +### `Failed to match an entry in the source and target systems Group 'Group-Name'` error -Group provisioning in Azure can fail with the `Failed to match an entry in the source and target systems Group 'Group-Name'` error message, -and the error response can include a HTML result of the GitLab URL `https://gitlab.com/users/sign_in`. +Group provisioning in Azure can fail with the `Failed to match an entry in the source and target systems Group 'Group-Name'` +error. The error response can include a HTML result of the GitLab URL `https://gitlab.com/users/sign_in`. -This error is harmless and occurs because Group provisioning was turned on but GitLab SCIM integration does not support it nor require it. To -remove the error, follow the instructions in the Azure configuration guide to disable the option -[`Synchronize Azure Active Directory Groups to AppName`](scim_setup.md#configure-azure-active-directory). +This error is harmless and occurs because group provisioning was turned on but GitLab SCIM integration does not support +it nor require it. To remove the error, follow the instructions in the Azure configuration guide to disable the option +to [synchronize Azure Active Directory groups to AppName](scim_setup.md#configure-azure-active-directory). diff --git a/doc/user/project/integrations/slack.md b/doc/user/project/integrations/slack.md index 17dd5652196..fd40b725598 100644 --- a/doc/user/project/integrations/slack.md +++ b/doc/user/project/integrations/slack.md @@ -4,9 +4,9 @@ group: Integrations info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Slack notifications service **(FREE)** +# Slack notifications integration **(FREE)** -The Slack notifications service enables your GitLab project to send events +The Slack notifications integration enables your GitLab project to send events (such as issue creation) to your existing Slack team as notifications. Setting up Slack notifications requires configuration changes for both Slack and GitLab. @@ -91,7 +91,7 @@ the error message and keep troubleshooting from there. You might see an entry like the following in your Sidekiq log: ```plaintext -2019-01-10_13:22:08.42572 2019-01-10T13:22:08.425Z 6877 TID-abcdefg Integrations::ExecuteWorker JID-3bade5fb3dd47a85db6d78c5 ERROR: {:class=>"Integrations::ExecuteWorker :service_class=>"SlackService", :message=>"SSL_connect returned=1 errno=0 state=error: certificate verify failed"} +2019-01-10_13:22:08.42572 2019-01-10T13:22:08.425Z 6877 TID-abcdefg Integrations::ExecuteWorker JID-3bade5fb3dd47a85db6d78c5 ERROR: {:class=>"Integrations::ExecuteWorker :integration_class=>"SlackService", :message=>"SSL_connect returned=1 errno=0 state=error: certificate verify failed"} ``` This issue occurs when there is a problem with GitLab communicating with Slack, @@ -128,7 +128,7 @@ the GitLab OpenSSL trust store is incorrect. Typical causes are: - Overriding the trust store with `gitlab_rails['env'] = {"SSL_CERT_FILE" => "/path/to/file.pem"}`. - Accidentally modifying the default CA bundle `/opt/gitlab/embedded/ssl/certs/cacert.pem`. -### Bulk update to disable the Slack Notification service +### Bulk update to disable the Slack Notification integration To disable notifications for all projects that have Slack integration enabled, [start a rails console session](../../../administration/operations/rails_console.md#starting-a-rails-console-session) and use a script similar to the following: @@ -140,8 +140,8 @@ Commands that change data can cause damage if not run correctly or under the rig # Grab all projects that have the Slack notifications enabled p = Project.find_by_sql("SELECT p.id FROM projects p LEFT JOIN integrations s ON p.id = s.project_id WHERE s.type_new = 'Slack' AND s.active = true") -# Disable the service on each of the projects that were found. +# Disable the integration on each of the projects that were found. p.each do |project| - project.slack_service.update!(:active, false) + project.slack_integration.update!(:active, false) end ``` diff --git a/lib/api/users.rb b/lib/api/users.rb index 0ce7e56f919..406daa871fe 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -29,6 +29,7 @@ module API end helpers Helpers::UsersHelpers + helpers Gitlab::Tracking::Helpers::WeakPasswordErrorEvent helpers do # rubocop: disable CodeReuse/ActiveRecord @@ -306,6 +307,8 @@ module API .by_username(user.username) .any? + track_weak_password_error(user, 'API::Users', 'create') + render_validation_error!(user) end end @@ -351,6 +354,7 @@ module API if result[:status] == :success present user, with: Entities::UserWithAdmin, current_user: current_user else + track_weak_password_error(user, 'API::Users', 'update') render_validation_error!(user) end end diff --git a/lib/gitlab/tracking/helpers/weak_password_error_event.rb b/lib/gitlab/tracking/helpers/weak_password_error_event.rb new file mode 100644 index 00000000000..beb6119e3f7 --- /dev/null +++ b/lib/gitlab/tracking/helpers/weak_password_error_event.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module Tracking + module Helpers + module WeakPasswordErrorEvent + # Tracks information if a user record has a weak password. + # No-op unless the error is present. + # + # Captures a minimal set of information, so that GitLab + # remains unaware of which users / demographics are attempting + # to choose weak passwords. + def track_weak_password_error(user, controller, method_name) + return unless user.errors[:password].grep(/must not contain commonly used combinations.*/).any? + + Gitlab::Tracking.event( + 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + 'track_weak_password_error', + controller: controller, + method: method_name + ) + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4bbb0913375..9b8a5311130 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8403,6 +8403,9 @@ msgstr "" msgid "CiVariables|Masked" msgstr "" +msgid "CiVariables|Options" +msgstr "" + msgid "CiVariables|Protected" msgstr "" diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index e4be2fbef3c..9494f55c631 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -78,6 +78,22 @@ RSpec.describe PasswordsController do end end + context 'password is weak' do + let(:password) { "password" } + + it 'tracks the event' do + subject + + expect(response.body).to have_content("must not contain commonly used combinations of words and letters") + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'PasswordsController', + method: 'create' + ) + end + end + it 'sets the username and caller_id in the context' do expect(controller).to receive(:update).and_wrap_original do |m, *args| m.call(*args) diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index ceb00660658..119a7be25ef 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -492,6 +492,16 @@ RSpec.describe RegistrationsController do expect(response).to render_template(:new) expect(response.body).to include(_('Password must not contain commonly used combinations of words and letters')) end + + it 'tracks the error' do + subject + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'RegistrationsController', + method: 'create' + ) + end end context 'when block_weak_passwords is disabled' do @@ -504,6 +514,16 @@ RSpec.describe RegistrationsController do end end end + + context 'when the password is not weak' do + it 'does not track a weak password error' do + subject + expect_no_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error' + ) + end + end end describe '#destroy' do diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb index 6c860740354..8887ff1746d 100644 --- a/spec/features/profiles/password_spec.rb +++ b/spec/features/profiles/password_spec.rb @@ -152,6 +152,33 @@ RSpec.describe 'Profile > Password' do it_behaves_like 'user enters an incorrect current password' end + context 'when the password is too weak' do + let(:new_password) { 'password' } + + subject do + page.within '.update-password' do + fill_in "user_password", with: user.password + fill_passwords(new_password, new_password) + end + end + + it 'tracks the error and does not change the password', :aggregate_failures do + expect { subject }.not_to change { user.reload.valid_password?(new_password) } + expect(user.failed_attempts).to eq(0) + + page.within '.gl-alert-danger' do + expect(page).to have_content('must not contain commonly used combinations of words and letters') + end + + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'Profiles::PasswordsController', + method: 'update' + ) + end + end + context 'when the password reset is successful' do subject do page.within '.update-password' do @@ -195,6 +222,23 @@ RSpec.describe 'Profile > Password' do expect(page).to have_current_path new_user_session_path, ignore_query: true end + it 'tracks weak password error' do + visit edit_profile_password_path + + expect(page).to have_current_path new_profile_password_path, ignore_query: true + + fill_in :user_password, with: user.password + fill_in :user_new_password, with: "password" + fill_in :user_password_confirmation, with: "password" + click_button 'Set new password' + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'Profiles::PasswordsController', + method: 'create' + ) + end + context 'when global require_two_factor_authentication is enabled' do it 'needs change user password' do stub_application_setting(require_two_factor_authentication: true) diff --git a/spec/frontend/ci_variable_list/components/ci_variable_popover_spec.js b/spec/frontend/ci_variable_list/components/ci_variable_popover_spec.js deleted file mode 100644 index 4d0c378d10e..00000000000 --- a/spec/frontend/ci_variable_list/components/ci_variable_popover_spec.js +++ /dev/null @@ -1,48 +0,0 @@ -import { GlButton } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; -import CiVariablePopover from '~/ci_variable_list/components/ci_variable_popover.vue'; -import mockData from '../services/mock_data'; - -describe('Ci Variable Popover', () => { - let wrapper; - - const defaultProps = { - target: 'ci-variable-value-22', - value: mockData.mockPemCert, - tooltipText: 'Copy value', - }; - - const createComponent = (props = defaultProps) => { - wrapper = shallowMount(CiVariablePopover, { - propsData: { ...props }, - }); - }; - - const findButton = () => wrapper.findComponent(GlButton); - - beforeEach(() => { - createComponent(); - }); - - afterEach(() => { - wrapper.destroy(); - wrapper = null; - }); - - it('displays max count plus ... when character count is over 95', () => { - expect(wrapper.text()).toHaveLength(98); - }); - - it('copies full value to clipboard', () => { - expect(findButton().attributes('data-clipboard-text')).toEqual(mockData.mockPemCert); - }); - - it('displays full value when count is less than max count', () => { - createComponent({ - target: 'ci-variable-value-22', - value: 'test_variable_value', - tooltipText: 'Copy value', - }); - expect(wrapper.text()).toEqual('test_variable_value'); - }); -}); diff --git a/spec/lib/gitlab/tracking/helpers/weak_password_error_event_spec.rb b/spec/lib/gitlab/tracking/helpers/weak_password_error_event_spec.rb new file mode 100644 index 00000000000..3df10f79e98 --- /dev/null +++ b/spec/lib/gitlab/tracking/helpers/weak_password_error_event_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Tracking::Helpers::WeakPasswordErrorEvent do + let(:user) { build(:user) } + + subject(:helper) { Class.new.include(described_class).new } + + context "when user has a weak password error" do + before do + user.password = "password" + user.valid? + end + + it "tracks the event" do + helper.track_weak_password_error(user, 'A', 'B') + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'A', + method: 'B' + ) + end + end + + context "when user does not have a weak password error" do + before do + user.password = "short" + user.valid? + end + + it "does not track the event" do + helper.track_weak_password_error(user, 'A', 'B') + expect_no_snowplow_event + end + end + + context "when user does not have any errors" do + it "does not track the event" do + helper.track_weak_password_error(user, 'A', 'B') + expect_no_snowplow_event + end + end +end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 44a6bec0130..7fbf0ede75b 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -309,7 +309,7 @@ RSpec.describe Ci::Bridge do end context 'when the pipeline runs from a pipeline schedule' do - let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) } + let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) } let(:pipeline) { create(:ci_pipeline, pipeline_schedule: pipeline_schedule) } let(:options) do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d9fb225e8c0..9c83c1df10c 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1763,7 +1763,7 @@ RSpec.describe Ci::Build do context 'and start action is defined' do before do - build.update!(options: { environment: { action: 'start' } } ) + build.update!(options: { environment: { action: 'start' } }) end it { is_expected.to be_truthy } @@ -1793,7 +1793,7 @@ RSpec.describe Ci::Build do context 'and stop action is defined' do before do - build.update!(options: { environment: { action: 'stop' } } ) + build.update!(options: { environment: { action: 'stop' } }) end it { is_expected.to be_truthy } diff --git a/spec/models/ci/build_trace_spec.rb b/spec/models/ci/build_trace_spec.rb index f2df4874b84..907b49dc180 100644 --- a/spec/models/ci/build_trace_spec.rb +++ b/spec/models/ci/build_trace_spec.rb @@ -38,9 +38,9 @@ RSpec.describe Ci::BuildTrace do let(:data) { StringIO.new("UTF-8 dashes here: āāā\nš¤š¤š¤š¤\xF0\x9F\x90\n") } it 'returns valid UTF-8 data', :aggregate_failures do - expect(subject.lines[0]).to eq({ offset: 0, content: [{ text: 'UTF-8 dashes here: āāā' }] } ) + expect(subject.lines[0]).to eq({ offset: 0, content: [{ text: 'UTF-8 dashes here: āāā' }] }) # Each of the dashes is 3 bytes, so we get 19 + 9 + 1 = 29 - expect(subject.lines[1]).to eq({ offset: 29, content: [{ text: 'š¤š¤š¤š¤ļæ½' }] } ) + expect(subject.lines[1]).to eq({ offset: 29, content: [{ text: 'š¤š¤š¤š¤ļæ½' }] }) end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b98772df47f..68dfcdbd85a 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -4173,7 +4173,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do let(:pipeline) { create(:ci_pipeline) } let!(:old_job) { create(:ci_build, name: 'rspec', retried: true, pipeline: pipeline) } let!(:job_without_artifacts) { create(:ci_build, name: 'rspec', pipeline: pipeline) } - let!(:expected_job) { create(:ci_build, :artifacts, name: 'rspec', pipeline: pipeline ) } + let!(:expected_job) { create(:ci_build, :artifacts, name: 'rspec', pipeline: pipeline) } let!(:different_job) { create(:ci_build, name: 'deploy', pipeline: pipeline) } subject { pipeline.find_job_with_archive_artifacts('rspec') } diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb index 20f64d40865..2445babb5f2 100644 --- a/spec/models/ci/secure_file_spec.rb +++ b/spec/models/ci/secure_file_spec.rb @@ -131,7 +131,7 @@ RSpec.describe Ci::SecureFile do describe '#update_metadata!' do it 'assigns the expected metadata when a parsable file is supplied' do file = create(:ci_secure_file, name: 'file1.cer', - file: CarrierWaveStringFile.new(fixture_file('ci_secure_files/sample.cer') )) + file: CarrierWaveStringFile.new(fixture_file('ci_secure_files/sample.cer'))) file.update_metadata! expect(file.expires_at).to eq(DateTime.parse('2022-04-26 19:20:40')) diff --git a/spec/models/ci/trigger_request_spec.rb b/spec/models/ci/trigger_request_spec.rb index 0d462741089..a6e8e8496ac 100644 --- a/spec/models/ci/trigger_request_spec.rb +++ b/spec/models/ci/trigger_request_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::TriggerRequest do describe 'validation' do it 'be invalid if saving a variable' do - trigger = build(:ci_trigger_request, variables: { TRIGGER_KEY_1: 'TRIGGER_VALUE_1' } ) + trigger = build(:ci_trigger_request, variables: { TRIGGER_KEY_1: 'TRIGGER_VALUE_1' }) expect(trigger).not_to be_valid end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 549a273e2d7..b60c5ec114d 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -130,7 +130,7 @@ RSpec.describe Clusters::Applications::Prometheus do end context 'with knative installed' do - let(:knative) { create(:clusters_applications_knative, :updated ) } + let(:knative) { create(:clusters_applications_knative, :updated) } let(:prometheus) { create(:clusters_applications_prometheus, cluster: knative.cluster) } subject { prometheus.install_command } @@ -161,7 +161,7 @@ RSpec.describe Clusters::Applications::Prometheus do end describe '#predelete' do - let(:knative) { create(:clusters_applications_knative, :updated ) } + let(:knative) { create(:clusters_applications_knative, :updated) } let(:prometheus) { create(:clusters_applications_prometheus, cluster: knative.cluster) } subject { prometheus.uninstall_command.predelete } diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 635326eeadc..04763accc42 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -427,7 +427,7 @@ RSpec.describe DeployToken do end describe '.gitlab_deploy_token' do - let(:project) { create(:project ) } + let(:project) { create(:project) } subject { project.deploy_tokens.gitlab_deploy_token } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index a442856d993..8a3d43f58e0 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -884,8 +884,8 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do describe '#actions_for' do let(:deployment) { create(:deployment, :success, environment: environment) } let(:pipeline) { deployment.deployable.pipeline } - let!(:review_action) { create(:ci_build, :manual, name: 'review-apps', pipeline: pipeline, environment: 'review/$CI_COMMIT_REF_NAME' ) } - let!(:production_action) { create(:ci_build, :manual, name: 'production', pipeline: pipeline, environment: 'production' ) } + let!(:review_action) { create(:ci_build, :manual, name: 'review-apps', pipeline: pipeline, environment: 'review/$CI_COMMIT_REF_NAME') } + let!(:production_action) { create(:ci_build, :manual, name: 'production', pipeline: pipeline, environment: 'production') } it 'returns a list of actions with matching environment' do expect(environment.actions_for('review/master')).to contain_exactly(review_action) diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index a9aa5698ebb..2f1edf9ab94 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -37,7 +37,7 @@ RSpec.describe EnvironmentStatus do context 'multiple deployments' do it { - new_deployment = create(:deployment, :succeed, environment: deployment.environment, sha: deployment.sha ) + new_deployment = create(:deployment, :succeed, environment: deployment.environment, sha: deployment.sha) is_expected.to eq(new_deployment) } end diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index 0f9d5bf3c2d..dc740ce8b0f 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -54,7 +54,7 @@ RSpec.describe Experiment do end context 'with an existing context' do - let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user, converted_at: 2.days.ago, context: { b: 1 } ) } + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user, converted_at: 2.days.ago, context: { b: 1 }) } it 'merges the context' do expect { record_conversion }.to change { experiment_subject.reload.context }.to('a' => 42, 'b' => 1) diff --git a/spec/models/exported_protected_branch_spec.rb b/spec/models/exported_protected_branch_spec.rb index 7886a522741..9f862de6ff8 100644 --- a/spec/models/exported_protected_branch_spec.rb +++ b/spec/models/exported_protected_branch_spec.rb @@ -11,7 +11,7 @@ RSpec.describe ExportedProtectedBranch do it 'returns the correct push access levels' do exported_branch = create(:exported_protected_branch, :developers_can_push) deploy_key = create(:deploy_key) - create(:deploy_keys_project, :write_access, project: exported_branch.project, deploy_key: deploy_key ) + create(:deploy_keys_project, :write_access, project: exported_branch.project, deploy_key: deploy_key) create(:protected_branch_push_access_level, protected_branch: exported_branch, deploy_key: deploy_key) dev_push_access_level = exported_branch.push_access_levels.first diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index eb9ef8a21e9..928f9d6d057 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2556,7 +2556,7 @@ RSpec.describe Group do end context 'when parent does not allow' do - let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false ) } + let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } it 'raises exception' do diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 4c8c19cb3e5..819dad9d46d 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Integrations::Jira do end before do - WebMock.stub_request(:get, /serverInfo/).to_return(body: server_info_results.to_json ) + WebMock.stub_request(:get, /serverInfo/).to_return(body: server_info_results.to_json) end it_behaves_like Integrations::ResetSecretFields do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index e7b2212ebff..08c05b614f6 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -654,7 +654,7 @@ RSpec.describe Issue do let_it_be(:authorized_issue_a) { create(:issue, project: authorized_project) } let_it_be(:authorized_issue_b) { create(:issue, project: authorized_project) } let_it_be(:authorized_issue_c) { create(:issue, project: authorized_project2) } - let_it_be(:authorized_incident_a) { create(:incident, project: authorized_project ) } + let_it_be(:authorized_incident_a) { create(:incident, project: authorized_project) } let_it_be(:unauthorized_issue) { create(:issue, project: unauthorized_project) } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 04df8ecc882..c70f0144fe6 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -213,7 +213,7 @@ RSpec.describe Member do describe 'Scopes & finders' do let_it_be(:project) { create(:project, :public) } let_it_be(:group) { create(:group) } - let_it_be(:blocked_pending_approval_user) { create(:user, :blocked_pending_approval ) } + let_it_be(:blocked_pending_approval_user) { create(:user, :blocked_pending_approval) } let_it_be(:blocked_pending_approval_project_member) { create(:project_member, :invited, :developer, project: project, invite_email: blocked_pending_approval_user.email) } let_it_be(:awaiting_group_member) { create(:group_member, :awaiting, group: group) } let_it_be(:awaiting_project_member) { create(:project_member, :awaiting, project: project) } diff --git a/spec/models/metrics/dashboard/annotation_spec.rb b/spec/models/metrics/dashboard/annotation_spec.rb index 4b7492016f3..9b8601e4052 100644 --- a/spec/models/metrics/dashboard/annotation_spec.rb +++ b/spec/models/metrics/dashboard/annotation_spec.rb @@ -63,7 +63,7 @@ RSpec.describe Metrics::Dashboard::Annotation do end context 'annotation with shared ownership' do - subject { build(:metrics_dashboard_annotation, :with_cluster, environment: build(:environment) ) } + subject { build(:metrics_dashboard_annotation, :with_cluster, environment: build(:environment)) } it 'reports error about both shared ownership' do subject.valid? diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index a4446bfedd1..17c49e13c85 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -106,7 +106,7 @@ RSpec.describe NamespaceSetting, type: :model do describe '#prevent_sharing_groups_outside_hierarchy' do let(:settings) { create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true) } - let!(:group) { create(:group, parent: parent, namespace_settings: settings ) } + let!(:group) { create(:group, parent: parent, namespace_settings: settings) } subject(:group_sharing_setting) { settings.prevent_sharing_groups_outside_hierarchy } @@ -133,7 +133,7 @@ RSpec.describe NamespaceSetting, type: :model do context 'when :show_diff_preview_in_email is false' do it 'returns false' do settings = create(:namespace_settings, show_diff_preview_in_email: false) - group = create(:group, namespace_settings: settings ) + group = create(:group, namespace_settings: settings) expect(group.show_diff_preview_in_email?).to be_falsey end @@ -142,7 +142,7 @@ RSpec.describe NamespaceSetting, type: :model do context 'when :show_diff_preview_in_email is true' do it 'returns true' do settings = create(:namespace_settings, show_diff_preview_in_email: true) - group = create(:group, namespace_settings: settings ) + group = create(:group, namespace_settings: settings) expect(group.show_diff_preview_in_email?).to be_truthy end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 838e5d700c1..4fc752a6a66 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1623,7 +1623,7 @@ RSpec.describe Namespace do describe '#share_with_group_lock with subgroups' do context 'when creating a subgroup' do - let(:subgroup) { create(:group, parent: root_group ) } + let(:subgroup) { create(:group, parent: root_group) } context 'under a parent with "Share with group lock" enabled' do let(:root_group) { create(:group, share_with_group_lock: true) } @@ -1644,7 +1644,7 @@ RSpec.describe Namespace do context 'when enabling the parent group "Share with group lock"' do let(:root_group) { create(:group) } - let!(:subgroup) { create(:group, parent: root_group ) } + let!(:subgroup) { create(:group, parent: root_group) } it 'the subgroup "Share with group lock" becomes enabled' do root_group.update!(share_with_group_lock: true) @@ -1657,7 +1657,7 @@ RSpec.describe Namespace do let(:root_group) { create(:group, share_with_group_lock: true) } context 'and the subgroup "Share with group lock" is enabled' do - let(:subgroup) { create(:group, parent: root_group, share_with_group_lock: true ) } + let(:subgroup) { create(:group, parent: root_group, share_with_group_lock: true) } it 'the subgroup "Share with group lock" does not change' do root_group.update!(share_with_group_lock: false) @@ -1667,7 +1667,7 @@ RSpec.describe Namespace do end context 'but the subgroup "Share with group lock" is disabled' do - let(:subgroup) { create(:group, parent: root_group ) } + let(:subgroup) { create(:group, parent: root_group) } it 'the subgroup "Share with group lock" does not change' do root_group.update!(share_with_group_lock: false) @@ -1682,7 +1682,7 @@ RSpec.describe Namespace do let(:root_group) { create(:group, share_with_group_lock: true) } context 'when the subgroup "Share with group lock" is enabled' do - let(:subgroup) { create(:group, share_with_group_lock: true ) } + let(:subgroup) { create(:group, share_with_group_lock: true) } it 'the subgroup "Share with group lock" does not change' do subgroup.parent = root_group @@ -1708,7 +1708,7 @@ RSpec.describe Namespace do let(:root_group) { create(:group) } context 'when the subgroup "Share with group lock" is enabled' do - let(:subgroup) { create(:group, share_with_group_lock: true ) } + let(:subgroup) { create(:group, share_with_group_lock: true) } it 'the subgroup "Share with group lock" does not change' do subgroup.parent = root_group diff --git a/spec/models/network/graph_spec.rb b/spec/models/network/graph_spec.rb index a393aace39c..5274526e81c 100644 --- a/spec/models/network/graph_spec.rb +++ b/spec/models/network/graph_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Network::Graph do it '#initialize' do graph = described_class.new(project, 'refs/heads/master', project.repository.commit, nil) - expect(graph.notes).to eq( { note_on_commit.commit_id => 1 } ) + expect(graph.notes).to eq({ note_on_commit.commit_id => 1 }) end describe '#commits' do @@ -19,7 +19,7 @@ RSpec.describe Network::Graph do commits = graph.commits expect(commits).not_to be_empty - expect(commits).to all( be_kind_of(Network::Commit) ) + expect(commits).to all(be_kind_of(Network::Commit)) end it 'sorts commits by commit date (descending)' do @@ -42,7 +42,7 @@ RSpec.describe Network::Graph do parent_indexes = commit.parent_ids.map { |parent_id| commit_ids.find_index(parent_id) }.compact # All parents of the current commit should appear after it - expect(parent_indexes).to all( be > index ) + expect(parent_indexes).to all(be > index) end end end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 0edb04224a3..241c585099c 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -969,12 +969,12 @@ RSpec.describe Packages::Package, type: :model do end context 'sorting' do - let_it_be(:project) { create(:project, name: 'aaa' ) } - let_it_be(:project2) { create(:project, name: 'bbb' ) } - let_it_be(:package1) { create(:package, project: project ) } - let_it_be(:package2) { create(:package, project: project2 ) } - let_it_be(:package3) { create(:package, project: project2 ) } - let_it_be(:package4) { create(:package, project: project ) } + let_it_be(:project) { create(:project, name: 'aaa') } + let_it_be(:project2) { create(:project, name: 'bbb') } + let_it_be(:package1) { create(:package, project: project) } + let_it_be(:package2) { create(:package, project: project2) } + let_it_be(:package3) { create(:package, project: project2) } + let_it_be(:package4) { create(:package, project: project) } it 'orders packages by their projects name ascending' do expect(Packages::Package.order_project_name).to eq([package1, package4, package2, package3]) diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 73ec51ea801..9d4c53f8d55 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -108,7 +108,7 @@ RSpec.describe PersonalAccessToken do describe '.last_used_before' do context 'last_used_*' do let_it_be(:date) { DateTime.new(2022, 01, 01) } - let_it_be(:token) { create(:personal_access_token, last_used_at: date ) } + let_it_be(:token) { create(:personal_access_token, last_used_at: date) } # This token should never occur in the following tests and indicates that filtering was done correctly with it let_it_be(:never_used_token) { create(:personal_access_token) } diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 5730ca58e9e..eb0292c7005 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -70,7 +70,7 @@ RSpec.describe ProjectSetting, type: :model do describe '#show_diff_preview_in_email?' do context 'when a project is a top-level namespace' do - let(:project_settings ) { create(:project_setting, show_diff_preview_in_email: false) } + let(:project_settings) { create(:project_setting, show_diff_preview_in_email: false) } let(:project) { create(:project, project_setting: project_settings) } context 'when show_diff_preview_in_email is disabled' do @@ -80,7 +80,7 @@ RSpec.describe ProjectSetting, type: :model do end context 'when show_diff_preview_in_email is enabled' do - let(:project_settings ) { create(:project_setting, show_diff_preview_in_email: true) } + let(:project_settings) { create(:project_setting, show_diff_preview_in_email: true) } it 'returns true' do settings = create(:project_setting, show_diff_preview_in_email: true) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 88d96a812e0..359fc4db01f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -4508,7 +4508,7 @@ RSpec.describe Project, factory_default: :keep do project_2 = create(:project, :public, :merge_requests_disabled) project_3 = create(:project, :public, :issues_disabled) project_4 = create(:project, :public) - project_4.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE ) + project_4.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE) project_ids = described_class.ids_with_issuables_available_for(user).pluck(:id) @@ -7125,7 +7125,7 @@ RSpec.describe Project, factory_default: :keep do describe '#export_in_progress?' do let(:project) { build(:project) } - let!(:project_export_job ) { create(:project_export_job, project: project) } + let!(:project_export_job) { create(:project_export_job, project: project) } context 'when project export is enqueued' do it { expect(project.export_in_progress?).to be false } @@ -7150,7 +7150,7 @@ RSpec.describe Project, factory_default: :keep do describe '#export_status' do let(:project) { build(:project) } - let!(:project_export_job ) { create(:project_export_job, project: project) } + let!(:project_export_job) { create(:project_export_job, project: project) } context 'when project export is enqueued' do it { expect(project.export_status).to eq :queued } @@ -7174,7 +7174,7 @@ RSpec.describe Project, factory_default: :keep do end context 'when project export is being regenerated' do - let!(:new_project_export_job ) { create(:project_export_job, project: project) } + let!(:new_project_export_job) { create(:project_export_job, project: project) } before do finish_job(project_export_job) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 6fbf69ec23a..adebacec352 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -463,7 +463,7 @@ RSpec.describe Repository do repository.delete_branch(branch) expect(subject).not_to be_empty - expect(subject).to all( be_a(::Commit) ) + expect(subject).to all(be_a(::Commit)) expect(subject.size).to eq(1) end end @@ -482,7 +482,7 @@ RSpec.describe Repository do end it 'returns only Commit instances' do - expect(subject).to all( be_a(Commit) ) + expect(subject).to all(be_a(Commit)) end context 'when some commits are not found ' do @@ -2978,7 +2978,7 @@ RSpec.describe Repository do it 'returns false for invalid commit IDs' do expect(repository.ancestor?(commit.id, Gitlab::Git::BLANK_SHA)).to eq(false) - expect(repository.ancestor?( Gitlab::Git::BLANK_SHA, commit.id)).to eq(false) + expect(repository.ancestor?(Gitlab::Git::BLANK_SHA, commit.id)).to eq(false) end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 18b0cb36cc6..23ba0be2fbc 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -498,7 +498,7 @@ RSpec.describe Todo do describe '.for_internal_notes' do it 'returns todos created from internal notes' do - internal_note = create(:note, confidential: true ) + internal_note = create(:note, confidential: true) todo = create(:todo, note: internal_note) create(:todo) diff --git a/spec/models/users/calloutable_spec.rb b/spec/models/users/calloutable_spec.rb index 791fe1c1bc4..7e186445c1b 100644 --- a/spec/models/users/calloutable_spec.rb +++ b/spec/models/users/calloutable_spec.rb @@ -15,8 +15,8 @@ RSpec.describe Users::Calloutable do describe '#dismissed_after?' do let(:some_feature_name) { Users::Callout.feature_names.keys.second } - let(:callout_dismissed_month_ago) { create(:callout, feature_name: some_feature_name, dismissed_at: 1.month.ago ) } - let(:callout_dismissed_day_ago) { create(:callout, feature_name: some_feature_name, dismissed_at: 1.day.ago ) } + let(:callout_dismissed_month_ago) { create(:callout, feature_name: some_feature_name, dismissed_at: 1.month.ago) } + let(:callout_dismissed_day_ago) { create(:callout, feature_name: some_feature_name, dismissed_at: 1.day.ago) } it 'returns whether a callout dismissed after specified date' do expect(callout_dismissed_month_ago.dismissed_after?(15.days.ago)).to eq(false) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index fb8706ab4c3..28ee557c996 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1291,6 +1291,20 @@ RSpec.describe API::Users do .to eq([Gitlab::PathRegex.namespace_format_message]) end + it 'tracks weak password errors' do + attributes = attributes_for(:user).merge({ password: "password" }) + post api('/users', admin), params: attributes + + expect(json_response['message']['password']) + .to eq(['must not contain commonly used combinations of words and letters']) + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'API::Users', + method: 'create' + ) + end + it "is not available for non admin users" do post api("/users", user), params: attributes_for(:user) expect(response).to have_gitlab_http_status(:forbidden) @@ -1492,6 +1506,21 @@ RSpec.describe API::Users do .not_to have_enqueued_mail(DeviseMailer, :password_change) end end + + context 'with a weak password' do + it 'tracks weak password errors' do + update_password(user, admin, "password") + + expect(json_response['message']['password']) + .to eq(['must not contain commonly used combinations of words and letters']) + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'API::Users', + method: 'update' + ) + end + end end it "updates user with new bio" do diff --git a/spec/support/shared_examples/features/variable_list_shared_examples.rb b/spec/support/shared_examples/features/variable_list_shared_examples.rb index d1e5046a39e..f0b72cfaee3 100644 --- a/spec/support/shared_examples/features/variable_list_shared_examples.rb +++ b/spec/support/shared_examples/features/variable_list_shared_examples.rb @@ -31,8 +31,8 @@ RSpec.shared_examples 'variable list' do |is_admin| wait_for_requests page.within('[data-testid="ci-variable-table"]') do - expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Key"]').text).to eq('key') - expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Protected"] svg[data-testid="mobile-issue-close-icon"]')).to be_present + expect(find(".js-ci-variable-row:nth-child(1) td[data-label='#{s_('CiVariables|Key')}']").text).to eq('key') + expect(find(".js-ci-variable-row:nth-child(1) td[data-label='#{s_('CiVariables|Options')}']")).to have_content(s_('CiVariables|Protected')) end end @@ -46,26 +46,26 @@ RSpec.shared_examples 'variable list' do |is_admin| wait_for_requests page.within('[data-testid="ci-variable-table"]') do - expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Key"]').text).to eq('key') - expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Masked"] svg[data-testid="close-icon"]')).to be_present + expect(find(".js-ci-variable-row:nth-child(1) td[data-label='#{s_('CiVariables|Key')}']").text).to eq('key') + expect(find(".js-ci-variable-row:nth-child(1) td[data-label='#{s_('CiVariables|Options')}']")).not_to have_content(s_('CiVariables|Masked')) end end it 'reveals and hides variables' do page.within('[data-testid="ci-variable-table"]') do expect(first('.js-ci-variable-row td[data-label="Key"]').text).to eq(variable.key) - expect(page).to have_content('*' * 17) + expect(page).to have_content('*' * 5) click_button('Reveal value') expect(first('.js-ci-variable-row td[data-label="Key"]').text).to eq(variable.key) expect(first('.js-ci-variable-row td[data-label="Value"]').text).to eq(variable.value) - expect(page).not_to have_content('*' * 17) + expect(page).not_to have_content('*' * 5) click_button('Hide value') expect(first('.js-ci-variable-row td[data-label="Key"]').text).to eq(variable.key) - expect(page).to have_content('*' * 17) + expect(page).to have_content('*' * 5) end end @@ -116,7 +116,8 @@ RSpec.shared_examples 'variable list' do |is_admin| wait_for_requests page.within('[data-testid="ci-variable-table"]') do - expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Masked"] svg[data-testid="close-icon"]')).to be_present + expect(find(".js-ci-variable-row:nth-child(1) td[data-label='#{s_('CiVariables|Options')}']")).to have_content(s_('CiVariables|Protected')) + expect(find(".js-ci-variable-row:nth-child(1) td[data-label='#{s_('CiVariables|Options')}']")).not_to have_content(s_('CiVariables|Masked')) end end @@ -144,7 +145,7 @@ RSpec.shared_examples 'variable list' do |is_admin| end page.within('[data-testid="ci-variable-table"]') do - expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Masked"] svg[data-testid="mobile-issue-close-icon"]')).to be_present + expect(find(".js-ci-variable-row:nth-child(1) td[data-label='#{s_('CiVariables|Options')}']")).to have_content(s_('CiVariables|Masked')) end end |
