summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-05-31 11:42:18 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-05-31 11:42:44 +0000
commit15c040a6bd71894260b66a90685070c0babfee76 (patch)
tree27021108f64428697744973cddaede55930f4ef7
parent6e4e4023b46c786a99e1cfe8832fa5eff2728e0d (diff)
downloadgitlab-ce-15c040a6bd71894260b66a90685070c0babfee76.tar.gz
Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee
-rw-r--r--app/policies/concerns/policy_actor.rb4
-rw-r--r--app/policies/global_policy.rb10
-rw-r--r--app/serializers/member_entity.rb4
-rw-r--r--config/initializers/lograge.rb1
-rw-r--r--lib/banzai/filter/absolute_link_filter.rb5
-rw-r--r--lib/banzai/filter/ascii_doc_post_processing_filter.rb10
-rw-r--r--lib/banzai/filter/base_relative_link_filter.rb5
-rw-r--r--lib/banzai/filter/color_filter.rb5
-rw-r--r--lib/banzai/filter/custom_emoji_filter.rb2
-rw-r--r--lib/banzai/filter/emoji_filter.rb2
-rw-r--r--lib/banzai/filter/footnote_filter.rb12
-rw-r--r--lib/banzai/filter/gollum_tags_filter.rb2
-rw-r--r--lib/banzai/filter/image_lazy_load_filter.rb5
-rw-r--r--lib/banzai/filter/inline_diff_filter.rb2
-rw-r--r--lib/banzai/filter/inline_metrics_redactor_filter.rb3
-rw-r--r--lib/banzai/filter/kroki_filter.rb5
-rw-r--r--lib/banzai/filter/markdown_post_escape_filter.rb9
-rw-r--r--lib/banzai/filter/math_filter.rb9
-rw-r--r--lib/banzai/filter/mermaid_filter.rb5
-rw-r--r--lib/banzai/filter/plantuml_filter.rb7
-rw-r--r--lib/banzai/filter/suggestion_filter.rb5
-rw-r--r--lib/banzai/filter/syntax_highlight_filter.rb5
-rw-r--r--lib/banzai/filter/table_of_contents_filter.rb5
-rw-r--r--lib/banzai/filter/wiki_link_filter.rb13
-rw-r--r--lib/gitlab/auth.rb12
-rw-r--r--lib/gitlab/auth/user_access_denied_reason.rb5
-rw-r--r--lib/gitlab/diff/suggestions_parser.rb5
-rw-r--r--lib/gitlab/utils.rb18
-rw-r--r--lib/gitlab/utils/nokogiri.rb24
-rw-r--r--spec/features/users/login_spec.rb19
-rw-r--r--spec/initializers/lograge_spec.rb21
-rw-r--r--spec/lib/banzai/pipeline/post_process_pipeline_spec.rb7
-rw-r--r--spec/lib/gitlab/auth/user_access_denied_reason_spec.rb8
-rw-r--r--spec/lib/gitlab/git_access_spec.rb14
-rw-r--r--spec/lib/gitlab/utils/nokogiri_spec.rb34
-rw-r--r--spec/lib/gitlab/utils_spec.rb23
-rw-r--r--spec/policies/global_policy_spec.rb24
-rw-r--r--spec/requests/git_http_spec.rb61
-rw-r--r--spec/requests/lfs_http_spec.rb4
39 files changed, 374 insertions, 40 deletions
diff --git a/app/policies/concerns/policy_actor.rb b/app/policies/concerns/policy_actor.rb
index cd19b46ad6c..08a26da6673 100644
--- a/app/policies/concerns/policy_actor.rb
+++ b/app/policies/concerns/policy_actor.rb
@@ -80,6 +80,10 @@ module PolicyActor
def can_read_all_resources?
false
end
+
+ def password_expired?
+ false
+ end
end
PolicyActor.prepend_mod_with('PolicyActor')
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
index 85263ec7c87..73757891cd6 100644
--- a/app/policies/global_policy.rb
+++ b/app/policies/global_policy.rb
@@ -15,6 +15,10 @@ class GlobalPolicy < BasePolicy
@user&.required_terms_not_accepted?
end
+ condition(:password_expired, scope: :user) do
+ @user&.password_expired?
+ end
+
condition(:project_bot, scope: :user) { @user&.project_bot? }
condition(:migration_bot, scope: :user) { @user&.migration_bot? }
@@ -73,6 +77,12 @@ class GlobalPolicy < BasePolicy
prevent :access_git
end
+ rule { password_expired }.policy do
+ prevent :access_api
+ prevent :access_git
+ prevent :use_slash_commands
+ end
+
rule { can_create_group }.policy do
enable :create_group
end
diff --git a/app/serializers/member_entity.rb b/app/serializers/member_entity.rb
index 7559a03bd3b..5100a41638e 100644
--- a/app/serializers/member_entity.rb
+++ b/app/serializers/member_entity.rb
@@ -40,7 +40,9 @@ class MemberEntity < Grape::Entity
expose :valid_level_roles, as: :valid_roles
- expose :user, if: -> (member) { member.user.present? }, using: MemberUserEntity
+ expose :user, if: -> (member) { member.user.present? } do |member, options|
+ MemberUserEntity.represent(member.user, source: options[:source])
+ end
expose :invite, if: -> (member) { member.invite? } do
expose :email do |member|
diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb
index e8479bc6aa4..61e357808d9 100644
--- a/config/initializers/lograge.rb
+++ b/config/initializers/lograge.rb
@@ -18,6 +18,7 @@ unless Gitlab::Runtime.sidekiq?
data[:db_duration_s] = Gitlab::Utils.ms_to_round_sec(data.delete(:db)) if data[:db]
data[:view_duration_s] = Gitlab::Utils.ms_to_round_sec(data.delete(:view)) if data[:view]
data[:duration_s] = Gitlab::Utils.ms_to_round_sec(data.delete(:duration)) if data[:duration]
+ data[:location] = Gitlab::Utils.removes_sensitive_data_from_url(data[:location]) if data[:location]
# Remove empty hashes to prevent type mismatches
# These are set to empty hashes in Lograge's ActionCable subscriber
diff --git a/lib/banzai/filter/absolute_link_filter.rb b/lib/banzai/filter/absolute_link_filter.rb
index a9bdb004c4b..cc7bf3ed556 100644
--- a/lib/banzai/filter/absolute_link_filter.rb
+++ b/lib/banzai/filter/absolute_link_filter.rb
@@ -6,10 +6,13 @@ module Banzai
module Filter
# HTML filter that converts relative urls into absolute ones.
class AbsoluteLinkFilter < HTML::Pipeline::Filter
+ CSS = 'a.gfm'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
def call
return doc unless context[:only_path] == false
- doc.search('a.gfm').each do |el|
+ doc.xpath(XPATH).each do |el|
process_link_attr el.attribute('href')
end
diff --git a/lib/banzai/filter/ascii_doc_post_processing_filter.rb b/lib/banzai/filter/ascii_doc_post_processing_filter.rb
index 09f0fd7df45..83c729e13b5 100644
--- a/lib/banzai/filter/ascii_doc_post_processing_filter.rb
+++ b/lib/banzai/filter/ascii_doc_post_processing_filter.rb
@@ -3,14 +3,20 @@
module Banzai
module Filter
class AsciiDocPostProcessingFilter < HTML::Pipeline::Filter
+ CSS_MATH = '[data-math-style]'
+ XPATH_MATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_MATH).freeze
+ CSS_MERM = '[data-mermaid-style]'
+ XPATH_MERM = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_MERM).freeze
+
def call
- doc.search('[data-math-style]').each do |node|
+ doc.xpath(XPATH_MATH).each do |node|
node.set_attribute('class', 'code math js-render-math')
end
- doc.search('[data-mermaid-style]').each do |node|
+ doc.xpath(XPATH_MERM).each do |node|
node.set_attribute('class', 'js-render-mermaid')
end
+
doc
end
end
diff --git a/lib/banzai/filter/base_relative_link_filter.rb b/lib/banzai/filter/base_relative_link_filter.rb
index 84a6e18e77b..3f775abb185 100644
--- a/lib/banzai/filter/base_relative_link_filter.rb
+++ b/lib/banzai/filter/base_relative_link_filter.rb
@@ -7,6 +7,9 @@ module Banzai
class BaseRelativeLinkFilter < HTML::Pipeline::Filter
include Gitlab::Utils::StrongMemoize
+ CSS = 'a:not(.gfm), img:not(.gfm), video:not(.gfm), audio:not(.gfm)'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
protected
def linkable_attributes
@@ -41,7 +44,7 @@ module Banzai
def fetch_linkable_attributes
attrs = []
- attrs += doc.search('a:not(.gfm), img:not(.gfm), video:not(.gfm), audio:not(.gfm)').flat_map do |el|
+ attrs += doc.xpath(XPATH).flat_map do |el|
[el.attribute('href'), el.attribute('src'), el.attribute('data-src')]
end
diff --git a/lib/banzai/filter/color_filter.rb b/lib/banzai/filter/color_filter.rb
index 0aca7441638..58e9b8cdba1 100644
--- a/lib/banzai/filter/color_filter.rb
+++ b/lib/banzai/filter/color_filter.rb
@@ -7,8 +7,11 @@ module Banzai
class ColorFilter < HTML::Pipeline::Filter
COLOR_CHIP_CLASS = 'gfm-color_chip'
+ CSS = 'code'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
def call
- doc.css('code').each do |node|
+ doc.xpath(XPATH).each do |node|
color = ColorParser.parse(node.content)
node << color_chip(color) if color
end
diff --git a/lib/banzai/filter/custom_emoji_filter.rb b/lib/banzai/filter/custom_emoji_filter.rb
index 3171231dc9b..a5f1a22c483 100644
--- a/lib/banzai/filter/custom_emoji_filter.rb
+++ b/lib/banzai/filter/custom_emoji_filter.rb
@@ -11,7 +11,7 @@ module Banzai
return doc unless context[:project]
return doc unless Feature.enabled?(:custom_emoji, context[:project])
- doc.search(".//text()").each do |node|
+ doc.xpath('descendant-or-self::text()').each do |node|
content = node.to_html
next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS)
diff --git a/lib/banzai/filter/emoji_filter.rb b/lib/banzai/filter/emoji_filter.rb
index 8952a3ff6b4..9d24bf028b6 100644
--- a/lib/banzai/filter/emoji_filter.rb
+++ b/lib/banzai/filter/emoji_filter.rb
@@ -11,7 +11,7 @@ module Banzai
IGNORE_UNICODE_EMOJIS = %w(™ © ®).freeze
def call
- doc.search(".//text()").each do |node|
+ doc.xpath('descendant-or-self::text()').each do |node|
content = node.to_html
next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS)
diff --git a/lib/banzai/filter/footnote_filter.rb b/lib/banzai/filter/footnote_filter.rb
index 5474242e03c..0f856dc0eb9 100644
--- a/lib/banzai/filter/footnote_filter.rb
+++ b/lib/banzai/filter/footnote_filter.rb
@@ -23,17 +23,23 @@ module Banzai
FOOTNOTE_LINK_REFERENCE_PATTERN = /\A#{FOOTNOTE_LINK_ID_PREFIX}\d+\z/.freeze
FOOTNOTE_START_NUMBER = 1
+ CSS_SECTION = "ol > li[id=#{FOOTNOTE_ID_PREFIX}#{FOOTNOTE_START_NUMBER}]"
+ XPATH_SECTION = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_SECTION).freeze
+ CSS_FOOTNOTE = 'sup > a[id]'
+ XPATH_FOOTNOTE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_FOOTNOTE).freeze
+
def call
- return doc unless first_footnote = doc.at_css("ol > li[id=#{fn_id(FOOTNOTE_START_NUMBER)}]")
+ return doc unless first_footnote = doc.at_xpath(XPATH_SECTION)
# Sanitization stripped off the section wrapper - add it back in
first_footnote.parent.wrap('<section class="footnotes">')
rand_suffix = "-#{random_number}"
modified_footnotes = {}
- doc.css('sup > a[id]').each do |link_node|
+ doc.xpath(XPATH_FOOTNOTE).each do |link_node|
ref_num = link_node[:id].delete_prefix(FOOTNOTE_LINK_ID_PREFIX)
- footnote_node = doc.at_css("li[id=#{fn_id(ref_num)}]")
+ node_xpath = Gitlab::Utils::Nokogiri.css_to_xpath("li[id=#{fn_id(ref_num)}]")
+ footnote_node = doc.at_xpath(node_xpath)
if INTEGER_PATTERN.match?(ref_num) && (footnote_node || modified_footnotes[ref_num])
link_node[:href] += rand_suffix
diff --git a/lib/banzai/filter/gollum_tags_filter.rb b/lib/banzai/filter/gollum_tags_filter.rb
index 6de9f2b86f6..0548d5a9997 100644
--- a/lib/banzai/filter/gollum_tags_filter.rb
+++ b/lib/banzai/filter/gollum_tags_filter.rb
@@ -60,7 +60,7 @@ module Banzai
IGNORED_ANCESTOR_TAGS = %w(pre code tt).to_set
def call
- doc.search(".//text()").each do |node|
+ doc.xpath('descendant-or-self::text()').each do |node|
next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS)
next unless node.content =~ TAGS_PATTERN
diff --git a/lib/banzai/filter/image_lazy_load_filter.rb b/lib/banzai/filter/image_lazy_load_filter.rb
index d8b9eb29cf5..916c135b777 100644
--- a/lib/banzai/filter/image_lazy_load_filter.rb
+++ b/lib/banzai/filter/image_lazy_load_filter.rb
@@ -6,8 +6,11 @@ module Banzai
# HTML filter that moves the value of image `src` attributes to `data-src`
# so they can be lazy loaded.
class ImageLazyLoadFilter < HTML::Pipeline::Filter
+ CSS = 'img'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
def call
- doc.xpath('descendant-or-self::img').each do |img|
+ doc.xpath(XPATH).each do |img|
img.add_class('lazy')
img['data-src'] = img['src']
img['src'] = LazyImageTagHelper.placeholder_image
diff --git a/lib/banzai/filter/inline_diff_filter.rb b/lib/banzai/filter/inline_diff_filter.rb
index 5a1c0bee32d..e47ff15e7b7 100644
--- a/lib/banzai/filter/inline_diff_filter.rb
+++ b/lib/banzai/filter/inline_diff_filter.rb
@@ -7,7 +7,7 @@ module Banzai
IGNORED_ANCESTOR_TAGS = %w(pre code tt).to_set
def call
- doc.search(".//text()").each do |node|
+ doc.xpath('descendant-or-self::text()').each do |node|
next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS)
content = node.to_html
diff --git a/lib/banzai/filter/inline_metrics_redactor_filter.rb b/lib/banzai/filter/inline_metrics_redactor_filter.rb
index 2259115acfc..b256815ae84 100644
--- a/lib/banzai/filter/inline_metrics_redactor_filter.rb
+++ b/lib/banzai/filter/inline_metrics_redactor_filter.rb
@@ -8,6 +8,7 @@ module Banzai
include Gitlab::Utils::StrongMemoize
METRICS_CSS_CLASS = '.js-render-metrics'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(METRICS_CSS_CLASS).freeze
EMBED_LIMIT = 100
Route = Struct.new(:regex, :permission)
@@ -41,7 +42,7 @@ module Banzai
# @return [Nokogiri::XML::NodeSet]
def nodes
strong_memoize(:nodes) do
- nodes = doc.css(METRICS_CSS_CLASS)
+ nodes = doc.xpath(XPATH)
nodes.drop(EMBED_LIMIT).each(&:remove)
nodes
diff --git a/lib/banzai/filter/kroki_filter.rb b/lib/banzai/filter/kroki_filter.rb
index dbd4de32a47..3803302c324 100644
--- a/lib/banzai/filter/kroki_filter.rb
+++ b/lib/banzai/filter/kroki_filter.rb
@@ -15,10 +15,11 @@ module Banzai
.map { |diagram_type| %(pre[lang="#{diagram_type}"] > code) }
.join(', ')
- return doc unless doc.at(diagram_selectors)
+ xpath = Gitlab::Utils::Nokogiri.css_to_xpath(diagram_selectors)
+ return doc unless doc.at_xpath(xpath)
diagram_format = "svg"
- doc.css(diagram_selectors).each do |node|
+ doc.xpath(xpath).each do |node|
diagram_type = node.parent['lang']
img_tag = Nokogiri::HTML::DocumentFragment.parse(%(<img src="#{create_image_src(diagram_type, diagram_format, node.content)}"/>))
node.parent.replace(img_tag)
diff --git a/lib/banzai/filter/markdown_post_escape_filter.rb b/lib/banzai/filter/markdown_post_escape_filter.rb
index ad32e9afbf5..b69afdcfebe 100644
--- a/lib/banzai/filter/markdown_post_escape_filter.rb
+++ b/lib/banzai/filter/markdown_post_escape_filter.rb
@@ -8,6 +8,11 @@ module Banzai
NOT_LITERAL_REGEX = %r{#{LITERAL_KEYWORD}-((%5C|\\).+?)-#{LITERAL_KEYWORD}}.freeze
SPAN_REGEX = %r{<span>(.*?)</span>}.freeze
+ CSS_A = 'a'
+ XPATH_A = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_A).freeze
+ CSS_CODE = 'code'
+ XPATH_CODE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_CODE).freeze
+
def call
return doc unless result[:escaped_literals]
@@ -24,12 +29,12 @@ module Banzai
# Banzai::Renderer::CommonMark::HTML. However, we eventually want to use
# the built-in compiled renderer, rather than the ruby version, for speed.
# So let's do this work here.
- doc.css('a').each do |node|
+ doc.xpath(XPATH_A).each do |node|
node.attributes['href'].value = node.attributes['href'].value.gsub(SPAN_REGEX, '\1') if node.attributes['href']
node.attributes['title'].value = node.attributes['title'].value.gsub(SPAN_REGEX, '\1') if node.attributes['title']
end
- doc.css('code').each do |node|
+ doc.xpath(XPATH_CODE).each do |node|
node.attributes['lang'].value = node.attributes['lang'].value.gsub(SPAN_REGEX, '\1') if node.attributes['lang']
end
diff --git a/lib/banzai/filter/math_filter.rb b/lib/banzai/filter/math_filter.rb
index 2247984b86d..53dafe45fb3 100644
--- a/lib/banzai/filter/math_filter.rb
+++ b/lib/banzai/filter/math_filter.rb
@@ -10,6 +10,11 @@ module Banzai
# HTML filter that adds class="code math" and removes the dollar sign in $`2+2`$.
#
class MathFilter < HTML::Pipeline::Filter
+ CSS_MATH = 'pre.code.language-math'
+ XPATH_MATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_MATH).freeze
+ CSS_CODE = 'code'
+ XPATH_CODE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_CODE).freeze
+
# Attribute indicating inline or display math.
STYLE_ATTRIBUTE = 'data-math-style'
@@ -21,7 +26,7 @@ module Banzai
DOLLAR_SIGN = '$'
def call
- doc.css('code').each do |code|
+ doc.xpath(XPATH_CODE).each do |code|
closing = code.next
opening = code.previous
@@ -39,7 +44,7 @@ module Banzai
end
end
- doc.css('pre.code.language-math').each do |el|
+ doc.xpath(XPATH_MATH).each do |el|
el[STYLE_ATTRIBUTE] = 'display'
el[:class] += " #{TAG_CLASS}"
end
diff --git a/lib/banzai/filter/mermaid_filter.rb b/lib/banzai/filter/mermaid_filter.rb
index f0adb83af8a..aaaf851ccf0 100644
--- a/lib/banzai/filter/mermaid_filter.rb
+++ b/lib/banzai/filter/mermaid_filter.rb
@@ -4,8 +4,11 @@
module Banzai
module Filter
class MermaidFilter < HTML::Pipeline::Filter
+ CSS = 'pre[lang="mermaid"] > code'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
def call
- doc.css('pre[lang="mermaid"] > code').add_class('js-render-mermaid')
+ doc.xpath(XPATH).add_class('js-render-mermaid')
doc
end
diff --git a/lib/banzai/filter/plantuml_filter.rb b/lib/banzai/filter/plantuml_filter.rb
index 37d4126c1ba..93370178a61 100644
--- a/lib/banzai/filter/plantuml_filter.rb
+++ b/lib/banzai/filter/plantuml_filter.rb
@@ -8,12 +8,15 @@ module Banzai
# HTML that replaces all `code plantuml` tags with PlantUML img tags.
#
class PlantumlFilter < HTML::Pipeline::Filter
+ CSS = 'pre > code[lang="plantuml"]'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
def call
- return doc unless settings.plantuml_enabled? && doc.at('pre > code[lang="plantuml"]')
+ return doc unless settings.plantuml_enabled? && doc.at_xpath(XPATH)
plantuml_setup
- doc.css('pre > code[lang="plantuml"]').each do |node|
+ doc.xpath(XPATH).each do |node|
img_tag = Nokogiri::HTML::DocumentFragment.parse(
Asciidoctor::PlantUml::Processor.plantuml_content(node.content, {}))
node.parent.replace(img_tag)
diff --git a/lib/banzai/filter/suggestion_filter.rb b/lib/banzai/filter/suggestion_filter.rb
index 56a14ec0737..aa1fcb1021c 100644
--- a/lib/banzai/filter/suggestion_filter.rb
+++ b/lib/banzai/filter/suggestion_filter.rb
@@ -7,10 +7,13 @@ module Banzai
# Class used for tagging elements that should be rendered
TAG_CLASS = 'js-render-suggestion'
+ CSS = 'pre.language-suggestion > code'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
def call
return doc unless suggestions_filter_enabled?
- doc.search('pre.language-suggestion > code').each do |node|
+ doc.xpath(XPATH).each do |node|
node.add_class(TAG_CLASS)
end
diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb
index b16ea689d2e..f1440c13d47 100644
--- a/lib/banzai/filter/syntax_highlight_filter.rb
+++ b/lib/banzai/filter/syntax_highlight_filter.rb
@@ -14,8 +14,11 @@ module Banzai
PARAMS_DELIMITER = ':'
LANG_PARAMS_ATTR = 'data-lang-params'
+ CSS = 'pre:not([data-math-style]):not([data-mermaid-style]):not([data-kroki-style]) > code'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
def call
- doc.search('pre:not([data-math-style]):not([data-mermaid-style]):not([data-kroki-style]) > code').each do |node|
+ doc.xpath(XPATH).each do |node|
highlight_node(node)
end
diff --git a/lib/banzai/filter/table_of_contents_filter.rb b/lib/banzai/filter/table_of_contents_filter.rb
index b362607aed2..13ca9cde567 100644
--- a/lib/banzai/filter/table_of_contents_filter.rb
+++ b/lib/banzai/filter/table_of_contents_filter.rb
@@ -19,6 +19,9 @@ module Banzai
class TableOfContentsFilter < HTML::Pipeline::Filter
include Gitlab::Utils::Markdown
+ CSS = 'h1, h2, h3, h4, h5, h6'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
def call
return doc if context[:no_header_anchors]
@@ -27,7 +30,7 @@ module Banzai
headers = Hash.new(0)
header_root = current_header = HeaderNode.new
- doc.css('h1, h2, h3, h4, h5, h6').each do |node|
+ doc.xpath(XPATH).each do |node|
if header_content = node.children.first
id = string_to_anchor(node.text)
diff --git a/lib/banzai/filter/wiki_link_filter.rb b/lib/banzai/filter/wiki_link_filter.rb
index 44f13612fde..2b95d87ff8e 100644
--- a/lib/banzai/filter/wiki_link_filter.rb
+++ b/lib/banzai/filter/wiki_link_filter.rb
@@ -10,14 +10,21 @@ module Banzai
class WikiLinkFilter < HTML::Pipeline::Filter
include Gitlab::Utils::SanitizeNodeLink
+ CSS_A = 'a:not(.gfm)'
+ XPATH_A = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_A).freeze
+ CSS_VA = 'video, audio'
+ XPATH_VA = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_VA).freeze
+ CSS_IMG = 'img'
+ XPATH_IMG = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_IMG).freeze
+
def call
return doc unless wiki?
- doc.search('a:not(.gfm)').each { |el| process_link(el.attribute('href'), el) }
+ doc.xpath(XPATH_A).each { |el| process_link(el.attribute('href'), el) }
- doc.search('video, audio').each { |el| process_link(el.attribute('src'), el) }
+ doc.xpath(XPATH_VA).each { |el| process_link(el.attribute('src'), el) }
- doc.search('img').each do |el|
+ doc.xpath(XPATH_IMG).each do |el|
attr = el.attribute('data-src') || el.attribute('src')
process_link(attr, el)
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index c6997288b65..4489fc9f3b2 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -84,7 +84,7 @@ module Gitlab
Gitlab::Auth::UniqueIpsLimiter.limit_user! do
user = User.by_login(login)
- break if user && !user.can?(:log_in)
+ break if user && !can_user_login_with_non_expired_password?(user)
authenticators = []
@@ -182,7 +182,7 @@ module Gitlab
if valid_oauth_token?(token)
user = User.id_in(token.resource_owner_id).first
- return unless user&.can?(:log_in)
+ return unless user && can_user_login_with_non_expired_password?(user)
Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities)
end
@@ -200,7 +200,7 @@ module Gitlab
return if project && token.user.project_bot? && !project.bots.include?(token.user)
- if token.user.can?(:log_in) || token.user.project_bot?
+ if can_user_login_with_non_expired_password?(token.user) || token.user.project_bot?
Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes))
end
end
@@ -285,7 +285,7 @@ module Gitlab
return unless build.project.builds_enabled?
if build.user
- return unless build.user.can?(:log_in) || (build.user.project_bot? && build.project.bots&.include?(build.user))
+ return unless can_user_login_with_non_expired_password?(build.user) || (build.user.project_bot? && build.project.bots&.include?(build.user))
# If user is assigned to build, use restricted credentials of user
Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities)
@@ -380,6 +380,10 @@ module Gitlab
user.increment_failed_attempts!
end
+
+ def can_user_login_with_non_expired_password?(user)
+ user.can?(:log_in) && !user.password_expired?
+ end
end
end
end
diff --git a/lib/gitlab/auth/user_access_denied_reason.rb b/lib/gitlab/auth/user_access_denied_reason.rb
index 36b54ba2e46..6639000dba8 100644
--- a/lib/gitlab/auth/user_access_denied_reason.rb
+++ b/lib/gitlab/auth/user_access_denied_reason.rb
@@ -23,6 +23,9 @@ module Gitlab
"Your primary email address is not confirmed. "\
"Please check your inbox for the confirmation instructions. "\
"In case the link is expired, you can request a new confirmation email at #{Rails.application.routes.url_helpers.new_user_confirmation_url}"
+ when :password_expired
+ "Your password expired. "\
+ "Please access GitLab from a web browser to update your password."
else
"Your account has been blocked."
end
@@ -41,6 +44,8 @@ module Gitlab
:deactivated
elsif !@user.confirmed?
:unconfirmed
+ elsif @user.password_expired?
+ :password_expired
else
:blocked
end
diff --git a/lib/gitlab/diff/suggestions_parser.rb b/lib/gitlab/diff/suggestions_parser.rb
index f3e6fc455ac..6f126147113 100644
--- a/lib/gitlab/diff/suggestions_parser.rb
+++ b/lib/gitlab/diff/suggestions_parser.rb
@@ -6,6 +6,9 @@ module Gitlab
# Matches for instance "-1", "+1" or "-1+2".
SUGGESTION_CONTEXT = /^(\-(?<above>\d+))?(\+(?<below>\d+))?$/.freeze
+ CSS = 'pre.language-suggestion'
+ XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze
+
class << self
# Returns an array of Gitlab::Diff::Suggestion which represents each
# suggestion in the given text.
@@ -17,7 +20,7 @@ module Gitlab
no_original_data: true,
suggestions_filter_enabled: supports_suggestion)
doc = Nokogiri::HTML(html)
- suggestion_nodes = doc.search('pre.language-suggestion')
+ suggestion_nodes = doc.xpath(XPATH)
return [] if suggestion_nodes.empty?
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index d70e5c3594c..77e0e2ca96c 100644
--- a/lib/gitlab/utils.rb
+++ b/lib/gitlab/utils.rb
@@ -197,6 +197,24 @@ module Gitlab
rescue Addressable::URI::InvalidURIError, TypeError
end
+ def removes_sensitive_data_from_url(uri_string)
+ uri = parse_url(uri_string)
+
+ return unless uri
+ return uri_string unless uri.fragment
+
+ stripped_params = CGI.parse(uri.fragment)
+ if stripped_params['access_token']
+ stripped_params['access_token'] = 'filtered'
+ filtered_query = Addressable::URI.new
+ filtered_query.query_values = stripped_params
+
+ uri.fragment = filtered_query.query
+ end
+
+ uri.to_s
+ end
+
# Invert a hash, collecting all keys that map to a given value in an array.
#
# Unlike `Hash#invert`, where the last encountered pair wins, and which has the
diff --git a/lib/gitlab/utils/nokogiri.rb b/lib/gitlab/utils/nokogiri.rb
new file mode 100644
index 00000000000..4b37bb7e5ea
--- /dev/null
+++ b/lib/gitlab/utils/nokogiri.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Utils
+ class Nokogiri
+ class << self
+ # Use Nokogiri to convert a css selector into an xpath selector.
+ # Nokogiri can use css selectors with `doc.search()`. However
+ # for large node trees, it is _much_ slower than using xpath,
+ # by several orders of magnitude.
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/329186
+ def css_to_xpath(css)
+ xpath = ::Nokogiri::CSS.xpath_for(css)
+
+ # Due to https://github.com/sparklemotion/nokogiri/issues/572,
+ # we remove the leading `//` and add `descendant-or-self::`
+ # in order to ensure we're searching from this node and all
+ # descendants.
+ xpath.map { |t| "descendant-or-self::#{t[2..-1]}" }.join('|')
+ end
+ end
+ end
+ end
+end
diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb
index e60d9d6ab69..1d7099ba443 100644
--- a/spec/features/users/login_spec.rb
+++ b/spec/features/users/login_spec.rb
@@ -394,6 +394,25 @@ RSpec.describe 'Login' do
gitlab_sign_in(user)
end
+
+ context 'when the users password is expired' do
+ before do
+ user.update!(password_expires_at: Time.parse('2018-05-08 11:29:46 UTC'))
+ end
+
+ it 'asks for a new password' do
+ expect(authentication_metrics)
+ .to increment(:user_authenticated_counter)
+
+ visit new_user_session_path
+
+ fill_in 'user_login', with: user.email
+ fill_in 'user_password', with: '12345678'
+ click_button 'Sign in'
+
+ expect(current_path).to eq(new_profile_password_path)
+ end
+ end
end
context 'with invalid username and password' do
diff --git a/spec/initializers/lograge_spec.rb b/spec/initializers/lograge_spec.rb
index abb1673bb88..421f6373eff 100644
--- a/spec/initializers/lograge_spec.rb
+++ b/spec/initializers/lograge_spec.rb
@@ -173,6 +173,27 @@ RSpec.describe 'lograge', type: :request do
end
end
+ describe 'with access token in url' do
+ before do
+ event.payload[:location] = 'http://example.com/auth.html#access_token=secret_token&token_type=Bearer'
+ end
+
+ it 'strips location from sensitive information' do
+ subscriber.redirect_to(event)
+ subscriber.process_action(event)
+
+ expect(log_data['location']).not_to include('secret_token')
+ expect(log_data['location']).to include('filtered')
+ end
+
+ it 'leaves non-sensitive information from location' do
+ subscriber.redirect_to(event)
+ subscriber.process_action(event)
+
+ expect(log_data['location']).to include('&token_type=Bearer')
+ end
+ end
+
context 'with db payload' do
context 'when RequestStore is enabled', :request_store do
it 'includes db counters' do
diff --git a/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb b/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb
index ebe1ca4d403..55038d58f22 100644
--- a/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb
@@ -36,9 +36,11 @@ RSpec.describe Banzai::Pipeline::PostProcessPipeline do
end
let(:doc) { HTML::Pipeline.parse(html) }
+ let(:non_related_xpath_calls) { 2 }
it 'searches for attributes only once' do
- expect(doc).to receive(:search).once.and_call_original
+ expect(doc).to receive(:xpath).exactly(non_related_xpath_calls + 1).times
+ .and_call_original
subject
end
@@ -49,7 +51,8 @@ RSpec.describe Banzai::Pipeline::PostProcessPipeline do
end
it 'searches for attributes twice' do
- expect(doc).to receive(:search).twice.and_call_original
+ expect(doc).to receive(:xpath).exactly(non_related_xpath_calls + 2).times
+ .and_call_original
subject
end
diff --git a/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb b/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb
index d3c6cde5590..102d6fba97f 100644
--- a/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb
+++ b/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb
@@ -57,5 +57,13 @@ RSpec.describe Gitlab::Auth::UserAccessDeniedReason do
it { is_expected.to eq('Your account is pending approval from your administrator and hence blocked.') }
end
+
+ context 'when the user has expired password' do
+ before do
+ user.update!(password_expires_at: 2.days.ago)
+ end
+
+ it { is_expected.to eq('Your password expired. Please access GitLab from a web browser to update your password.') }
+ end
end
end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index ae9c697e0b9..3d6c04fd484 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -433,6 +433,13 @@ RSpec.describe Gitlab::GitAccess do
expect { pull_access_check }.to raise_forbidden("Your account has been deactivated by your administrator. Please log back in from a web browser to reactivate your account at #{Gitlab.config.gitlab.url}")
end
+ it 'disallows users with expired password to pull' do
+ project.add_maintainer(user)
+ user.update!(password_expires_at: 2.minutes.ago)
+
+ expect { pull_access_check }.to raise_forbidden("Your password expired. Please access GitLab from a web browser to update your password.")
+ end
+
context 'when the project repository does not exist' do
before do
project.add_guest(user)
@@ -969,6 +976,13 @@ RSpec.describe Gitlab::GitAccess do
expect { push_access_check }.to raise_forbidden("Your account has been deactivated by your administrator. Please log back in from a web browser to reactivate your account at #{Gitlab.config.gitlab.url}")
end
+ it 'disallows users with expired password to push' do
+ project.add_maintainer(user)
+ user.update!(password_expires_at: 2.minutes.ago)
+
+ expect { push_access_check }.to raise_forbidden("Your password expired. Please access GitLab from a web browser to update your password.")
+ end
+
it 'cleans up the files' do
expect(project.repository).to receive(:clean_stale_repository_files).and_call_original
expect { push_access_check }.not_to raise_error
diff --git a/spec/lib/gitlab/utils/nokogiri_spec.rb b/spec/lib/gitlab/utils/nokogiri_spec.rb
new file mode 100644
index 00000000000..90f137f53c8
--- /dev/null
+++ b/spec/lib/gitlab/utils/nokogiri_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Utils::Nokogiri do
+ describe '#css_to_xpath' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:css, :xpath) do
+ 'img' | "descendant-or-self::img"
+ 'a.gfm' | "descendant-or-self::a[contains(concat(' ',normalize-space(@class),' '),' gfm ')]"
+ 'a:not(.gfm)' | "descendant-or-self::a[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]"
+ 'video, audio' | "descendant-or-self::video|descendant-or-self::audio"
+ '[data-math-style]' | "descendant-or-self::*[@data-math-style]"
+ '[data-mermaid-style]' | "descendant-or-self::*[@data-mermaid-style]"
+ '.js-render-metrics' | "descendant-or-self::*[contains(concat(' ',normalize-space(@class),' '),' js-render-metrics ')]"
+ 'h1, h2, h3, h4, h5, h6' | "descendant-or-self::h1|descendant-or-self::h2|descendant-or-self::h3|descendant-or-self::h4|descendant-or-self::h5|descendant-or-self::h6"
+ 'pre.code.language-math' | "descendant-or-self::pre[contains(concat(' ',normalize-space(@class),' '),' code ') and contains(concat(' ',normalize-space(@class),' '),' language-math ')]"
+ 'pre > code[lang="plantuml"]' | "descendant-or-self::pre/code[@lang=\"plantuml\"]"
+ 'pre[lang="mermaid"] > code' | "descendant-or-self::pre[@lang=\"mermaid\"]/code"
+ 'pre.language-suggestion' | "descendant-or-self::pre[contains(concat(' ',normalize-space(@class),' '),' language-suggestion ')]"
+ 'pre.language-suggestion > code' | "descendant-or-self::pre[contains(concat(' ',normalize-space(@class),' '),' language-suggestion ')]/code"
+ 'a.gfm[data-reference-type="user"]' | "descendant-or-self::a[contains(concat(' ',normalize-space(@class),' '),' gfm ') and @data-reference-type=\"user\"]"
+ 'a:not(.gfm), img:not(.gfm), video:not(.gfm), audio:not(.gfm)' | "descendant-or-self::a[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]|descendant-or-self::img[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]|descendant-or-self::video[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]|descendant-or-self::audio[not(contains(concat(' ',normalize-space(@class),' '),' gfm '))]"
+ 'pre:not([data-math-style]):not([data-mermaid-style]):not([data-kroki-style]) > code' | "descendant-or-self::pre[not(@data-math-style) and not(@data-mermaid-style) and not(@data-kroki-style)]/code"
+ end
+
+ with_them do
+ it 'generates the xpath' do
+ expect(described_class.css_to_xpath(css)).to eq xpath
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 11dba610faf..a7ccce0aaab 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -417,6 +417,29 @@ RSpec.describe Gitlab::Utils do
end
end
+ describe '.removes_sensitive_data_from_url' do
+ it 'returns string object' do
+ expect(described_class.removes_sensitive_data_from_url('http://gitlab.com')).to be_instance_of(String)
+ end
+
+ it 'returns nil when URI cannot be parsed' do
+ expect(described_class.removes_sensitive_data_from_url('://gitlab.com')).to be nil
+ end
+
+ it 'returns nil with invalid parameter' do
+ expect(described_class.removes_sensitive_data_from_url(1)).to be nil
+ end
+
+ it 'returns string with filtered access_token param' do
+ expect(described_class.removes_sensitive_data_from_url('http://gitlab.com/auth.html#access_token=secret_token')).to eq('http://gitlab.com/auth.html#access_token=filtered')
+ end
+
+ it 'returns string with filtered access_token param but other params preserved' do
+ expect(described_class.removes_sensitive_data_from_url('http://gitlab.com/auth.html#access_token=secret_token&token_type=Bearer&state=test'))
+ .to include('&token_type=Bearer', '&state=test')
+ end
+ end
+
describe 'multiple_key_invert' do
it 'invert keys with array values' do
hash = {
diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb
index e677f5558fd..bbbc5d08c07 100644
--- a/spec/policies/global_policy_spec.rb
+++ b/spec/policies/global_policy_spec.rb
@@ -239,6 +239,14 @@ RSpec.describe GlobalPolicy do
it { is_expected.not_to be_allowed(:access_api) }
end
+ context 'user with expired password' do
+ before do
+ current_user.update!(password_expires_at: 2.minutes.ago)
+ end
+
+ it { is_expected.not_to be_allowed(:access_api) }
+ end
+
context 'when terms are enforced' do
before do
enforce_terms
@@ -418,6 +426,14 @@ RSpec.describe GlobalPolicy do
it { is_expected.not_to be_allowed(:access_git) }
end
+
+ context 'user with expired password' do
+ before do
+ current_user.update!(password_expires_at: 2.minutes.ago)
+ end
+
+ it { is_expected.not_to be_allowed(:access_git) }
+ end
end
describe 'read instance metadata' do
@@ -494,6 +510,14 @@ RSpec.describe GlobalPolicy do
it { is_expected.not_to be_allowed(:use_slash_commands) }
end
+
+ context 'user with expired password' do
+ before do
+ current_user.update!(password_expires_at: 2.minutes.ago)
+ end
+
+ it { is_expected.not_to be_allowed(:use_slash_commands) }
+ end
end
describe 'create_snippet' do
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index a1e28c18769..279c65fc2f4 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -35,6 +35,26 @@ RSpec.describe 'Git HTTP requests' do
expect(response.header['WWW-Authenticate']).to start_with('Basic ')
end
end
+
+ context "when password is expired" do
+ it "responds to downloads with status 401 Unauthorized" do
+ user.update!(password_expires_at: 2.days.ago)
+
+ download(path, user: user.username, password: user.password) do |response|
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+ end
+ end
+
+ context "when user is blocked" do
+ let(:user) { create(:user, :blocked) }
+
+ it "responds to downloads with status 401 Unauthorized" do
+ download(path, user: user.username, password: user.password) do |response|
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+ end
+ end
end
context "when authentication succeeds" do
@@ -75,6 +95,15 @@ RSpec.describe 'Git HTTP requests' do
expect(response.header['WWW-Authenticate']).to start_with('Basic ')
end
end
+
+ context "when password is expired" do
+ it "responds to uploads with status 401 Unauthorized" do
+ user.update!(password_expires_at: 2.days.ago)
+ upload(path, user: user.username, password: user.password) do |response|
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+ end
+ end
end
context "when authentication succeeds" do
@@ -576,6 +605,16 @@ RSpec.describe 'Git HTTP requests' do
it_behaves_like 'pulls are allowed'
it_behaves_like 'pushes are allowed'
+
+ context "when password is expired" do
+ it "responds to downloads with status 401 unauthorized" do
+ user.update!(password_expires_at: 2.days.ago)
+
+ download(path, **env) do |response|
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+ end
+ end
end
context 'when user has 2FA enabled' do
@@ -649,6 +688,18 @@ RSpec.describe 'Git HTTP requests' do
expect(response).to have_gitlab_http_status(:ok)
end
end
+
+ context "when password is expired" do
+ it "responds to uploads with status 401 unauthorized" do
+ user.update!(password_expires_at: 2.days.ago)
+
+ write_access_token = create(:personal_access_token, user: user, scopes: [:write_repository])
+
+ upload(path, user: user.username, password: write_access_token.token) do |response|
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+ end
+ end
end
end
@@ -860,6 +911,16 @@ RSpec.describe 'Git HTTP requests' do
expect(response).to have_gitlab_http_status(:not_found)
end
+
+ context 'when users password is expired' do
+ it 'rejects pulls with 401 unauthorized' do
+ user.update!(password_expires_at: 2.days.ago)
+
+ download(path, user: 'gitlab-ci-token', password: build.token) do |response|
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+ end
+ end
end
end
end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 4e18c9cb4ca..0e3a0252638 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -346,9 +346,7 @@ RSpec.describe 'Git LFS API and storage' do
let_it_be(:user) { create(:user, password_expires_at: 1.minute.ago)}
let(:role) { :reporter}
- # TODO: This should return a 404 response
- # https://gitlab.com/gitlab-org/gitlab/-/issues/292006
- it_behaves_like 'LFS http 200 response'
+ it_behaves_like 'LFS http 401 response'
end
context 'when user is blocked' do