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

Update upstream source from tag 'upstream/14.2.6+ds1'

Browse source 
Update to upstream version '14.2.6+ds1'
with Debian dir 7d2b7c02e9
This commit is contained in:
Pirate Praveen 2021-10-29 20:45:47 +05:30
commit 43416ee97d
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)

11
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,9 +164,14 @@ 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\-+/, "")
path.gsub!(/\A\-+/, "")
# Users with the great usernames of "." or ".." would end up with a blank username.
# Work around that by setting their username to "blank", followed by a counter.

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
# check that user is allowed to set specified visibility_level
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)
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

54
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) }
subject(:resolve) do
mutation.resolve(
project_path: issue.project.full_path,
iid: issue.iid,
severity: severity
)
end
it_behaves_like 'permission level for issue mutation is correctly verified'
context 'as guest' do
let(:user) { guest }
context 'when the user can update the issue' do
before do
issue.project.add_developer(user)
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

25
spec/models/user_spec.rb
View file

@ -5511,16 +5511,29 @@ 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)
}
expect(user.hook_attrs).to eq(user_attributes)
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

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)