Commit 2d6b4d05 authored by John T Skarbek's avatar John T Skarbek

Merge remote-tracking branch 'dev/master'

parents b1527121 69a9f4fe
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
## 13.0.1 (2020-05-27)
### Security (3 changes)
- Change the mirror user along with pull mirror settings.
- Allow only users with a verified email to be member of a group when the group has restricted membership based on email domain.
- Do not auto-confirm email in Trial registration.
## 13.0.0 (2020-05-22) ## 13.0.0 (2020-05-22)
### Security (1 change) ### Security (1 change)
...@@ -326,6 +335,15 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -326,6 +335,15 @@ Please view this file on the master branch, on stable branches it's out of date.
- Translate unauthenticated user string for Audit Event. !31856 (Sashi Kumar) - Translate unauthenticated user string for Audit Event. !31856 (Sashi Kumar)
## 12.10.7 (2020-05-27)
### Security (3 changes)
- Change the mirror user along with pull mirror settings.
- Allow only users with a verified email to be member of a group when the group has restricted membership based on email domain.
- Do not auto-confirm email in Trial registration.
## 12.10.6 (2020-05-15) ## 12.10.6 (2020-05-15)
- No changes. - No changes.
...@@ -400,6 +418,15 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -400,6 +418,15 @@ Please view this file on the master branch, on stable branches it's out of date.
- Add health status counts to usage data. !28964 - Add health status counts to usage data. !28964
## 12.9.8 (2020-05-27)
### Security (3 changes)
- Change the mirror user along with pull mirror settings.
- Allow only users with a verified email to be member of a group when the group has restricted membership based on email domain.
- Do not auto-confirm email in Trial registration.
## 12.9.6 (2020-05-05) ## 12.9.6 (2020-05-05)
- No changes. - No changes.
......
...@@ -2,6 +2,24 @@ ...@@ -2,6 +2,24 @@
documentation](doc/development/changelog.md) for instructions on adding your own documentation](doc/development/changelog.md) for instructions on adding your own
entry. entry.
## 13.0.1 (2020-05-27)
### Security (12 changes)
- Add an extra validation to Static Site Editor payload.
- Hide EKS secret key in admin integrations settings.
- Added data integrity check before updating a deploy key.
- Display only verified emails on notifications and profile page.
- Require confirmed email address for GitLab OAuth authentication.
- Kubernetes cluster details page no longer exposes Service Token.
- Fix confirming unverified emails with soft email confirmation flow enabled.
- Disallow user to control PUT request using mermaid markdown in issue description.
- Check forked project permissions before allowing fork.
- Limit memory footprint of a command that generates ZIP artifacts metadata.
- Fix file enuming using Group Import.
- Prevent XSS in the monitoring dashboard.
## 13.0.0 (2020-05-22) ## 13.0.0 (2020-05-22)
### Removed (20 changes, 5 of them are from the community) ### Removed (20 changes, 5 of them are from the community)
...@@ -571,6 +589,26 @@ entry. ...@@ -571,6 +589,26 @@ entry.
- Use visitUrl in Alert management. !32414 - Use visitUrl in Alert management. !32414
## 12.10.7 (2020-05-27)
### Security (14 changes)
- Add an extra validation to Static Site Editor payload.
- Hide EKS secret key in admin integrations settings.
- Added data integrity check before updating a deploy key.
- Display only verified emails on notifications and profile page.
- Disable caching on repo/blobs/[sha]/raw endpoint.
- Require confirmed email address for GitLab OAuth authentication.
- Kubernetes cluster details page no longer exposes Service Token.
- Fix confirming unverified emails with soft email confirmation flow enabled.
- Disallow user to control PUT request using mermaid markdown in issue description.
- Check forked project permissions before allowing fork.
- Limit memory footprint of a command that generates ZIP artifacts metadata.
- Fix file enuming using Group Import.
- Prevent XSS in the monitoring dashboard.
- Use `gsub` instead of the Ruby `%` operator to perform variable substitution in Prometheus proxy API.
## 12.10.6 (2020-05-15) ## 12.10.6 (2020-05-15)
### Fixed (5 changes) ### Fixed (5 changes)
...@@ -1071,6 +1109,25 @@ entry. ...@@ -1071,6 +1109,25 @@ entry.
- Remove store_mentions! in Snippets::CreateService. !29581 (Sashi Kumar) - Remove store_mentions! in Snippets::CreateService. !29581 (Sashi Kumar)
## 12.9.8 (2020-05-27)
### Security (13 changes)
- Hide EKS secret key in admin integrations settings.
- Added data integrity check before updating a deploy key.
- Display only verified emails on notifications and profile page.
- Disable caching on repo/blobs/[sha]/raw endpoint.
- Require confirmed email address for GitLab OAuth authentication.
- Kubernetes cluster details page no longer exposes Service Token.
- Fix confirming unverified emails with soft email confirmation flow enabled.
- Disallow user to control PUT request using mermaid markdown in issue description.
- Check forked project permissions before allowing fork.
- Limit memory footprint of a command that generates ZIP artifacts metadata.
- Fix file enuming using Group Import.
- Prevent XSS in the monitoring dashboard.
- Use `gsub` instead of the Ruby `%` operator to perform variable substitution in Prometheus proxy API.
## 12.9.6 (2020-05-05) ## 12.9.6 (2020-05-05)
### Fixed (1 change) ### Fixed (1 change)
......
...@@ -108,7 +108,6 @@ export default class Clusters { ...@@ -108,7 +108,6 @@ export default class Clusters {
}); });
this.installApplication = this.installApplication.bind(this); this.installApplication = this.installApplication.bind(this);
this.showToken = this.showToken.bind(this);
this.errorContainer = document.querySelector('.js-cluster-error'); this.errorContainer = document.querySelector('.js-cluster-error');
this.successContainer = document.querySelector('.js-cluster-success'); this.successContainer = document.querySelector('.js-cluster-success');
...@@ -119,7 +118,6 @@ export default class Clusters { ...@@ -119,7 +118,6 @@ export default class Clusters {
); );
this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason'); this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason');
this.successApplicationContainer = document.querySelector('.js-cluster-application-notice'); this.successApplicationContainer = document.querySelector('.js-cluster-application-notice');
this.showTokenButton = document.querySelector('.js-show-cluster-token');
this.tokenField = document.querySelector('.js-cluster-token'); this.tokenField = document.querySelector('.js-cluster-token');
this.ingressDomainHelpText = document.querySelector('.js-ingress-domain-help-text'); this.ingressDomainHelpText = document.querySelector('.js-ingress-domain-help-text');
this.ingressDomainSnippet = this.ingressDomainSnippet =
...@@ -258,7 +256,6 @@ export default class Clusters { ...@@ -258,7 +256,6 @@ export default class Clusters {
} }
addListeners() { addListeners() {
if (this.showTokenButton) this.showTokenButton.addEventListener('click', this.showToken);
eventHub.$on('installApplication', this.installApplication); eventHub.$on('installApplication', this.installApplication);
eventHub.$on('updateApplication', data => this.updateApplication(data)); eventHub.$on('updateApplication', data => this.updateApplication(data));
eventHub.$on('saveKnativeDomain', data => this.saveKnativeDomain(data)); eventHub.$on('saveKnativeDomain', data => this.saveKnativeDomain(data));
...@@ -275,7 +272,6 @@ export default class Clusters { ...@@ -275,7 +272,6 @@ export default class Clusters {
} }
removeListeners() { removeListeners() {
if (this.showTokenButton) this.showTokenButton.removeEventListener('click', this.showToken);
eventHub.$off('installApplication', this.installApplication); eventHub.$off('installApplication', this.installApplication);
eventHub.$off('updateApplication', this.updateApplication); eventHub.$off('updateApplication', this.updateApplication);
eventHub.$off('saveKnativeDomain'); eventHub.$off('saveKnativeDomain');
...@@ -344,18 +340,6 @@ export default class Clusters { ...@@ -344,18 +340,6 @@ export default class Clusters {
} }
} }
showToken() {
const type = this.tokenField.getAttribute('type');
if (type === 'password') {
this.tokenField.setAttribute('type', 'text');
this.showTokenButton.textContent = s__('ClusterIntegration|Hide');
} else {
this.tokenField.setAttribute('type', 'password');
this.showTokenButton.textContent = s__('ClusterIntegration|Show');
}
}
hideAll() { hideAll() {
this.errorContainer.classList.add('hidden'); this.errorContainer.classList.add('hidden');
this.successContainer.classList.add('hidden'); this.successContainer.classList.add('hidden');
......
...@@ -119,7 +119,11 @@ export default class Issue { ...@@ -119,7 +119,11 @@ export default class Issue {
} else { } else {
this.disableCloseReopenButton($button); this.disableCloseReopenButton($button);
const url = $button.attr('href'); const url = $button.data('close-reopen-url');
if (!url) {
return;
}
return axios return axios
.put(url) .put(url)
.then(({ data }) => { .then(({ data }) => {
......
<script> <script>
import { __, s__, sprintf } from '~/locale'; import { __, s__, sprintf } from '~/locale';
import { GlFormGroup, GlFormInput, GlFormRadioGroup, GlFormTextarea } from '@gitlab/ui'; import { GlFormGroup, GlFormInput, GlFormRadioGroup, GlFormTextarea } from '@gitlab/ui';
import { escape as esc } from 'lodash';
const defaultFileName = dashboard => dashboard.path.split('/').reverse()[0]; const defaultFileName = dashboard => dashboard.path.split('/').reverse()[0];
...@@ -42,7 +43,7 @@ export default { ...@@ -42,7 +43,7 @@ export default {
html: sprintf( html: sprintf(
__('Commit to %{branchName} branch'), __('Commit to %{branchName} branch'),
{ {
branchName: `<strong>${this.defaultBranch}</strong>`, branchName: `<strong>${esc(this.defaultBranch)}</strong>`,
}, },
false, false,
), ),
......
...@@ -40,7 +40,9 @@ export default class Profile { ...@@ -40,7 +40,9 @@ export default class Profile {
bindEvents() { bindEvents() {
$('.js-preferences-form').on('change.preference', 'input[type=radio]', this.submitForm); $('.js-preferences-form').on('change.preference', 'input[type=radio]', this.submitForm);
$('.js-group-notification-email').on('change', this.submitForm); $('.js-group-notification-email').on('change', this.submitForm);
$('#user_notification_email').on('change', this.submitForm); $('#user_notification_email').on('select2-selecting', event => {
setTimeout(this.submitForm.bind(event.currentTarget));
});
$('#user_notified_of_own_activity').on('change', this.submitForm); $('#user_notified_of_own_activity').on('change', this.submitForm);
this.form.on('submit', this.onSubmitForm); this.form.on('submit', this.onSubmitForm);
} }
......
...@@ -191,8 +191,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -191,8 +191,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
params[:application_setting][:import_sources]&.delete("") params[:application_setting][:import_sources]&.delete("")
params[:application_setting][:restricted_visibility_levels]&.delete("") params[:application_setting][:restricted_visibility_levels]&.delete("")
params[:application_setting].delete(:elasticsearch_aws_secret_access_key) if params[:application_setting][:elasticsearch_aws_secret_access_key].blank?
params[:application_setting][:required_instance_ci_template] = nil if params[:application_setting][:required_instance_ci_template].blank? params[:application_setting][:required_instance_ci_template] = nil if params[:application_setting][:required_instance_ci_template].blank?
remove_blank_params_for!(:elasticsearch_aws_secret_access_key, :eks_secret_access_key)
# TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204) # TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204)
params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file] params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file]
params.delete(:domain_blacklist_raw) if params[:domain_blacklist] params.delete(:domain_blacklist_raw) if params[:domain_blacklist]
...@@ -262,6 +264,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -262,6 +264,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
render action render action
end end
def remove_blank_params_for!(*keys)
params[:application_setting].delete_if { |setting, value| setting.to_sym.in?(keys) && value.blank? }
end
# overridden in EE # overridden in EE
def valid_setting_panels def valid_setting_panels
VALID_SETTING_PANELS VALID_SETTING_PANELS
......
...@@ -53,10 +53,16 @@ module MembershipActions ...@@ -53,10 +53,16 @@ module MembershipActions
end end
def request_access def request_access
membershipable.request_access(current_user) access_requester = membershipable.request_access(current_user)
redirect_to polymorphic_path(membershipable), if access_requester.persisted?
notice: _('Your request for access has been queued for review.') redirect_to polymorphic_path(membershipable),
notice: _('Your request for access has been queued for review.')
else
redirect_to polymorphic_path(membershipable),
alert: _("Your request for access could not be processed: %{error_meesage}") %
{ error_meesage: access_requester.errors.full_messages.to_sentence }
end
end end
def approve_access_request def approve_access_request
......
...@@ -4,6 +4,8 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController ...@@ -4,6 +4,8 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
include Gitlab::Experimentation::ControllerConcern include Gitlab::Experimentation::ControllerConcern
include InitializesCurrentUserMode include InitializesCurrentUserMode
before_action :verify_confirmed_email!, only: [:new]
layout 'profile' layout 'profile'
# Overridden from Doorkeeper::AuthorizationsController to # Overridden from Doorkeeper::AuthorizationsController to
...@@ -21,4 +23,13 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController ...@@ -21,4 +23,13 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
render "doorkeeper/authorizations/error" render "doorkeeper/authorizations/error"
end end
end end
private
def verify_confirmed_email!
return if current_user&.confirmed?
pre_auth.error = :unconfirmed_email
render "doorkeeper/authorizations/error"
end
end end
...@@ -37,6 +37,8 @@ class Projects::DeployKeysController < Projects::ApplicationController ...@@ -37,6 +37,8 @@ class Projects::DeployKeysController < Projects::ApplicationController
end end
def update def update
access_denied! unless deploy_key
if deploy_key.update(update_params) if deploy_key.update(update_params)
flash[:notice] = _('Deploy key was successfully updated.') flash[:notice] = _('Deploy key was successfully updated.')
redirect_to_repository redirect_to_repository
...@@ -85,10 +87,12 @@ class Projects::DeployKeysController < Projects::ApplicationController ...@@ -85,10 +87,12 @@ class Projects::DeployKeysController < Projects::ApplicationController
end end
def update_params def update_params
permitted_params = [deploy_keys_projects_attributes: [:id, :can_push]] permitted_params = [deploy_keys_projects_attributes: [:can_push]]
permitted_params << :title if can?(current_user, :update_deploy_key, deploy_key) permitted_params << :title if can?(current_user, :update_deploy_key, deploy_key)
params.require(:deploy_key).permit(*permitted_params) key_update_params = params.require(:deploy_key).permit(*permitted_params)
key_update_params.dig(:deploy_keys_projects_attributes, '0')&.merge!(id: deploy_keys_project.id)
key_update_params
end end
def authorize_update_deploy_key! def authorize_update_deploy_key!
......
...@@ -14,6 +14,7 @@ class NotificationSetting < ApplicationRecord ...@@ -14,6 +14,7 @@ class NotificationSetting < ApplicationRecord
validates :user_id, uniqueness: { scope: [:source_type, :source_id], validates :user_id, uniqueness: { scope: [:source_type, :source_id],
message: "already exists in source", message: "already exists in source",
allow_nil: true } allow_nil: true }
validate :owns_notification_email, if: :notification_email_changed?
scope :for_groups, -> { where(source_type: 'Namespace') } scope :for_groups, -> { where(source_type: 'Namespace') }
...@@ -97,6 +98,13 @@ class NotificationSetting < ApplicationRecord ...@@ -97,6 +98,13 @@ class NotificationSetting < ApplicationRecord
def event_enabled?(event) def event_enabled?(event)
respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend
end end
def owns_notification_email
return if user.temp_oauth_email?
return if notification_email.empty?
errors.add(:notification_email, _("is not an email you own")) unless user.verified_emails.include?(notification_email)
end
end end
NotificationSetting.prepend_if_ee('EE::NotificationSetting') NotificationSetting.prepend_if_ee('EE::NotificationSetting')
...@@ -238,9 +238,10 @@ class User < ApplicationRecord ...@@ -238,9 +238,10 @@ class User < ApplicationRecord
if previous_changes.key?('email') if previous_changes.key?('email')
# Grab previous_email here since previous_changes changes after # Grab previous_email here since previous_changes changes after
# #update_emails_with_primary_email and #update_notification_email are called # #update_emails_with_primary_email and #update_notification_email are called
previous_confirmed_at = previous_changes.key?('confirmed_at') ? previous_changes['confirmed_at'][0] : confirmed_at
previous_email = previous_changes[:email][0] previous_email = previous_changes[:email][0]
update_emails_with_primary_email(previous_email) update_emails_with_primary_email(previous_confirmed_at, previous_email)
update_invalid_gpg_signatures update_invalid_gpg_signatures
if previous_email == notification_email if previous_email == notification_email
...@@ -756,15 +757,15 @@ class User < ApplicationRecord ...@@ -756,15 +757,15 @@ class User < ApplicationRecord
end end
def owns_notification_email def owns_notification_email
return if temp_oauth_email? return if new_record? || temp_oauth_email?
errors.add(:notification_email, _("is not an email you own")) unless all_emails.include?(notification_email) errors.add(:notification_email, _("is not an email you own")) unless verified_emails.include?(notification_email)
end end
def owns_public_email def owns_public_email
return if public_email.blank? return if public_email.blank?
errors.add(:public_email, _("is not an email you own")) unless all_emails.include?(public_email) errors.add(:public_email, _("is not an email you own")) unless verified_emails.include?(public_email)
end end
def owns_commit_email def owns_commit_email
...@@ -812,13 +813,15 @@ class User < ApplicationRecord ...@@ -812,13 +813,15 @@ class User < ApplicationRecord
# By using an `after_commit` instead of `after_update`, we avoid the recursive callback # By using an `after_commit` instead of `after_update`, we avoid the recursive callback
# scenario, though it then requires us to use the `previous_changes` hash # scenario, though it then requires us to use the `previous_changes` hash
# rubocop: disable CodeReuse/ServiceClass # rubocop: disable CodeReuse/ServiceClass
def update_emails_with_primary_email(previous_email) def update_emails_with_primary_email(previous_confirmed_at, previous_email)
primary_email_record = emails.find_by(email: email) primary_email_record = emails.find_by(email: email)
Emails::DestroyService.new(self, user: self).execute(primary_email_record) if primary_email_record Emails::DestroyService.new(self, user: self).execute(primary_email_record) if primary_email_record
# the original primary email was confirmed, and we want that to carry over. We don't # the original primary email was confirmed, and we want that to carry over. We don't
# have access to the original confirmation values at this point, so just set confirmed_at # have access to the original confirmation values at this point, so just set confirmed_at
Emails::CreateService.new(self, user: self, email: previous_email).execute(confirmed_at: confirmed_at) Emails::CreateService.new(self, user: self, email: previous_email).execute(confirmed_at: previous_confirmed_at)
update_columns(confirmed_at: primary_email_record.confirmed_at) if primary_email_record&.confirmed_at
end end
# rubocop: enable CodeReuse/ServiceClass # rubocop: enable CodeReuse/ServiceClass
...@@ -1202,18 +1205,20 @@ class User < ApplicationRecord ...@@ -1202,18 +1205,20 @@ class User < ApplicationRecord
all_emails all_emails
end end
def all_public_emails def verified_emails(include_private_email: true)
all_emails(include_private_email: false)
end
def verified_emails
verified_emails = [] verified_emails = []
verified_emails << email if primary_email_verified? verified_emails << email if primary_email_verified?
verified_emails << private_commit_email verified_emails << private_commit_email if include_private_email
verified_emails.concat(emails.confirmed.pluck(:email)) verified_emails.concat(emails.confirmed.pluck(:email))
verified_emails verified_emails
end end
def public_verified_emails
emails = verified_emails(include_private_email: false)
emails << email unless temp_oauth_email?
emails.uniq
end
def any_email?(check_email) def any_email?(check_email)
downcased = check_email.downcase downcased = check_email.downcase
......
...@@ -10,6 +10,12 @@ module Clusters ...@@ -10,6 +10,12 @@ module Clusters
def execute(cluster) def execute(cluster)
if validate_params(cluster) if validate_params(cluster)
token = params.dig(:platform_kubernetes_attributes, :token)
if token.blank?
params[:platform_kubernetes_attributes]&.delete(:token)
end
cluster.update(params) cluster.update(params)
else else
false false
......
...@@ -26,6 +26,6 @@ ...@@ -26,6 +26,6 @@
= f.text_field :eks_access_key_id, class: 'form-control' = f.text_field :eks_access_key_id, class: 'form-control'
.form-group .form-group
= f.label :eks_secret_access_key, 'Secret access key', class: 'label-bold' = f.label :eks_secret_access_key, 'Secret access key', class: 'label-bold'
= f.password_field :eks_secret_access_key, value: @application_setting.eks_secret_access_key, class: 'form-control' = f.password_field :eks_secret_access_key, autocomplete: 'off', class: 'form-control'
= f.submit 'Save changes', class: "btn btn-success" = f.submit 'Save changes', class: "btn btn-success"
...@@ -25,16 +25,10 @@ ...@@ -25,16 +25,10 @@
label: s_('ClusterIntegration|CA Certificate'), label_class: 'label-bold', label: s_('ClusterIntegration|CA Certificate'), label_class: 'label-bold',
input_group_class: 'gl-field-error-anchor', append: copy_ca_cert_btn input_group_class: 'gl-field-error-anchor', append: copy_ca_cert_btn
- show_token_btn = (platform_field.button s_('ClusterIntegration|Show'), = platform_field.password_field :token, type: 'password', class: 'js-select-on-focus js-cluster-token',
type: 'button', class: 'js-show-cluster-token btn btn-default') readonly: cluster.read_only_kubernetes_platform_fields?, autocomplete: 'new-password',
- copy_token_btn = clipboard_button(text: platform.token, title: s_('ClusterIntegration|Copy Service Token'), label: s_('ClusterIntegration|Enter new Service Token'), label_class: 'label-bold',
class: 'input-group-text btn-default') if cluster.read_only_kubernetes_platform_fields? input_group_class: 'gl-field-error-anchor'
= platform_field.text_field :token, type: 'password', class: 'js-select-on-focus js-cluster-token',
required: true, title: s_('ClusterIntegration|Service token is required.'),
readonly: cluster.read_only_kubernetes_platform_fields?,
label: s_('ClusterIntegration|Service Token'), label_class: 'label-bold',
input_group_class: 'gl-field-error-anchor', append: show_token_btn + copy_token_btn
= platform_field.form_group :authorization_type do = platform_field.form_group :authorization_type do
= platform_field.check_box :authorization_type, { disabled: true, label: s_('ClusterIntegration|RBAC-enabled cluster'), = platform_field.check_box :authorization_type, { disabled: true, label: s_('ClusterIntegration|RBAC-enabled cluster'),
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
- help_text = email_change_disabled ? s_("Your account uses dedicated credentials for the \"%{group_name}\" group and can only be updated through SSO.") % { group_name: @user.managing_group.name } : read_only_help_text - help_text = email_change_disabled ? s_("Your account uses dedicated credentials for the \"%{group_name}\" group and can only be updated through SSO.") % { group_name: @user.managing_group.name } : read_only_help_text
= form.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), help: help_text.html_safe, readonly: readonly || email_change_disabled = form.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), help: help_text.html_safe, readonly: readonly || email_change_disabled
= form.select :public_email, options_for_select(@user.all_public_emails, selected: @user.public_email), = form.select :public_email, options_for_select(@user.public_verified_emails, selected: @user.public_email),
{ help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") },
control_class: 'select2 input-lg', disabled: email_change_disabled control_class: 'select2 input-lg', disabled: email_change_disabled
- commit_email_link_url = help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') - commit_email_link_url = help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank')
......
- form = local_assigns.fetch(:form) - form = local_assigns.fetch(:form)
.form-group .form-group
= form.label :notification_email, class: "label-bold" = form.label :notification_email, class: "label-bold"
= form.select :notification_email, @user.all_public_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil) = form.select :notification_email, @user.public_verified_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil)
.help-block .help-block
= local_assigns.fetch(:help_text, nil) = local_assigns.fetch(:help_text, nil)
...@@ -13,4 +13,4 @@ ...@@ -13,4 +13,4 @@
.table-section.section-30 .table-section.section-30
= form_for setting, url: profile_notifications_group_path(group), method: :put, html: { class: 'update-notifications' } do |f| = form_for setting, url: profile_notifications_group_path(group), method: :put, html: { class: 'update-notifications' } do |f|
= f.select :notification_email, @user.all_public_emails, { include_blank: 'Global notification email' }, class: 'select2 js-group-notification-email' = f.select :notification_email, @user.public_verified_emails, { include_blank: 'Global notification email' }, class: 'select2 js-group-notification-email'
- page_title 'Edit Deploy Key' - page_title 'Edit Deploy Key'
%h3.page-title Edit Deploy Key %h3.page-title= _('Edit Deploy Key')
%hr %hr
%div %div
= form_for [@project.namespace.becomes(Namespace), @project, @deploy_key], html: { class: 'js-requires-input' } do |f| = form_for [@project.namespace.becomes(Namespace), @project, @deploy_key], include_id: false, html: { class: 'js-requires-input' } do |f|
= render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key } = render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key }
.form-actions .form-actions
= f.submit 'Save changes', class: 'btn-success btn' = f.submit 'Save changes', class: 'btn-success btn'
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
.float-left.btn-group.prepend-left-10.issuable-close-dropdown.droplab-dropdown.js-issuable-close-dropdown .float-left.btn-group.prepend-left-10.issuable-close-dropdown.droplab-dropdown.js-issuable-close-dropdown
= link_to "#{display_button_action} #{display_issuable_type}", close_reopen_issuable_path(issuable), = link_to "#{display_button_action} #{display_issuable_type}", close_reopen_issuable_path(issuable),
method: button_method, class: "#{button_class} btn-#{button_action} #{(add_blocked_class ? 'btn-issue-blocked' : '')}", title: "#{display_button_action} #{display_issuable_type}", data: { qa_selector: 'close_issue_button' } method: button_method, class: "#{button_class} btn-#{button_action} #{(add_blocked_class ? 'btn-issue-blocked' : '')}", title: "#{display_button_action} #{display_issuable_type}", data: { qa_selector: 'close_issue_button', 'close-reopen-url': close_reopen_issuable_path(issuable) }
= button_tag type: 'button', class: "#{toggle_class} btn-#{button_action}-color", = button_tag type: 'button', class: "#{toggle_class} btn-#{button_action}-color",
data: { 'dropdown-trigger' => '#issuable-close-menu' }, 'aria-label' => _('Toggle dropdown') do data: { 'dropdown-trigger' => '#issuable-close-menu' }, 'aria-label' => _('Toggle dropdown') do
......
...@@ -36,6 +36,7 @@ en: ...@@ -36,6 +36,7 @@ en:
access_denied: 'The resource owner or authorization server denied the request.' access_denied: 'The resource owner or authorization server denied the request.'
invalid_scope: 'The requested scope is invalid, unknown, or malformed.' invalid_scope: 'The requested scope is invalid, unknown, or malformed.'
server_error: 'The authorization server encountered an unexpected condition which prevented it from fulfilling the request.' server_error: 'The authorization server encountered an unexpected condition which prevented it from fulfilling the request.'
unconfirmed_email: 'Verify the email address in your account profile before you sign in.'
temporarily_unavailable: 'The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server.' temporarily_unavailable: 'The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server.'
#configuration error messages #configuration error messages
......
...@@ -1204,7 +1204,7 @@ PUT /projects/:id ...@@ -1204,7 +1204,7 @@ PUT /projects/:id
| `approvals_before_merge` | integer | no | **(STARTER)** How many approvers should approve merge request by default | | `approvals_before_merge` | integer | no | **(STARTER)** How many approvers should approve merge request by default |
| `external_authorization_classification_label` | string | no | **(PREMIUM)** The classification label for the project | | `external_authorization_classification_label` | string | no | **(PREMIUM)** The classification label for the project |
| `mirror` | boolean | no | **(STARTER)** Enables pull mirroring in a project | | `mirror` | boolean | no | **(STARTER)** Enables pull mirroring in a project |
| `mirror_user_id` | integer | no | **(STARTER)** User responsible for all the activity surrounding a pull mirror event | | `mirror_user_id` | integer | no | **(STARTER)** User responsible for all the activity surrounding a pull mirror event. Can only be set by admins. |
| `mirror_trigger_builds` | boolean | no | **(STARTER)** Pull mirroring triggers builds | | `mirror_trigger_builds` | boolean | no | **(STARTER)** Pull mirroring triggers builds |
| `only_mirror_protected_branches` | boolean | no | **(STARTER)** Only mirror protected branches | | `only_mirror_protected_branches` | boolean | no | **(STARTER)** Only mirror protected branches |
| `mirror_overwrites_diverged_branches` | boolean | no | **(STARTER)** Pull mirror overwrites diverged branches | | `mirror_overwrites_diverged_branches` | boolean | no | **(STARTER)** Pull mirror overwrites diverged branches |
......
# frozen_string_literal: true
module SafeMirrorParams
extend ActiveSupport::Concern
included do
helper_method :default_mirror_users
end
private
def valid_mirror_user?(mirror_params)
return true unless mirror_params[:mirror_user_id].present?
default_mirror_users.map(&:id).include?(mirror_params[:mirror_user_id].to_i)
end
def default_mirror_users
[current_user, project.mirror_user].compact.uniq
end
end
...@@ -6,23 +6,16 @@ module EE ...@@ -6,23 +6,16 @@ module EE
extend ActiveSupport::Concern extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
prepended do
include SafeMirrorParams
end
private private
override :import_params_attributes override :import_params_attributes
def import_params_attributes def import_params_attributes
super + [:mirror, :mirror_user_id] super + [:mirror]
end end
override :import_params override :import_params
def import_params def import_params
base_import_params = super super.merge(mirror_user_id: current_user&.id)
return base_import_params if valid_mirror_user?(base_import_params)
base_import_params.merge(mirror_user_id: current_user.id)
end end
end end
end end
......
...@@ -59,16 +59,10 @@ module EE ...@@ -59,16 +59,10 @@ module EE
private private
def mirror_params_attributes_ee def mirror_params_attributes_ee
[ attrs = Projects::UpdateService::PULL_MIRROR_ATTRIBUTES.dup
:mirror, attrs.delete(:mirror_user_id) # Cannot be set by the frontend
:import_url, attrs.delete(:import_data_attributes) # We need more detail here
:username_only_import_url, attrs.push(
:mirror_user_id,
:mirror_trigger_builds,
:only_mirror_protected_branches,
:mirror_overwrites_diverged_branches,
:pull_mirror_branch_prefix,
import_data_attributes: %i[ import_data_attributes: %i[
id id
auth_method auth_method
...@@ -76,7 +70,7 @@ module EE ...@@ -76,7 +70,7 @@ module EE
ssh_known_hosts ssh_known_hosts
regenerate_ssh_private_key regenerate_ssh_private_key
] ]
] )
end end
def safe_mirror_params def safe_mirror_params
......
...@@ -7,8 +7,6 @@ module EE ...@@ -7,8 +7,6 @@ module EE
extend ActiveSupport::Concern extend ActiveSupport::Concern
prepended do prepended do
include SafeMirrorParams
before_action :push_rule, only: [:show, :create_deploy_token] before_action :push_rule, only: [:show, :create_deploy_token]
end end
......
...@@ -106,7 +106,6 @@ module EE ...@@ -106,7 +106,6 @@ module EE
%i[ %i[
mirror mirror
mirror_trigger_builds mirror_trigger_builds
mirror_user_id
] ]
end end
......
...@@ -9,7 +9,6 @@ class TrialRegistrationsController < RegistrationsController ...@@ -9,7 +9,6 @@ class TrialRegistrationsController < RegistrationsController
before_action :check_if_gl_com before_action :check_if_gl_com
before_action :set_redirect_url, only: [:new] before_action :set_redirect_url, only: [:new]
before_action :skip_confirmation, only: [:create]
def new def new
end end
...@@ -26,10 +25,6 @@ class TrialRegistrationsController < RegistrationsController ...@@ -26,10 +25,6 @@ class TrialRegistrationsController < RegistrationsController
end end
end end
def skip_confirmation
params[:user][:skip_confirmation] = true
end
override :sign_up_params override :sign_up_params
def sign_up_params def sign_up_params
if params[:user] if params[:user]
......
...@@ -29,7 +29,7 @@ module EE ...@@ -29,7 +29,7 @@ module EE
end end
def options_for_mirror_user def options_for_mirror_user
options_from_collection_for_select(default_mirror_users, :id, :name, @project.mirror_user_id || current_user.id) options_from_collection_for_select([current_user], :id, :name, current_user.id)
end end
def mirrored_repositories_count(project = @project) def mirrored_repositories_count(project = @project)
......
...@@ -36,7 +36,23 @@ module EE ...@@ -36,7 +36,23 @@ module EE
end end
def group_domain_limitations def group_domain_limitations
user ? validate_users_email : validate_invitation_email if user
validate_users_email
validate_email_verified
else
validate_invitation_email
end
end
def validate_email_verified
return if user.primary_email_verified?
# Do not validate if emails are verified
# for users created via SAML/SCIM.
return if group_saml_identity.present?
return if source.scim_identities.for_user(user).exists?
errors.add(:user, email_not_verified)
end end
def validate_users_email def validate_users_email
...@@ -67,6 +83,10 @@ module EE ...@@ -67,6 +83,10 @@ module EE
_("email '%{email}' does not match the allowed domain of '%{email_domain}'" % { email: email, email_domain: group_allowed_email_domain.domain }) _("email '%{email}' does not match the allowed domain of '%{email_domain}'" % { email: email, email_domain: group_allowed_email_domain.domain })
end end
def email_not_verified
_("email '%{email}' is not a verified email." % { email: user.email })
end
def group_allowed_email_domain def group_allowed_email_domain
group.root_ancestor_allowed_email_domain group.root_ancestor_allowed_email_domain
end end
......
...@@ -6,19 +6,27 @@ module EE ...@@ -6,19 +6,27 @@ module EE
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
include CleanupApprovers include CleanupApprovers
PULL_MIRROR_ATTRIBUTES = %i[
mirror
mirror_user_id
import_url
username_only_import_url
mirror_trigger_builds
only_mirror_protected_branches
mirror_overwrites_diverged_branches
pull_mirror_branch_prefix
import_data_attributes
].freeze
override :execute override :execute
def execute def execute
should_remove_old_approvers = params.delete(:remove_old_approvers) should_remove_old_approvers = params.delete(:remove_old_approvers)
wiki_was_enabled = project.wiki_enabled?
limit = params.delete(:repository_size_limit) limit = params.delete(:repository_size_limit)
wiki_was_enabled = project.wiki_enabled?
unless valid_mirror_user? mirror_user_setting
project.errors.add(:mirror_user_id, 'is invalid')
return project
end
compliance_framework_setting compliance_framework_setting
return update_failed! if project.errors.any?
result = super do result = super do
# Repository size limit comes as MB from the view # Repository size limit comes as MB from the view
...@@ -41,13 +49,19 @@ module EE ...@@ -41,13 +49,19 @@ module EE
private private
def valid_mirror_user? # A user who changes any aspect of pull mirroring settings must be made
return true unless params[:mirror_user_id].present? # into the mirror user, to prevent them from acquiring capabilities
# owned by the previous user, such as writing to a protected branch.
mirror_user_id = params[:mirror_user_id].to_i #
# Only admins can set the mirror user to be an arbitrary user.
mirror_user_id == current_user.id || def mirror_user_setting
mirror_user_id == project.mirror_user&.id return unless PULL_MIRROR_ATTRIBUTES.any? { |symbol| params.key?(symbol) }
if params[:mirror_user_id] && params[:mirror_user_id] != project.mirror_user_id
project.errors.add(:mirror_user_id, 'is invalid') unless current_user&.admin?
else
params[:mirror_user_id] = current_user.id
end
end end
def compliance_framework_setting def compliance_framework_setting
......
...@@ -10,6 +10,6 @@ ...@@ -10,6 +10,6 @@
= _('This issue is currently blocked by the following issues: %{issues}.').html_safe % { issues: blocked_by_issues_links } = _('This issue is currently blocked by the following issues: %{issues}.').html_safe % { issues: blocked_by_issues_links }
.gl-alert-actions .gl-alert-actions
= link_to _("Yes, close issue"), close_issuable_path(issue), rel: 'nofollow', method: '', = link_to _("Yes, close issue"), close_issuable_path(issue), rel: 'nofollow', method: '',
class: "btn btn-close-anyway gl-alert-action btn-warning btn-md gl-button", title: _("Yes, close issue") class: "btn btn-close-anyway gl-alert-action btn-warning btn-md gl-button", title: _("Yes, close issue"), 'data-close-reopen-url' => close_issuable_path(issue)
%button.btn.gl-alert-action.btn-warning.btn-md.gl-button.btn-secondary %button.btn.gl-alert-action.btn-warning.btn-md.gl-button.btn-secondary
= s_('Cancel') = s_('Cancel')
- import_data = @project.import_data || @project.build_import_data - import_data = @project.import_data || @project.build_import_data
- is_one_user_option = default_mirror_users.count == 1
- protocols = Gitlab::UrlSanitizer::ALLOWED_SCHEMES.join('|') - protocols = Gitlab::UrlSanitizer::ALLOWED_SCHEMES.join('|')
- direction_options = [[_('Push'), 'push']] - direction_options = [[_('Push'), 'push']]
- has_existing_pull_mirror = @project.mirror.present? - has_existing_pull_mirror = @project.mirror.present?
...@@ -29,15 +28,10 @@ ...@@ -29,15 +28,10 @@
= render partial: 'projects/mirrors/authentication_method', locals: { f: import_form } = render partial: 'projects/mirrors/authentication_method', locals: { f: import_form }
.form-group .form-group
= f.label :mirror_user_id, _('Mirror user'), class: 'label-light' = f.label :mirror_user_id_select, _('Mirror user'), class: 'label-light'
- if is_one_user_option = select_tag(:mirror_user_id_select, options_for_mirror_user, class: 'js-mirror-user select2 lg append-bottom-5', required: true, disabled: true)
= select_tag(:mirror_user_id_select, options_for_mirror_user, class: 'js-mirror-user select2 lg append-bottom-5', required: true, disabled: true)
= f.hidden_field :mirror_user_id, value: default_mirror_users.first.id if is_one_user_option
- else
= f.select(:mirror_user_id, options_for_mirror_user, {}, class: 'js-mirror-user select2 lg append-bottom-5', required: true)
.help-block .help-block
= _('This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches. Upon creation or when reassigning you can only assign yourself to be the mirror user.') = _('You will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches.')
- if Feature.enabled?(:pull_mirror_branch_prefix, @project) - if Feature.enabled?(:pull_mirror_branch_prefix, @project)
.form-group .form-group
......
---
title: Ensure passwords and access tokens don't appear in SCIM errors
merge_request:
author:
type: security
...@@ -43,6 +43,11 @@ module API ...@@ -43,6 +43,11 @@ module API
unauthorized! unless token && ScimOauthAccessToken.token_matches_for_group?(token, group) unauthorized! unless token && ScimOauthAccessToken.token_matches_for_group?(token, group)
end end
def sanitize_request_parameters(parameters)
filter = ActiveSupport::ParameterFilter.new(::Rails.application.config.filter_parameters)
filter.filter(parameters)
end
# Instance variable `@group` is necessary for the # Instance variable `@group` is necessary for the
# Gitlab::ApplicationContext in API::API # Gitlab::ApplicationContext in API::API
def find_and_authenticate_group!(group_path) def find_and_authenticate_group!(group_path)
...@@ -178,9 +183,9 @@ module API ...@@ -178,9 +183,9 @@ module API
present result.identity, with: ::EE::API::Entities::Scim::User present result.identity, with: ::EE::API::Entities::Scim::User
when :conflict when :conflict
scim_conflict!(message: "Error saving user with #{params.inspect}: #{result.message}") scim_conflict!(message: "Error saving user with #{sanitize_request_parameters(params).inspect}: #{result.message}")
when :error when :error
scim_error!(message: ["Error saving user with #{params.inspect}", result.message].compact.join(": ")) scim_error!(message: ["Error saving user with #{sanitize_request_parameters(params).inspect}", result.message].compact.join(": "))
end end
end end
...@@ -200,7 +205,7 @@ module API ...@@ -200,7 +205,7 @@ module API
if updated if updated
no_content! no_content!
else else
scim_error!(message: "Error updating #{identity.user.name} with #{params.inspect}") scim_error!(message: "Error updating #{identity.user.name} with #{sanitize_request_parameters(params).inspect}")
end end
end end
......
...@@ -29,7 +29,7 @@ module EE ...@@ -29,7 +29,7 @@ module EE
end end
params :optional_update_params_ee do params :optional_update_params_ee do
optional :mirror_user_id, type: Integer, desc: 'User responsible for all the activity surrounding a pull mirror event' optional :mirror_user_id, type: Integer, desc: 'User responsible for all the activity surrounding a pull mirror event. Can only be set by admins'
optional :only_mirror_protected_branches, type: Grape::API::Boolean, desc: 'Only mirror protected branches' optional :only_mirror_protected_branches, type: Grape::API::Boolean, desc: 'Only mirror protected branches'
optional :mirror_overwrites_diverged_branches, type: Grape::API::Boolean, desc: 'Pull mirror overwrites diverged branches' optional :mirror_overwrites_diverged_branches, type: Grape::API::Boolean, desc: 'Pull mirror overwrites diverged branches'
optional :import_url, type: String, desc: 'URL from which the project is imported' optional :import_url, type: String, desc: 'URL from which the project is imported'
......
...@@ -43,12 +43,9 @@ module EE ...@@ -43,12 +43,9 @@ module EE
def verify_mirror_attrs!(project, attrs) def verify_mirror_attrs!(project, attrs)
unless can?(current_user, :admin_mirror, project) unless can?(current_user, :admin_mirror, project)
attrs.delete(:mirror) ::Projects::UpdateService::PULL_MIRROR_ATTRIBUTES.each do |attr_name|
attrs.delete(:mirror_user_id) attrs.delete(attr_name)
attrs.delete(:mirror_trigger_builds) end
attrs.delete(:only_mirror_protected_branches)
attrs.delete(:mirror_overwrites_diverged_branches)
attrs.delete(:import_data_attributes)
end end
end end
......
...@@ -76,4 +76,134 @@ describe Groups::GroupMembersController do ...@@ -76,4 +76,134 @@ describe Groups::GroupMembersController do
end end
end end
end end
describe 'POST request_access' do
before do
create(:allowed_email_domain, group: group)
sign_in(requesting_user)
end
shared_examples_for 'creates a new access request' do
it 'creates a new access request to the group' do
post :request_access, params: { group_id: group }
expect(response).to set_flash.to 'Your request for access has been queued for review.'
expect(response).to redirect_to(group_path(group))
expect(group.requesters.exists?(user_id: requesting_user)).to be_truthy
expect(group.users).not_to include requesting_user
end
end
shared_examples_for 'creates access request for a verified user with email belonging to the allowed domain' do
context 'for a user with a verified email belonging to the allowed domain' do
let(:email) { 'verified@gitlab.com' }
let(:requesting_user) { create(:user, email: email, confirmed_at: Time.current) }
it_behaves_like 'creates a new access request'
end
end
context 'when users with unconfirmed emails are allowed to log-in' do
before do
stub_feature_flags(soft_email_confirmation: true)
end
context 'when group has email domain feature enabled' do
before do
stub_licensed_features(group_allowed_email_domains: true)
end
context 'for a user with an un-verified email belonging to the allowed domain' do
let(:email) { 'unverified@gitlab.com' }
let(:requesting_user) { create(:user, email: email, confirmed_at: nil) }
it 'does not create a new access request' do
post :request_access, params: { group_id: group }
expect(response).to set_flash.to "Your request for access could not be processed: "\
"User email 'unverified@gitlab.com' is not a verified email."
expect(response).to redirect_to(group_path(group))
expect(group.requesters.exists?(user_id: requesting_user)).to be_falsey
expect(group.users).not_to include requesting_user
end
end
it_behaves_like 'creates access request for a verified user with email belonging to the allowed domain'
end
context 'when group has email domain feature disabled' do
let(:email) { 'unverified@gitlab.com' }
let(:requesting_user) { create(:user, email: email, confirmed_at: nil) }
before do
stub_licensed_features(group_allowed_email_domains: false)
end
context 'for a user with an un-verified email belonging to the allowed domain' do
it_behaves_like 'creates a new access request'
end
context 'for a user with an un-verified email belonging to a domain different from the allowed domain' do
let(:email) { 'unverified@gmail.com' }
it_behaves_like 'creates a new access request'
end
it_behaves_like 'creates access request for a verified user with email belonging to the allowed domain'
end
end
context 'when users with unconfirmed emails are not allowed to log-in' do
before do
stub_feature_flags(soft_email_confirmation: false)
end
shared_examples_for 'does not create a new access request due to user pending confirmation' do
it 'does not create a new access request due to user pending confirmation' do
post :request_access, params: { group_id: group }
expect(response).to redirect_to(new_user_session_path)
expect(response).to set_flash.to 'You have to confirm your email address before continuing.'
expect(group.requesters.exists?(user_id: requesting_user)).to be_falsey
expect(group.users).not_to include requesting_user
end
end
context 'when group has email domain feature enabled' do
before do
stub_licensed_features(group_allowed_email_domains: true)
end
context 'for a user with an un-verified email belonging to the allowed domain' do
let(:email) { 'unverified@gitlab.com' }
let(:requesting_user) { create(:user, email: email, confirmed_at: nil) }
it_behaves_like 'does not create a new access request due to user pending confirmation'
end
it_behaves_like 'creates access request for a verified user with email belonging to the allowed domain'
end
context 'when group has email domain feature disabled' do
let(:email) { 'unverified@gitlab.com' }
let(:requesting_user) { create(:user, email: email, confirmed_at: nil) }
before do
stub_licensed_features(group_allowed_email_domains: false)
end
context 'for a user with an un-verified email belonging to the allowed domain' do
it_behaves_like 'does not create a new access request due to user pending confirmation'
end
context 'for a user with an un-verified email belonging to a domain different from the allowed domain' do
let(:email) { 'unverified@gmail.com' }
it_behaves_like 'does not create a new access request due to user pending confirmation'
end
it_behaves_like 'creates access request for a verified user with email belonging to the allowed domain'
end
end
end
end end
...@@ -30,33 +30,29 @@ describe Projects::MirrorsController do ...@@ -30,33 +30,29 @@ describe Projects::MirrorsController do
sign_in(project.owner) sign_in(project.owner)
end end
context 'when trying to create a mirror with the same URL' do context 'mirror_user is unset' do
it 'does not set up the mirror' do it 'sets up a pull mirror with the mirror user set to the signed-in user' do
do_put(project, mirror: true, import_url: remote_mirror.url) expect(project.mirror_user).to be_nil
expect(project.reload.mirror).to be_falsey do_put(project, mirror: true, import_url: 'http://local.dev')
expect(project.reload.import_url).to be_blank project.reload
end
end
context 'when trying to create a mirror with a different URL' do
it 'sets up the mirror' do
do_put(project, mirror: true, mirror_user_id: project.owner.id, import_url: 'http://local.dev')
expect(project.reload.mirror).to eq(true) expect(project.mirror).to eq(true)
expect(project.reload.import_url).to eq('http://local.dev') expect(project.import_url).to eq('http://local.dev')
expect(project.mirror_user).to eq(project.owner)
end end
end
context 'mirror user is not the current user' do context 'mirror_user is not the current user' do
it 'does not set up the mirror' do it 'sets up a pull mirror with the mirror user set to the signed-in user' do
new_user = create(:user) new_user = create(:user)
project.add_maintainer(new_user) project.add_maintainer(new_user)
do_put(project, mirror: true, mirror_user_id: new_user.id, import_url: 'http://local.dev') do_put(project, mirror: true, mirror_user_id: new_user.id, import_url: 'http://local.dev')
expect(project.reload.mirror).to be_falsey expect(project.mirror).to eq(true)
expect(project.reload.import_url).to be_blank expect(project.import_url).to eq('http://local.dev')
end expect(project.mirror_user).to eq(project.owner)
end end
end end
end end
...@@ -78,7 +74,7 @@ describe Projects::MirrorsController do ...@@ -78,7 +74,7 @@ describe Projects::MirrorsController do
sign_in(admin) sign_in(admin)
expect do expect do
do_put(project, mirror: true, mirror_user_id: admin.id, import_url: url) do_put(project, mirror: true, import_url: url)
end.to change { Project.mirror.count }.to(1) end.to change { Project.mirror.count }.to(1)
end end
end end
...@@ -88,7 +84,7 @@ describe Projects::MirrorsController do ...@@ -88,7 +84,7 @@ describe Projects::MirrorsController do
sign_in(project.owner) sign_in(project.owner)
expect do expect do
do_put(project, mirror: true, mirror_user_id: project.owner.id, import_url: url) do_put(project, mirror: true, import_url: url)
end.not_to change { Project.mirror.count } end.not_to change { Project.mirror.count }
end end
end end
...@@ -194,8 +190,7 @@ describe Projects::MirrorsController do ...@@ -194,8 +190,7 @@ describe Projects::MirrorsController do
do_put(project, { mirror_user_id: other_user.id }, format: :json) do_put(project, { mirror_user_id: other_user.id }, format: :json)
expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(project.reload.mirror_user).to eq(project.owner)
expect(json_response['mirror_user_id'].first).to eq("is invalid")
end end
end end
......
...@@ -271,7 +271,6 @@ describe ProjectsController do ...@@ -271,7 +271,6 @@ describe ProjectsController do
{ {
mirror: true, mirror: true,
mirror_trigger_builds: true, mirror_trigger_builds: true,
mirror_user_id: user.id,
import_url: 'https://example.com' import_url: 'https://example.com'
} }
end end
...@@ -297,6 +296,20 @@ describe ProjectsController do ...@@ -297,6 +296,20 @@ describe ProjectsController do
expect(project.mirror_user).to eq(user) expect(project.mirror_user).to eq(user)
expect(project.import_url).to eq('https://example.com') expect(project.import_url).to eq('https://example.com')
end end
it 'ignores mirror_user_id' do
other_user = create(:user)
put :update,
params: {
namespace_id: project.namespace,
id: project,
project: params.merge(mirror_user_id: other_user.id)
}
project.reload
expect(project.mirror_user).to eq(user)
end
end end
context 'when unlicensed' do context 'when unlicensed' do
......
...@@ -71,10 +71,10 @@ describe TrialRegistrationsController do ...@@ -71,10 +71,10 @@ describe TrialRegistrationsController do
allow(Gitlab).to receive(:com?).and_return(true) allow(Gitlab).to receive(:com?).and_return(true)
end end
it 'marks the account as confirmed' do it 'marks the account as unconfirmed' do
post :create, params: { user: user_params } post :create, params: { user: user_params }
expect(User.last).to be_confirmed expect(User.last).not_to be_confirmed
end end
context 'derivation of name' do context 'derivation of name' do
......
# frozen_string_literal: true
require 'spec_helper'
describe 'JIRA OAuth Provider' do
describe 'JIRA DVCS OAuth Authorization' do
let(:application) { create(:oauth_application, redirect_uri: oauth_jira_callback_url, scopes: 'read_user') }
before do
sign_in(user)
visit oauth_jira_authorize_path(client_id: application.uid,
redirect_uri: oauth_jira_callback_url,
response_type: 'code',
state: 'my_state',
scope: 'read_user')
end
it_behaves_like 'Secure OAuth Authorizations'
end
end
...@@ -23,7 +23,7 @@ describe 'Project settings > [EE] repository' do ...@@ -23,7 +23,7 @@ describe 'Project settings > [EE] repository' do
expect(page).to have_selector('#url') expect(page).to have_selector('#url')
expect(page).to have_selector('#mirror_direction') expect(page).to have_selector('#mirror_direction')
expect(page).to have_no_selector('#project_mirror', visible: false) expect(page).to have_no_selector('#project_mirror', visible: false)
expect(page).to have_no_selector('#project_mirror_user_id', visible: false) expect(page).to have_no_selector('#mirror_user_id_select', visible: false)
expect(page).to have_no_selector('#project_mirror_overwrites_diverged_branches') expect(page).to have_no_selector('#project_mirror_overwrites_diverged_branches')
expect(page).to have_no_selector('#project_mirror_trigger_builds') expect(page).to have_no_selector('#project_mirror_trigger_builds')
expect(page).to have_no_selector('#project_pull_mirror_branch_prefix') expect(page).to have_no_selector('#project_pull_mirror_branch_prefix')
...@@ -43,7 +43,7 @@ describe 'Project settings > [EE] repository' do ...@@ -43,7 +43,7 @@ describe 'Project settings > [EE] repository' do
expect(page).to have_selector('#url') expect(page).to have_selector('#url')
expect(page).to have_selector('#mirror_direction') expect(page).to have_selector('#mirror_direction')
expect(page).to have_selector('#project_mirror', visible: false) expect(page).to have_selector('#project_mirror', visible: false)
expect(page).to have_selector('#project_mirror_user_id', visible: false) expect(page).to have_selector('#mirror_user_id_select', visible: false)
expect(page).to have_selector('#project_mirror_overwrites_diverged_branches') expect(page).to have_selector('#project_mirror_overwrites_diverged_branches')
expect(page).to have_selector('#project_mirror_trigger_builds') expect(page).to have_selector('#project_mirror_trigger_builds')
expect(page).to have_selector('#project_pull_mirror_branch_prefix') expect(page).to have_selector('#project_pull_mirror_branch_prefix')
......
...@@ -92,7 +92,7 @@ describe Gitlab::CodeOwners::Entry do ...@@ -92,7 +92,7 @@ describe Gitlab::CodeOwners::Entry do
it 'only adds users mentioned in the owner line' do it 'only adds users mentioned in the owner line' do
other_user = create(:user) other_user = create(:user)
other_user.emails other_user.emails.load
entry.add_matching_users_from([other_user, user]) entry.add_matching_users_from([other_user, user])
...@@ -109,7 +109,7 @@ describe Gitlab::CodeOwners::Entry do ...@@ -109,7 +109,7 @@ describe Gitlab::CodeOwners::Entry do
it 'adds users by primary email, case-insensitively' do it 'adds users by primary email, case-insensitively' do
second_user = create(:user, email: 'JANE@GITLAB.ORG') second_user = create(:user, email: 'JANE@GITLAB.ORG')
second_user.emails second_user.emails.load
entry.add_matching_users_from([second_user, user]) entry.add_matching_users_from([second_user, user])
...@@ -119,7 +119,7 @@ describe Gitlab::CodeOwners::Entry do ...@@ -119,7 +119,7 @@ describe Gitlab::CodeOwners::Entry do
it 'adds users by secondary email, case-insensitively' do it 'adds users by secondary email, case-insensitively' do
second_user = create(:user) second_user = create(:user)
second_user.emails.create!(email: 'JaNe@GitLab.org') second_user.emails.create!(email: 'JaNe@GitLab.org')
second_user.emails second_user.emails.load
entry.add_matching_users_from([second_user, user]) entry.add_matching_users_from([second_user, user])
......
...@@ -11,6 +11,7 @@ describe GroupMember do ...@@ -11,6 +11,7 @@ describe GroupMember do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:user) { create(:user, email: 'test@gitlab.com') } let(:user) { create(:user, email: 'test@gitlab.com') }
let(:user_2) { create(:user, email: 'test@gmail.com') } let(:user_2) { create(:user, email: 'test@gmail.com') }
let(:user_3) { create(:user, email: 'unverified@gitlab.com', confirmed_at: nil) }
before do before do
create(:allowed_email_domain, group: group) create(:allowed_email_domain, group: group)
...@@ -31,6 +32,35 @@ describe GroupMember do ...@@ -31,6 +32,35 @@ describe GroupMember do
expect(build(:group_member, group: group, user: nil, invite_email: 'user@gitlab.com')).to be_valid expect(build(:group_member, group: group, user: nil, invite_email: 'user@gitlab.com')).to be_valid
end end
it 'user emails matching allowed domain must be verified' do
group_member = build(:group_member, group: group, user: user_3)
expect(group_member).to be_invalid
expect(group_member.errors[:user]).to include("email 'unverified@gitlab.com' is not a verified email.")
end
context 'with group SAML users' do
let(:saml_provider) { create(:saml_provider, group: group) }
let!(:group_related_identity) do
create(:group_saml_identity, user: user_3, saml_provider: saml_provider)
end
it 'user emails does not have to be verified' do
expect(build(:group_member, group: group, user: user_3)).to be_valid
end
end
context 'with group SCIM users' do
let!(:scim_identity) do
create(:scim_identity, user: user_3, group: group)
end
it 'user emails does not have to be verified' do
expect(build(:group_member, group: group, user: user_3)).to be_valid
end
end
context 'when group is subgroup' do context 'when group is subgroup' do
let(:subgroup) { create(:group, parent: group) } let(:subgroup) { create(:group, parent: group) }
...@@ -43,6 +73,13 @@ describe GroupMember do ...@@ -43,6 +73,13 @@ describe GroupMember do
expect(build(:group_member, group: subgroup, user: nil, invite_email: 'user@gmail.com')).to be_invalid expect(build(:group_member, group: subgroup, user: nil, invite_email: 'user@gmail.com')).to be_invalid
expect(build(:group_member, group: subgroup, user: nil, invite_email: 'user@gitlab.com')).to be_valid expect(build(:group_member, group: subgroup, user: nil, invite_email: 'user@gitlab.com')).to be_valid
end end
it 'user emails matching allowed domain must be verified' do
group_member = build(:group_member, group: subgroup, user: user_3)
expect(group_member).to be_invalid
expect(group_member.errors[:user]).to include("email 'unverified@gitlab.com' is not a verified email.")
end
end end
end end
...@@ -56,6 +93,10 @@ describe GroupMember do ...@@ -56,6 +93,10 @@ describe GroupMember do
expect(build(:group_member, group: group, invite_email: 'user@gmail.com')).to be_valid expect(build(:group_member, group: group, invite_email: 'user@gmail.com')).to be_valid
expect(build(:group_member, group: group, invite_email: 'user@gitlab.com')).to be_valid expect(build(:group_member, group: group, invite_email: 'user@gitlab.com')).to be_valid
end end
it 'user emails does not have to be verified' do
expect(build(:group_member, group: group, user: user_3)).to be_valid
end
end end
end end
end end
......
...@@ -567,7 +567,6 @@ describe API::Projects do ...@@ -567,7 +567,6 @@ describe API::Projects do
{ {
mirror: true, mirror: true,
import_url: import_url, import_url: import_url,
mirror_user_id: user.id,
mirror_trigger_builds: true, mirror_trigger_builds: true,
only_mirror_protected_branches: true, only_mirror_protected_branches: true,
mirror_overwrites_diverged_branches: true mirror_overwrites_diverged_branches: true
...@@ -588,7 +587,9 @@ describe API::Projects do ...@@ -588,7 +587,9 @@ describe API::Projects do
it 'updates mirror related attributes when user is admin' do it 'updates mirror related attributes when user is admin' do
admin = create(:admin) admin = create(:admin)
mirror_params[:mirror_user_id] = admin.id unrelated_user = create(:user)
mirror_params[:mirror_user_id] = unrelated_user.id
project.add_maintainer(admin) project.add_maintainer(admin)
expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once
...@@ -599,7 +600,7 @@ describe API::Projects do ...@@ -599,7 +600,7 @@ describe API::Projects do
expect(project.reload).to have_attributes( expect(project.reload).to have_attributes(
mirror: true, mirror: true,
import_url: import_url, import_url: import_url,
mirror_user_id: admin.id, mirror_user_id: unrelated_user.id,
mirror_trigger_builds: true, mirror_trigger_builds: true,
only_mirror_protected_branches: true, only_mirror_protected_branches: true,
mirror_overwrites_diverged_branches: true mirror_overwrites_diverged_branches: true
......
...@@ -6,6 +6,9 @@ describe API::Scim do ...@@ -6,6 +6,9 @@ describe API::Scim do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:scim_token) { create(:scim_oauth_access_token, group: group) } let(:scim_token) { create(:scim_oauth_access_token, group: group) }
let_it_be(:password) { 'secret_pass' }
let_it_be(:access_token) { 'secret_token' }
before do before do
stub_licensed_features(group_allowed_email_domains: true, group_saml: true) stub_licensed_features(group_allowed_email_domains: true, group_saml: true)
...@@ -26,6 +29,16 @@ describe API::Scim do ...@@ -26,6 +29,16 @@ describe API::Scim do
end end
end end
shared_examples 'filtered params in errors' do
it 'does not expose the password in error response' do
expect(json_response.fetch('detail')).to include("\"password\"=>\"[FILTERED]\"")
end
it 'does not expose the access token in error response' do
expect(json_response.fetch('detail')).to include("\"access_token\"=>\"[FILTERED]\"")
end
end
shared_examples 'SCIM API endpoints' do shared_examples 'SCIM API endpoints' do
describe 'GET api/scim/v2/groups/:group/Users' do describe 'GET api/scim/v2/groups/:group/Users' do
it_behaves_like 'SCIM token authenticated' it_behaves_like 'SCIM token authenticated'
...@@ -102,10 +115,24 @@ describe API::Scim do ...@@ -102,10 +115,24 @@ describe API::Scim do
emails: [ emails: [
{ primary: true, type: 'work', value: 'work@example.com' } { primary: true, type: 'work', value: 'work@example.com' }
], ],
name: { formatted: 'Test Name', familyName: 'Name', givenName: 'Test' } name: { formatted: 'Test Name', familyName: 'Name', givenName: 'Test' },
access_token: access_token,
password: password
}.to_query }.to_query
end end
context 'when a provisioning error occurs' do
before do
allow_next_instance_of(::EE::Gitlab::Scim::ProvisioningService) do |instance|
allow(instance).to receive(:execute).and_return(::EE::Gitlab::Scim::ProvisioningResponse.new(status: :error))
end
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it_behaves_like 'filtered params in errors'
end
context 'without an existing user' do context 'without an existing user' do
let(:new_user) { User.find_by_email('work@example.com') } let(:new_user) { User.find_by_email('work@example.com') }
let(:member) { GroupMember.find_by(user: new_user, group: group) } let(:member) { GroupMember.find_by(user: new_user, group: group) }
...@@ -319,7 +346,9 @@ describe API::Scim do ...@@ -319,7 +346,9 @@ describe API::Scim do
active: nil, active: nil,
userName: 'username', userName: 'username',
emails: [{ primary: true, type: 'work', value: 'work@example.com' }], emails: [{ primary: true, type: 'work', value: 'work@example.com' }],
name: { formatted: 'Test Name', familyName: 'Name', givenName: 'Test' } name: { formatted: 'Test Name', familyName: 'Name', givenName: 'Test' },
access_token: access_token,
password: password
}.to_query }.to_query
end end
context 'without an existing user' do context 'without an existing user' do
...@@ -341,7 +370,7 @@ describe API::Scim do ...@@ -341,7 +370,7 @@ describe API::Scim do
end end
end end
context 'existing user' do context 'existing user with group saml identity' do
before do before do
old_user = create(:user, email: 'work@example.com') old_user = create(:user, email: 'work@example.com')
...@@ -359,6 +388,20 @@ describe API::Scim do ...@@ -359,6 +388,20 @@ describe API::Scim do
expect(json_response['id']).to eq('test_uid') expect(json_response['id']).to eq('test_uid')
end end
end end
context 'existing user without a group saml identity' do
before do
create(:user, email: 'work@example.com')
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'responds with 409' do
expect(response).to have_gitlab_http_status(:conflict)
end
it_behaves_like 'filtered params in errors'
end
end end
describe 'PATCH api/scim/v2/groups/:group/Users/:id' do describe 'PATCH api/scim/v2/groups/:group/Users/:id' do
......
...@@ -23,7 +23,6 @@ describe Projects::MirrorsController do ...@@ -23,7 +23,6 @@ describe Projects::MirrorsController do
project: { project: {
mirror: '1', mirror: '1',
import_url: '', import_url: '',
mirror_user_id: user.id,
mirror_trigger_builds: '0' mirror_trigger_builds: '0'
} }
} }
......
...@@ -6,26 +6,48 @@ describe Projects::UpdateService, '#execute' do ...@@ -6,26 +6,48 @@ describe Projects::UpdateService, '#execute' do
include EE::GeoHelpers include EE::GeoHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:admin) { create(:user, :admin) }
let(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } let(:project) { create(:project, :repository, creator: user, namespace: user.namespace) }
context 'repository mirror' do context 'repository mirror' do
let!(:opts) do let(:opts) { { mirror: true, import_url: 'http://foo.com' } }
{
}
end
before do before do
stub_licensed_features(repository_mirrors: true) stub_licensed_features(repository_mirrors: true)
end end
it 'forces an import job' do it 'sets mirror attributes' do
opts = { result = update_project(project, user, opts)
import_url: 'http://foo.com',
mirror: true,
mirror_user_id: user.id,
mirror_trigger_builds: true
}
expect(result).to eq(status: :success)
expect(project).to have_attributes(opts)
expect(project.mirror_user).to eq(user)
end
it 'does not touch mirror_user_id for non-mirror changes' do
result = update_project(project, user, description: 'anything')
expect(result).to eq(status: :success)
expect(project.mirror_user).to be_nil
end
it 'forbids non-admins from setting mirror_user_id explicitly' do
project.team.add_maintainer(admin)
result = update_project(project, user, opts.merge(mirror_user_id: admin.id))
expect(result).to eq(status: :error, message: 'Mirror user is invalid')
expect(project.mirror_user).to be_nil
end
it 'allows admins to set mirror_user_id' do
project.team.add_maintainer(admin)
result = update_project(project, admin, opts.merge(mirror_user_id: user.id))
expect(result).to eq(status: :success)
expect(project.mirror_user).to eq(user)
end
it 'forces an import job' do
expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once
update_project(project, user, opts) update_project(project, user, opts)
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module API module API
class GroupImport < Grape::API class GroupImport < Grape::API
helpers Helpers::FileUploadHelpers
helpers do helpers do
def parent_group def parent_group
find_group!(params[:parent_id]) if params[:parent_id].present? find_group!(params[:parent_id]) if params[:parent_id].present?
...@@ -49,29 +51,20 @@ module API ...@@ -49,29 +51,20 @@ module API
params do params do
requires :path, type: String, desc: 'Group path' requires :path, type: String, desc: 'Group path'
requires :name, type: String, desc: 'Group name' requires :name, type: String, desc: 'Group name'
requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The group export file to be imported'
optional :parent_id, type: Integer, desc: "The ID of the parent group that the group will be imported into. Defaults to the current user's namespace." optional :parent_id, type: Integer, desc: "The ID of the parent group that the group will be imported into. Defaults to the current user's namespace."
optional 'file.path', type: String, desc: 'Path to locally stored body (generated by Workhorse)'
optional 'file.name', type: String, desc: 'Real filename as send in Content-Disposition (generated by Workhorse)'
optional 'file.type', type: String, desc: 'Real content type as send in Content-Type (generated by Workhorse)'
optional 'file.size', type: Integer, desc: 'Real size of file (generated by Workhorse)'
optional 'file.md5', type: String, desc: 'MD5 checksum of the file (generated by Workhorse)'
optional 'file.sha1', type: String, desc: 'SHA1 checksum of the file (generated by Workhorse)'
optional 'file.sha256', type: String, desc: 'SHA256 checksum of the file (generated by Workhorse)'
end end
post 'import' do post 'import' do
authorize_create_group! authorize_create_group!
require_gitlab_workhorse! require_gitlab_workhorse!
validate_file!
uploaded_file = UploadedFile.from_params(params, :file, ImportExportUploader.workhorse_local_upload_path)
bad_request!('Unable to process group import file') unless uploaded_file
group_params = { group_params = {
path: params[:path], path: params[:path],
name: params[:name], name: params[:name],
parent_id: params[:parent_id], parent_id: params[:parent_id],
visibility_level: closest_allowed_visibility_level, visibility_level: closest_allowed_visibility_level,
import_export_upload: ImportExportUpload.new(import_file: uploaded_file) import_export_upload: ImportExportUpload.new(import_file: params[:file])
} }
group = ::Groups::CreateService.new(current_user, group_params).execute group = ::Groups::CreateService.new(current_user, group_params).execute
......
...@@ -444,6 +444,8 @@ module API ...@@ -444,6 +444,8 @@ module API
not_found!("Source Project") unless fork_from_project not_found!("Source Project") unless fork_from_project
authorize! :fork_project, fork_from_project
result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project) result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project)
if result if result
......
...@@ -6,6 +6,8 @@ module API ...@@ -6,6 +6,8 @@ module API
class Repositories < Grape::API class Repositories < Grape::API
include PaginationParams include PaginationParams
helpers ::API::Helpers::HeadersHelpers
before { authorize! :download_code, user_project } before { authorize! :download_code, user_project }
params do params do
...@@ -67,6 +69,8 @@ module API ...@@ -67,6 +69,8 @@ module API
get ':id/repository/blobs/:sha/raw' do get ':id/repository/blobs/:sha/raw' do
assign_blob_vars! assign_blob_vars!
no_cache_headers
send_git_blob @repo, @blob send_git_blob @repo, @blob
end end
......
...@@ -21,7 +21,7 @@ module Gitlab ...@@ -21,7 +21,7 @@ module Gitlab
project_id: project.id, project_id: project.id,
project: project.path, project: project.path,
namespace: project.namespace.path, namespace: project.namespace.path,
return_url: return_url, return_url: sanitize_url(return_url),
is_supported_content: supported_content?.to_s, is_supported_content: supported_content?.to_s,
base_url: Gitlab::Routing.url_helpers.project_show_sse_path(project, full_path) base_url: Gitlab::Routing.url_helpers.project_show_sse_path(project, full_path)
} }
...@@ -52,6 +52,10 @@ module Gitlab ...@@ -52,6 +52,10 @@ module Gitlab
def full_path def full_path
"#{ref}/#{file_path}" "#{ref}/#{file_path}"
end end
def sanitize_url(url)
url if Gitlab::UrlSanitizer.valid_web?(url)
end
end end
end end
end end
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
module Gitlab module Gitlab
class UrlSanitizer class UrlSanitizer
ALLOWED_SCHEMES = %w[http https ssh git].freeze ALLOWED_SCHEMES = %w[http https ssh git].freeze
ALLOWED_WEB_SCHEMES = %w[http https].freeze
def self.sanitize(content) def self.sanitize(content)
regexp = URI::DEFAULT_PARSER.make_regexp(ALLOWED_SCHEMES) regexp = URI::DEFAULT_PARSER.make_regexp(ALLOWED_SCHEMES)
...@@ -12,17 +13,21 @@ module Gitlab ...@@ -12,17 +13,21 @@ module Gitlab
content.gsub(regexp, '') content.gsub(regexp, '')
end end
def self.valid?(url) def self.valid?(url, allowed_schemes: ALLOWED_SCHEMES)
return false unless url.present? return false unless url.present?
return false unless url.is_a?(String) return false unless url.is_a?(String)
uri = Addressable::URI.parse(url.strip) uri = Addressable::URI.parse(url.strip)
ALLOWED_SCHEMES.include?(uri.scheme) allowed_schemes.include?(uri.scheme)
rescue Addressable::URI::InvalidURIError rescue Addressable::URI::InvalidURIError
false false
end end
def self.valid_web?(url)
valid?(url, allowed_schemes: ALLOWED_WEB_SCHEMES)
end
def initialize(url, credentials: nil) def initialize(url, credentials: nil)
%i[user password].each do |symbol| %i[user password].each do |symbol|
credentials[symbol] = credentials[symbol].presence if credentials&.key?(symbol) credentials[symbol] = credentials[symbol].presence if credentials&.key?(symbol)
......
...@@ -4673,9 +4673,6 @@ msgstr "" ...@@ -4673,9 +4673,6 @@ msgstr ""
msgid "ClusterIntegration|Copy Kubernetes cluster name" msgid "ClusterIntegration|Copy Kubernetes cluster name"
msgstr "" msgstr ""
msgid "ClusterIntegration|Copy Service Token"
msgstr ""
msgid "ClusterIntegration|Could not load IAM roles" msgid "ClusterIntegration|Could not load IAM roles"
msgstr "" msgstr ""
...@@ -4754,6 +4751,9 @@ msgstr "" ...@@ -4754,6 +4751,9 @@ msgstr ""
msgid "ClusterIntegration|Enabled stack" msgid "ClusterIntegration|Enabled stack"
msgstr "" msgstr ""
msgid "ClusterIntegration|Enter new Service Token"
msgstr ""
msgid "ClusterIntegration|Enter the details for your Amazon EKS Kubernetes cluster" msgid "ClusterIntegration|Enter the details for your Amazon EKS Kubernetes cluster"
msgstr "" msgstr ""
...@@ -4838,9 +4838,6 @@ msgstr "" ...@@ -4838,9 +4838,6 @@ msgstr ""
msgid "ClusterIntegration|Helm streamlines installing and managing Kubernetes applications. Tiller runs inside of your Kubernetes Cluster, and manages releases of your charts." msgid "ClusterIntegration|Helm streamlines installing and managing Kubernetes applications. Tiller runs inside of your Kubernetes Cluster, and manages releases of your charts."
msgstr "" msgstr ""
msgid "ClusterIntegration|Hide"
msgstr ""
msgid "ClusterIntegration|If you are setting up multiple clusters and are using Auto DevOps, %{help_link_start}read this first%{help_link_end}." msgid "ClusterIntegration|If you are setting up multiple clusters and are using Auto DevOps, %{help_link_start}read this first%{help_link_end}."
msgstr "" msgstr ""
...@@ -5237,9 +5234,6 @@ msgstr "" ...@@ -5237,9 +5234,6 @@ msgstr ""
msgid "ClusterIntegration|Set the global mode for the WAF in this cluster. This can be overridden at the environmental level." msgid "ClusterIntegration|Set the global mode for the WAF in this cluster. This can be overridden at the environmental level."
msgstr "" msgstr ""
msgid "ClusterIntegration|Show"
msgstr ""
msgid "ClusterIntegration|Something went wrong on our end." msgid "ClusterIntegration|Something went wrong on our end."
msgstr "" msgstr ""
...@@ -22495,9 +22489,6 @@ msgstr "" ...@@ -22495,9 +22489,6 @@ msgstr ""
msgid "This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches." msgid "This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches."
msgstr "" msgstr ""
msgid "This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches. Upon creation or when reassigning you can only assign yourself to be the mirror user."
msgstr ""
msgid "This variable can not be masked." msgid "This variable can not be masked."
msgstr "" msgstr ""
...@@ -25322,6 +25313,9 @@ msgstr "" ...@@ -25322,6 +25313,9 @@ msgstr ""
msgid "You will be removed from existing projects/groups" msgid "You will be removed from existing projects/groups"
msgstr "" msgstr ""
msgid "You will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches."
msgstr ""
msgid "You will first need to set up Jira Integration to use this feature." msgid "You will first need to set up Jira Integration to use this feature."
msgstr "" msgstr ""
...@@ -25586,6 +25580,9 @@ msgstr "" ...@@ -25586,6 +25580,9 @@ msgstr ""
msgid "Your projects" msgid "Your projects"
msgstr "" msgstr ""
msgid "Your request for access could not be processed: %{error_meesage}"
msgstr ""
msgid "Your request for access has been queued for review." msgid "Your request for access has been queued for review."
msgstr "" msgstr ""
...@@ -26024,6 +26021,9 @@ msgstr "" ...@@ -26024,6 +26021,9 @@ msgstr ""
msgid "email '%{email}' does not match the allowed domain of '%{email_domain}'" msgid "email '%{email}' does not match the allowed domain of '%{email_domain}'"
msgstr "" msgstr ""
msgid "email '%{email}' is not a verified email."
msgstr ""
msgid "enabled" msgid "enabled"
msgstr "" msgstr ""
......
...@@ -162,6 +162,46 @@ describe Admin::ApplicationSettingsController do ...@@ -162,6 +162,46 @@ describe Admin::ApplicationSettingsController do
end end
end end
describe 'PATCH #integrations' do
before do
stub_feature_flags(instance_level_integrations: false)
sign_in(admin)
end
describe 'EKS integration' do
let(:application_setting) { ApplicationSetting.current }
let(:settings_params) do
{
eks_integration_enabled: '1',
eks_account_id: '123456789012',
eks_access_key_id: 'dummy access key',
eks_secret_access_key: 'dummy secret key'
}
end
it 'updates EKS settings' do
patch :integrations, params: { application_setting: settings_params }
expect(application_setting.eks_integration_enabled).to be_truthy
expect(application_setting.eks_account_id).to eq '123456789012'
expect(application_setting.eks_access_key_id).to eq 'dummy access key'
expect(application_setting.eks_secret_access_key).to eq 'dummy secret key'
end
context 'secret access key is blank' do
let(:settings_params) { { eks_secret_access_key: '' } }
it 'does not update the secret key' do
application_setting.update!(eks_secret_access_key: 'dummy secret key')
patch :integrations, params: { application_setting: settings_params }
expect(application_setting.reload.eks_secret_access_key).to eq 'dummy secret key'
end
end
end
end
describe 'PUT #reset_registration_token' do describe 'PUT #reset_registration_token' do
before do before do
sign_in(admin) sign_in(admin)
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
require 'spec_helper' require 'spec_helper'
describe Oauth::AuthorizationsController do describe Oauth::AuthorizationsController do
let(:user) { create(:user) }
let!(:application) { create(:oauth_application, scopes: 'api read_user', redirect_uri: 'http://example.com') } let!(:application) { create(:oauth_application, scopes: 'api read_user', redirect_uri: 'http://example.com') }
let(:params) do let(:params) do
{ {
...@@ -19,53 +18,68 @@ describe Oauth::AuthorizationsController do ...@@ -19,53 +18,68 @@ describe Oauth::AuthorizationsController do
end end
describe 'GET #new' do describe 'GET #new' do
context 'without valid params' do context 'when the user is confirmed' do
it 'returns 200 code and renders error view' do let(:user) { create(:user) }
get :new
expect(response).to have_gitlab_http_status(:ok) context 'without valid params' do
expect(response).to render_template('doorkeeper/authorizations/error') it 'returns 200 code and renders error view' do
get :new
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template('doorkeeper/authorizations/error')
end
end end
end
context 'with valid params' do context 'with valid params' do
render_views render_views
it 'returns 200 code and renders view' do it 'returns 200 code and renders view' do
get :new, params: params get :new, params: params
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template('doorkeeper/authorizations/new') expect(response).to render_template('doorkeeper/authorizations/new')
end end
it 'deletes session.user_return_to and redirects when skip authorization' do it 'deletes session.user_return_to and redirects when skip authorization' do
application.update(trusted: true) application.update(trusted: true)
request.session['user_return_to'] = 'http://example.com' request.session['user_return_to'] = 'http://example.com'
get :new, params: params get :new, params: params
expect(request.session['user_return_to']).to be_nil expect(request.session['user_return_to']).to be_nil
expect(response).to have_gitlab_http_status(:found) expect(response).to have_gitlab_http_status(:found)
end end
context 'when there is already an access token for the application' do context 'when there is already an access token for the application' do
context 'when the request scope matches any of the created token scopes' do context 'when the request scope matches any of the created token scopes' do
before do before do
scopes = Doorkeeper::OAuth::Scopes.from_string('api') scopes = Doorkeeper::OAuth::Scopes.from_string('api')
allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes) allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes)
create :oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes create :oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes
end end
it 'authorizes the request and redirects' do it 'authorizes the request and redirects' do
get :new, params: params get :new, params: params
expect(request.session['user_return_to']).to be_nil expect(request.session['user_return_to']).to be_nil
expect(response).to have_gitlab_http_status(:found) expect(response).to have_gitlab_http_status(:found)
end
end end
end end
end end
end end
context 'when the user is unconfirmed' do
let(:user) { create(:user, confirmed_at: nil) }
it 'returns 200 and renders error view' do
get :new, params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template('doorkeeper/authorizations/error')
end
end
end end
end end
...@@ -5,8 +5,8 @@ require 'spec_helper' ...@@ -5,8 +5,8 @@ require 'spec_helper'
describe Profiles::NotificationsController do describe Profiles::NotificationsController do
let(:user) do let(:user) do
create(:user) do |user| create(:user) do |user|
user.emails.create(email: 'original@example.com') user.emails.create(email: 'original@example.com', confirmed_at: Time.current)
user.emails.create(email: 'new@example.com') user.emails.create(email: 'new@example.com', confirmed_at: Time.current)
user.notification_email = 'original@example.com' user.notification_email = 'original@example.com'
user.save! user.save!
end end
......
...@@ -256,7 +256,7 @@ describe Projects::DeployKeysController do ...@@ -256,7 +256,7 @@ describe Projects::DeployKeysController do
end end
def deploy_key_params(title, can_push) def deploy_key_params(title, can_push)
deploy_keys_projects_attributes = { '0' => { id: deploy_keys_project, can_push: can_push } } deploy_keys_projects_attributes = { '0' => { can_push: can_push } }
{ deploy_key: { title: title, deploy_keys_projects_attributes: deploy_keys_projects_attributes } } { deploy_key: { title: title, deploy_keys_projects_attributes: deploy_keys_projects_attributes } }
end end
...@@ -300,6 +300,42 @@ describe Projects::DeployKeysController do ...@@ -300,6 +300,42 @@ describe Projects::DeployKeysController do
expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true)
end end
end end
context 'when a different deploy key id param is injected' do
let(:extra_params) { deploy_key_params('updated title', '1') }
let(:hacked_params) do
extra_params.reverse_merge(id: other_deploy_key_id,
namespace_id: project.namespace,
project_id: project)
end
subject { put :update, params: hacked_params }
context 'and that deploy key id exists' do
let(:other_project) { create(:project) }
let(:other_deploy_key) do
key = create(:deploy_key)
project.deploy_keys << key
key
end
let(:other_deploy_key_id) { other_deploy_key.id }
it 'does not update the can_push attribute' do
expect { subject }.not_to change { deploy_key.deploy_keys_project_for(project).can_push }
end
end
context 'and that deploy key id does not exist' do
let(:other_deploy_key_id) { 9999 }
it 'returns 404' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
end end
context 'with admin as project maintainer' do context 'with admin as project maintainer' do
......
...@@ -48,6 +48,10 @@ FactoryBot.define do ...@@ -48,6 +48,10 @@ FactoryBot.define do
after(:build) { |user, _| user.block! } after(:build) { |user, _| user.block! }
end end
trait :unconfirmed do
confirmed_at { nil }
end
trait :with_avatar do trait :with_avatar do
avatar { fixture_file_upload('spec/fixtures/dk.png') } avatar { fixture_file_upload('spec/fixtures/dk.png') }
end end
......
...@@ -39,7 +39,7 @@ describe 'User Cluster', :js do ...@@ -39,7 +39,7 @@ describe 'User Cluster', :js do
expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value) expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value)
.to have_content('http://example.com') .to have_content('http://example.com')
expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value) expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value)
.to have_content('my-token') .to be_empty
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe 'OAuth Provider' do
describe 'Standard OAuth Authorization' do
let(:application) { create(:oauth_application, scopes: 'read_user') }
before do
sign_in(user)
visit oauth_authorization_path(client_id: application.uid,
redirect_uri: application.redirect_uri.split.first,
response_type: 'code',
state: 'my_state',
scope: 'read_user')
end
it_behaves_like 'Secure OAuth Authorizations'
end
end
...@@ -46,7 +46,7 @@ describe 'User Cluster', :js do ...@@ -46,7 +46,7 @@ describe 'User Cluster', :js do
expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value) expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value)
.to have_content('http://example.com') .to have_content('http://example.com')
expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value) expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value)
.to have_content('my-token') .to be_empty
end end
it 'user sees RBAC is enabled by default' do it 'user sees RBAC is enabled by default' do
......
...@@ -82,28 +82,6 @@ describe('Clusters', () => { ...@@ -82,28 +82,6 @@ describe('Clusters', () => {
}); });
}); });
describe('showToken', () => {
it('should update token field type', () => {
cluster.showTokenButton.click();
expect(cluster.tokenField.getAttribute('type')).toEqual('text');
cluster.showTokenButton.click();
expect(cluster.tokenField.getAttribute('type')).toEqual('password');
});
it('should update show token button text', () => {
cluster.showTokenButton.click();
expect(cluster.showTokenButton.textContent).toEqual('Hide');
cluster.showTokenButton.click();
expect(cluster.showTokenButton.textContent).toEqual('Show');
});
});
describe('checkForNewInstalls', () => { describe('checkForNewInstalls', () => {
const INITIAL_APP_MAP = { const INITIAL_APP_MAP = {
helm: { status: null, title: 'Helm Tiller' }, helm: { status: null, title: 'Helm Tiller' },
......
<div class="description" updated-at="">
<div class="md issue-realtime-trigger-pulse">
<svg
id="mermaid-1587752414912"
width="100%"
xmlns="http://www.w3.org/2000/svg"
style="max-width: 185.35000610351562px;"
viewBox="0 0 185.35000610351562 50.5"
class="mermaid"
>
<g transform="translate(0, 0)">
<g class="output">
<g class="clusters"></g>
<g class="edgePaths"></g>
<g class="edgeLabels"></g>
<g class="nodes">
<g
class="node js-issuable-actions btn-close clickable"
style="opacity: 1;"
id="A"
transform="translate(92.67500305175781,25.25)"
title="click to PUT"
>
<a
class="js-issuable-actions btn-close clickable"
href="https://invalid"
rel="noopener"
>
<rect
rx="0"
ry="0"
x="-84.67500305175781"
y="-17.25"
width="169.35000610351562"
height="34.5"
class="label-container"
></rect>
<g class="label" transform="translate(0,0)">
<g transform="translate(-74.67500305175781,-7.25)">
<text style="">
<tspan xml:space="preserve" dy="1em" x="1">Click to send a PUT request</tspan>
</text>
</g>
</g>
</a>
</g>
</g>
</g>
</g>
<text class="source" display="none">
Click to send a PUT request
</text>
</svg>
</div>
<textarea
data-update-url="/h5bp/html5-boilerplate/-/issues/35.json"
dir="auto"
class="hidden js-task-list-field"
></textarea>
<div class="modal-open recaptcha-modal js-recaptcha-modal" style="display: none;">
<div role="dialog" tabindex="-1" class="modal d-block">
<div role="document" class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h4 class="modal-title float-left">Please solve the reCAPTCHA</h4>
<button type="button" data-dismiss="modal" aria-label="Close" class="close float-right">
<span aria-hidden="true">×</span>
</button>
</div>
<div class="modal-body">
<div>
<p>We want to be sure it is you, please confirm you are not a robot.</p>
<div></div>
</div>
</div>
<!---->
</div>
</div>
</div>
<div class="modal-backdrop fade show"></div>
</div>
</div>
...@@ -18,6 +18,7 @@ describe('Issue', () => { ...@@ -18,6 +18,7 @@ describe('Issue', () => {
preloadFixtures('issues/closed-issue.html'); preloadFixtures('issues/closed-issue.html');
preloadFixtures('issues/issue-with-task-list.html'); preloadFixtures('issues/issue-with-task-list.html');
preloadFixtures('issues/open-issue.html'); preloadFixtures('issues/open-issue.html');
preloadFixtures('static/issue_with_mermaid_graph.html');
function expectErrorMessage() { function expectErrorMessage() {
const $flashMessage = $('div.flash-alert'); const $flashMessage = $('div.flash-alert');
...@@ -228,4 +229,30 @@ describe('Issue', () => { ...@@ -228,4 +229,30 @@ describe('Issue', () => {
}); });
}); });
}); });
describe('when not displaying blocked warning', () => {
describe('when clicking a mermaid graph inside an issue description', () => {
let mock;
let spy;
beforeEach(() => {
loadFixtures('static/issue_with_mermaid_graph.html');
mock = new MockAdapter(axios);
spy = jest.spyOn(axios, 'put');
});
afterEach(() => {
mock.restore();
jest.clearAllMocks();
});
it('does not make a PUT request', () => {
Issue.prototype.initIssueBtnEventListeners();
$('svg a.js-issuable-actions').trigger('click');
expect(spy).not.toHaveBeenCalled();
});
});
});
}); });
...@@ -3,9 +3,17 @@ import DuplicateDashboardForm from '~/monitoring/components/duplicate_dashboard_ ...@@ -3,9 +3,17 @@ import DuplicateDashboardForm from '~/monitoring/components/duplicate_dashboard_
import { dashboardGitResponse } from '../mock_data'; import { dashboardGitResponse } from '../mock_data';
describe('DuplicateDashboardForm', () => { let wrapper;
let wrapper;
const createMountedWrapper = (props = {}) => {
// Use `mount` to render native input elements
wrapper = mount(DuplicateDashboardForm, {
propsData: { ...props },
sync: false,
});
};
describe('DuplicateDashboardForm', () => {
const defaultBranch = 'master'; const defaultBranch = 'master';
const findByRef = ref => wrapper.find({ ref }); const findByRef = ref => wrapper.find({ ref });
...@@ -20,14 +28,7 @@ describe('DuplicateDashboardForm', () => { ...@@ -20,14 +28,7 @@ describe('DuplicateDashboardForm', () => {
}; };
beforeEach(() => { beforeEach(() => {
// Use `mount` to render native input elements createMountedWrapper({ dashboard: dashboardGitResponse[0], defaultBranch });
wrapper = mount(DuplicateDashboardForm, {
propsData: {
dashboard: dashboardGitResponse[0],
defaultBranch,
},
sync: false,
});
}); });
it('renders correctly', () => { it('renders correctly', () => {
...@@ -146,3 +147,18 @@ describe('DuplicateDashboardForm', () => { ...@@ -146,3 +147,18 @@ describe('DuplicateDashboardForm', () => {
}); });
}); });
}); });
describe('DuplicateDashboardForm escapes elements', () => {
const branchToEscape = "<img/src='x'onerror=alert(document.domain)>";
beforeEach(() => {
createMountedWrapper({ dashboard: dashboardGitResponse[0], defaultBranch: branchToEscape });
});
it('should escape branch name data', () => {
const branchOptionHtml = wrapper.vm.branchOptions[0].html;
const escapedBranch = '&lt;img/src=&#39;x&#39;onerror=alert(document.domain)&gt';
expect(branchOptionHtml).toEqual(expect.stringContaining(escapedBranch));
});
});
...@@ -65,5 +65,23 @@ describe Gitlab::StaticSiteEditor::Config do ...@@ -65,5 +65,23 @@ describe Gitlab::StaticSiteEditor::Config do
it { is_expected.to include(is_supported_content: 'false') } it { is_expected.to include(is_supported_content: 'false') }
end end
context 'when return_url is not a valid URL' do
let(:return_url) { 'example.com' }
it { is_expected.to include(return_url: nil) }
end
context 'when return_url has a javascript scheme' do
let(:return_url) { 'javascript:alert(document.domain)' }
it { is_expected.to include(return_url: nil) }
end
context 'when return_url is missing' do
let(:return_url) { nil }
it { is_expected.to include(return_url: nil) }
end
end end
end end
...@@ -60,6 +60,30 @@ describe Gitlab::UrlSanitizer do ...@@ -60,6 +60,30 @@ describe Gitlab::UrlSanitizer do
end end
end end
describe '.valid_web?' do
where(:value, :url) do
false | nil
false | ''
false | '123://invalid:url'
false | 'valid@project:url.git'
false | 'valid:pass@project:url.git'
false | %w(test array)
false | 'ssh://example.com'
false | 'ssh://:@example.com'
false | 'ssh://foo@example.com'
false | 'ssh://foo:bar@example.com'
false | 'ssh://foo:bar@example.com/group/group/project.git'
false | 'git://example.com/group/group/project.git'
false | 'git://foo:bar@example.com/group/group/project.git'
true | 'http://foo:bar@example.com/group/group/project.git'
true | 'https://foo:bar@example.com/group/group/project.git'
end
with_them do
it { expect(described_class.valid_web?(url)).to eq(value) }
end
end
describe '#sanitized_url' do describe '#sanitized_url' do
context 'credentials in hash' do context 'credentials in hash' do
where(username: ['foo', '', nil], password: ['bar', '', nil]) where(username: ['foo', '', nil], password: ['bar', '', nil])
......
...@@ -110,6 +110,11 @@ describe Group do ...@@ -110,6 +110,11 @@ describe Group do
let(:group_notification_email) { 'user+group@example.com' } let(:group_notification_email) { 'user+group@example.com' }
let(:subgroup_notification_email) { 'user+subgroup@example.com' } let(:subgroup_notification_email) { 'user+subgroup@example.com' }
before do
create(:email, :confirmed, user: user, email: group_notification_email)
create(:email, :confirmed, user: user, email: subgroup_notification_email)
end
subject { subgroup.notification_email_for(user) } subject { subgroup.notification_email_for(user) }
context 'when both group notification emails are set' do context 'when both group notification emails are set' do
......
...@@ -48,6 +48,33 @@ RSpec.describe NotificationSetting do ...@@ -48,6 +48,33 @@ RSpec.describe NotificationSetting do
expect(notification_setting.reopen_merge_request).to eq(false) expect(notification_setting.reopen_merge_request).to eq(false)
end end
end end
context 'notification_email' do
let_it_be(:user) { create(:user) }
subject { described_class.new(source_id: 1, source_type: 'Project', user_id: user.id) }
it 'allows to change email to verified one' do
email = create(:email, :confirmed, user: user)
subject.update(notification_email: email.email)
expect(subject).to be_valid
end
it 'does not allow to change email to not verified one' do
email = create(:email, user: user)
subject.update(notification_email: email.email)
expect(subject).to be_invalid
end
it 'allows to change email to empty one' do
subject.update(notification_email: '')
expect(subject).to be_valid
end
end
end end
describe '#for_projects' do describe '#for_projects' do
......
...@@ -311,7 +311,7 @@ describe User do ...@@ -311,7 +311,7 @@ describe User do
end end
it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :public_email, :notification_email do it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :public_email, :notification_email do
subject { build(:user).tap { |user| user.emails << build(:email, email: email_value) } } subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } }
end end
describe '#commit_email' do describe '#commit_email' do
...@@ -568,6 +568,32 @@ describe User do ...@@ -568,6 +568,32 @@ describe User do
user = build(:user, email: "temp-email-for-oauth@example.com") user = build(:user, email: "temp-email-for-oauth@example.com")
expect(user).to be_valid expect(user).to be_valid
end end
it 'does not accept not verified emails' do
email = create(:email)
user = email.user
user.update(notification_email: email.email)
expect(user).to be_invalid
end
end
context 'owns_public_email' do
it 'accepts verified emails' do
email = create(:email, :confirmed, email: 'test@test.com')
user = email.user
user.update(public_email: email.email)
expect(user).to be_valid
end
it 'does not accept not verified emails' do
email = create(:email)
user = email.user
user.update(public_email: email.email)
expect(user).to be_invalid
end
end end
context 'set_commit_email' do context 'set_commit_email' do
...@@ -917,6 +943,108 @@ describe User do ...@@ -917,6 +943,108 @@ describe User do
expect(@user.emails.count).to eq 1 expect(@user.emails.count).to eq 1
expect(@user.emails.first.confirmed_at).not_to eq nil expect(@user.emails.first.confirmed_at).not_to eq nil
end end
context 'when the first email was unconfirmed and the second email gets confirmed' do
let(:user) { create(:user, :unconfirmed, email: 'should-be-unconfirmed@test.com') }
before do
user.update!(email: 'should-be-confirmed@test.com')
user.confirm
end
it 'updates user.email' do
expect(user.email).to eq('should-be-confirmed@test.com')
end
it 'confirms user.email' do
expect(user).to be_confirmed
end
it 'keeps the unconfirmed email unconfirmed' do
email = user.emails.first
expect(email.email).to eq('should-be-unconfirmed@test.com')
expect(email).not_to be_confirmed
end
it 'has only one email association' do
expect(user.emails.size).to eq(1)
end
end
end
context 'when an existing email record is set as primary' do
let(:user) { create(:user, email: 'confirmed@test.com') }
context 'when it is unconfirmed' do
let(:originally_unconfirmed_email) { 'should-stay-unconfirmed@test.com' }
before do
user.emails << create(:email, email: originally_unconfirmed_email, confirmed_at: nil)
user.update!(email: originally_unconfirmed_email)
end
it 'keeps the user confirmed' do
expect(user).to be_confirmed
end
it 'keeps the original email' do
expect(user.email).to eq('confirmed@test.com')
end
context 'when the email gets confirmed' do
before do
user.confirm
end
it 'keeps the user confirmed' do
expect(user).to be_confirmed
end
it 'updates the email' do
expect(user.email).to eq(originally_unconfirmed_email)
end
end
end
context 'when it is confirmed' do
let!(:old_confirmed_email) { user.email }
let(:confirmed_email) { 'already-confirmed@test.com' }
before do
user.emails << create(:email, :confirmed, email: confirmed_email)
user.update!(email: confirmed_email)
end
it 'keeps the user confirmed' do
expect(user).to be_confirmed
end
it 'updates the email' do
expect(user.email).to eq(confirmed_email)
end
it 'moves the old email' do
email = user.reload.emails.first
expect(email.email).to eq(old_confirmed_email)
expect(email).to be_confirmed
end
end
end
context 'when unconfirmed user deletes a confirmed additional email' do
let(:user) { create(:user, :unconfirmed) }
before do
user.emails << create(:email, :confirmed)
end
it 'does not affect the confirmed status' do
expect { user.emails.confirmed.destroy_all }.not_to change { user.confirmed? } # rubocop: disable Cop/DestroyAll
end
end end
describe '#update_notification_email' do describe '#update_notification_email' do
...@@ -2070,6 +2198,31 @@ describe User do ...@@ -2070,6 +2198,31 @@ describe User do
end end
end end
describe '#public_verified_emails' do
let(:user) { create(:user) }
it 'returns only confirmed public emails' do
email_confirmed = create :email, user: user, confirmed_at: Time.current
create :email, user: user
expect(user.public_verified_emails).to contain_exactly(
user.email,
email_confirmed.email
)
end
it 'returns confirmed public emails plus main user email when user is not confirmed' do
user = create(:user, confirmed_at: nil)
email_confirmed = create :email, user: user, confirmed_at: Time.current
create :email, user: user
expect(user.public_verified_emails).to contain_exactly(
user.email,
email_confirmed.email
)
end
end
describe '#verified_email?' do describe '#verified_email?' do
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -4200,9 +4353,10 @@ describe User do ...@@ -4200,9 +4353,10 @@ describe User do
context 'when an ancestor has a level other than Global' do context 'when an ancestor has a level other than Global' do
let(:ancestor) { create(:group) } let(:ancestor) { create(:group) }
let(:group) { create(:group, parent: ancestor) } let(:group) { create(:group, parent: ancestor) }
let(:email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) }
before do before do
create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: 'ancestor@example.com') create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: email.email)
end end
it 'has the same level set' do it 'has the same level set' do
...@@ -4227,10 +4381,12 @@ describe User do ...@@ -4227,10 +4381,12 @@ describe User do
let(:grand_ancestor) { create(:group) } let(:grand_ancestor) { create(:group) }
let(:ancestor) { create(:group, parent: grand_ancestor) } let(:ancestor) { create(:group, parent: grand_ancestor) }
let(:group) { create(:group, parent: ancestor) } let(:group) { create(:group, parent: ancestor) }
let(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) }
let(:grand_email) { create(:email, :confirmed, email: 'grand@example.com', user: user) }
before do before do
create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: 'grand@example.com') create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: grand_email.email)
create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: 'ancestor@example.com') create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: ancestor_email.email)
end end
it 'has the same email set' do it 'has the same email set' do
...@@ -4268,7 +4424,7 @@ describe User do ...@@ -4268,7 +4424,7 @@ describe User do
context 'when group has notification email set' do context 'when group has notification email set' do
it 'returns group notification email' do it 'returns group notification email' do
group_notification_email = 'user+group@example.com' group_notification_email = 'user+group@example.com'
create(:email, :confirmed, user: user, email: group_notification_email)
create(:notification_setting, user: user, source: group, notification_email: group_notification_email) create(:notification_setting, user: user, source: group, notification_email: group_notification_email)
is_expected.to eq(group_notification_email) is_expected.to eq(group_notification_email)
......
...@@ -11,7 +11,7 @@ describe API::GroupImport do ...@@ -11,7 +11,7 @@ describe API::GroupImport do
let(:file) { File.join('spec', 'fixtures', 'group_export.tar.gz') } let(:file) { File.join('spec', 'fixtures', 'group_export.tar.gz') }
let(:export_path) { "#{Dir.tmpdir}/group_export_spec" } let(:export_path) { "#{Dir.tmpdir}/group_export_spec" }
let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } let(:workhorse_headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } }
before do before do
allow_next_instance_of(Gitlab::ImportExport) do |import_export| allow_next_instance_of(Gitlab::ImportExport) do |import_export|
...@@ -35,7 +35,7 @@ describe API::GroupImport do ...@@ -35,7 +35,7 @@ describe API::GroupImport do
} }
end end
subject { post api('/groups/import', user), params: params, headers: workhorse_header } subject { upload_archive(file_upload, workhorse_headers, params) }
shared_examples 'when all params are correct' do shared_examples 'when all params are correct' do
context 'when user is authorized to create new group' do context 'when user is authorized to create new group' do
...@@ -151,7 +151,7 @@ describe API::GroupImport do ...@@ -151,7 +151,7 @@ describe API::GroupImport do
params[:file] = file_upload params[:file] = file_upload
expect do expect do
post api('/groups/import', user), params: params, headers: workhorse_header upload_archive(file_upload, workhorse_headers, params)
end.not_to change { Group.count }.from(1) end.not_to change { Group.count }.from(1)
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
...@@ -171,7 +171,7 @@ describe API::GroupImport do ...@@ -171,7 +171,7 @@ describe API::GroupImport do
context 'without a file from workhorse' do context 'without a file from workhorse' do
it 'rejects the request' do it 'rejects the request' do
subject upload_archive(nil, workhorse_headers, params)
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
...@@ -179,7 +179,7 @@ describe API::GroupImport do ...@@ -179,7 +179,7 @@ describe API::GroupImport do
context 'without a workhorse header' do context 'without a workhorse header' do
it 'rejects request without a workhorse header' do it 'rejects request without a workhorse header' do
post api('/groups/import', user), params: params upload_archive(file_upload, {}, params)
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
end end
...@@ -189,9 +189,7 @@ describe API::GroupImport do ...@@ -189,9 +189,7 @@ describe API::GroupImport do
let(:params) do let(:params) do
{ {
path: 'test-import-group', path: 'test-import-group',
name: 'test-import-group', name: 'test-import-group'
'file.path' => file_upload.path,
'file.name' => file_upload.original_filename
} }
end end
...@@ -229,9 +227,7 @@ describe API::GroupImport do ...@@ -229,9 +227,7 @@ describe API::GroupImport do
{ {
path: 'test-import-group', path: 'test-import-group',
name: 'test-import-group', name: 'test-import-group',
file: fog_file, file: fog_file
'file.remote_id' => file_name,
'file.size' => fog_file.size
} }
end end
...@@ -245,10 +241,21 @@ describe API::GroupImport do ...@@ -245,10 +241,21 @@ describe API::GroupImport do
include_examples 'when some params are missing' include_examples 'when some params are missing'
end end
end end
def upload_archive(file, headers = {}, params = {})
workhorse_finalize(
api('/groups/import', user),
method: :post,
file_key: :file,
params: params.merge(file: file),
headers: headers,
send_rewritten_field: true
)
end
end end
describe 'POST /groups/import/authorize' do describe 'POST /groups/import/authorize' do
subject { post api('/groups/import/authorize', user), headers: workhorse_header } subject { post api('/groups/import/authorize', user), headers: workhorse_headers }
it 'authorizes importing group with workhorse header' do it 'authorizes importing group with workhorse header' do
subject subject
...@@ -258,7 +265,7 @@ describe API::GroupImport do ...@@ -258,7 +265,7 @@ describe API::GroupImport do
end end
it 'rejects requests that bypassed gitlab-workhorse' do it 'rejects requests that bypassed gitlab-workhorse' do
workhorse_header.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
subject subject
......
...@@ -19,7 +19,7 @@ describe API::NotificationSettings do ...@@ -19,7 +19,7 @@ describe API::NotificationSettings do
end end
describe "PUT /notification_settings" do describe "PUT /notification_settings" do
let(:email) { create(:email, user: user) } let(:email) { create(:email, :confirmed, user: user) }
it "updates global notification settings for the current user" do it "updates global notification settings for the current user" do
put api("/notification_settings", user), params: { level: 'watch', notification_email: email.email } put api("/notification_settings", user), params: { level: 'watch', notification_email: email.email }
......
...@@ -1891,6 +1891,17 @@ describe API::Projects do ...@@ -1891,6 +1891,17 @@ describe API::Projects do
expect(project_fork_target).to be_forked expect(project_fork_target).to be_forked
end end
it 'fails without permission from forked_from project' do
project_fork_source.project_feature.update_attribute(:forking_access_level, ProjectFeature::PRIVATE)
post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", user)
expect(response).to have_gitlab_http_status(:forbidden)
expect(project_fork_target.forked_from_project).to be_nil
expect(project_fork_target.fork_network_member).not_to be_present
expect(project_fork_target).not_to be_forked
end
it 'denies project to be forked from a private project' do it 'denies project to be forked from a private project' do
post api("/projects/#{project_fork_target.id}/fork/#{private_project_fork_source.id}", user) post api("/projects/#{project_fork_target.id}/fork/#{private_project_fork_source.id}", user)
......
...@@ -177,6 +177,12 @@ describe API::Repositories do ...@@ -177,6 +177,12 @@ describe API::Repositories do
expect(headers['Content-Disposition']).to eq 'inline' expect(headers['Content-Disposition']).to eq 'inline'
end end
it_behaves_like 'uncached response' do
before do
get api(route, current_user)
end
end
context 'when sha does not exist' do context 'when sha does not exist' do
it_behaves_like '404 response' do it_behaves_like '404 response' do
let(:request) { get api(route.sub(sample_blob.oid, 'abcd9876'), current_user) } let(:request) { get api(route.sub(sample_blob.oid, 'abcd9876'), current_user) }
......
...@@ -9,15 +9,11 @@ describe 'OpenID Connect requests' do ...@@ -9,15 +9,11 @@ describe 'OpenID Connect requests' do
name: 'Alice', name: 'Alice',
username: 'alice', username: 'alice',
email: 'private@example.com', email: 'private@example.com',
emails: [public_email],
public_email: public_email.email,
website_url: 'https://example.com', website_url: 'https://example.com',
avatar: fixture_file_upload('spec/fixtures/dk.png') avatar: fixture_file_upload('spec/fixtures/dk.png')
) )
end end
let(:public_email) { build :email, email: 'public@example.com' }
let(:access_grant) { create :oauth_access_grant, application: application, resource_owner_id: user.id } let(:access_grant) { create :oauth_access_grant, application: application, resource_owner_id: user.id }
let(:access_token) { create :oauth_access_token, application: application, resource_owner_id: user.id } let(:access_token) { create :oauth_access_token, application: application, resource_owner_id: user.id }
...@@ -37,7 +33,7 @@ describe 'OpenID Connect requests' do ...@@ -37,7 +33,7 @@ describe 'OpenID Connect requests' do
'name' => 'Alice', 'name' => 'Alice',
'nickname' => 'alice', 'nickname' => 'alice',
'email' => 'public@example.com', 'email' => 'public@example.com',
'email_verified' => false, 'email_verified' => true,
'website' => 'https://example.com', 'website' => 'https://example.com',
'profile' => 'http://localhost/alice', 'profile' => 'http://localhost/alice',
'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png", 'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png",
...@@ -62,6 +58,11 @@ describe 'OpenID Connect requests' do ...@@ -62,6 +58,11 @@ describe 'OpenID Connect requests' do
get '/oauth/userinfo', params: {}, headers: { 'Authorization' => "Bearer #{access_token.token}" } get '/oauth/userinfo', params: {}, headers: { 'Authorization' => "Bearer #{access_token.token}" }
end end
before do
email = create(:email, :confirmed, email: 'public@example.com', user: user)
user.update!(public_email: email.email)
end
context 'Application without OpenID scope' do context 'Application without OpenID scope' do
let(:application) { create :oauth_application, scopes: 'api' } let(:application) { create :oauth_application, scopes: 'api' }
...@@ -123,7 +124,7 @@ describe 'OpenID Connect requests' do ...@@ -123,7 +124,7 @@ describe 'OpenID Connect requests' do
end end
it 'has false in email_verified claim' do it 'has false in email_verified claim' do
expect(json_response['email_verified']).to eq(false) expect(json_response['email_verified']).to eq(true)
end end
end end
......
...@@ -5,8 +5,8 @@ require 'spec_helper' ...@@ -5,8 +5,8 @@ require 'spec_helper'
describe 'view user notifications' do describe 'view user notifications' do
let(:user) do let(:user) do
create(:user) do |user| create(:user) do |user|
user.emails.create(email: 'original@example.com') user.emails.create(email: 'original@example.com', confirmed_at: Time.current)
user.emails.create(email: 'new@example.com') user.emails.create(email: 'new@example.com', confirmed_at: Time.current)
user.notification_email = 'original@example.com' user.notification_email = 'original@example.com'
user.save! user.save!
end end
......
...@@ -47,6 +47,39 @@ describe Clusters::UpdateService do ...@@ -47,6 +47,39 @@ describe Clusters::UpdateService do
expect(cluster.platform.namespace).to eq('custom-namespace') expect(cluster.platform.namespace).to eq('custom-namespace')
end end
end end
context 'when service token is empty' do
let(:params) do
{
platform_kubernetes_attributes: {
token: ''
}
}
end
it 'does not update the token' do
current_token = cluster.platform.token
is_expected.to eq(true)
cluster.platform.reload
expect(cluster.platform.token).to eq(current_token)
end
end
context 'when service token is not empty' do
let(:params) do
{
platform_kubernetes_attributes: {
token: 'new secret token'
}
}
end
it 'updates the token' do
is_expected.to eq(true)
expect(cluster.platform.token).to eq('new secret token')
end
end
end end
context 'when invalid params' do context 'when invalid params' do
......
...@@ -2457,6 +2457,8 @@ describe NotificationService, :mailer do ...@@ -2457,6 +2457,8 @@ describe NotificationService, :mailer do
group = create(:group) group = create(:group)
project.update(group: group) project.update(group: group)
create(:email, :confirmed, user: u_custom_notification_enabled, email: group_notification_email)
create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email) create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email)
end end
...@@ -2491,6 +2493,7 @@ describe NotificationService, :mailer do ...@@ -2491,6 +2493,7 @@ describe NotificationService, :mailer do
group = create(:group) group = create(:group)
project.update(group: group) project.update(group: group)
create(:email, :confirmed, user: u_member, email: group_notification_email)
create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email) create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
end end
...@@ -2584,6 +2587,7 @@ describe NotificationService, :mailer do ...@@ -2584,6 +2587,7 @@ describe NotificationService, :mailer do
group = create(:group) group = create(:group)
project.update(group: group) project.update(group: group)
create(:email, :confirmed, user: u_member, email: group_notification_email)
create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email) create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
end end
......
# frozen_string_literal: true
RSpec.shared_examples 'Secure OAuth Authorizations' do
context 'when user is confirmed' do
let(:user) { create(:user) }
it 'asks the user to authorize the application' do
expect(page).to have_text "Authorize #{application.name} to use your account?"
end
end
context 'when user is unconfirmed' do
let(:user) { create(:user, confirmed_at: nil) }
it 'displays an error' do
expect(page).to have_text I18n.t('doorkeeper.errors.messages.unconfirmed_email')
end
end
end
...@@ -28,6 +28,7 @@ RSpec.shared_examples 'an email sent to a user' do ...@@ -28,6 +28,7 @@ RSpec.shared_examples 'an email sent to a user' do
it 'is sent to user\'s group notification email' do it 'is sent to user\'s group notification email' do
group_notification_email = 'user+group@example.com' group_notification_email = 'user+group@example.com'
create(:email, :confirmed, user: recipient, email: group_notification_email)
create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email) create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email)
expect(subject).to deliver_to(group_notification_email) expect(subject).to deliver_to(group_notification_email)
......
# frozen_string_literal: true
#
# Pairs with lib/gitlab/no_cache_headers.rb
#
RSpec.shared_examples 'uncached response' do
it 'defines an uncached header response' do
expect(response.headers["Cache-Control"]).to include("no-store", "no-cache")
expect(response.headers["Pragma"]).to eq("no-cache")
expect(response.headers["Expires"]).to eq("Fri, 01 Jan 1990 00:00:00 GMT")
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'admin/application_settings/_eks' do
let_it_be(:admin) { create(:admin) }
let(:page) { Capybara::Node::Simple.new(rendered) }
before do
assign(:application_setting, application_setting)
allow(view).to receive(:current_user) { admin }
allow(view).to receive(:expanded) { true }
end
shared_examples 'EKS secret access key input' do
it 'renders an empty password field' do
render
expect(rendered).to have_field('Secret access key', type: 'password')
expect(page.find_field('Secret access key').value).to be_blank
end
end
context 'when eks_secret_access_key is not set' do
let(:application_setting) { build(:application_setting) }
include_examples 'EKS secret access key input'
end
context 'when eks_secret_access_key is set' do
let(:application_setting) { build(:application_setting, eks_secret_access_key: 'eks_secret_access_key') }
include_examples 'EKS secret access key input'
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment