realaravinth/debian-mirror-gitlab
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

New upstream version 14.2.6+ds1

Browse source
This commit is contained in:
Pirate Praveen 2021-10-29 20:43:33 +05:30
parent edb4c77020
commit ee1142ef34
76 changed files with 859 additions and 106 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
CHANGELOG.md
View file

@ -2,6 +2,24 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
## 14.2.6 (2021-10-28)
### Security (13 changes)
- [Highlight usage of unicode bidi characters](gitlab-org/security/gitlab@18a768bb3cd19b6dc780bb85d91a93605ec8aa4f) ([merge request](gitlab-org/security/gitlab!1939))
- [Fix dompurify.js to prevent path traversal attacks](gitlab-org/security/gitlab@cfd7c715162c22060b9b80268ef501a9e604421a) ([merge request](gitlab-org/security/gitlab!1931))
- [Refresh authorizations on transfer of groups having project shares](gitlab-org/security/gitlab@3fc08eb869156a090b015e78da79c8ced16a7162) ([merge request](gitlab-org/security/gitlab!1918))
- [Do not allow Applications API to create apps with blank scopes](gitlab-org/security/gitlab@c4ffc8c0ee5356bcb9b76dbfa92517589b4225a8) ([merge request](gitlab-org/security/gitlab!1924))
- [Don't allow author to resolve discussions when MR is locked via GraphQL](gitlab-org/security/gitlab@fe2d0b6f250b60619da97f162c93c9e645daf4af) ([merge request](gitlab-org/security/gitlab!1921))
- [Workhorse: Allow uploading only a single file](gitlab-org/security/gitlab@89b04599592b7dfc0e4883cfde5d3ecd9ea855b2) ([merge request](gitlab-org/security/gitlab!1915))
- [Group owners should see SCIM token only once](gitlab-org/security/gitlab@d52c1e41f38039db075a7a3418b8eb9ed8474c2a) ([merge request](gitlab-org/security/gitlab!1908)) **GitLab Enterprise Edition**
- [Respect visibility level settings when updating project via API](gitlab-org/security/gitlab@3051d6a00d1a56133a77ecd24313bafb4565d576) ([merge request](gitlab-org/security/gitlab!1905))
- [Avoid decoding the whole tiff image on isTIFF check](gitlab-org/security/gitlab@bab7f45def8fc81fe4b0961a21b4c90a60358ff9) ([merge request](gitlab-org/security/gitlab!1901))
- [Adding a '[redacted]' to mask private email addresses](gitlab-org/security/gitlab@8eb9749f40b87b9b49b034bceb263219a4d3b114) ([merge request](gitlab-org/security/gitlab!1895))
- [Do not display the root password by default](gitlab-org/security/gitlab@4ccf08b6645b9f616657edd266d9d31e3602d170) ([merge request](gitlab-org/security/gitlab!1802))
- [Set PipelineSchedules to inactive](gitlab-org/security/gitlab@ebee16945325d22ceb5c07b7ba48df6fd0b2f067) ([merge request](gitlab-org/security/gitlab!1878))
- [Remove external_webhook_token from exported project](gitlab-org/security/gitlab@f3ef12185902f3ed5c9d62ffce07418fd704a753) ([merge request](gitlab-org/security/gitlab!1865))
## 14.2.5 (2021-09-30)
### Security (28 changes)

2
GITALY_SERVER_VERSION
View file

@ -1 +1 @@
14.2.5
14.2.6

2
VERSION
View file

@ -1 +1 @@
14.2.5
14.2.6

19
app/assets/javascripts/lib/dompurify.js
View file

@ -1,5 +1,5 @@
import { sanitize as dompurifySanitize, addHook } from 'dompurify';
import { getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility';
import { getNormalizedURL, getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility';
const defaultConfig = {
// Safely allow SVG <use> tags
@ -11,12 +11,14 @@ const defaultConfig = {
// Only icons urls from `gon` are allowed
const getAllowedIconUrls = (gon = window.gon) =>
[gon.sprite_file_icons, gon.sprite_icons].filter(Boolean);
[gon.sprite_file_icons, gon.sprite_icons]
.filter(Boolean)
.map((path) => relativePathToAbsolute(path, getBaseURL()));
const isUrlAllowed = (url) => getAllowedIconUrls().some((allowedUrl) => url.startsWith(allowedUrl));
const isUrlAllowed = (url) =>
getAllowedIconUrls().some((allowedUrl) => getNormalizedURL(url).startsWith(allowedUrl));
const isHrefSafe = (url) =>
isUrlAllowed(url) || isUrlAllowed(relativePathToAbsolute(url, getBaseURL()));
const isHrefSafe = (url) => url.match(/^#/) || isUrlAllowed(url);
const removeUnsafeHref = (node, attr) => {
if (!node.hasAttribute(attr)) {
@ -36,13 +38,14 @@ const removeUnsafeHref = (node, attr) => {
* <use href="/assets/icons-xxx.svg#icon_name"></use>
* </svg>
*
* It validates both href & xlink:href attributes.
* Note that `xlink:href` is deprecated, but still in use
* https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href
*
* @param {Object} node - Node to sanitize
*/
const sanitizeSvgIcon = (node) => {
removeUnsafeHref(node, 'href');
// Note: `xlink:href` is deprecated, but still in use
// https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href
removeUnsafeHref(node, 'xlink:href');
};

18
app/assets/javascripts/lib/utils/url_utility.js
View file

@ -397,6 +397,24 @@ export function isSafeURL(url) {
}
}
/**
* Returns a normalized url
*
* https://gitlab.com/foo/../baz => https://gitlab.com/baz
*
* @param {String} url - URL to be transformed
* @param {String?} baseUrl - current base URL
* @returns {String}
*/
export const getNormalizedURL = (url, baseUrl) => {
const base = baseUrl || getBaseURL();
try {
return new URL(url, base).href;
} catch (e) {
return '';
}
};
export function getWebSocketProtocol() {
return window.location.protocol.replace('http', 'ws');
}

19
app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
View file

@ -37,6 +37,10 @@ export default {
securityAndComplianceLabel: s__('ProjectSettings|Security & Compliance'),
snippetsLabel: s__('ProjectSettings|Snippets'),
wikiLabel: s__('ProjectSettings|Wiki'),
pucWarningLabel: s__('ProjectSettings|Warn about Potentially Unwanted Characters'),
pucWarningHelpText: s__(
'ProjectSettings|Highlight the usage of hidden unicode characters. These have innocent uses for right-to-left languages, but can also be used in potential exploits.',
),
},
components: {
@ -178,6 +182,7 @@ export default {
securityAndComplianceAccessLevel: featureAccessLevel.PROJECT_MEMBERS,
operationsAccessLevel: featureAccessLevel.EVERYONE,
containerRegistryAccessLevel: featureAccessLevel.EVERYONE,
warnAboutPotentiallyUnwantedCharacters: true,
lfsEnabled: true,
requestAccessEnabled: true,
highlightChangesClass: false,
@ -752,5 +757,19 @@ export default {
}}</template>
</gl-form-checkbox>
</project-setting-row>
<project-setting-row class="gl-mb-5">
<input
:value="warnAboutPotentiallyUnwantedCharacters"
type="hidden"
name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"
/>
<gl-form-checkbox
v-model="warnAboutPotentiallyUnwantedCharacters"
name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"
>
{{ $options.i18n.pucWarningLabel }}
<template #help>{{ $options.i18n.pucWarningHelpText }}</template>
</gl-form-checkbox>
</project-setting-row>
</div>
</template>

8
app/assets/javascripts/snippets/components/show.vue
View file

@ -66,7 +66,13 @@ export default {
data-qa-selector="clone_button"
/>
</div>
<snippet-blob v-for="blob in blobs" :key="blob.path" :snippet="snippet" :blob="blob" />
<snippet-blob
v-for="blob in blobs"
:key="blob.path"
:snippet="snippet"
:blob="blob"
class="project-highlight-puc"
/>
</template>
</div>
</template>

6
app/assets/stylesheets/framework/highlight.scss
View file

@ -85,3 +85,9 @@
td.line-numbers {
line-height: 1;
}
.project-highlight-puc .unicode-bidi::before {
content: '<EFBFBD>';
cursor: pointer;
text-decoration: underline wavy $red-500;
}

1
app/controllers/projects_controller.rb
View file

@ -398,6 +398,7 @@ class ProjectsController < Projects::ApplicationController
show_default_award_emojis
squash_option
mr_default_target_self
warn_about_potentially_unwanted_characters
]
end

2
app/graphql/mutations/issues/set_severity.rb
View file

@ -8,6 +8,8 @@ module Mutations
argument :severity, Types::IssuableSeverityEnum, required: true,
description: 'Set the incident severity level.'
authorize :admin_issue
def resolve(project_path:, iid:, severity:)
issue = authorized_find!(project_path: project_path, iid: iid)
project = issue.project

7
app/helpers/projects_helper.rb
View file

@ -377,6 +377,12 @@ module ProjectsHelper
}
end
def project_classes(project)
return "project-highlight-puc" if project.warn_about_potentially_unwanted_characters?
""
end
private
def tab_ability_map
@ -533,6 +539,7 @@ module ProjectsHelper
metricsDashboardAccessLevel: feature.metrics_dashboard_access_level,
operationsAccessLevel: feature.operations_access_level,
showDefaultAwardEmojis: project.show_default_award_emojis?,
warnAboutPotentiallyUnwantedCharacters: project.warn_about_potentially_unwanted_characters?,
securityAndComplianceAccessLevel: project.security_and_compliance_access_level,
containerRegistryAccessLevel: feature.container_registry_access_level
}

3
app/models/concerns/resolvable_discussion.rb
View file

@ -81,8 +81,7 @@ module ResolvableDiscussion
return false unless current_user
return false unless resolvable?
current_user == self.noteable.try(:author) ||
current_user.can?(:resolve_note, self.project)
current_user.can?(:resolve_note, self.noteable)
end
def resolve!(current_user)

9
app/models/namespace.rb
View file

@ -26,6 +26,8 @@ class Namespace < ApplicationRecord
SHARED_RUNNERS_SETTINGS = %w[disabled_and_unoverridable disabled_with_override enabled].freeze
URL_MAX_LENGTH = 255
PATH_TRAILING_VIOLATIONS = %w[.git .atom .].freeze
cache_markdown_field :description, pipeline: :description
has_many :projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
@ -162,7 +164,12 @@ class Namespace < ApplicationRecord
# Remove everything that's not in the list of allowed characters.
path.gsub!(/[^a-zA-Z0-9_\-\.]/, "")
# Remove trailing violations ('.atom', '.git', or '.')
path.gsub!(/(\.atom|\.git|\.)*\z/, "")
loop do
orig = path
PATH_TRAILING_VIOLATIONS.each { |ext| path = path.chomp(ext) }
break if orig == path
end
# Remove leading violations ('-')
path.gsub!(/\A\-+/, "")

19
app/models/project.rb
View file

@ -412,8 +412,8 @@ class Project < ApplicationRecord
:container_registry_access_level, :container_registry_enabled?,
to: :project_feature, allow_nil: true
alias_method :container_registry_enabled, :container_registry_enabled?
delegate :show_default_award_emojis, :show_default_award_emojis=,
:show_default_award_emojis?,
delegate :show_default_award_emojis, :show_default_award_emojis=, :show_default_award_emojis?,
:warn_about_potentially_unwanted_characters, :warn_about_potentially_unwanted_characters=, :warn_about_potentially_unwanted_characters?,
to: :project_setting, allow_nil: true
delegate :scheduled?, :started?, :in_progress?, :failed?, :finished?,
prefix: :import, to: :import_state, allow_nil: true
@ -2669,8 +2669,23 @@ class Project < ApplicationRecord
ci_cd_settings.group_runners_enabled?
end
def visible_group_links(for_user:)
user = for_user
links = project_group_links_with_preload
user.max_member_access_for_group_ids(links.map(&:group_id)) if user && links.any?
DeclarativePolicy.user_scope do
links.select { Ability.allowed?(user, :read_group, _1.group) }
end
end
private
# overridden in EE
def project_group_links_with_preload
project_group_links
end
def find_integration(integrations, name)
integrations.find { _1.to_param == name }
end

1
app/models/project_group_link.rb
View file

@ -15,6 +15,7 @@ class ProjectGroupLink < ApplicationRecord
validate :different_group
scope :non_guests, -> { where('group_access > ?', Gitlab::Access::GUEST) }
scope :in_group, -> (group_ids) { where(group_id: group_ids) }
alias_method :shared_with_group, :group

2
app/models/user.rb
View file

@ -1459,7 +1459,7 @@ class User < ApplicationRecord
name: name,
username: username,
avatar_url: avatar_url(only_path: false),
email: email
email: public_email.presence || _('[REDACTED]')
}
end

6
app/policies/issuable_policy.rb
View file

@ -11,6 +11,8 @@ class IssuablePolicy < BasePolicy
@user && @subject.assignee_or_author?(@user)
end
condition(:is_author) { @subject&.author == @user }
rule { can?(:guest_access) & assignee_or_author }.policy do
enable :read_issue
enable :update_issue
@ -20,6 +22,10 @@ class IssuablePolicy < BasePolicy
enable :reopen_merge_request
end
rule { is_author }.policy do
enable :resolve_note
end
rule { locked & ~is_project_member }.policy do
prevent :create_note
prevent :admin_note

1
app/policies/project_policy.rb
View file

@ -221,6 +221,7 @@ class ProjectPolicy < BasePolicy
enable :set_note_created_at
enable :set_emails_disabled
enable :set_show_default_award_emojis
enable :set_warn_about_potentially_unwanted_characters
end
rule { can?(:guest_access) }.policy do

14
app/services/concerns/update_visibility_level.rb
View file

@ -1,13 +1,17 @@
# frozen_string_literal: true
module UpdateVisibilityLevel
def valid_visibility_level_change?(target, new_visibility)
# check that user is allowed to set specified visibility_level
if new_visibility && new_visibility.to_i != target.visibility_level
unless can?(current_user, :change_visibility_level, target) &&
Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
def valid_visibility_level_change?(target, new_visibility)
return true unless new_visibility
deny_visibility_level(target, new_visibility)
new_visibility_level = Gitlab::VisibilityLevel.level_value(new_visibility)
if new_visibility_level != target.visibility_level_value
unless can?(current_user, :change_visibility_level, target) &&
Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility_level)
deny_visibility_level(target, new_visibility_level)
return false
end
end

8
app/services/groups/transfer_service.rb
View file

@ -177,6 +177,14 @@ module Groups
# schedule refreshing projects for all the members of the group
@group.refresh_members_authorized_projects
# When a group is transferred, it also affects who gets access to the projects shared to
# the subgroups within its hierarchy, so we also schedule jobs that refresh authorizations for all such shared projects.
project_group_shares_within_the_hierarchy = ProjectGroupLink.in_group(group.self_and_descendants.select(:id))
project_group_shares_within_the_hierarchy.find_each do |project_group_link|
AuthorizedProjectUpdate::ProjectRecalculateWorker.perform_async(project_group_link.project_id)
end
end
def raise_transfer_error(message)

4
app/services/groups/update_service.rb
View file

@ -15,7 +15,7 @@ module Groups
return false
end
return false unless valid_visibility_level_change?(group, params[:visibility_level])
return false unless valid_visibility_level_change?(group, group.visibility_attribute_value(params))
return false unless valid_share_with_group_lock_change?
@ -77,7 +77,7 @@ module Groups
end
def after_update
if group.previous_changes.include?(:visibility_level) && group.private?
if group.previous_changes.include?(group.visibility_level_field) && group.private?
# don't enqueue immediately to prevent todos removal in case of a mistake
TodosDestroyer::GroupPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, group.id)
end

1
app/services/issuable_base_service.rb
View file

@ -139,6 +139,7 @@ class IssuableBaseService < ::BaseProjectService
def filter_severity(issuable)
severity = params.delete(:severity)
return unless severity && issuable.supports_severity?
return unless can_admin_issuable?(issuable)
severity = IssuableSeverity::DEFAULT unless IssuableSeverity.severities.key?(severity)
return if severity == issuable.severity

2
app/services/projects/update_service.rb
View file

@ -49,7 +49,7 @@ module Projects
private
def validate!
unless valid_visibility_level_change?(project, params[:visibility_level])
unless valid_visibility_level_change?(project, project.visibility_attribute_value(params))
raise ValidationError, s_('UpdateProject|New visibility level not allowed!')
end

1
app/views/layouts/project.html.haml
View file

@ -6,6 +6,7 @@
- display_subscription_banner!
- display_namespace_storage_limit_alert!
- @left_sidebar = true
- @content_class = [@content_class, project_classes(@project)].compact.join(" ")
- content_for :project_javascripts do
- project = @target_project || @project

6
config/gitlab.yml.example
View file

@ -176,8 +176,10 @@ production: &base
## Application settings cache expiry in seconds (default: 60)
# application_settings_cache_seconds: 60
## Print initial root password to stdout during initialization (default: true)
# display_initial_root_password: true
## Print initial root password to stdout during initialization (default: false)
# WARNING: setting this to true means that the root password will be printed in
# plaintext. This can be a security risk.
# display_initial_root_password: false
## Reply by email
# Allow users to comment on issues and merge requests by replying to notification emails.

3
config/initializers/1_settings.rb
View file

@ -217,8 +217,7 @@ Settings.gitlab['no_todos_messages'] ||= YAML.load_file(Rails.root.join('config'
Settings.gitlab['impersonation_enabled'] ||= true if Settings.gitlab['impersonation_enabled'].nil?
Settings.gitlab['usage_ping_enabled'] = true if Settings.gitlab['usage_ping_enabled'].nil?
Settings.gitlab['max_request_duration_seconds'] ||= 57
Settings.gitlab['display_initial_root_password'] = true if Settings.gitlab['display_initial_root_password'].nil?
Settings.gitlab['display_initial_root_password'] = false if Settings.gitlab['display_initial_root_password'].nil?
Gitlab.ee do
Settings.gitlab['mirror_max_delay'] ||= 300

2
db/fixtures/production/002_admin.rb
View file

@ -26,7 +26,7 @@ if user.persisted?
if ::Settings.gitlab['display_initial_root_password']
puts "password: #{user_args[:password]}".color(:green)
else
puts "password: *** - You opted not to display initial root password to STDOUT."
puts "password: ******".color(:green)
end
else
puts "password: You'll be prompted to create one on your first visit.".color(:green)

22
db/migrate/20210929144453_add_warn_about_potentially_unwanted_characters_to_project_settings.rb Normal file
View file

@ -0,0 +1,22 @@
# frozen_string_literal: true
# See https://docs.gitlab.com/ee/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddWarnAboutPotentiallyUnwantedCharactersToProjectSettings < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
add_column :project_settings, :warn_about_potentially_unwanted_characters, :boolean, null: false, default: true
end
end
def down
with_lock_retries do
remove_column :project_settings, :warn_about_potentially_unwanted_characters
end
end
end

1
db/schema_migrations/20210929144453 Normal file
View file

@ -0,0 +1 @@
0f808c27d19e6a38d4aa31f2dd820fe226681af84e05c4af47213409b2043e5a

1
db/structure.sql
View file

@ -17302,6 +17302,7 @@ CREATE TABLE project_settings (
cve_id_request_enabled boolean DEFAULT true NOT NULL,
mr_default_target_self boolean DEFAULT false NOT NULL,
previous_default_branch text,
warn_about_potentially_unwanted_characters boolean DEFAULT true NOT NULL,
CONSTRAINT check_3a03e7557a CHECK ((char_length(previous_default_branch) <= 4096)),
CONSTRAINT check_bde223416c CHECK ((show_default_award_emojis IS NOT NULL))
);

2
lib/api/applications.rb
View file

@ -15,7 +15,7 @@ module API
params do
requires :name, type: String, desc: 'Application name'
requires :redirect_uri, type: String, desc: 'Application redirect URI'
requires :scopes, type: String, desc: 'Application scopes'
requires :scopes, type: String, desc: 'Application scopes', allow_blank: false
optional :confidential, type: Boolean, default: true,
desc: 'Application will be used where the client secret is confidential'

4
lib/api/entities/project.rb
View file

@ -100,7 +100,9 @@ module API
expose :build_coverage_regex
expose :ci_config_path, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) }
expose :shared_with_groups do |project, options|
SharedGroupWithProject.represent(project.project_group_links, options)
user = options[:current_user]
SharedGroupWithProject.represent(project.visible_group_links(for_user: user), options)
end
expose :only_allow_merge_if_pipeline_succeeds
expose :allow_merge_on_skipped_pipeline

1
lib/api/helpers/projects_helpers.rb
View file

@ -39,6 +39,7 @@ module API
optional :emails_disabled, type: Boolean, desc: 'Disable email notifications'
optional :show_default_award_emojis, type: Boolean, desc: 'Show default award emojis'
optional :warn_about_potentially_unwanted_characters, type: Boolean, desc: 'Warn about Potentially Unwanted Characters'
optional :shared_runners_enabled, type: Boolean, desc: 'Flag indication if shared runners are enabled for that project'
optional :resolve_outdated_diff_discussions, type: Boolean, desc: 'Automatically resolve merge request diffs discussions on lines changed with a push'
optional :remove_source_branch_after_merge, type: Boolean, desc: 'Remove the source branch by default after merge'

3
lib/api/project_import.rb
View file

@ -10,6 +10,8 @@ module API
feature_category :importers
before { authenticate! unless route.settings[:skip_authentication] }
helpers do
def import_params
declared_params(include_missing: false)
@ -110,6 +112,7 @@ module API
detail 'This feature was introduced in GitLab 10.6.'
success Entities::ProjectImportStatus
end
route_setting :skip_authentication, true
get ':id/import' do
present user_project, with: Entities::ProjectImportStatus
end

2
lib/api/projects.rb
View file

@ -423,7 +423,7 @@ module API
authorize_admin_project
attrs = declared_params(include_missing: false)
authorize! :rename_project, user_project if attrs[:name].present?
authorize! :change_visibility_level, user_project if attrs[:visibility].present?
authorize! :change_visibility_level, user_project if user_project.visibility_attribute_present?(attrs)
attrs = translate_params_for_compatibility(attrs)
filter_attributes_using_license!(attrs)

4
lib/gitlab/background_migration/user_mentions/models/group.rb
View file

@ -11,6 +11,10 @@ module Gitlab
has_one :saml_provider
def root_saml_provider
root_ancestor.saml_provider
end
def self.declarative_policy_class
"GroupPolicy"
end

4
lib/gitlab/import_export/project/import_export.yml
View file

@ -171,9 +171,11 @@ excluded_attributes:
- :marked_for_deletion_by_user_id
- :compliance_framework_setting
- :show_default_award_emojis
- :warn_about_potentially_unwanted_characters
- :services
- :exported_protected_branches
- :repository_size_limit
- :external_webhook_token
namespaces:
- :runners_token
- :runners_token_encrypted
@ -377,6 +379,8 @@ excluded_attributes:
system_note_metadata:
- :description_version_id
- :note_id
pipeline_schedules:
- :active
methods:
notes:
- :type

5
lib/gitlab/import_export/project/relation_factory.rb
View file

@ -84,6 +84,7 @@ module Gitlab
when :'Ci::Pipeline' then setup_pipeline
when *BUILD_MODELS then setup_build
when :issues then setup_issue
when :'Ci::PipelineSchedule' then setup_pipeline_schedule
end
update_project_references
@ -143,6 +144,10 @@ module Gitlab
@relation_hash['relative_position'] = compute_relative_position
end
def setup_pipeline_schedule
@relation_hash['active'] = false
end
def compute_relative_position
return unless max_relative_position

19
lib/gitlab/unicode.rb Normal file
View file

@ -0,0 +1,19 @@
# frozen_string_literal: true
module Gitlab
class Unicode
# Regular expression for identifying bidirectional control
# characters in UTF-8 strings
#
# Documentation on how this works:
# https://idiosyncratic-ruby.com/41-proper-unicoding.html
BIDI_REGEXP = /\p{Bidi Control}/.freeze
class << self
# Warning message used to highlight bidi characters in the GUI
def bidi_warning
_("Potentially unwanted character detected: Unicode BiDi Control")
end
end
end
end

8
lib/gitlab/visibility_level.rb
View file

@ -155,6 +155,14 @@ module Gitlab
false
end
def visibility_attribute_value(attributes)
visibility_level_attributes.each do |attr|
return attributes[attr] if attributes.has_key?(attr)
end
nil
end
def visibility_level_attributes
[visibility_level_field, visibility_level_field.to_s,
:visibility, 'visibility']

14
lib/rouge/formatters/html_gitlab.rb
View file

@ -21,12 +21,24 @@ module Rouge
is_first = false
yield %(<span id="LC#{@line_number}" class="line" lang="#{@tag}">)
line.each { |token, value| yield span(token, value.chomp! || value) }
line.each do |token, value|
yield highlight_unicode_control_characters(span(token, value.chomp! || value))
end
yield %(</span>)
@line_number += 1
end
end
private
def highlight_unicode_control_characters(text)
text.gsub(Gitlab::Unicode::BIDI_REGEXP) do |char|
%(<span class="unicode-bidi has-tooltip" data-toggle="tooltip" title="#{Gitlab::Unicode.bidi_warning}">#{char}</span>)
end
end
end
end
end

12
locale/gitlab.pot
View file

@ -25164,6 +25164,9 @@ msgstr ""
msgid "Postman collection file path or URL"
msgstr ""
msgid "Potentially unwanted character detected: Unicode BiDi Control"
msgstr ""
msgid "Pre-defined push rules."
msgstr ""
@ -26253,6 +26256,9 @@ msgstr ""
msgid "ProjectSettings|Global"
msgstr ""
msgid "ProjectSettings|Highlight the usage of hidden unicode characters. These have innocent uses for right-to-left languages, but can also be used in potential exploits."
msgstr ""
msgid "ProjectSettings|Internal"
msgstr ""
@ -26442,6 +26448,9 @@ msgstr ""
msgid "ProjectSettings|Visualize the project's performance metrics."
msgstr ""
msgid "ProjectSettings|Warn about Potentially Unwanted Characters"
msgstr ""
msgid "ProjectSettings|What are badges?"
msgstr ""
@ -38764,6 +38773,9 @@ msgstr ""
msgid "[No reason]"
msgstr ""
msgid "[REDACTED]"
msgstr ""
msgid "[Redacted]"
msgstr ""

28
spec/controllers/projects_controller_spec.rb
View file

@ -312,6 +312,34 @@ RSpec.describe ProjectsController do
expect { get_show }.not_to change { Gitlab::GitalyClient.get_request_count }
end
describe "PUC highlighting" do
render_views
before do
expect(controller).to receive(:find_routable!).and_return(public_project)
end
context "option is enabled" do
it "adds the highlighting class" do
expect(public_project).to receive(:warn_about_potentially_unwanted_characters?).and_return(true)
get_show
expect(response.body).to have_css(".project-highlight-puc")
end
end
context "option is disabled" do
it "doesn't add the highlighting class" do
expect(public_project).to receive(:warn_about_potentially_unwanted_characters?).and_return(false)
get_show
expect(response.body).not_to have_css(".project-highlight-puc")
end
end
end
end
context "when the url contains .atom" do

4
spec/factories/users.rb
View file

@ -19,6 +19,10 @@ FactoryBot.define do
admin { true }
end
trait :public_email do
public_email { email }
end
trait :blocked do
after(:build) { |user, _| user.block! }
end

6
spec/frontend/lib/dompurify_spec.js
View file

@ -22,12 +22,16 @@ const safeUrls = {
const unsafeUrls = [
'/an/evil/url',
'../../../evil/url',
'https://evil.url/assets/icons-123a.svg',
'https://evil.url/assets/icons-123a.svg#test',
'https://evil.url/assets/icons-456b.svg',
`https://evil.url/${rootGon.sprite_icons}`,
`https://evil.url/${rootGon.sprite_file_icons}`,
`https://evil.url/${absoluteGon.sprite_icons}`,
`https://evil.url/${absoluteGon.sprite_file_icons}`,
`${rootGon.sprite_icons}/../evil/path`,
`${rootGon.sprite_file_icons}/../../evil/path`,
`${absoluteGon.sprite_icons}/../evil/path`,
`${absoluteGon.sprite_file_icons}/../../https://evil.url`,
];
const forbiddenDataAttrs = ['data-remote', 'data-url', 'data-type', 'data-method'];

21
spec/frontend/lib/utils/url_utility_spec.js
View file

@ -607,6 +607,27 @@ describe('URL utility', () => {
});
});
describe('getNormalizedURL', () => {
it.each`
url | base | result
${'./foo'} | ${''} | ${'http://test.host/foo'}
${'../john.md'} | ${''} | ${'http://test.host/john.md'}
${'/images/img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/images/img.png'}
${'/images/../img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/img.png'}
${'/images/./img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/images/img.png'}
${'./images/img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/user/images/img.png'}
${'../images/../img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/img.png'}
${'/images/img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/images/img.png'}
${'/images/../img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/img.png'}
${'/images/./img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/images/img.png'}
`(
'converts url "$url" with base "$base" to normalized url => "expected"',
({ url, base, result }) => {
expect(urlUtils.getNormalizedURL(url, base)).toBe(result);
},
);
});
describe('getWebSocketProtocol', () => {
it.each`
protocol | expectation

13
spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
View file

@ -27,6 +27,7 @@ const defaultProps = {
emailsDisabled: false,
packagesEnabled: true,
showDefaultAwardEmojis: true,
warnAboutPotentiallyUnwantedCharacters: true,
},
isGitlabCom: true,
canDisableEmails: true,
@ -97,6 +98,10 @@ describe('Settings Panel', () => {
const findEmailSettings = () => wrapper.find({ ref: 'email-settings' });
const findShowDefaultAwardEmojis = () =>
wrapper.find('input[name="project[project_setting_attributes][show_default_award_emojis]"]');
const findWarnAboutPuc = () =>
wrapper.find(
'input[name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"]',
);
const findMetricsVisibilitySettings = () => wrapper.find({ ref: 'metrics-visibility-settings' });
const findOperationsSettings = () => wrapper.find({ ref: 'operations-settings' });
@ -539,6 +544,14 @@ describe('Settings Panel', () => {
});
});
describe('Warn about potentially unwanted characters', () => {
it('should have a "Warn about Potentially Unwanted Characters" input', () => {
wrapper = mountComponent();
expect(findWarnAboutPuc().exists()).toBe(true);
});
});
describe('Metrics dashboard', () => {
it('should show the metrics dashboard access toggle', () => {
wrapper = mountComponent();

22
spec/graphql/mutations/discussions/toggle_resolve_spec.rb
View file

@ -7,6 +7,7 @@ RSpec.describe Mutations::Discussions::ToggleResolve do
described_class.new(object: nil, context: { current_user: user }, field: nil)
end
let_it_be(:author) { create(:user) }
let_it_be(:project) { create(:project, :repository) }
describe '#resolve' do
@ -136,20 +137,37 @@ RSpec.describe Mutations::Discussions::ToggleResolve do
end
end
end
context 'when user is the author and discussion is locked' do
let(:user) { author }
before do
issuable.update!(discussion_locked: true)
end
it 'raises an error' do
expect { mutation.resolve(id: id_arg, resolve: resolve_arg) }.to raise_error(
Gitlab::Graphql::Errors::ResourceNotAvailable,
"The resource that you are attempting to access does not exist or you don't have permission to perform this action"
)
end
end
end
context 'when discussion is on a merge request' do
let_it_be(:noteable) { create(:merge_request, source_project: project) }
let_it_be(:noteable) { create(:merge_request, source_project: project, author: author) }
let(:discussion) { create(:diff_note_on_merge_request, noteable: noteable, project: project).to_discussion }
let(:issuable) { noteable }
it_behaves_like 'a working resolve method'
end
context 'when discussion is on a design' do
let_it_be(:noteable) { create(:design, :with_file, issue: create(:issue, project: project)) }
let_it_be(:noteable) { create(:design, :with_file, issue: create(:issue, project: project, author: author)) }
let(:discussion) { create(:diff_note_on_design, noteable: noteable, project: project).to_discussion }
let(:issuable) { noteable.issue }
it_behaves_like 'a working resolve method'
end

58
spec/graphql/mutations/issues/set_severity_spec.rb
View file

@ -3,26 +3,58 @@
require 'spec_helper'
RSpec.describe Mutations::Issues::SetSeverity do
let_it_be(:user) { create(:user) }
let_it_be(:issue) { create(:incident) }
let_it_be(:project) { create(:project) }
let_it_be(:guest) { create(:user) }
let_it_be(:reporter) { create(:user) }
let_it_be(:issue) { create(:incident, project: project) }
let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
specify { expect(described_class).to require_graphql_authorizations(:update_issue) }
specify { expect(described_class).to require_graphql_authorizations(:update_issue, :admin_issue) }
before_all do
project.add_guest(guest)
project.add_reporter(reporter)
end
describe '#resolve' do
let(:severity) { 'critical' }
let(:mutated_incident) { subject[:issue] }
subject(:resolve) { mutation.resolve(project_path: issue.project.full_path, iid: issue.iid, severity: severity) }
it_behaves_like 'permission level for issue mutation is correctly verified'
context 'when the user can update the issue' do
before do
issue.project.add_developer(user)
subject(:resolve) do
mutation.resolve(
project_path: issue.project.full_path,
iid: issue.iid,
severity: severity
)
end
context 'as guest' do
let(:user) { guest }
it 'raises an error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
context 'and also author' do
let!(:issue) { create(:incident, project: project, author: user) }
it 'raises an error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
context 'and also assignee' do
let!(:issue) { create(:incident, project: project, assignee_ids: [user.id]) }
it 'raises an error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
end
context 'as reporter' do
let(:user) { reporter }
context 'when issue type is incident' do
context 'when severity has a correct value' do
it 'updates severity' do
@ -48,9 +80,9 @@ RSpec.describe Mutations::Issues::SetSeverity do
end
context 'when issue type is not incident' do
let!(:issue) { create(:issue) }
let!(:issue) { create(:issue, project: project) }
it 'does not updates the issue' do
it 'does not update the issue' do
expect { resolve }.not_to change { issue.updated_at }
end
end

22
spec/helpers/projects_helper_spec.rb
View file

@ -953,4 +953,26 @@ RSpec.describe ProjectsHelper do
)
end
end
describe '#project_classes' do
subject { helper.project_classes(project) }
it { is_expected.to be_a(String) }
context 'PUC highlighting enabled' do
before do
project.warn_about_potentially_unwanted_characters = true
end
it { is_expected.to include('project-highlight-puc') }
end
context 'PUC highlighting disabled' do
before do
project.warn_about_potentially_unwanted_characters = false
end
it { is_expected.not_to include('project-highlight-puc') }
end
end
end

39
spec/lib/api/entities/project_spec.rb Normal file
View file

@ -0,0 +1,39 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ::API::Entities::Project do
let(:project) { create(:project, :public) }
let(:current_user) { create(:user) }
let(:options) { { current_user: current_user } }
let(:entity) do
::API::Entities::Project.new(project, options)
end
subject(:json) { entity.as_json }
describe '.shared_with_groups' do
let(:group) { create(:group, :private) }
before do
project.project_group_links.create!(group: group)
end
context 'when the current user does not have access to the group' do
it 'is empty' do
expect(json[:shared_with_groups]).to be_empty
end
end
context 'when the current user has access to the group' do
before do
group.add_guest(current_user)
end
it 'contains information about the shared group' do
expect(json[:shared_with_groups]).to contain_exactly(include(group_id: group.id))
end
end
end
end

2
spec/lib/gitlab/data_builder/build_spec.rb
View file

@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Gitlab::DataBuilder::Build do
let!(:tag_names) { %w(tag-1 tag-2) }
let(:runner) { create(:ci_runner, :instance, tag_list: tag_names.map { |n| ActsAsTaggableOn::Tag.create!(name: n)}) }
let(:user) { create(:user) }
let(:user) { create(:user, :public_email) }
let(:build) { create(:ci_build, :running, runner: runner, user: user) }
describe '.build' do

4
spec/lib/gitlab/data_builder/pipeline_spec.rb
View file

@ -3,7 +3,7 @@
require 'spec_helper'
RSpec.describe Gitlab::DataBuilder::Pipeline do
let_it_be(:user) { create(:user) }
let_it_be(:user) { create(:user, :public_email) }
let_it_be(:project) { create(:project, :repository) }
let_it_be_with_reload(:pipeline) do
@ -46,7 +46,7 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do
name: user.name,
username: user.username,
avatar_url: user.avatar_url(only_path: false),
email: user.email
email: user.public_email
})
end

29
spec/lib/gitlab/import_export/project/relation_factory_spec.rb
View file

@ -267,6 +267,35 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_
end
end
context 'pipeline_schedule' do
let(:relation_sym) { :pipeline_schedules }
let(:relation_hash) do
{
"id": 3,
"created_at": "2016-07-22T08:55:44.161Z",
"updated_at": "2016-07-22T08:55:44.161Z",
"description": "pipeline schedule",
"ref": "main",
"cron": "0 4 * * 0",
"cron_timezone": "UTC",
"active": value,
"project_id": project.id
}
end
subject { created_object.active }
[true, false].each do |v|
context "when relation_hash has active set to #{v}" do
let(:value) { v }
it "the created object is not active" do
expect(created_object.active).to eq(false)
end
end
end
end
# `project_id`, `described_class.USER_REFERENCES`, noteable_id, target_id, and some project IDs are already
# re-assigned by described_class.
context 'Potentially hazardous foreign keys' do

2
spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
View file

@ -376,7 +376,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
expect(pipeline_schedule.ref).to eq('master')
expect(pipeline_schedule.cron).to eq('0 4 * * 0')
expect(pipeline_schedule.cron_timezone).to eq('UTC')
expect(pipeline_schedule.active).to eq(true)
expect(pipeline_schedule.active).to eq(false)
end
end

1
spec/lib/gitlab/import_export/safe_model_attributes.yml
View file

@ -555,7 +555,6 @@ Project:
- merge_requests_rebase_enabled
- jobs_cache_index
- external_authorization_classification_label
- external_webhook_token
- pages_https_only
- merge_requests_disable_committers_approval
- require_password_to_approve

33
spec/lib/gitlab/unicode_spec.rb Normal file
View file

@ -0,0 +1,33 @@
# frozen_string_literal: true
require "spec_helper"
RSpec.describe Gitlab::Unicode do
describe described_class::BIDI_REGEXP do
using RSpec::Parameterized::TableSyntax
where(:bidi_string, :match) do
"\u2066" | true # left-to-right isolate
"\u2067" | true # right-to-left isolate
"\u2068" | true # first strong isolate
"\u2069" | true # pop directional isolate
"\u202a" | true # left-to-right embedding
"\u202b" | true # right-to-left embedding
"\u202c" | true # pop directional formatting
"\u202d" | true # left-to-right override
"\u202e" | true # right-to-left override
"\u2066foobar" | true
"" | false
"foo" | false
"\u2713" | false # checkmark
end
with_them do
let(:utf8_string) { bidi_string.encode("utf-8") }
it "matches only the bidi characters" do
expect(utf8_string.match?(subject)).to eq(match)
end
end
end
end

21
spec/lib/rouge/formatters/html_gitlab_spec.rb
View file

@ -36,5 +36,26 @@ RSpec.describe Rouge::Formatters::HTMLGitlab do
is_expected.to eq(code)
end
end
context 'when unicode control characters are used' do
let(:lang) { 'javascript' }
let(:tokens) { lexer.lex(code, continue: false) }
let(:code) do
<<~JS
#!/usr/bin/env node
var accessLevel = "user";
if (accessLevel != "user‮ ⁦// Check if admin⁩ ⁦") {
console.log("You are an admin.");
}
JS
end
it 'highlights the control characters' do
message = "Potentially unwanted character detected: Unicode BiDi Control"
is_expected.to include(%{<span class="unicode-bidi has-tooltip" data-toggle="tooltip" title="#{message}">}).exactly(4).times
end
end
end
end

2
spec/models/ci/pipeline_spec.rb
View file

@ -7,7 +7,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
include StubRequests
include Ci::SourcePipelineHelpers
let_it_be(:user) { create(:user) }
let_it_be(:user) { create(:user, :public_email) }
let_it_be(:namespace) { create_default(:namespace).freeze }
let_it_be(:project) { create_default(:project, :repository).freeze }

20
spec/models/concerns/resolvable_discussion_spec.rb
View file

@ -188,6 +188,16 @@ RSpec.describe Discussion, ResolvableDiscussion do
it "returns true" do
expect(subject.can_resolve?(current_user)).to be true
end
context 'when noteable is locked' do
before do
allow(subject.noteable).to receive(:discussion_locked?).and_return(true)
end
it 'returns false' do
expect(subject.can_resolve?(current_user)).to be_falsey
end
end
end
context "when the signed in user can push to the project" do
@ -200,8 +210,11 @@ RSpec.describe Discussion, ResolvableDiscussion do
end
context "when the noteable has no author" do
before do
subject.noteable.author = nil
end
it "returns true" do
expect(noteable).to receive(:author).and_return(nil)
expect(subject.can_resolve?(current_user)).to be true
end
end
@ -213,8 +226,11 @@ RSpec.describe Discussion, ResolvableDiscussion do
end
context "when the noteable has no author" do
before do
subject.noteable.author = nil
end
it "returns false" do
expect(noteable).to receive(:author).and_return(nil)
expect(subject.can_resolve?(current_user)).to be false
end
end

13
spec/models/project_spec.rb
View file

@ -660,6 +660,19 @@ RSpec.describe Project, factory_default: :keep do
it { is_expected.to delegate_method(:container_registry_enabled?).to(:project_feature) }
it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) }
describe 'project settings' do
%i(
show_default_award_emojis
show_default_award_emojis=
show_default_award_emojis?
warn_about_potentially_unwanted_characters
warn_about_potentially_unwanted_characters=
warn_about_potentially_unwanted_characters?
).each do |method|
it { is_expected.to delegate_method(method).to(:project_setting).with_arguments(allow_nil: true) }
end
end
include_examples 'ci_cd_settings delegation' do
# Skip attributes defined in EE code
let(:exclude_attributes) do

23
spec/models/user_spec.rb
View file

@ -5511,19 +5511,32 @@ RSpec.describe User do
end
describe '#hook_attrs' do
it 'includes id, name, username, avatar_url, and email' do
user = create(:user)
user_attributes = {
let(:user) { create(:user) }
let(:user_attributes) do
{
id: user.id,
name: user.name,
username: user.username,
avatar_url: user.avatar_url(only_path: false),
email: user.email
avatar_url: user.avatar_url(only_path: false)
}
end
context 'with a public email' do
it 'includes id, name, username, avatar_url, and email' do
user.public_email = "hello@hello.com"
user_attributes[:email] = user.public_email
expect(user.hook_attrs).to eq(user_attributes)
end
end
context 'without a public email' do
it "does not include email if user's email is private" do
user_attributes[:email] = "[REDACTED]"
expect(user.hook_attrs).to eq(user_attributes)
end
end
end
describe 'user credit card validation' do
context 'when user is initialized' do
let(:user) { build(:user) }

14
spec/policies/issuable_policy_spec.rb
View file

@ -13,6 +13,10 @@ RSpec.describe IssuablePolicy, models: true do
let(:merge_request) { create(:merge_request, source_project: project, author: user) }
let(:policies) { described_class.new(user, merge_request) }
it 'allows resolving notes' do
expect(policies).to be_allowed(:resolve_note)
end
context 'when user is able to read project' do
it 'enables user to read and update issuables' do
expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
@ -43,16 +47,24 @@ RSpec.describe IssuablePolicy, models: true do
it 'can not create a note nor award emojis' do
expect(policies).to be_disallowed(:create_note, :award_emoji)
end
it 'does not allow resolving note' do
expect(policies).to be_disallowed(:resolve_note)
end
end
context 'when the user is a project member' do
before do
project.add_guest(user)
project.add_developer(user)
end
it 'can create a note and award emojis' do
expect(policies).to be_allowed(:create_note, :award_emoji)
end
it 'allows resolving notes' do
expect(policies).to be_allowed(:resolve_note)
end
end
end
end

62
spec/requests/api/applications_spec.rb
View file

@ -5,13 +5,14 @@ require 'spec_helper'
RSpec.describe API::Applications, :api do
let(:admin_user) { create(:user, admin: true) }
let(:user) { create(:user, admin: false) }
let!(:application) { create(:application, name: 'another_application', owner: nil, redirect_uri: 'http://other_application.url', scopes: '') }
let(:scopes) { 'api' }
let!(:application) { create(:application, name: 'another_application', owner: nil, redirect_uri: 'http://other_application.url', scopes: scopes) }
describe 'POST /applications' do
context 'authenticated and authorized user' do
it 'creates and returns an OAuth application' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '' }
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes }
end.to change { Doorkeeper::Application.count }.by 1
application = Doorkeeper::Application.find_by(name: 'application_name', redirect_uri: 'http://application.url')
@ -22,11 +23,12 @@ RSpec.describe API::Applications, :api do
expect(json_response['secret']).to eq application.secret
expect(json_response['callback_url']).to eq application.redirect_uri
expect(json_response['confidential']).to eq application.confidential
expect(application.scopes.to_s).to eq('api')
end
it 'does not allow creating an application with the wrong redirect_uri format' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://', scopes: '' }
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://', scopes: scopes }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:bad_request)
@ -36,7 +38,7 @@ RSpec.describe API::Applications, :api do
it 'does not allow creating an application with a forbidden URI format' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'javascript://alert()', scopes: '' }
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'javascript://alert()', scopes: scopes }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:bad_request)
@ -46,7 +48,7 @@ RSpec.describe API::Applications, :api do
it 'does not allow creating an application without a name' do
expect do
post api('/applications', admin_user), params: { redirect_uri: 'http://application.url', scopes: '' }
post api('/applications', admin_user), params: { redirect_uri: 'http://application.url', scopes: scopes }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:bad_request)
@ -56,7 +58,7 @@ RSpec.describe API::Applications, :api do
it 'does not allow creating an application without a redirect_uri' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', scopes: '' }
post api('/applications', admin_user), params: { name: 'application_name', scopes: scopes }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:bad_request)
@ -64,19 +66,59 @@ RSpec.describe API::Applications, :api do
expect(json_response['error']).to eq('redirect_uri is missing')
end
it 'does not allow creating an application without scopes' do
it 'does not allow creating an application without specifying `scopes`' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url' }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response).to be_a Hash
expect(json_response['error']).to eq('scopes is missing')
expect(json_response['error']).to eq('scopes is missing, scopes is empty')
end
it 'does not allow creating an application with blank `scopes`' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '' }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq('scopes is empty')
end
it 'does not allow creating an application with invalid `scopes`' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'non_existent_scope' }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']['scopes'][0]).to eq('doesn\'t match configured on the server.')
end
context 'multiple scopes' do
it 'creates an application with multiple `scopes` when each scope specified is seperated by a space' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api read_user' }
end.to change { Doorkeeper::Application.count }.by 1
application = Doorkeeper::Application.last
expect(response).to have_gitlab_http_status(:created)
expect(application.scopes.to_s).to eq('api read_user')
end
it 'does not allow creating an application with multiple `scopes` when one of the scopes is invalid' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api non_existent_scope' }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']['scopes'][0]).to eq('doesn\'t match configured on the server.')
end
end
it 'defaults to creating an application with confidential' do
expect do
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '', confidential: nil }
post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes, confidential: nil }
end.to change { Doorkeeper::Application.count }.by(1)
expect(response).to have_gitlab_http_status(:created)
@ -89,7 +131,7 @@ RSpec.describe API::Applications, :api do
context 'authorized user without authorization' do
it 'does not create application' do
expect do
post api('/applications', user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '' }
post api('/applications', user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes }
end.not_to change { Doorkeeper::Application.count }
expect(response).to have_gitlab_http_status(:forbidden)

18
spec/requests/api/groups_spec.rb
View file

@ -758,6 +758,15 @@ RSpec.describe API::Groups do
expect(json_response['prevent_sharing_groups_outside_hierarchy']).to eq(true)
end
it 'does not update visibility_level if it is restricted' do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
put api("/groups/#{group1.id}", user1), params: { visibility: 'internal' }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']['visibility_level']).to include('internal has been restricted by your GitLab administrator')
end
context 'updating the `default_branch_protection` attribute' do
subject do
put api("/groups/#{group1.id}", user1), params: { default_branch_protection: ::Gitlab::Access::PROTECTION_NONE }
@ -845,6 +854,15 @@ RSpec.describe API::Groups do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['name']).to eq(new_group_name)
end
it 'ignores visibility level restrictions' do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
put api("/groups/#{group1.id}", admin), params: { visibility: 'internal' }
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['visibility']).to eq('internal')
end
end
context 'when authenticated as an user that can see the group' do

1
spec/requests/api/project_attributes.yml
View file

@ -137,6 +137,7 @@ project_setting:
- has_confluence
- has_vulnerabilities
- prevent_merge_without_jira_issue
- warn_about_potentially_unwanted_characters
- previous_default_branch
- project_id
- push_rule_id

34
spec/requests/api/project_import_spec.rb
View file

@ -16,6 +16,16 @@ RSpec.describe API::ProjectImport do
namespace.add_owner(user)
end
shared_examples 'requires authentication' do
let(:user) { nil }
it 'returns 401' do
subject
expect(response).to have_gitlab_http_status(:unauthorized)
end
end
describe 'POST /projects/import' do
subject { upload_archive(file_upload, workhorse_headers, params) }
@ -32,6 +42,8 @@ RSpec.describe API::ProjectImport do
allow(ImportExportUploader).to receive(:workhorse_upload_path).and_return('/')
end
it_behaves_like 'requires authentication'
it 'executes a limited number of queries' do
control_count = ActiveRecord::QueryRecorder.new { subject }.count
@ -281,6 +293,10 @@ RSpec.describe API::ProjectImport do
end
describe 'POST /projects/remote-import' do
subject do
post api('/projects/remote-import', user), params: params
end
let(:params) do
{
path: 'test-import',
@ -288,10 +304,12 @@ RSpec.describe API::ProjectImport do
}
end
it_behaves_like 'requires authentication'
it 'returns NOT FOUND when the feature is disabled' do
stub_feature_flags(import_project_from_remote_file: false)
post api('/projects/remote-import', user), params: params
subject
expect(response).to have_gitlab_http_status(:not_found)
end
@ -315,7 +333,7 @@ RSpec.describe API::ProjectImport do
.to receive(:execute)
.and_return(service_response)
post api('/projects/remote-import', user), params: params
subject
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to include({
@ -338,7 +356,7 @@ RSpec.describe API::ProjectImport do
.to receive(:execute)
.and_return(service_response)
post api('/projects/remote-import', user), params: params
subject
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response).to eq({
@ -350,6 +368,14 @@ RSpec.describe API::ProjectImport do
end
describe 'GET /projects/:id/import' do
it 'public project accessible for an unauthenticated user' do
project = create(:project, :public)
get api("/projects/#{project.id}/import", nil)
expect(response).to have_gitlab_http_status(:ok)
end
it 'returns the import status' do
project = create(:project, :import_started)
project.add_maintainer(user)
@ -376,6 +402,8 @@ RSpec.describe API::ProjectImport do
describe 'POST /projects/import/authorize' do
subject { post api('/projects/import/authorize', user), headers: workhorse_headers }
it_behaves_like 'requires authentication'
it 'authorizes importing project with workhorse header' do
subject

24
spec/requests/api/projects_spec.rb
View file

@ -990,7 +990,7 @@ RSpec.describe API::Projects do
expect do
get api('/projects', admin)
end.not_to exceed_query_limit(control.count)
end.not_to exceed_query_limit(control)
end
end
end
@ -3203,6 +3203,15 @@ RSpec.describe API::Projects do
expect(json_response['visibility']).to eq('private')
end
it 'does not update visibility_level if it is restricted' do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
put api("/projects/#{project3.id}", user), params: { visibility: 'internal' }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']['visibility_level']).to include('internal has been restricted by your GitLab administrator')
end
it 'does not update name to existing name' do
project_param = { name: project3.name }
@ -3526,6 +3535,19 @@ RSpec.describe API::Projects do
end
end
context 'when authenticated as the admin' do
let_it_be(:admin) { create(:admin) }
it 'ignores visibility level restrictions' do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
put api("/projects/#{project3.id}", admin), params: { visibility: 'internal' }
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['visibility']).to eq('internal')
end
end
context 'when updating repository storage' do
let(:unknown_storage) { 'new-storage' }
let(:new_project) { create(:project, :repository, namespace: user.namespace) }

55
spec/services/groups/transfer_service_spec.rb
View file

@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Groups::TransferService do
RSpec.describe Groups::TransferService, :sidekiq_inline do
let_it_be(:user) { create(:user) }
let_it_be(:new_parent_group) { create(:group, :public) }
@ -594,6 +594,59 @@ RSpec.describe Groups::TransferService do
transfer_service.execute(new_parent_group)
end
end
context 'transferring groups with shared_projects' do
let_it_be_with_reload(:shared_project) { create(:project, :public) }
shared_examples_for 'drops the authorizations of ancestor members from the old hierarchy' do
it 'drops the authorizations of ancestor members from the old hierarchy' do
expect { transfer_service.execute(new_parent_group) }.to change {
ProjectAuthorization.where(project: shared_project, user: old_group_member).size
}.from(1).to(0)
end
end
context 'when the group that has existing project share is transferred' do
before do
create(:project_group_link, :maintainer, project: shared_project, group: group)
end
it_behaves_like 'drops the authorizations of ancestor members from the old hierarchy'
end
context 'when the group whose subgroup has an existing project share is transferred' do
let_it_be_with_reload(:subgroup) { create(:group, :private, parent: group) }
before do
create(:project_group_link, :maintainer, project: shared_project, group: subgroup)
end
it_behaves_like 'drops the authorizations of ancestor members from the old hierarchy'
end
end
context 'when a group that has existing group share is transferred' do
let(:shared_with_group) { group }
let_it_be(:member_of_shared_with_group) { create(:user) }
let_it_be(:shared_group) { create(:group, :private) }
let_it_be(:project_in_shared_group) { create(:project, namespace: shared_group) }
before do
shared_with_group.add_developer(member_of_shared_with_group)
create(:group_group_link, :maintainer, shared_group: shared_group, shared_with_group: shared_with_group)
shared_with_group.refresh_members_authorized_projects
end
it 'retains the authorizations of direct members' do
expect { transfer_service.execute(new_parent_group) }.not_to change {
ProjectAuthorization.where(
project: project_in_shared_group,
user: member_of_shared_with_group,
access_level: Gitlab::Access::DEVELOPER).size
}.from(1)
end
end
end
end

36
spec/services/issues/update_service_spec.rb
View file

@ -6,6 +6,7 @@ RSpec.describe Issues::UpdateService, :mailer do
let_it_be(:user) { create(:user) }
let_it_be(:user2) { create(:user) }
let_it_be(:user3) { create(:user) }
let_it_be(:guest) { create(:user) }
let_it_be(:group) { create(:group, :public) }
let_it_be(:project, reload: true) { create(:project, :repository, group: group) }
let_it_be(:label) { create(:label, project: project) }
@ -24,6 +25,7 @@ RSpec.describe Issues::UpdateService, :mailer do
project.add_maintainer(user)
project.add_developer(user2)
project.add_developer(user3)
project.add_guest(guest)
end
describe 'execute' do
@ -95,9 +97,7 @@ RSpec.describe Issues::UpdateService, :mailer do
end
context 'user is a guest' do
before do
project.add_guest(user)
end
let(:user) { guest }
it 'does not assign the sentry error' do
update_issue(opts)
@ -245,11 +245,7 @@ RSpec.describe Issues::UpdateService, :mailer do
context 'from issue to restricted issue types' do
context 'without sufficient permissions' do
let(:user) { create(:user) }
before do
project.add_guest(user)
end
let(:user) { guest }
it 'does nothing to the labels' do
expect { update_issue(issue_type: 'issue') }.not_to change(issue.labels, :count)
@ -394,12 +390,6 @@ RSpec.describe Issues::UpdateService, :mailer do
end
context 'when current user cannot admin issues in the project' do
let(:guest) { create(:user) }
before do
project.add_guest(guest)
end
it 'filters out params that cannot be set without the :admin_issue permission' do
described_class.new(
project: project, current_user: guest, params: opts.merge(
@ -1100,6 +1090,24 @@ RSpec.describe Issues::UpdateService, :mailer do
it_behaves_like 'does not change the severity'
end
context 'as guest' do
let(:user) { guest }
it_behaves_like 'does not change the severity'
context 'and also author' do
let(:issue) { create(:incident, project: project, author: user) }
it_behaves_like 'does not change the severity'
end
context 'and also assignee' do
let(:issue) { create(:incident, project: project, assignee_ids: [user.id]) }
it_behaves_like 'does not change the severity'
end
end
end
context 'when severity has been set before' do

2
workhorse/internal/artifacts/artifacts_upload_test.go
View file

@ -270,7 +270,7 @@ func TestUploadHandlerForMultipleFiles(t *testing.T) {
require.NoError(t, s.writer.Close())
response := testUploadArtifacts(t, s.writer.FormDataContentType(), s.url, s.buffer)
require.Equal(t, http.StatusInternalServerError, response.Code)
require.Equal(t, http.StatusBadRequest, response.Code)
}
func TestUploadFormProcessing(t *testing.T) {

BIN
workhorse/internal/upload/exif/testdata/takes_lot_of_memory_to_decode.tiff vendored Normal file
View file

Binary file not shown.

11
workhorse/internal/upload/rewrite.go
View file

@ -25,7 +25,10 @@ import (
)
// ErrInjectedClientParam means that the client sent a parameter that overrides one of our own fields
var ErrInjectedClientParam = errors.New("injected client parameter")
var (
ErrInjectedClientParam = errors.New("injected client parameter")
ErrMultipleFilesUploaded = errors.New("upload request contains more than one file")
)
var (
multipartUploadRequests = promauto.NewCounterVec(
@ -114,6 +117,10 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr
}
func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipart.Part, opts *filestore.SaveFileOpts) error {
if rew.filter.Count() > 0 {
return ErrMultipleFilesUploaded
}
multipartFiles.WithLabelValues(rew.filter.Name()).Inc()
filename := filepath.Base(p.FileName())
@ -226,7 +233,7 @@ func handleExifUpload(ctx context.Context, r io.Reader, filename string, imageTy
}
func isTIFF(r io.Reader) bool {
_, err := tiff.Decode(r)
_, err := tiff.DecodeConfig(r)
if err == nil {
return true
}

13
workhorse/internal/upload/rewrite_test.go
View file

@ -2,6 +2,7 @@ package upload
import (
"os"
"runtime"
"testing"
"github.com/stretchr/testify/require"
@ -29,6 +30,10 @@ func TestImageTypeRecongition(t *testing.T) {
filename: "exif/testdata/sample_exif_invalid.jpg",
isJPEG: false,
isTIFF: false,
}, {
filename: "exif/testdata/takes_lot_of_memory_to_decode.tiff", // File from https://gitlab.com/gitlab-org/gitlab/-/issues/341363
isJPEG: false,
isTIFF: true,
},
}
@ -36,8 +41,16 @@ func TestImageTypeRecongition(t *testing.T) {
t.Run(test.filename, func(t *testing.T) {
input, err := os.Open(test.filename)
require.NoError(t, err)
var m runtime.MemStats
runtime.ReadMemStats(&m)
start := m.TotalAlloc
require.Equal(t, test.isJPEG, isJPEG(input))
require.Equal(t, test.isTIFF, isTIFF(input))
runtime.ReadMemStats(&m)
require.Less(t, m.TotalAlloc-start, uint64(50000), "must take reasonable amount of memory to recognise the type")
})
}
}

3
workhorse/internal/upload/uploads.go
View file

@ -21,6 +21,7 @@ type MultipartFormProcessor interface {
ProcessField(ctx context.Context, formName string, writer *multipart.Writer) error
Finalize(ctx context.Context) error
Name() string
Count() int
}
func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, preauth *api.Response, filter MultipartFormProcessor, opts *filestore.SaveFileOpts) {
@ -34,6 +35,8 @@ func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, p
switch err {
case ErrInjectedClientParam:
helper.CaptureAndFail(w, r, err, "Bad Request", http.StatusBadRequest)
case ErrMultipleFilesUploaded:
helper.CaptureAndFail(w, r, err, "Uploading multiple files is not allowed", http.StatusBadRequest)
case http.ErrNotMultipart:
h.ServeHTTP(w, r)
case filestore.ErrEntityTooLarge:

43
workhorse/internal/upload/uploads_test.go
View file

@ -18,7 +18,6 @@ import (
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/filestore"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/helper"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/objectstore/test"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/proxy"
@ -28,11 +27,7 @@ import (
var nilHandler = http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})
type testFormProcessor struct{}
func (a *testFormProcessor) ProcessFile(ctx context.Context, formName string, file *filestore.FileHandler, writer *multipart.Writer) error {
return nil
}
type testFormProcessor struct{ SavedFileTracker }
func (a *testFormProcessor) ProcessField(ctx context.Context, formName string, writer *multipart.Writer) error {
if formName != "token" && !strings.HasPrefix(formName, "file.") && !strings.HasPrefix(formName, "other.") {
@ -45,10 +40,6 @@ func (a *testFormProcessor) Finalize(ctx context.Context) error {
return nil
}
func (a *testFormProcessor) Name() string {
return ""
}
func TestUploadTempPathRequirement(t *testing.T) {
apiResponse := &api.Response{}
preparer := &DefaultPreparer{}
@ -259,6 +250,38 @@ func TestUploadProcessingField(t *testing.T) {
require.Equal(t, 500, response.Code)
}
func TestUploadingMultipleFiles(t *testing.T) {
testhelper.ConfigureSecret()
tempPath, err := ioutil.TempDir("", "uploads")
require.NoError(t, err)
defer os.RemoveAll(tempPath)
var buffer bytes.Buffer
writer := multipart.NewWriter(&buffer)
_, err = writer.CreateFormFile("file", "my.file")
require.NoError(t, err)
_, err = writer.CreateFormFile("file", "my.file")
require.NoError(t, err)
require.NoError(t, writer.Close())
httpRequest, err := http.NewRequest("PUT", "/url/path", &buffer)
require.NoError(t, err)
httpRequest.Header.Set("Content-Type", writer.FormDataContentType())
response := httptest.NewRecorder()
apiResponse := &api.Response{TempPath: tempPath}
preparer := &DefaultPreparer{}
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
require.Equal(t, 400, response.Code)
require.Equal(t, "Uploading multiple files is not allowed\n", response.Body.String())
}
func TestUploadProcessingFile(t *testing.T) {
tempPath, err := ioutil.TempDir("", "uploads")
require.NoError(t, err)