summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-03-24 13:36:59 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-03-24 13:36:59 +0000
commitf39b4225cba29695319bc51b5c04bf06d4cb409a (patch)
tree8302d2c7405cfe78b8b34bf63ee46cb3aae3ce47
parent2774ddc308f96f49a0f26871ff544681229f4eee (diff)
downloadgitlab-ce-f39b4225cba29695319bc51b5c04bf06d4cb409a.tar.gz
Add latest changes from gitlab-org/security/gitlab@12-8-stable-ee
-rw-r--r--GITLAB_WORKHORSE_VERSION2
-rw-r--r--Gemfile.lock2
-rw-r--r--app/assets/javascripts/frequent_items/utils.js21
-rw-r--r--app/controllers/import/fogbugz_controller.rb18
-rw-r--r--app/controllers/projects/mirrors_controller.rb2
-rw-r--r--app/models/group.rb3
-rw-r--r--app/models/issue.rb6
-rw-r--r--app/serializers/merge_request_poll_widget_entity.rb2
-rw-r--r--app/uploaders/object_storage.rb2
-rw-r--r--app/views/projects/mirrors/_mirror_repos.html.haml50
-rw-r--r--changelogs/unreleased/security-120026-redact-notes-in-moved-confidential-issues.yml5
-rw-r--r--changelogs/unreleased/security-193100-ignore-duplicate-multipart-params.yml5
-rw-r--r--changelogs/unreleased/security-59-prevent-create-api-snippet.yml5
-rw-r--r--changelogs/unreleased/security-backend-xss-admin-email.yml5
-rw-r--r--changelogs/unreleased/security-disable-mirroring-fix.yml5
-rw-r--r--changelogs/unreleased/security-docker-blocked-users.yml5
-rw-r--r--changelogs/unreleased/security-fogbugz-importer-deny-localhost-requests.yml5
-rw-r--r--changelogs/unreleased/security-mask-gh-service-password.yml5
-rw-r--r--changelogs/unreleased/security-mr-pipeline-status-permission-check.yml5
-rw-r--r--changelogs/unreleased/security-path-traversal-master.yml5
-rw-r--r--changelogs/unreleased/security-restrict-project-pipeline-metrics.yml5
-rw-r--r--changelogs/unreleased/security-update-nokogiri-cve-2020-7595.yml5
-rw-r--r--changelogs/unreleased/security-updating-description-of-trigger-by-other-maintainer.yml5
-rw-r--r--changelogs/unreleased/security-xss-vulnerability-in-admin-send-email-notification.yml5
-rw-r--r--lib/api/snippets.rb2
-rw-r--r--lib/api/triggers.rb2
-rw-r--r--lib/gitlab/auth.rb6
-rw-r--r--lib/gitlab/gfm/uploads_rewriter.rb2
-rw-r--r--lib/gitlab/regex.rb8
-rw-r--r--locale/gitlab.pot6
-rw-r--r--spec/controllers/groups_controller_spec.rb22
-rw-r--r--spec/controllers/import/fogbugz_controller_spec.rb29
-rw-r--r--spec/controllers/projects/mirrors_controller_spec.rb66
-rw-r--r--spec/features/projects/settings/repository_settings_spec.rb43
-rw-r--r--spec/javascripts/frequent_items/utils_spec.js18
-rw-r--r--spec/lib/banzai/filter/label_reference_filter_spec.rb7
-rw-r--r--spec/lib/banzai/filter/reference_redactor_filter_spec.rb64
-rw-r--r--spec/lib/gitlab/auth_spec.rb41
-rw-r--r--spec/lib/gitlab/gfm/uploads_rewriter_spec.rb10
-rw-r--r--spec/lib/gitlab/regex_spec.rb32
-rw-r--r--spec/models/group_spec.rb3
-rw-r--r--spec/models/issue_spec.rb333
-rw-r--r--spec/policies/issue_policy_spec.rb12
-rw-r--r--spec/requests/api/groups_spec.rb28
-rw-r--r--spec/requests/api/project_snippets_spec.rb24
-rw-r--r--spec/requests/api/snippets_spec.rb10
-rw-r--r--spec/requests/api/triggers_spec.rb44
-rw-r--r--spec/requests/jwt_controller_spec.rb15
-rw-r--r--spec/serializers/merge_request_poll_widget_entity_spec.rb11
-rw-r--r--spec/support/helpers/workhorse_helpers.rb2
-rw-r--r--spec/uploaders/object_storage_spec.rb13
51 files changed, 793 insertions, 238 deletions
diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION
index 72963fb08c2..a4ea2549bb6 100644
--- a/GITLAB_WORKHORSE_VERSION
+++ b/GITLAB_WORKHORSE_VERSION
@@ -1 +1 @@
-8.21.0
+8.21.1
diff --git a/Gemfile.lock b/Gemfile.lock
index aa33bd4cd68..9735b9be935 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -652,7 +652,7 @@ GEM
netrc (0.11.0)
nio4r (2.5.2)
no_proxy_fix (0.1.2)
- nokogiri (1.10.7)
+ nokogiri (1.10.8)
mini_portile2 (~> 2.4.0)
nokogumbo (1.5.0)
nokogiri
diff --git a/app/assets/javascripts/frequent_items/utils.js b/app/assets/javascripts/frequent_items/utils.js
index 5188d6118ac..76741a8b238 100644
--- a/app/assets/javascripts/frequent_items/utils.js
+++ b/app/assets/javascripts/frequent_items/utils.js
@@ -45,8 +45,19 @@ export const updateExistingFrequentItem = (frequentItem, item) => {
};
};
-export const sanitizeItem = item => ({
- ...item,
- name: sanitize(item.name.toString(), { allowedTags: [] }),
- namespace: sanitize(item.namespace.toString(), { allowedTags: [] }),
-});
+export const sanitizeItem = item => {
+ // Only sanitize if the key exists on the item
+ const maybeSanitize = key => {
+ if (!Object.prototype.hasOwnProperty.call(item, key)) {
+ return {};
+ }
+
+ return { [key]: sanitize(item[key].toString(), { allowedTags: [] }) };
+ };
+
+ return {
+ ...item,
+ ...maybeSanitize('name'),
+ ...maybeSanitize('namespace'),
+ };
+};
diff --git a/app/controllers/import/fogbugz_controller.rb b/app/controllers/import/fogbugz_controller.rb
index 28ead8d44da..4fb6efde7ff 100644
--- a/app/controllers/import/fogbugz_controller.rb
+++ b/app/controllers/import/fogbugz_controller.rb
@@ -3,6 +3,7 @@
class Import::FogbugzController < Import::BaseController
before_action :verify_fogbugz_import_enabled
before_action :user_map, only: [:new_user_map, :create_user_map]
+ before_action :verify_blocked_uri, only: :callback
rescue_from Fogbugz::AuthenticationException, with: :fogbugz_unauthorized
@@ -106,4 +107,21 @@ class Import::FogbugzController < Import::BaseController
def verify_fogbugz_import_enabled
render_404 unless fogbugz_import_enabled?
end
+
+ def verify_blocked_uri
+ Gitlab::UrlBlocker.validate!(
+ params[:uri],
+ {
+ allow_localhost: allow_local_requests?,
+ allow_local_network: allow_local_requests?,
+ schemes: %w(http https)
+ }
+ )
+ rescue Gitlab::UrlBlocker::BlockedUrlError => e
+ redirect_to new_import_fogbugz_url, alert: _('Specified URL cannot be used: "%{reason}"') % { reason: e.message }
+ end
+
+ def allow_local_requests?
+ Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
+ end
end
diff --git a/app/controllers/projects/mirrors_controller.rb b/app/controllers/projects/mirrors_controller.rb
index dd1ea151de7..936f89e58e7 100644
--- a/app/controllers/projects/mirrors_controller.rb
+++ b/app/controllers/projects/mirrors_controller.rb
@@ -67,7 +67,7 @@ class Projects::MirrorsController < Projects::ApplicationController
end
def check_mirror_available!
- Gitlab::CurrentSettings.current_application_settings.mirror_available || current_user&.admin?
+ render_404 unless can?(current_user, :admin_remote_mirror, project)
end
def mirror_params_attributes
diff --git a/app/models/group.rb b/app/models/group.rb
index bf771bd0409..83b2d17cb92 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -67,6 +67,9 @@ class Group < Namespace
validates :variables, variable_duplicates: true
validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 }
+ validates :name,
+ format: { with: Gitlab::Regex.group_name_regex,
+ message: Gitlab::Regex.group_name_regex_message }
add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
diff --git a/app/models/issue.rb b/app/models/issue.rb
index be702134ced..4c5bd3da67e 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -319,10 +319,8 @@ class Issue < ApplicationRecord
true
elsif project.owner == user
true
- elsif confidential?
- author == user ||
- assignees.include?(user) ||
- project.team.member?(user, Gitlab::Access::REPORTER)
+ elsif confidential? && !assignee_or_author?(user)
+ project.team.member?(user, Gitlab::Access::REPORTER)
else
project.public? ||
project.internal? && !user.external? ||
diff --git a/app/serializers/merge_request_poll_widget_entity.rb b/app/serializers/merge_request_poll_widget_entity.rb
index a45026ea016..18e8ec0e7d1 100644
--- a/app/serializers/merge_request_poll_widget_entity.rb
+++ b/app/serializers/merge_request_poll_widget_entity.rb
@@ -53,7 +53,7 @@ class MergeRequestPollWidgetEntity < Grape::Entity
# CI related
expose :has_ci?, as: :has_ci
- expose :ci_status do |merge_request|
+ expose :ci_status, if: -> (mr, _) { presenter(mr).can_read_pipeline? } do |merge_request|
presenter(merge_request).ci_status
end
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb
index 450ebb00b49..f4a4adc7342 100644
--- a/app/uploaders/object_storage.rb
+++ b/app/uploaders/object_storage.rb
@@ -318,7 +318,7 @@ module ObjectStorage
def cache!(new_file = sanitized_file)
# We intercept ::UploadedFile which might be stored on remote storage
# We use that for "accelerated" uploads, where we store result on remote storage
- if new_file.is_a?(::UploadedFile) && new_file.remote_id
+ if new_file.is_a?(::UploadedFile) && new_file.remote_id.present?
return cache_remote_file!(new_file.remote_id, new_file.original_filename)
end
diff --git a/app/views/projects/mirrors/_mirror_repos.html.haml b/app/views/projects/mirrors/_mirror_repos.html.haml
index 80d2d2afada..4004c4f4b07 100644
--- a/app/views/projects/mirrors/_mirror_repos.html.haml
+++ b/app/views/projects/mirrors/_mirror_repos.html.haml
@@ -1,7 +1,9 @@
- expanded = expanded_by_default?
- protocols = Gitlab::UrlSanitizer::ALLOWED_SCHEMES.join('|')
+- mirror_settings_enabled = can?(current_user, :admin_remote_mirror, @project)
+- mirror_settings_class = "#{'expanded' if expanded} #{'js-mirror-settings' if mirror_settings_enabled}".strip
-%section.settings.project-mirror-settings.js-mirror-settings.no-animate#js-push-remote-settings{ class: ('expanded' if expanded), data: { qa_selector: 'mirroring_repositories_settings_section' } }
+%section.settings.project-mirror-settings.no-animate#js-push-remote-settings{ class: mirror_settings_class, data: { qa_selector: 'mirroring_repositories_settings_section' } }
.settings-header
%h4= _('Mirroring repositories')
%button.btn.js-settings-toggle
@@ -11,26 +13,32 @@
= link_to _('Read more'), help_page_path('workflow/repository_mirroring'), target: '_blank'
.settings-content
- = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f|
- .panel.panel-default
- .panel-body
- %div= form_errors(@project)
+ - if mirror_settings_enabled
+ = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f|
+ .panel.panel-default
+ .panel-body
+ %div= form_errors(@project)
- .form-group.has-feedback
- = label_tag :url, _('Git repository URL'), class: 'label-light'
- = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password'
+ .form-group.has-feedback
+ = label_tag :url, _('Git repository URL'), class: 'label-light'
+ = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password'
- = render 'projects/mirrors/instructions'
+ = render 'projects/mirrors/instructions'
- = render 'projects/mirrors/mirror_repos_form', f: f
+ = render 'projects/mirrors/mirror_repos_form', f: f
- .form-check.append-bottom-10
- = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input'
- = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label'
- = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank'
+ .form-check.append-bottom-10
+ = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input'
+ = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label'
+ = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank'
- .panel-footer
- = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror
+ .panel-footer
+ = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror
+ - else
+ .gl-alert.gl-alert-info{ role: 'alert' }
+ = sprite_icon('information-o', size: 16, css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
+ .gl-alert-body
+ = _('Mirror settings are only available to GitLab administrators.')
.panel.panel-default
.table-responsive
@@ -61,8 +69,10 @@
- if mirror.last_error.present?
.badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true', qa_selector: 'mirror_error_badge' }, title: html_escape(mirror.last_error.try(:strip)) }= _('Error')
%td
- .btn-group.mirror-actions-group.pull-right{ role: 'group' }
- - if mirror.ssh_key_auth?
- = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button')
- = render 'shared/remote_mirror_update_button', remote_mirror: mirror
+ - if mirror_settings_enabled
+ .btn-group.mirror-actions-group.pull-right{ role: 'group' }
+ - if mirror.ssh_key_auth?
+ = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button')
+ = render 'shared/remote_mirror_update_button', remote_mirror: mirror
%button.js-delete-mirror.qa-delete-mirror.rspec-delete-mirror.btn.btn-danger{ type: 'button', data: { mirror_id: mirror.id, toggle: 'tooltip', container: 'body' }, title: _('Remove') }= icon('trash-o')
+
diff --git a/changelogs/unreleased/security-120026-redact-notes-in-moved-confidential-issues.yml b/changelogs/unreleased/security-120026-redact-notes-in-moved-confidential-issues.yml
new file mode 100644
index 00000000000..54ee6ac9048
--- /dev/null
+++ b/changelogs/unreleased/security-120026-redact-notes-in-moved-confidential-issues.yml
@@ -0,0 +1,5 @@
+---
+title: Redact notes in moved confidential issues
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-193100-ignore-duplicate-multipart-params.yml b/changelogs/unreleased/security-193100-ignore-duplicate-multipart-params.yml
new file mode 100644
index 00000000000..c871e1615e0
--- /dev/null
+++ b/changelogs/unreleased/security-193100-ignore-duplicate-multipart-params.yml
@@ -0,0 +1,5 @@
+---
+title: Ignore empty remote_id params from Workhorse accelerated uploads
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-59-prevent-create-api-snippet.yml b/changelogs/unreleased/security-59-prevent-create-api-snippet.yml
new file mode 100644
index 00000000000..135fdfe7153
--- /dev/null
+++ b/changelogs/unreleased/security-59-prevent-create-api-snippet.yml
@@ -0,0 +1,5 @@
+---
+title: External user can not create personal snippet through API
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-backend-xss-admin-email.yml b/changelogs/unreleased/security-backend-xss-admin-email.yml
new file mode 100644
index 00000000000..82f97cd719a
--- /dev/null
+++ b/changelogs/unreleased/security-backend-xss-admin-email.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent malicious entry for group name
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-disable-mirroring-fix.yml b/changelogs/unreleased/security-disable-mirroring-fix.yml
new file mode 100644
index 00000000000..1b0a6a87515
--- /dev/null
+++ b/changelogs/unreleased/security-disable-mirroring-fix.yml
@@ -0,0 +1,5 @@
+---
+title: Restrict mirroring changes to admins only when mirroring is disabled
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-docker-blocked-users.yml b/changelogs/unreleased/security-docker-blocked-users.yml
new file mode 100644
index 00000000000..6e34506e7fd
--- /dev/null
+++ b/changelogs/unreleased/security-docker-blocked-users.yml
@@ -0,0 +1,5 @@
+---
+title: Reject all container registry requests from blocked users
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-fogbugz-importer-deny-localhost-requests.yml b/changelogs/unreleased/security-fogbugz-importer-deny-localhost-requests.yml
new file mode 100644
index 00000000000..ecc05470717
--- /dev/null
+++ b/changelogs/unreleased/security-fogbugz-importer-deny-localhost-requests.yml
@@ -0,0 +1,5 @@
+---
+title: Deny localhost requests on fogbugz importer
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-mask-gh-service-password.yml b/changelogs/unreleased/security-mask-gh-service-password.yml
new file mode 100644
index 00000000000..cabbee204eb
--- /dev/null
+++ b/changelogs/unreleased/security-mask-gh-service-password.yml
@@ -0,0 +1,5 @@
+---
+title: Change GitHub service integration token input to password
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-mr-pipeline-status-permission-check.yml b/changelogs/unreleased/security-mr-pipeline-status-permission-check.yml
new file mode 100644
index 00000000000..598804bd0a7
--- /dev/null
+++ b/changelogs/unreleased/security-mr-pipeline-status-permission-check.yml
@@ -0,0 +1,5 @@
+---
+title: Add permission check for pipeline status of MR
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-path-traversal-master.yml b/changelogs/unreleased/security-path-traversal-master.yml
new file mode 100644
index 00000000000..d5e269823ea
--- /dev/null
+++ b/changelogs/unreleased/security-path-traversal-master.yml
@@ -0,0 +1,5 @@
+---
+title: Fix UploadRewriter Path Traversal vulnerability
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-restrict-project-pipeline-metrics.yml b/changelogs/unreleased/security-restrict-project-pipeline-metrics.yml
new file mode 100644
index 00000000000..20c24aa6bdf
--- /dev/null
+++ b/changelogs/unreleased/security-restrict-project-pipeline-metrics.yml
@@ -0,0 +1,5 @@
+---
+title: Restrict access to project pipeline metrics reports
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-update-nokogiri-cve-2020-7595.yml b/changelogs/unreleased/security-update-nokogiri-cve-2020-7595.yml
new file mode 100644
index 00000000000..58ad219f0eb
--- /dev/null
+++ b/changelogs/unreleased/security-update-nokogiri-cve-2020-7595.yml
@@ -0,0 +1,5 @@
+---
+title: Update Nokogiri to fix CVE-2020-7595
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-updating-description-of-trigger-by-other-maintainer.yml b/changelogs/unreleased/security-updating-description-of-trigger-by-other-maintainer.yml
new file mode 100644
index 00000000000..f7bef1589a2
--- /dev/null
+++ b/changelogs/unreleased/security-updating-description-of-trigger-by-other-maintainer.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent updating trigger by other maintainers
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-xss-vulnerability-in-admin-send-email-notification.yml b/changelogs/unreleased/security-xss-vulnerability-in-admin-send-email-notification.yml
new file mode 100644
index 00000000000..fe31f1167eb
--- /dev/null
+++ b/changelogs/unreleased/security-xss-vulnerability-in-admin-send-email-notification.yml
@@ -0,0 +1,5 @@
+---
+title: Fix XSS vulnerability in `admin/email` "Recipient Group" dropdown
+merge_request:
+author:
+type: security
diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb
index b5df036c5ca..0aaab9a812f 100644
--- a/lib/api/snippets.rb
+++ b/lib/api/snippets.rb
@@ -74,6 +74,8 @@ module API
desc: 'The visibility of the snippet'
end
post do
+ authorize! :create_snippet
+
attrs = declared_params(include_missing: false).merge(request: request, api: true)
service_response = ::Snippets::CreateService.new(nil, current_user, attrs).execute
snippet = service_response.payload[:snippet]
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index 76af29b2977..e1829403941 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -109,6 +109,8 @@ module API
trigger = user_project.triggers.find(params.delete(:trigger_id))
break not_found!('Trigger') unless trigger
+ authorize! :admin_trigger, trigger
+
if trigger.update(declared_params(include_missing: false))
present trigger, with: Entities::Trigger, current_user: current_user
else
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 1329357d0b8..a1f6de338da 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -171,6 +171,8 @@ module Gitlab
if valid_oauth_token?(token)
user = User.find_by(id: token.resource_owner_id)
+ return unless user.can?(:log_in)
+
Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities)
end
end
@@ -182,7 +184,7 @@ module Gitlab
token = PersonalAccessTokensFinder.new(state: 'active').find_by_token(password)
- if token && valid_scoped_token?(token, all_available_scopes)
+ if token && valid_scoped_token?(token, all_available_scopes) && token.user.can?(:log_in)
Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes))
end
end
@@ -260,6 +262,8 @@ module Gitlab
return unless build.project.builds_enabled?
if build.user
+ return unless build.user.can?(:log_in)
+
# If user is assigned to build, use restricted credentials of user
Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities)
else
diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb
index 6b52d6e88e5..23af0a9bb18 100644
--- a/lib/gitlab/gfm/uploads_rewriter.rb
+++ b/lib/gitlab/gfm/uploads_rewriter.rb
@@ -22,6 +22,8 @@ module Gitlab
return @text unless needs_rewrite?
@text.gsub(@pattern) do |markdown|
+ Gitlab::Utils.check_path_traversal!($~[:file])
+
file = find_file(@source_project, $~[:secret], $~[:file])
break markdown unless file.try(:exists?)
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index fd6e24a96d8..b64603d0dde 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -16,6 +16,14 @@ module Gitlab
"It must start with letter, digit, emoji or '_'."
end
+ def group_name_regex
+ project_name_regex
+ end
+
+ def group_name_regex_message
+ project_name_regex_message
+ end
+
##
# Docker Distribution Registry repository / tag name rules
#
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index e1e04c38804..28cea78c8c5 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -12357,6 +12357,9 @@ msgstr ""
msgid "Mirror repository"
msgstr ""
+msgid "Mirror settings are only available to GitLab administrators."
+msgstr ""
+
msgid "Mirror user"
msgstr ""
@@ -18176,6 +18179,9 @@ msgstr ""
msgid "Specific Runners"
msgstr ""
+msgid "Specified URL cannot be used: \"%{reason}\""
+msgstr ""
+
msgid "Specify an e-mail address regex pattern to identify default internal users."
msgstr ""
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 1c58c2b5c97..35f5a8e8728 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -258,6 +258,18 @@ describe GroupsController do
end
end
end
+
+ context "malicious group name" do
+ subject { post :create, params: { group: { name: "<script>alert('Mayday!');</script>", path: "invalid_group_url" } } }
+
+ before do
+ sign_in(user)
+ end
+
+ it { expect { subject }.not_to change { Group.count } }
+
+ it { expect(subject).to render_template(:new) }
+ end
end
describe 'GET #index' do
@@ -829,6 +841,16 @@ describe GroupsController do
put :update, params: { id: group.to_param, group: { name: 'world' } }
end.to change { group.reload.name }
end
+
+ context "malicious group name" do
+ subject { put :update, params: { id: group.to_param, group: { name: "<script>alert('Attack!');</script>" } } }
+
+ it { is_expected.to render_template(:edit) }
+
+ it 'does not update name' do
+ expect { subject }.not_to change { group.reload.name }
+ end
+ end
end
describe 'DELETE #destroy' do
diff --git a/spec/controllers/import/fogbugz_controller_spec.rb b/spec/controllers/import/fogbugz_controller_spec.rb
index 9a647b8caae..c833fbfaea5 100644
--- a/spec/controllers/import/fogbugz_controller_spec.rb
+++ b/spec/controllers/import/fogbugz_controller_spec.rb
@@ -25,6 +25,35 @@ describe Import::FogbugzController do
expect(session[:fogbugz_uri]).to eq(uri)
expect(response).to redirect_to(new_user_map_import_fogbugz_path)
end
+
+ context 'verify url' do
+ shared_examples 'denies local request' do |reason|
+ it 'does not allow requests' do
+ post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword' }
+
+ expect(response).to redirect_to(new_import_fogbugz_url)
+ expect(flash[:alert]).to eq("Specified URL cannot be used: \"#{reason}\"")
+ end
+ end
+
+ context 'when host is localhost' do
+ let(:uri) { 'https://localhost:3000' }
+
+ include_examples 'denies local request', 'Requests to localhost are not allowed'
+ end
+
+ context 'when host is on local network' do
+ let(:uri) { 'http://192.168.0.1/' }
+
+ include_examples 'denies local request', 'Requests to the local network are not allowed'
+ end
+
+ context 'when host is ftp protocol' do
+ let(:uri) { 'ftp://testing' }
+
+ include_examples 'denies local request', 'Only allowed schemes are http, https'
+ end
+ end
end
describe 'POST #create_user_map' do
diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb
index 4362febda5c..3579e4aa2cf 100644
--- a/spec/controllers/projects/mirrors_controller_spec.rb
+++ b/spec/controllers/projects/mirrors_controller_spec.rb
@@ -5,6 +5,72 @@ require 'spec_helper'
describe Projects::MirrorsController do
include ReactiveCachingHelpers
+ shared_examples 'only admin is allowed when mirroring is disabled' do
+ let(:subject_action) { raise 'subject_action is required' }
+ let(:user) { project.owner }
+ let(:project_settings_path) { project_settings_repository_path(project, anchor: 'js-push-remote-settings') }
+
+ context 'when project mirroring is enabled' do
+ it 'allows requests from a maintainer' do
+ sign_in(user)
+
+ subject_action
+ expect(response).to redirect_to(project_settings_path)
+ end
+
+ it 'allows requests from an admin user' do
+ user.update!(admin: true)
+ sign_in(user)
+
+ subject_action
+ expect(response).to redirect_to(project_settings_path)
+ end
+ end
+
+ context 'when project mirroring is disabled' do
+ before do
+ stub_application_setting(mirror_available: false)
+ end
+
+ it 'disallows requests from a maintainer' do
+ sign_in(user)
+
+ subject_action
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
+
+ it 'allows requests from an admin user' do
+ user.update!(admin: true)
+ sign_in(user)
+
+ subject_action
+ expect(response).to redirect_to(project_settings_path)
+ end
+ end
+ end
+
+ describe 'Access control' do
+ let(:project) { create(:project, :repository) }
+
+ describe '#update' do
+ include_examples 'only admin is allowed when mirroring is disabled' do
+ let(:subject_action) do
+ do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com' } })
+ end
+ end
+ end
+
+ describe '#update_now' do
+ include_examples 'only admin is allowed when mirroring is disabled' do
+ let(:options) { { namespace_id: project.namespace, project_id: project } }
+
+ let(:subject_action) do
+ get :update_now, params: options.merge(sync_remote: true)
+ end
+ end
+ end
+ end
+
describe 'setting up a remote mirror' do
let_it_be(:project) { create(:project, :repository) }
diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb
index 18031a40f15..a104feece59 100644
--- a/spec/features/projects/settings/repository_settings_spec.rb
+++ b/spec/features/projects/settings/repository_settings_spec.rb
@@ -142,11 +142,7 @@ describe 'Projects > Settings > Repository settings' do
end
context 'remote mirror settings' do
- let(:user2) { create(:user) }
-
before do
- project.add_maintainer(user2)
-
visit project_settings_repository_path(project)
end
@@ -206,6 +202,18 @@ describe 'Projects > Settings > Repository settings' do
expect(page).to have_selector('[title="Copy SSH public key"]')
end
+ context 'when project mirroring is disabled' do
+ before do
+ stub_application_setting(mirror_available: false)
+ visit project_settings_repository_path(project)
+ end
+
+ it 'hides remote mirror settings' do
+ expect(page.find('.project-mirror-settings')).not_to have_selector('form')
+ expect(page).to have_content('Mirror settings are only available to GitLab administrators.')
+ end
+ end
+
def select_direction(direction = 'push')
direction_select = find('#mirror_direction')
@@ -270,4 +278,31 @@ describe 'Projects > Settings > Repository settings' do
expect(mirror).not_to have_selector('.rspec-update-now-button')
end
end
+
+ context 'for admin' do
+ shared_examples_for 'shows mirror settings' do
+ it 'shows mirror settings' do
+ expect(page.find('.project-mirror-settings')).to have_selector('form')
+ expect(page).not_to have_content('Changing mirroring setting is disabled for non-admin users.')
+ end
+ end
+
+ before do
+ stub_application_setting(mirror_available: mirror_available)
+ user.update!(admin: true)
+ visit project_settings_repository_path(project)
+ end
+
+ context 'when project mirroring is enabled' do
+ let(:mirror_available) { true }
+
+ include_examples 'shows mirror settings'
+ end
+
+ context 'when project mirroring is disabled' do
+ let(:mirror_available) { false }
+
+ include_examples 'shows mirror settings'
+ end
+ end
end
diff --git a/spec/javascripts/frequent_items/utils_spec.js b/spec/javascripts/frequent_items/utils_spec.js
index fd5bd002428..2939b46bc31 100644
--- a/spec/javascripts/frequent_items/utils_spec.js
+++ b/spec/javascripts/frequent_items/utils_spec.js
@@ -108,5 +108,23 @@ describe('Frequent Items utils spec', () => {
expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 });
});
+
+ it("skips `name` key if it doesn't exist on the item", () => {
+ const input = {
+ namespace: '<br>test',
+ id: 1,
+ };
+
+ expect(sanitizeItem(input)).toEqual({ namespace: 'test', id: 1 });
+ });
+
+ it("skips `namespace` key if it doesn't exist on the item", () => {
+ const input = {
+ name: '<br><b>test</b>',
+ id: 1,
+ };
+
+ expect(sanitizeItem(input)).toEqual({ name: 'test', id: 1 });
+ });
});
});
diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb
index 82df5064896..f4d017df2d2 100644
--- a/spec/lib/banzai/filter/label_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb
@@ -523,7 +523,12 @@ describe Banzai::Filter::LabelReferenceFilter do
end
context 'when group name has HTML entities' do
- let(:another_group) { create(:group, name: '<img src=x onerror=alert(1)>', path: 'another_group') }
+ let(:another_group) { create(:group, name: 'random', path: 'another_group') }
+
+ before do
+ another_group.name = "<img src=x onerror=alert(1)>"
+ another_group.save!(validate: false)
+ end
it 'escapes the HTML entities' do
expect(result.text)
diff --git a/spec/lib/banzai/filter/reference_redactor_filter_spec.rb b/spec/lib/banzai/filter/reference_redactor_filter_spec.rb
index 9739afd3d57..a68581b3000 100644
--- a/spec/lib/banzai/filter/reference_redactor_filter_spec.rb
+++ b/spec/lib/banzai/filter/reference_redactor_filter_spec.rb
@@ -20,8 +20,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
it 'skips when the skip_redaction flag is set' do
user = create(:user)
project = create(:project)
-
link = reference_link(project: project.id, reference_type: 'test')
+
doc = filter(link, current_user: user, skip_redaction: true)
expect(doc.css('a').length).to eq 1
@@ -51,8 +51,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
user = create(:user)
project = create(:project)
project.add_maintainer(user)
-
link = reference_link(project: project.id, reference_type: 'test')
+
doc = filter(link, current_user: user)
expect(doc.css('a').length).to eq 1
@@ -69,8 +69,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
it 'removes unpermitted references' do
user = create(:user)
project = create(:project)
-
link = reference_link(project: project.id, reference_type: 'test')
+
doc = filter(link, current_user: user)
expect(doc.css('a').length).to eq 0
@@ -90,8 +90,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
non_member = create(:user)
project = create(:project, :public)
issue = create(:issue, :confidential, project: project)
-
link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
doc = filter(link, current_user: non_member)
expect(doc.css('a').length).to eq 0
@@ -124,8 +124,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
assignee = create(:user)
project = create(:project, :public)
issue = create(:issue, :confidential, project: project, assignees: [assignee])
-
link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
doc = filter(link, current_user: assignee)
expect(doc.css('a').length).to eq 1
@@ -136,8 +136,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
project = create(:project, :public)
project.add_developer(member)
issue = create(:issue, :confidential, project: project)
-
link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
doc = filter(link, current_user: member)
expect(doc.css('a').length).to eq 1
@@ -147,20 +147,62 @@ describe Banzai::Filter::ReferenceRedactorFilter do
admin = create(:admin)
project = create(:project, :public)
issue = create(:issue, :confidential, project: project)
-
link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
doc = filter(link, current_user: admin)
expect(doc.css('a').length).to eq 1
end
+
+ context "when a confidential issue is moved from a public project to a private one" do
+ let(:public_project) { create(:project, :public) }
+ let(:private_project) { create(:project, :private) }
+
+ it 'removes references for author' do
+ author = create(:user)
+ issue = create(:issue, :confidential, project: public_project, author: author)
+ issue.update!(project: private_project) # move issue to private project
+ link = reference_link(project: private_project.id, issue: issue.id, reference_type: 'issue')
+
+ doc = filter(link, current_user: author)
+
+ expect(doc.css('a').length).to eq 0
+ end
+
+ it 'removes references for assignee' do
+ assignee = create(:user)
+ issue = create(:issue, :confidential, project: public_project, assignees: [assignee])
+ issue.update!(project: private_project) # move issue to private project
+ link = reference_link(project: private_project.id, issue: issue.id, reference_type: 'issue')
+
+ doc = filter(link, current_user: assignee)
+
+ expect(doc.css('a').length).to eq 0
+ end
+
+ it 'allows references for project members' do
+ member = create(:user)
+ project = create(:project, :public)
+ project_2 = create(:project, :private)
+ project.add_developer(member)
+ project_2.add_developer(member)
+ issue = create(:issue, :confidential, project: project)
+ issue.update!(project: project_2) # move issue to private project
+ link = reference_link(project: project_2.id, issue: issue.id, reference_type: 'issue')
+
+ doc = filter(link, current_user: member)
+
+ expect(doc.css('a').length).to eq 1
+ end
+ end
end
it 'allows references for non confidential issues' do
user = create(:user)
project = create(:project, :public)
issue = create(:issue, project: project)
-
link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
doc = filter(link, current_user: user)
expect(doc.css('a').length).to eq 1
@@ -172,8 +214,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
it 'removes unpermitted Group references' do
user = create(:user)
group = create(:group, :private)
-
link = reference_link(group: group.id, reference_type: 'user')
+
doc = filter(link, current_user: user)
expect(doc.css('a').length).to eq 0
@@ -183,8 +225,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
user = create(:user)
group = create(:group, :private)
group.add_developer(user)
-
link = reference_link(group: group.id, reference_type: 'user')
+
doc = filter(link, current_user: user)
expect(doc.css('a').length).to eq 1
@@ -200,8 +242,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
context 'with data-user' do
it 'allows any User reference' do
user = create(:user)
-
link = reference_link(user: user.id, reference_type: 'user')
+
doc = filter(link)
expect(doc.css('a').length).to eq 1
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index ed763f63756..76be2c0cd22 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -165,6 +165,12 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
expect(subject).to eq(Gitlab::Auth::Result.new(build.user, build.project, :build, described_class.build_authentication_abilities))
end
+
+ it 'fails with blocked user token' do
+ build.update(user: create(:user, :blocked))
+
+ expect(subject).to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+ end
end
(HasStatus::AVAILABLE_STATUSES - ['running']).each do |build_status|
@@ -260,6 +266,15 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')
end
+
+ context 'blocked user' do
+ let(:user) { create(:user, :blocked) }
+
+ it 'fails' do
+ expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip'))
+ .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+ end
+ end
end
context 'while using personal access tokens as passwords' do
@@ -308,9 +323,35 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it 'fails if password is nil' do
expect_results_with_abilities(nil, nil, false)
end
+
+ context 'when user is blocked' do
+ let(:user) { create(:user, :blocked) }
+ let(:personal_access_token) { create(:personal_access_token, scopes: ['read_registry'], user: user) }
+
+ before do
+ stub_container_registry_config(enabled: true)
+ end
+
+ it 'fails if user is blocked' do
+ expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip'))
+ .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+ end
+ end
end
context 'while using regular user and password' do
+ it 'fails for a blocked user' do
+ user = create(
+ :user,
+ :blocked,
+ username: 'normal_user',
+ password: 'my-secret'
+ )
+
+ expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip'))
+ .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+ end
+
it 'goes through lfs authentication' do
user = create(
:user,
diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
index 5a930d44dcb..ebd7c7af265 100644
--- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
+++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
@@ -68,6 +68,16 @@ describe Gitlab::Gfm::UploadsRewriter do
expect(moved_text.scan(/\A\[.*?\]/).count).to eq(1)
end
+ context 'path traversal in file name' do
+ let(:text) do
+ "![a](/uploads/11111111111111111111111111111111/../../../../../../../../../../../../../../etc/passwd)"
+ end
+
+ it 'throw an error' do
+ expect { rewriter.rewrite(new_project) }.to raise_error(an_instance_of(StandardError).and having_attributes(message: "Invalid path"))
+ end
+ end
+
context "file are stored locally" do
include_examples "files are accessible"
end
diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb
index c580b46cf8d..f1b5393a2d8 100644
--- a/spec/lib/gitlab/regex_spec.rb
+++ b/spec/lib/gitlab/regex_spec.rb
@@ -3,9 +3,7 @@
require 'spec_helper'
describe Gitlab::Regex do
- describe '.project_name_regex' do
- subject { described_class.project_name_regex }
-
+ shared_examples_for 'project/group name regex' do
it { is_expected.to match('gitlab-ce') }
it { is_expected.to match('GitLab CE') }
it { is_expected.to match('100 lines') }
@@ -15,6 +13,34 @@ describe Gitlab::Regex do
it { is_expected.not_to match('?gitlab') }
end
+ shared_examples_for 'project/group name error message' do
+ it { is_expected.to eq("can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'.") }
+ end
+
+ describe '.project_name_regex' do
+ subject { described_class.project_name_regex }
+
+ it_behaves_like 'project/group name regex'
+ end
+
+ describe '.group_name_regex' do
+ subject { described_class.group_name_regex }
+
+ it_behaves_like 'project/group name regex'
+ end
+
+ describe '.project_name_regex_message' do
+ subject { described_class.project_name_regex_message }
+
+ it_behaves_like 'project/group name error message'
+ end
+
+ describe '.group_name_regex_message' do
+ subject { described_class.group_name_regex_message }
+
+ it_behaves_like 'project/group name error message'
+ end
+
describe '.environment_name_regex' do
subject { described_class.environment_name_regex }
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index d42888e1d54..66df7792a1b 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -48,6 +48,9 @@ describe Group do
describe 'validations' do
it { is_expected.to validate_presence_of :name }
+ it { is_expected.to allow_value('group test_4').for(:name) }
+ it { is_expected.not_to allow_value('test/../foo').for(:name) }
+ it { is_expected.not_to allow_value('<script>alert("Attack!")</script>').for(:name) }
it { is_expected.to validate_presence_of :path }
it { is_expected.not_to validate_presence_of :owner }
it { is_expected.to validate_presence_of :two_factor_grace_period }
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index c0501fb16c6..3f1bc8a2de6 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -536,222 +536,258 @@ describe Issue do
end
describe '#visible_to_user?' do
+ let(:project) { build(:project) }
+ let(:issue) { build(:issue, project: project) }
+ let(:user) { create(:user) }
+
+ subject { issue.visible_to_user?(user) }
+
+ context 'with a project' do
+ it 'returns false when feature is disabled' do
+ project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED)
+
+ is_expected.to eq(false)
+ end
+
+ it 'returns false when restricted for members' do
+ project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE)
+
+ is_expected.to eq(false)
+ end
+ end
+
context 'without a user' do
- let(:issue) { build(:issue) }
+ let(:user) { nil }
it 'returns true when the issue is publicly visible' do
expect(issue).to receive(:publicly_visible?).and_return(true)
- expect(issue.visible_to_user?).to eq(true)
+ is_expected.to eq(true)
end
it 'returns false when the issue is not publicly visible' do
expect(issue).to receive(:publicly_visible?).and_return(false)
- expect(issue.visible_to_user?).to eq(false)
+ is_expected.to eq(false)
end
end
context 'with a user' do
- let(:user) { create(:user) }
- let(:issue) { build(:issue) }
-
- it 'returns true when the issue is readable' do
- expect(issue).to receive(:readable_by?).with(user).and_return(true)
-
- expect(issue.visible_to_user?(user)).to eq(true)
+ shared_examples 'issue readable by user' do
+ it { is_expected.to eq(true) }
end
- it 'returns false when the issue is not readable' do
- expect(issue).to receive(:readable_by?).with(user).and_return(false)
-
- expect(issue.visible_to_user?(user)).to eq(false)
+ shared_examples 'issue not readable by user' do
+ it { is_expected.to eq(false) }
end
- it 'returns false when feature is disabled' do
- expect(issue).not_to receive(:readable_by?)
-
- issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED)
+ shared_examples 'confidential issue readable by user' do
+ specify do
+ issue.confidential = true
- expect(issue.visible_to_user?(user)).to eq(false)
+ is_expected.to eq(true)
+ end
end
- it 'returns false when restricted for members' do
- expect(issue).not_to receive(:readable_by?)
-
- issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE)
+ shared_examples 'confidential issue not readable by user' do
+ specify do
+ issue.confidential = true
- expect(issue.visible_to_user?(user)).to eq(false)
+ is_expected.to eq(false)
+ end
end
- end
-
- describe 'with a regular user that is not a team member' do
- let(:user) { create(:user) }
- context 'using a public project' do
- let(:project) { create(:project, :public) }
+ context 'with an admin user' do
+ let(:user) { build(:admin) }
- it 'returns true for a regular issue' do
- issue = build(:issue, project: project)
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue readable by user'
+ end
- expect(issue.visible_to_user?(user)).to eq(true)
+ context 'with an owner' do
+ before do
+ project.add_maintainer(user)
end
- it 'returns false for a confidential issue' do
- issue = build(:issue, project: project, confidential: true)
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue readable by user'
+ end
- expect(issue.visible_to_user?(user)).to eq(false)
+ context 'with a reporter user' do
+ before do
+ project.add_reporter(user)
end
+
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue readable by user'
end
- context 'using an internal project' do
- let(:project) { create(:project, :internal) }
+ context 'with a guest user' do
+ before do
+ project.add_guest(user)
+ end
- context 'using an internal user' do
- it 'returns true for a regular issue' do
- issue = build(:issue, project: project)
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue not readable by user'
- expect(issue.visible_to_user?(user)).to eq(true)
+ context 'when user is an assignee' do
+ before do
+ issue.update!(assignees: [user])
end
- it 'returns false for a confidential issue' do
- issue = build(:issue, :confidential, project: project)
-
- expect(issue.visible_to_user?(user)).to eq(false)
- end
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue readable by user'
end
- context 'using an external user' do
+ context 'when user is the author' do
before do
- allow(user).to receive(:external?).and_return(true)
- end
-
- it 'returns false for a regular issue' do
- issue = build(:issue, project: project)
-
- expect(issue.visible_to_user?(user)).to eq(false)
+ issue.update!(author: user)
end
- it 'returns false for a confidential issue' do
- issue = build(:issue, :confidential, project: project)
-
- expect(issue.visible_to_user?(user)).to eq(false)
- end
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue readable by user'
end
end
- context 'using a private project' do
- let(:project) { create(:project, :private) }
-
- it 'returns false for a regular issue' do
- issue = build(:issue, project: project)
+ context 'with a user that is not a member' do
+ context 'using a public project' do
+ let(:project) { build(:project, :public) }
- expect(issue.visible_to_user?(user)).to eq(false)
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue not readable by user'
end
- it 'returns false for a confidential issue' do
- issue = build(:issue, :confidential, project: project)
+ context 'using an internal project' do
+ let(:project) { build(:project, :internal) }
- expect(issue.visible_to_user?(user)).to eq(false)
- end
+ context 'using an internal user' do
+ before do
+ allow(user).to receive(:external?).and_return(false)
+ end
- context 'when the user is the project owner' do
- before do
- project.add_maintainer(user)
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue not readable by user'
end
- it 'returns true for a regular issue' do
- issue = build(:issue, project: project)
+ context 'using an external user' do
+ before do
+ allow(user).to receive(:external?).and_return(true)
+ end
- expect(issue.visible_to_user?(user)).to eq(true)
+ it_behaves_like 'issue not readable by user'
+ it_behaves_like 'confidential issue not readable by user'
end
+ end
- it 'returns true for a confidential issue' do
- issue = build(:issue, :confidential, project: project)
-
- expect(issue.visible_to_user?(user)).to eq(true)
+ context 'using an external user' do
+ before do
+ allow(user).to receive(:external?).and_return(true)
end
+
+ it_behaves_like 'issue not readable by user'
+ it_behaves_like 'confidential issue not readable by user'
end
end
- end
-
- context 'with a regular user that is a team member' do
- let(:user) { create(:user) }
- let(:project) { create(:project, :public) }
- context 'using a public project' do
+ context 'with an external authentication service' do
before do
- project.add_developer(user)
+ enable_external_authorization_service_check
end
- it 'returns true for a regular issue' do
- issue = build(:issue, project: project)
+ it 'is `false` when an external authorization service is enabled' do
+ issue = build(:issue, project: build(:project, :public))
- expect(issue.visible_to_user?(user)).to eq(true)
+ expect(issue).not_to be_visible_to_user
end
- it 'returns true for a confidential issue' do
- issue = build(:issue, :confidential, project: project)
-
- expect(issue.visible_to_user?(user)).to eq(true)
- end
- end
-
- context 'using an internal project' do
- let(:project) { create(:project, :internal) }
+ it 'checks the external service to determine if an issue is readable by a user' do
+ project = build(:project, :public,
+ external_authorization_classification_label: 'a-label')
+ issue = build(:issue, project: project)
+ user = build(:user)
- before do
- project.add_developer(user)
+ expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false }
+ expect(issue.visible_to_user?(user)).to be_falsy
end
- it 'returns true for a regular issue' do
+ it 'does not check the external service if a user does not have access to the project' do
+ project = build(:project, :private,
+ external_authorization_classification_label: 'a-label')
issue = build(:issue, project: project)
+ user = build(:user)
- expect(issue.visible_to_user?(user)).to eq(true)
+ expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
+ expect(issue.visible_to_user?(user)).to be_falsy
end
- it 'returns true for a confidential issue' do
- issue = build(:issue, :confidential, project: project)
+ it 'does not check the external webservice for admins' do
+ issue = build(:issue)
+ user = build(:admin)
- expect(issue.visible_to_user?(user)).to eq(true)
+ expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
+
+ issue.visible_to_user?(user)
end
end
- context 'using a private project' do
- let(:project) { create(:project, :private) }
+ context 'when issue is moved to a private project' do
+ let(:private_project) { build(:project, :private)}
before do
- project.add_developer(user)
+ issue.update(project: private_project) # move issue to private project
end
- it 'returns true for a regular issue' do
- issue = build(:issue, project: project)
+ shared_examples 'issue visible if user has guest access' do
+ context 'when user is not a member' do
+ it_behaves_like 'issue not readable by user'
+ it_behaves_like 'confidential issue not readable by user'
+ end
- expect(issue.visible_to_user?(user)).to eq(true)
+ context 'when user is a guest' do
+ before do
+ private_project.add_guest(user)
+ end
+
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue readable by user'
+ end
end
- it 'returns true for a confidential issue' do
- issue = build(:issue, :confidential, project: project)
+ context 'when user is the author of the original issue' do
+ before do
+ issue.update!(author: user)
+ end
- expect(issue.visible_to_user?(user)).to eq(true)
+ it_behaves_like 'issue visible if user has guest access'
end
- end
- end
- context 'with an admin user' do
- let(:project) { create(:project) }
- let(:user) { create(:admin) }
+ context 'when user is an assignee in the original issue' do
+ before do
+ issue.update!(assignees: [user])
+ end
- it 'returns true for a regular issue' do
- issue = build(:issue, project: project)
+ it_behaves_like 'issue visible if user has guest access'
+ end
- expect(issue.visible_to_user?(user)).to eq(true)
- end
+ context 'when user is not the author or an assignee in original issue' do
+ context 'when user is a guest' do
+ before do
+ private_project.add_guest(user)
+ end
- it 'returns true for a confidential issue' do
- issue = build(:issue, :confidential, project: project)
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue not readable by user'
+ end
- expect(issue.visible_to_user?(user)).to eq(true)
+ context 'when user is a reporter' do
+ before do
+ private_project.add_reporter(user)
+ end
+
+ it_behaves_like 'issue readable by user'
+ it_behaves_like 'confidential issue readable by user'
+ end
+ end
end
end
end
@@ -875,49 +911,6 @@ describe Issue do
subject { create(:issue, updated_at: 1.hour.ago) }
end
- context 'when an external authentication service' do
- before do
- enable_external_authorization_service_check
- end
-
- describe '#visible_to_user?' do
- it 'is `false` when an external authorization service is enabled' do
- issue = build(:issue, project: build(:project, :public))
-
- expect(issue).not_to be_visible_to_user
- end
-
- it 'checks the external service to determine if an issue is readable by a user' do
- project = build(:project, :public,
- external_authorization_classification_label: 'a-label')
- issue = build(:issue, project: project)
- user = build(:user)
-
- expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false }
- expect(issue.visible_to_user?(user)).to be_falsy
- end
-
- it 'does not check the external service if a user does not have access to the project' do
- project = build(:project, :private,
- external_authorization_classification_label: 'a-label')
- issue = build(:issue, project: project)
- user = build(:user)
-
- expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
- expect(issue.visible_to_user?(user)).to be_falsy
- end
-
- it 'does not check the external webservice for admins' do
- issue = build(:issue)
- user = build(:admin)
-
- expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
-
- issue.visible_to_user?(user)
- end
- end
- end
-
describe "#labels_hook_attrs" do
let(:label) { create(:label) }
let(:issue) { create(:labeled_issue, labels: [label]) }
diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb
index 89fcf3c10df..242a002bc23 100644
--- a/spec/policies/issue_policy_spec.rb
+++ b/spec/policies/issue_policy_spec.rb
@@ -103,12 +103,24 @@ describe IssuePolicy do
expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
end
+ it 'does not allow issue author to read or update confidential issue moved to an private project' do
+ confidential_issue.project = build(:project, :private)
+
+ expect(permissions(author, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue)
+ end
+
it 'allows issue assignees to read and update their confidential issues' do
expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue)
expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue)
expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
end
+
+ it 'does not allow issue assignees to read or update confidential issue moved to an private project' do
+ confidential_issue.project = build(:project, :private)
+
+ expect(permissions(assignee, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue)
+ end
end
end
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index 35b77832c73..4421f0f1181 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -568,6 +568,20 @@ describe API::Groups do
expect(json_response['shared_projects'].length).to eq(0)
end
+ context 'malicious group name' do
+ subject { put api("/groups/#{group1.id}", user1), params: { name: "<SCRIPT>alert('DOUBLE-ATTACK!')</SCRIPT>" } }
+
+ it 'returns bad request' do
+ subject
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ end
+
+ it 'does not update group name' do
+ expect { subject }.not_to change { group1.reload.name }
+ end
+ end
+
it 'returns 404 for a non existing group' do
put api('/groups/1328', user1), params: { name: new_group_name }
@@ -999,6 +1013,20 @@ describe API::Groups do
expect(json_response["parent_id"]).to eq(parent.id)
end
+ context 'malicious group name' do
+ subject { post api("/groups", user3), params: group_params }
+
+ let(:group_params) { attributes_for_group_api name: "<SCRIPT>alert('ATTACKED!')</SCRIPT>", path: "unique-url" }
+
+ it 'returns bad request' do
+ subject
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ end
+
+ it { expect { subject }.not_to change { Group.count } }
+ end
+
it "does not create group, duplicate" do
post api("/groups", user3), params: { name: 'Duplicate Test', path: group2.path }
diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb
index 2c6a13efc12..b618800857b 100644
--- a/spec/requests/api/project_snippets_spec.rb
+++ b/spec/requests/api/project_snippets_spec.rb
@@ -98,6 +98,30 @@ describe API::ProjectSnippets do
}
end
+ context 'with an external user' do
+ let(:user) { create(:user, :external) }
+
+ context 'that belongs to the project' do
+ before do
+ project.add_developer(user)
+ end
+
+ it 'creates a new snippet' do
+ post api("/projects/#{project.id}/snippets/", user), params: params
+
+ expect(response).to have_gitlab_http_status(201)
+ end
+ end
+
+ context 'that does not belong to the project' do
+ it 'does not create a new snippet' do
+ post api("/projects/#{project.id}/snippets/", user), params: params
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+ end
+
context 'with a regular user' do
let(:user) { create(:user) }
diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb
index 21565265b99..c93f9a34e01 100644
--- a/spec/requests/api/snippets_spec.rb
+++ b/spec/requests/api/snippets_spec.rb
@@ -224,6 +224,16 @@ describe API::Snippets do
it_behaves_like 'snippet creation'
+ context 'with an external user' do
+ let(:user) { create(:user, :external) }
+
+ it 'does not create a new snippet' do
+ post api("/snippets/", user), params: params
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+
it 'returns 400 for missing parameters' do
params.delete(:title)
diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb
index 1042e4e970d..e60318d1c3e 100644
--- a/spec/requests/api/triggers_spec.rb
+++ b/spec/requests/api/triggers_spec.rb
@@ -238,24 +238,44 @@ describe API::Triggers do
end
describe 'PUT /projects/:id/triggers/:trigger_id' do
- context 'authenticated user with valid permissions' do
- let(:new_description) { 'new description' }
+ context 'user is maintainer of the project' do
+ context 'the trigger belongs to user' do
+ let(:new_description) { 'new description' }
- it 'updates description' do
- put api("/projects/#{project.id}/triggers/#{trigger.id}", user),
- params: { description: new_description }
+ it 'updates description' do
+ put api("/projects/#{project.id}/triggers/#{trigger.id}", user),
+ params: { description: new_description }
- expect(response).to have_gitlab_http_status(200)
- expect(json_response).to include('description' => new_description)
- expect(trigger.reload.description).to eq(new_description)
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response).to include('description' => new_description)
+ expect(trigger.reload.description).to eq(new_description)
+ end
+ end
+
+ context 'the trigger does not belong to user' do
+ it 'does not update trigger' do
+ put api("/projects/#{project.id}/triggers/#{trigger2.id}", user)
+
+ expect(response).to have_gitlab_http_status(:forbidden)
+ end
end
end
- context 'authenticated user with invalid permissions' do
- it 'does not update trigger' do
- put api("/projects/#{project.id}/triggers/#{trigger.id}", user2)
+ context 'user is developer of the project' do
+ context 'the trigger belongs to user' do
+ it 'does not update trigger' do
+ put api("/projects/#{project.id}/triggers/#{trigger2.id}", user2)
- expect(response).to have_gitlab_http_status(403)
+ expect(response).to have_gitlab_http_status(:forbidden)
+ end
+ end
+
+ context 'the trigger does not belong to user' do
+ it 'does not update trigger' do
+ put api("/projects/#{project.id}/triggers/#{trigger.id}", user2)
+
+ expect(response).to have_gitlab_http_status(:forbidden)
+ end
end
end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index 754ab3e6a45..73dc9d8c63e 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -25,6 +25,17 @@ describe JwtController do
end
context 'when using authenticated request' do
+ shared_examples 'rejecting a blocked user' do
+ context 'with blocked user' do
+ let(:user) { create(:user, :blocked) }
+
+ it 'rejects the request as unauthorized' do
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ expect(response.body).to include('HTTP Basic: Access denied')
+ end
+ end
+ end
+
context 'using CI token' do
let(:build) { create(:ci_build, :running) }
let(:project) { build.project }
@@ -61,6 +72,8 @@ describe JwtController do
expect(response).to have_gitlab_http_status(:ok)
expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!)
end
+
+ it_behaves_like 'rejecting a blocked user'
end
end
@@ -72,6 +85,8 @@ describe JwtController do
it { expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!) }
+ it_behaves_like 'rejecting a blocked user'
+
context 'when passing a flat array of scopes' do
# We use this trick to make rails to generate a query_string:
# scope=scope1&scope=scope2
diff --git a/spec/serializers/merge_request_poll_widget_entity_spec.rb b/spec/serializers/merge_request_poll_widget_entity_spec.rb
index 0593dd527cc..29d35fdc811 100644
--- a/spec/serializers/merge_request_poll_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_poll_widget_entity_spec.rb
@@ -138,7 +138,7 @@ describe MergeRequestPollWidgetEntity do
end
describe 'pipeline' do
- let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
+ let!(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
before do
allow_any_instance_of(MergeRequestPresenter).to receive(:can?).and_call_original
@@ -158,6 +158,10 @@ describe MergeRequestPollWidgetEntity do
expect(subject[:pipeline]).to eq(pipeline_payload)
end
+
+ it 'returns ci_status' do
+ expect(subject[:ci_status]).to eq('pending')
+ end
end
context 'when is not up to date' do
@@ -171,10 +175,15 @@ describe MergeRequestPollWidgetEntity do
context 'when user does not have access to pipelines' do
let(:result) { false }
+ let(:req) { double('request', current_user: user, project: project) }
it 'does not have pipeline' do
expect(subject[:pipeline]).to eq(nil)
end
+
+ it 'does not return ci_status' do
+ expect(subject[:ci_status]).to eq(nil)
+ end
end
end
end
diff --git a/spec/support/helpers/workhorse_helpers.rb b/spec/support/helpers/workhorse_helpers.rb
index e0fba191deb..0de8b382403 100644
--- a/spec/support/helpers/workhorse_helpers.rb
+++ b/spec/support/helpers/workhorse_helpers.rb
@@ -76,7 +76,7 @@ module WorkhorseHelpers
"#{key}.size" => file.size
}.tap do |params|
params["#{key}.path"] = file.path if file.path
- params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id
+ params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id.present?
end
end
diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb
index 2f2ed28891a..2b9f55aa8f2 100644
--- a/spec/uploaders/object_storage_spec.rb
+++ b/spec/uploaders/object_storage_spec.rb
@@ -714,6 +714,19 @@ describe ObjectStorage do
end
end
+ context 'when empty remote_id is specified' do
+ let(:uploaded_file) do
+ UploadedFile.new(temp_file.path, remote_id: '')
+ end
+
+ it 'uses local storage' do
+ subject
+
+ expect(uploader).to be_file_storage
+ expect(uploader.object_store).to eq(described_class::Store::LOCAL)
+ end
+ end
+
context 'when valid file is specified' do
let(:uploaded_file) do
UploadedFile.new(temp_file.path, filename: "my_file.txt", remote_id: "test/123123")