New upstream version 11.3.11+dfsg

CHANGELOG.md

@@ -2,6 +2,45 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 11.3.11 (2018-11-26)
+
+### Security (33 changes)
+
+- Filter user sensitive data from discussions JSON. !2537
+- Escape entity title while autocomplete template rendering to prevent XSS. !2557
+- Restrict Personal Access Tokens to API scope on web requests.
+- Fix XSS in merge request source branch name.
+- Escape user fullname while rendering autocomplete template to prevent XSS.
+- Fix CRLF vulnerability in Project hooks.
+- Fix possible XSS attack in Markdown urls with spaces.
+- Redact sensitive information on gitlab-workhorse log.
+- Set timeout for syntax highlighting.
+- Do not follow redirects in Prometheus service when making http requests to the configured api url.
+- Persist only SHA digest of PersonalAccessToken#token.
+- Sanitize JSON data properly to fix XSS on Issue details page.
+- Don't expose confidential information in commit message list.
+- Markdown API no longer displays confidential title references unless authorized.
+- Provide email notification when a user changes their email address.
+- Properly filter private references from system notes.
+- Redact personal tokens in unsubscribe links.
+- Resolve reflected XSS in Ouath authorize window.
+- Fix SSRF in project integrations.
+- Fix stored XSS in merge requests from imported repository.
+- Fixed ability to comment on locked/confidential issues.
+- Fixed ability of guest users to edit/delete comments on locked or confidential issues.
+- Fix milestone promotion authorization check.
+- Monkey kubeclient to not follow any redirects.
+- Configure mermaid to not render HTML content in diagrams.
+- Redact confidential events in the API.
+- Fix xss vulnerability sourced from package.json.
+- Fix a possible symlink time of check to time of use race condition in GitLab Pages.
+- Removed ability to see private group names when the group id is entered in the url.
+- Fix stored XSS for Environments.
+- Block loopback addresses in UrlBlocker.
+- Prevent SSRF attacks in HipChat integration.
+- Validate Wiki attachments are valid temporary files.
+
+
 ## 11.3.10 (2018-11-18)
 
 ### Security (1 change)

GITLAB_PAGES_VERSION

@@ -1 +1 @@
-1.1.0
+1.1.1

GITLAB_WORKHORSE_VERSION

@@ -1 +1 @@
-6.1.0
+6.1.2

VERSION

@@ -1 +1 @@
-11.3.10
+11.3.11

app/assets/javascripts/behaviors/markdown/render_mermaid.js

@@ -25,6 +25,9 @@ export default function renderMermaid($els) {
       },
       // mermaidAPI options
       theme: 'neutral',
+      flowchart: {
+        htmlLabels: false,
+      },
     });
 
     $els.each((i, el) => {

app/controllers/application_controller.rb

@@ -10,8 +10,8 @@ class ApplicationController < ActionController::Base
   include WorkhorseHelper
   include EnforcesTwoFactorAuthentication
   include WithPerformanceBar
+  include SessionlessAuthentication
 
-  before_action :authenticate_sessionless_user!
   before_action :authenticate_user!
   before_action :enforce_terms!, if: :should_enforce_terms?
   before_action :validate_user_service_ticket!
@@ -140,13 +140,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  # This filter handles personal access tokens, and atom requests with rss tokens
-  def authenticate_sessionless_user!
-    user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user
-
-    sessionless_sign_in(user) if user
-  end
-
   def log_exception(exception)
     Raven.capture_exception(exception) if sentry_enabled?
 
@@ -412,25 +405,11 @@ class ApplicationController < ActionController::Base
     Gitlab::I18n.with_user_locale(current_user, &block)
   end
 
-  def sessionless_sign_in(user)
-    if user && can?(user, :log_in)
-      # Notice we are passing store false, so the user is not
-      # actually stored in the session and a token is needed
-      # for every request. If you want the token to work as a
-      # sign in token, you can simply remove store: false.
-      sign_in(user, store: false, message: :sessionless_sign_in)
-    end
-  end
-
   def set_page_title_header
     # Per https://tools.ietf.org/html/rfc5987, headers need to be ISO-8859-1, not UTF-8
     response.headers['Page-Title'] = URI.escape(page_title('GitLab'))
   end
 
-  def sessionless_user?
-    current_user && !session.keys.include?('warden.user.user.key')
-  end
-
   def peek_request?
     request.path.start_with?('/-/peek')
   end

app/controllers/concerns/notes_actions.rb

@@ -4,6 +4,7 @@ module NotesActions
   extend ActiveSupport::Concern
 
   included do
+    prepend_before_action :normalize_create_params, only: [:create]
     before_action :set_polling_interval_header, only: [:index]
     before_action :require_noteable!, only: [:index, :create]
     before_action :authorize_admin_note!, only: [:update, :destroy]
@@ -216,6 +217,15 @@ module NotesActions
     ProjectNoteSerializer.new(project: project, noteable: noteable, current_user: current_user)
   end
 
+  # Avoids checking permissions in the wrong object - this ensures that the object we checked permissions for
+  # is the object we're actually creating a note in.
+  def normalize_create_params
+    params[:note].try do |note|
+      note[:noteable_id] = params[:target_id]
+      note[:noteable_type] = params[:target_type].classify
+    end
+  end
+
   def note_project
     strong_memoize(:note_project) do
       next nil unless project

app/controllers/concerns/sessionless_authentication.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+# == SessionlessAuthentication
+#
+# Controller concern to handle PAT and RSS token authentication methods
+#
+module SessionlessAuthentication
+  # This filter handles personal access tokens, and atom requests with rss tokens
+  def authenticate_sessionless_user!(request_format)
+    user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user(request_format)
+
+    sessionless_sign_in(user) if user
+  end
+
+  def sessionless_user?
+    current_user && !session.keys.include?('warden.user.user.key')
+  end
+
+  def sessionless_sign_in(user)
+    if user && can?(user, :log_in)
+      # Notice we are passing store false, so the user is not
+      # actually stored in the session and a token is needed
+      # for every request. If you want the token to work as a
+      # sign in token, you can simply remove store: false.
+      sign_in(user, store: false, message: :sessionless_sign_in)
+    end
+  end
+end

app/controllers/dashboard/projects_controller.rb

@@ -2,6 +2,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   include ParamsBackwardCompatibility
   include RendersMemberAccess
 
+  prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
   before_action :set_non_archived_param
   before_action :default_sorting
   skip_cross_project_access_check :index, :starred

app/controllers/dashboard/todos_controller.rb

@@ -2,6 +2,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
   include ActionView::Helpers::NumberHelper
 
   before_action :authorize_read_project!, only: :index
+  before_action :authorize_read_group!, only: :index
   before_action :find_todos, only: [:index, :destroy_all]
 
   def index
@@ -58,6 +59,15 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     end
   end
 
+  def authorize_read_group!
+    group_id = params[:group_id]
+
+    if group_id.present?
+      group = Group.find(group_id)
+      render_404 unless can?(current_user, :read_group, group)
+    end
+  end
+
   def find_todos
     @todos ||= TodosFinder.new(current_user, todo_params).execute
   end

app/controllers/dashboard_controller.rb

@@ -9,6 +9,9 @@ class DashboardController < Dashboard::ApplicationController
     :label_name
   ].freeze
 
+  prepend_before_action(only: [:issues]) { authenticate_sessionless_user!(:rss) }
+  prepend_before_action(only: [:issues_calendar]) { authenticate_sessionless_user!(:ics) }
+
   before_action :event_filter, only: :activity
   before_action :projects, only: [:issues, :merge_requests]
   before_action :set_show_full_reference, only: [:issues, :merge_requests]

app/controllers/graphql_controller.rb

@@ -1,6 +1,7 @@
 class GraphqlController < ApplicationController
   # Unauthenticated users have access to the API for public data
   skip_before_action :authenticate_user!
+  prepend_before_action(only: [:execute]) { authenticate_sessionless_user!(:api) }
 
   before_action :check_graphql_feature_flag!
 

app/controllers/groups_controller.rb

@@ -7,6 +7,9 @@ class GroupsController < Groups::ApplicationController
 
   respond_to :html
 
+  prepend_before_action(only: [:show, :issues]) { authenticate_sessionless_user!(:rss) }
+  prepend_before_action(only: [:issues_calendar]) { authenticate_sessionless_user!(:ics) }
+
   before_action :authenticate_user!, only: [:new, :create]
   before_action :group, except: [:index, :new, :create]
 

app/controllers/oauth/applications_controller.rb

@@ -7,7 +7,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
   before_action :verify_user_oauth_applications_enabled
   before_action :authenticate_user!
   before_action :add_gon_variables
-  before_action :load_scopes, only: [:index, :create, :edit]
+  before_action :load_scopes, only: [:index, :create, :edit, :update]
 
   helper_method :can?
 

app/controllers/projects/commits_controller.rb

@@ -4,6 +4,7 @@ class Projects::CommitsController < Projects::ApplicationController
   include ExtractsPath
   include RendersCommits
 
+  prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
   before_action :whitelist_query_limiting, except: :commits_root
   before_action :require_non_empty_project
   before_action :assign_ref_vars, except: :commits_root

app/controllers/projects/issues_controller.rb

@@ -7,7 +7,10 @@ class Projects::IssuesController < Projects::ApplicationController
   include IssuesCalendar
   include SpammableActions
 
-  prepend_before_action :authenticate_user!, only: [:new]
+  prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
+  prepend_before_action(only: [:calendar]) { authenticate_sessionless_user!(:ics) }
+  prepend_before_action :authenticate_new_issue!, only: [:new]
+  prepend_before_action :store_uri, only: [:new, :show]
 
   before_action :whitelist_query_limiting, only: [:create, :create_merge_request, :move, :bulk_update]
   before_action :check_issues_available!
@@ -213,16 +216,18 @@ class Projects::IssuesController < Projects::ApplicationController
     ] + [{ label_ids: [], assignee_ids: [] }]
   end
 
-  def authenticate_user!
+  def authenticate_new_issue!
     return if current_user
 
     notice = "Please sign in to create the new issue."
 
+    redirect_to new_user_session_path, notice: notice
+  end
+
+  def store_uri
     if request.get? && !request.xhr?
       store_location_for :user, request.fullpath
     end
-
-    redirect_to new_user_session_path, notice: notice
   end
 
   def serializer

app/controllers/projects/milestones_controller.rb

@@ -9,7 +9,10 @@ class Projects::MilestonesController < Projects::ApplicationController
   before_action :authorize_read_milestone!
 
   # Allow admin milestone
-  before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels, :promote]
+  before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels]
+
+  # Allow to promote milestone
+  before_action :authorize_promote_milestone!, only: :promote
 
   respond_to :html
 
@@ -76,7 +79,7 @@ class Projects::MilestonesController < Projects::ApplicationController
 
   def promote
     promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone)
-    flash[:notice] = flash_notice_for(promoted_milestone, project.group)
+    flash[:notice] = flash_notice_for(promoted_milestone, project_group)
 
     respond_to do |format|
       format.html do
@@ -112,6 +115,12 @@ class Projects::MilestonesController < Projects::ApplicationController
 
   protected
 
+  def project_group
+    strong_memoize(:project_group) do
+      project.group
+    end
+  end
+
   def milestones
     strong_memoize(:milestones) do
       MilestonesFinder.new(search_params).execute
@@ -126,13 +135,17 @@ class Projects::MilestonesController < Projects::ApplicationController
     return render_404 unless can?(current_user, :admin_milestone, @project)
   end
 
+  def authorize_promote_milestone!
+    return render_404 unless can?(current_user, :admin_milestone, project_group)
+  end
+
   def milestone_params
     params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event)
   end
 
   def search_params
-    if request.format.json? && @project.group && can?(current_user, :read_group, @project.group)
-      groups = @project.group.self_and_ancestors_ids
+    if request.format.json? && project_group && can?(current_user, :read_group, project_group)
+      groups = project_group.self_and_ancestors_ids
     end
 
     params.permit(:state).merge(project_ids: @project.id, group_ids: groups)

app/controllers/projects/tags_controller.rb

@@ -1,6 +1,8 @@
 class Projects::TagsController < Projects::ApplicationController
   include SortingHelper
 
+  prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
+
   # Authorize
   before_action :require_non_empty_project
   before_action :authorize_download_code!

app/controllers/projects_controller.rb

@@ -5,6 +5,8 @@ class ProjectsController < Projects::ApplicationController
   include PreviewMarkdown
   include SendFileUpload
 
+  prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
+
   before_action :whitelist_query_limiting, only: [:create]
   before_action :authenticate_user!, except: [:index, :show, :activity, :refs]
   before_action :redirect_git_extension, only: [:show]

app/controllers/users_controller.rb

@@ -12,6 +12,7 @@ class UsersController < ApplicationController
                                 calendar_activities: true
 
   skip_before_action :authenticate_user!
+  prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
   before_action :user, except: [:exists]
   before_action :authorize_read_user_profile!,
                 only: [:calendar, :calendar_activities, :groups, :projects, :contributed_projects, :snippets]

app/helpers/milestones_helper.rb

@@ -1,5 +1,6 @@
 module MilestonesHelper
   include EntityDateHelper
+  include Gitlab::Utils::StrongMemoize
 
   def milestones_filter_path(opts = {})
     if @project
@@ -241,4 +242,16 @@ module MilestonesHelper
       dashboard_milestone_path(milestone.safe_title, title: milestone.title)
     end
   end
+
+  def can_admin_project_milestones?
+    strong_memoize(:can_admin_project_milestones) do
+      can?(current_user, :admin_milestone, @project)
+    end
+  end
+
+  def can_admin_group_milestones?
+    strong_memoize(:can_admin_group_milestones) do
+      can?(current_user, :admin_milestone, @project.group)
+    end
+  end
 end

app/mailers/emails/notes.rb

@@ -26,7 +26,7 @@ module Emails
       mail_answer_note_thread(@merge_request, @note, note_thread_options(recipient_id))
     end
 
-    def note_snippet_email(recipient_id, note_id)
+    def note_project_snippet_email(recipient_id, note_id)
       setup_note_mail(note_id, recipient_id)
 
       @snippet = @note.noteable

app/models/concerns/cache_markdown_field.rb

@@ -15,7 +15,7 @@ module CacheMarkdownField
   # Increment this number every time the renderer changes its output
   CACHE_REDCARPET_VERSION         = 3
   CACHE_COMMONMARK_VERSION_START  = 10
-  CACHE_COMMONMARK_VERSION        = 11
+  CACHE_COMMONMARK_VERSION        = 12
 
   # changes to these attributes cause the cache to be invalidates
   INVALIDATED_BY = %w[author project].freeze

app/models/note.rb

@@ -310,7 +310,7 @@ class Note < ActiveRecord::Base
   end
 
   def to_ability_name
-    for_personal_snippet? ? 'personal_snippet' : noteable_type.underscore
+    for_snippet? ? noteable.class.name.underscore : noteable_type.underscore
   end
 
   def can_be_discussion_note?

app/models/project_services/prometheus_service.rb

@@ -72,7 +72,7 @@ class PrometheusService < MonitoringService
   end
 
   def prometheus_client
-    RestClient::Resource.new(api_url) if api_url && manual_configuration? && active?
+    RestClient::Resource.new(api_url, max_redirects: 0) if api_url && manual_configuration? && active?
   end
 
   def prometheus_installed?

app/policies/commit_policy.rb

@@ -2,4 +2,6 @@
 
 class CommitPolicy < BasePolicy
   delegate { @subject.project }
+
+  rule { can?(:download_code) }.enable :read_commit
 end

app/policies/note_policy.rb

@@ -9,8 +9,17 @@ class NotePolicy < BasePolicy
 
   condition(:editable, scope: :subject) { @subject.editable? }
 
+  condition(:can_read_noteable) { can?(:"read_#{@subject.to_ability_name}") }
+
   rule { ~editable }.prevent :admin_note
 
+  # If user can't read the issue/MR/etc then they should not be allowed to do anything to their own notes
+  rule { ~can_read_noteable }.policy do
+    prevent :read_note
+    prevent :admin_note
+    prevent :resolve_note
+  end
+
   rule { is_author }.policy do
     enable :read_note
     enable :admin_note

app/validators/url_validator.rb

@@ -41,12 +41,13 @@ class UrlValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
     @record = record
 
-    if value.present?
-      value.strip!
-    else
+    unless value.present?
       record.errors.add(attribute, 'must be a valid URL')
+      return
     end
 
+    value = strip_value!(record, attribute, value)
+
     Gitlab::UrlBlocker.validate!(value, blocker_args)
   rescue Gitlab::UrlBlocker::BlockedUrlError => e
     record.errors.add(attribute, "is blocked: #{e.message}")
@@ -54,6 +55,13 @@ class UrlValidator < ActiveModel::EachValidator
 
   private
 
+  def strip_value!(record, attribute, value)
+    new_value = value.strip
+    return value if new_value == value
+
+    record.public_send("#{attribute}=", new_value) # rubocop:disable GitlabSecurity/PublicSend
+  end
+
   def default_options
     # By default the validator doesn't block any url based on the ip address
     {

app/views/devise/mailer/email_changed.html.haml

@@ -0,0 +1,12 @@
+= email_default_heading("Hello, #{@resource.name}!")
+
+- if @resource.try(:unconfirmed_email?)
+  %p
+    We're contacting you to notify you that your email is being changed to #{@resource.reload.unconfirmed_email}.
+- else
+  %p
+    We're contacting you to notify you that your email has been changed to #{@resource.email}.
+
+%p
+  If you did not initiate this change, please contact your administrator
+  immediately.

app/views/devise/mailer/email_changed.text.erb

@@ -0,0 +1,10 @@
+Hello, <%= @resource.name %>!
+
+<% if @resource.try(:unconfirmed_email?) %>
+We're contacting you to notify you that your email is being changed to <%= @resource.reload.unconfirmed_email %>.
+<% else %>
+We're contacting you to notify you that your email has been changed to <%= @resource.email %>.
+<% end %>
+
+If you did not initiate this change, please contact your administrator
+immediately.

app/views/projects/commits/_commit.html.haml

@@ -4,18 +4,6 @@
 - ref           = local_assigns.fetch(:ref) { merge_request&.source_branch }
 
 - link = commit_path(project, commit, merge_request: merge_request)
-- cache_key = [project.full_path,
-               ref,
-               commit.id,
-               Gitlab::CurrentSettings.current_application_settings,
-               @path.presence,
-               current_controller?(:commits),
-               merge_request&.iid,
-               view_details,
-               commit.status(ref),
-               I18n.locale].compact
-
-= cache(cache_key, expires_in: 1.day) do
 %li.commit.flex-row.js-toggle-container{ id: "commit-#{commit.short_id}" }
 
   .avatar-cell.d-none.d-sm-block
@@ -37,7 +25,7 @@
         %button.text-expander.js-toggle-button
           = sprite_icon('ellipsis_h', size: 12)
 
-        .commiter
+      .committer
         - commit_author_link = commit_author_link(commit, avatar: false, size: 24)
         - commit_timeago = time_ago_with_tooltip(commit.authored_date, placement: 'bottom')
         - commit_text =  _('%{commit_author_link} authored %{commit_timeago}') % { commit_author_link: commit_author_link, commit_timeago: commit_timeago }

app/views/shared/milestones/_milestone.html.haml

@@ -35,8 +35,8 @@
     .col-sm-2
       .milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end
         - if @project
-          - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
-            - if @project.group
+          - if can_admin_project_milestones? and milestone.active?
+            - if can_admin_group_milestones?
               %button.js-promote-project-milestone-button.btn.btn-blank.btn-sm.btn-grouped.has-tooltip{ title: _('Promote to Group Milestone'),
                 disabled: true,
                 type: 'button',

config/application.rb

@@ -93,7 +93,9 @@ module Gitlab
     # - Sentry DSN (:sentry_dsn)
     # - Deploy keys (:key)
     # - File content from Web Editor (:content)
-    config.filter_parameters += [/token$/, /password/, /secret/]
+    #
+    # NOTE: It is **IMPORTANT** to also update gitlab-workhorse's filter when adding parameters here!
+    config.filter_parameters += [/token$/, /password/, /secret/, /key$/]
     config.filter_parameters += %i(
       certificate
       encrypted_key

config/initializers/devise.rb

@@ -103,6 +103,9 @@ Devise.setup do |config|
   # Send a notification email when the user's password is changed
   config.send_password_change_notification = true
 
+  # Send a notification email when the user's email is changed
+  config.send_email_changed_notification = true
+
   # ==> Configuration for :validatable
   # Range for password length. Default is 6..128.
   config.password_length = 8..128

config/initializers/doorkeeper.rb

@@ -48,6 +48,13 @@ Doorkeeper.configure do
   #
   force_ssl_in_redirect_uri false
 
+  # Specify what redirect URI's you want to block during Application creation.
+  # Any redirect URI is whitelisted by default.
+  #
+  # You can use this option in order to forbid URI's with 'javascript' scheme
+  # for example.
+  forbid_redirect_uri { |uri| %w[data vbscript javascript].include?(uri.scheme.to_s.downcase) }
+
   # Provide support for an owner to be assigned to each registered application (disabled by default)
   # Optional parameter confirmation: true (default false) if you want to enforce ownership of
   # a registered application

config/initializers/rack_attack_global.rb

@@ -33,22 +33,22 @@ class Rack::Attack
   throttle('throttle_authenticated_api', Gitlab::Throttle.authenticated_api_options) do |req|
     Gitlab::Throttle.settings.throttle_authenticated_api_enabled &&
       req.api_request? &&
-      req.authenticated_user_id
+      req.authenticated_user_id([:api])
   end
 
   throttle('throttle_authenticated_web', Gitlab::Throttle.authenticated_web_options) do |req|
     Gitlab::Throttle.settings.throttle_authenticated_web_enabled &&
       req.web_request? &&
-      req.authenticated_user_id
+      req.authenticated_user_id([:api, :rss, :ics])
   end
 
   class Request
     def unauthenticated?
-      !authenticated_user_id
+      !authenticated_user_id([:api, :rss, :ics])
     end
 
-    def authenticated_user_id
-      Gitlab::Auth::RequestAuthenticator.new(self).user&.id
+    def authenticated_user_id(request_formats)
+      Gitlab::Auth::RequestAuthenticator.new(self).user(request_formats)&.id
     end
 
     def api_request?

db/migrate/20181108091549_cleanup_environments_external_url.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class CleanupEnvironmentsExternalUrl < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    update_column_in_batches(:environments, :external_url, nil) do |table, query|
+      query.where(table[:external_url].matches('javascript://%'))
+    end
+  end
+
+  def down
+  end
+end

db/post_migrate/20181026091631_migrate_forbidden_redirect_uris.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class MigrateForbiddenRedirectUris < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  FORBIDDEN_SCHEMES = %w[data:// vbscript:// javascript://]
+  NEW_URI = 'http://forbidden-scheme-has-been-overwritten'
+
+  disable_ddl_transaction!
+
+  def up
+    update_forbidden_uris(:oauth_applications)
+    update_forbidden_uris(:oauth_access_grants)
+  end
+
+  def down
+    # noop
+  end
+
+  private
+
+  def update_forbidden_uris(table_name)
+    update_column_in_batches(table_name, :redirect_uri, NEW_URI) do |table, query|
+      where_clause = FORBIDDEN_SCHEMES.map do |scheme|
+        table[:redirect_uri].matches("#{scheme}%")
+      end.inject(&:or)
+
+      query.where(where_clause)
+    end
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181014121030) do
+ActiveRecord::Schema.define(version: 20181108091549) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

doc/workflow/notifications.md

@@ -63,6 +63,8 @@ Below is the table of events users can be notified of:
 |------------------------------|-------------------------------------------------------------------|------------------------------|
 | New SSH key added            | User                                                              | Security email, always sent. |
 | New email added              | User                                                              | Security email, always sent. |
+| Email changed                | User                                                              | Security email, always sent. |
+| Password changed             | User                                                              | Security email, always sent. |
 | New user created             | User                                                              | Sent on user creation, except for omniauth (LDAP)|
 | User added to project        | User                                                              | Sent when user is added to project |
 | Project access level changed | User                                                              | Sent when user project access level is changed |

lib/banzai/filter/spaced_link_filter.rb

@@ -17,6 +17,9 @@ module Banzai
     # This is a small extension to the CommonMark spec.  If they start allowing
     # spaces in urls, we could then remove this filter.
     #
+    # Note: Filter::SanitizationFilter should always be run sometime after this filter
+    # to prevent XSS attacks
+    #
     class SpacedLinkFilter < HTML::Pipeline::Filter
       include ActionView::Helpers::TagHelper
 

lib/banzai/pipeline/gfm_pipeline.rb

@@ -10,13 +10,16 @@ module Banzai
       def self.filters
         @filters ||= FilterArray[
           Filter::PlantumlFilter,
+
+          # Must always be before the SanitizationFilter to prevent XSS attacks
+          Filter::SpacedLinkFilter,
+
           Filter::SanitizationFilter,
           Filter::SyntaxHighlightFilter,
 
           Filter::MathFilter,
           Filter::ColorFilter,
           Filter::MermaidFilter,
-          Filter::SpacedLinkFilter,
           Filter::VideoLinkFilter,
           Filter::ImageLazyLoadFilter,
           Filter::ImageLinkFilter,

lib/gitlab/auth/request_authenticator.rb

@@ -11,12 +11,18 @@ module Gitlab
         @request = request
       end
 
-      def user
-        find_sessionless_user || find_user_from_warden
+      def user(request_formats)
+        request_formats.each do |format|
+          user = find_sessionless_user(format)
+
+          return user if user
         end
 
-      def find_sessionless_user
-        find_user_from_access_token || find_user_from_feed_token
+        find_user_from_warden
+      end
+
+      def find_sessionless_user(request_format)
+        find_user_from_web_access_token(request_format) || find_user_from_feed_token(request_format)
       rescue Gitlab::Auth::AuthenticationError
         nil
       end

lib/gitlab/auth/user_auth_finders.rb

@@ -25,8 +25,8 @@ module Gitlab
         current_request.env['warden']&.authenticate if verified_request?
       end
 
-      def find_user_from_feed_token
-        return unless rss_request? || ics_request?
+      def find_user_from_feed_token(request_format)
+        return unless valid_rss_format?(request_format)
 
         # NOTE: feed_token was renamed from rss_token but both needs to be supported because
         #       users might have already added the feed to their RSS reader before the rename
@@ -36,6 +36,17 @@ module Gitlab
         User.find_by_feed_token(token) || raise(UnauthorizedError)
       end
 
+      # We only allow Private Access Tokens with `api` scope to be used by web
+      # requests on RSS feeds or ICS files for backwards compatibility.
+      # It is also used by GraphQL/API requests.
+      def find_user_from_web_access_token(request_format)
+        return unless access_token && valid_web_access_format?(request_format)
+
+        validate_access_token!(scopes: [:api])
+
+        access_token.user || raise(UnauthorizedError)
+      end
+
       def find_user_from_access_token
         return unless access_token
 
@@ -107,6 +118,26 @@ module Gitlab
         @current_request ||= ensure_action_dispatch_request(request)
       end
 
+      def valid_web_access_format?(request_format)
+        case request_format
+        when :rss
+          rss_request?
+        when :ics
+          ics_request?
+        when :api
+          api_request?
+        end
+      end
+
+      def valid_rss_format?(request_format)
+        case request_format
+        when :rss
+          rss_request?
+        when :ics
+          ics_request?
+        end
+      end
+
       def rss_request?
         current_request.path.ends_with?('.atom') || current_request.format.atom?
       end
@@ -114,6 +145,10 @@ module Gitlab
       def ics_request?
         current_request.path.ends_with?('.ics') || current_request.format.ics?
       end
+
+      def api_request?
+        current_request.path.starts_with?("/api/")
+      end
     end
   end
 end

lib/gitlab/url_blocker.rb

@@ -1,4 +1,5 @@
 require 'resolv'
+require 'ipaddress'
 
 module Gitlab
   class UrlBlocker
@@ -8,11 +9,8 @@ module Gitlab
       def validate!(url, allow_localhost: false, allow_local_network: true, enforce_user: false, ports: [], protocols: [])
         return true if url.nil?
 
-        begin
-          uri = Addressable::URI.parse(url)
-        rescue Addressable::URI::InvalidURIError
-          raise BlockedUrlError, "URI is invalid"
-        end
+        # Param url can be a string, URI or Addressable::URI
+        uri = parse_url(url)
 
         # Allow imports from the GitLab instance itself but only from the configured ports
         return true if internal?(uri)
@@ -24,7 +22,9 @@ module Gitlab
         validate_hostname!(uri.hostname)
 
         begin
-          addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM)
+          addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM).map do |addr|
+            addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
+          end
         rescue SocketError
           return true
         end
@@ -47,6 +47,18 @@ module Gitlab
 
       private
 
+      def parse_url(url)
+        raise Addressable::URI::InvalidURIError if multiline?(url)
+
+        Addressable::URI.parse(url)
+      rescue Addressable::URI::InvalidURIError, URI::InvalidURIError
+        raise BlockedUrlError, 'URI is invalid'
+      end
+
+      def multiline?(url)
+        CGI.unescape(url.to_s) =~ /\n|\r/
+      end
+
       def validate_port!(port, ports)
         return if port.blank?
         # Only ports under 1024 are restricted
@@ -71,13 +83,14 @@ module Gitlab
 
       def validate_hostname!(value)
         return if value.blank?
+        return if IPAddress.valid?(value)
         return if value =~ /\A\p{Alnum}/
 
-        raise BlockedUrlError, "Hostname needs to start with an alphanumeric character"
+        raise BlockedUrlError, "Hostname or IP address invalid"
       end
 
       def validate_localhost!(addrs_info)
-        local_ips = ["127.0.0.1", "::1", "0.0.0.0"]
+        local_ips = ["::", "0.0.0.0"]
         local_ips.concat(Socket.ip_address_list.map(&:ip_address))
 
         return if (local_ips & addrs_info.map(&:ip_address)).empty?
@@ -92,7 +105,7 @@ module Gitlab
       end
 
       def validate_local_network!(addrs_info)
-        return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
+        return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? || addr.ipv6_unique_local? }
 
         raise BlockedUrlError, "Requests to the local network are not allowed"
       end
@@ -109,11 +122,13 @@ module Gitlab
       end
 
       def internal_web?(uri)
+        uri.scheme == config.gitlab.protocol &&
           uri.hostname == config.gitlab.host &&
           (uri.port.blank? || uri.port == config.gitlab.port)
       end
 
       def internal_shell?(uri)
+        uri.scheme == 'ssh' &&
           uri.hostname == config.gitlab_shell.ssh_host &&
           (uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
       end

spec/controllers/application_controller_spec.rb

@@ -107,59 +107,6 @@ describe ApplicationController do
     end
   end
 
-  describe "#authenticate_user_from_personal_access_token!" do
-    before do
-      stub_authentication_activity_metrics(debug: false)
-    end
-
-    controller(described_class) do
-      def index
-        render text: 'authenticated'
-      end
-    end
-
-    let(:personal_access_token) { create(:personal_access_token, user: user) }
-
-    context "when the 'personal_access_token' param is populated with the personal access token" do
-      it "logs the user in" do
-        expect(authentication_metrics)
-          .to increment(:user_authenticated_counter)
-          .and increment(:user_session_override_counter)
-          .and increment(:user_sessionless_authentication_counter)
-
-        get :index, private_token: personal_access_token.token
-
-        expect(response).to have_gitlab_http_status(200)
-        expect(response.body).to eq('authenticated')
-      end
-    end
-
-    context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do
-      it "logs the user in" do
-        expect(authentication_metrics)
-          .to increment(:user_authenticated_counter)
-          .and increment(:user_session_override_counter)
-          .and increment(:user_sessionless_authentication_counter)
-
-        @request.headers["PRIVATE-TOKEN"] = personal_access_token.token
-        get :index
-
-        expect(response).to have_gitlab_http_status(200)
-        expect(response.body).to eq('authenticated')
-      end
-    end
-
-    it "doesn't log the user in otherwise" do
-      expect(authentication_metrics)
-        .to increment(:user_unauthenticated_counter)
-
-      get :index, private_token: "token"
-
-      expect(response.status).not_to eq(200)
-      expect(response.body).not_to eq('authenticated')
-    end
-  end
-
   describe 'session expiration' do
     controller(described_class) do
       # The anonymous controller will report 401 and fail to run any actions.
@@ -248,74 +195,6 @@ describe ApplicationController do
     end
   end
 
-  describe '#authenticate_sessionless_user!' do
-    before do
-      stub_authentication_activity_metrics(debug: false)
-    end
-
-    describe 'authenticating a user from a feed token' do
-      controller(described_class) do
-        def index
-          render text: 'authenticated'
-        end
-      end
-
-      context "when the 'feed_token' param is populated with the feed token" do
-        context 'when the request format is atom' do
-          it "logs the user in" do
-            expect(authentication_metrics)
-              .to increment(:user_authenticated_counter)
-              .and increment(:user_session_override_counter)
-              .and increment(:user_sessionless_authentication_counter)
-
-            get :index, feed_token: user.feed_token, format: :atom
-
-            expect(response).to have_gitlab_http_status 200
-            expect(response.body).to eq 'authenticated'
-          end
-        end
-
-        context 'when the request format is ics' do
-          it "logs the user in" do
-            expect(authentication_metrics)
-              .to increment(:user_authenticated_counter)
-              .and increment(:user_session_override_counter)
-              .and increment(:user_sessionless_authentication_counter)
-
-            get :index, feed_token: user.feed_token, format: :ics
-
-            expect(response).to have_gitlab_http_status 200
-            expect(response.body).to eq 'authenticated'
-          end
-        end
-
-        context 'when the request format is neither atom nor ics' do
-          it "doesn't log the user in" do
-            expect(authentication_metrics)
-              .to increment(:user_unauthenticated_counter)
-
-            get :index, feed_token: user.feed_token
-
-            expect(response.status).not_to have_gitlab_http_status 200
-            expect(response.body).not_to eq 'authenticated'
-          end
-        end
-      end
-
-      context "when the 'feed_token' param is populated with an invalid feed token" do
-        it "doesn't log the user" do
-          expect(authentication_metrics)
-            .to increment(:user_unauthenticated_counter)
-
-          get :index, feed_token: 'token', format: :atom
-
-          expect(response.status).not_to eq 200
-          expect(response.body).not_to eq 'authenticated'
-        end
-      end
-    end
-  end
-
   describe '#route_not_found' do
     it 'renders 404 if authenticated' do
       allow(controller).to receive(:current_user).and_return(user)
@@ -581,36 +460,6 @@ describe ApplicationController do
 
         expect(response).to have_gitlab_http_status(200)
       end
-
-      context 'for sessionless users' do
-        render_views
-
-        before do
-          sign_out user
-        end
-
-        it 'renders a 403 when the sessionless user did not accept the terms' do
-          get :index, feed_token: user.feed_token, format: :atom
-
-          expect(response).to have_gitlab_http_status(403)
-        end
-
-        it 'renders the error message when the format was html' do
-          get :index,
-              private_token: create(:personal_access_token, user: user).token,
-              format: :html
-
-          expect(response.body).to have_content /accept the terms of service/i
-        end
-
-        it 'renders a 200 when the sessionless user accepted the terms' do
-          accept_terms(user)
-
-          get :index, feed_token: user.feed_token, format: :atom
-
-          expect(response).to have_gitlab_http_status(200)
-        end
-      end
     end
   end
 

spec/controllers/dashboard/projects_controller_spec.rb

@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe Dashboard::ProjectsController do
+  it_behaves_like 'authenticates sessionless user', :index, :atom
+end

spec/controllers/dashboard/todos_controller_spec.rb

@@ -42,6 +42,16 @@ describe Dashboard::TodosController do
       end
     end
 
+    context 'group authorization' do
+      it 'renders 404 when user does not have read access on given group' do
+        unauthorized_group = create(:group, :private)
+
+        get :index, group_id: unauthorized_group.id
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
     context 'when using pagination' do
       let(:last_page) { user.todos.page.total_pages }
       let!(:issues) { create_list(:issue, 3, project: project, assignees: [user]) }

spec/controllers/dashboard_controller_spec.rb

@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe DashboardController do
+  context 'signed in' do
     let(:user) { create(:user) }
     let(:project) { create(:project) }
 
@@ -19,3 +20,7 @@ describe DashboardController do
       it_behaves_like 'issuables requiring filter', :merge_requests
     end
   end
+
+  it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first
+  it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics
+end

spec/controllers/graphql_controller_spec.rb

@@ -52,15 +52,58 @@ describe GraphqlController do
     end
   end
 
+  context 'token authentication' do
+    before do
+      stub_authentication_activity_metrics(debug: false)
+    end
+
+    let(:user) { create(:user, username: 'Simon') }
+    let(:personal_access_token) { create(:personal_access_token, user: user) }
+
+    context "when the 'personal_access_token' param is populated with the personal access token" do
+      it 'logs the user in' do
+        expect(authentication_metrics)
+          .to increment(:user_authenticated_counter)
+                .and increment(:user_session_override_counter)
+                       .and increment(:user_sessionless_authentication_counter)
+
+        run_test_query!(private_token: personal_access_token.token)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(query_response).to eq('echo' => '"Simon" says: test success')
+      end
+    end
+
+    context 'when the personal access token has no api scope' do
+      it 'does not log the user in' do
+        personal_access_token.update(scopes: [:read_user])
+
+        run_test_query!(private_token: personal_access_token.token)
+
+        expect(response).to have_gitlab_http_status(200)
+
+        expect(query_response).to eq('echo' => 'nil says: test success')
+      end
+    end
+
+    context 'without token' do
+      it 'shows public data' do
+        run_test_query!
+
+        expect(query_response).to eq('echo' => 'nil says: test success')
+      end
+    end
+  end
+
   # Chosen to exercise all the moving parts in GraphqlController#execute
-  def run_test_query!(variables: { 'text' => 'test success' })
+  def run_test_query!(variables: { 'text' => 'test success' }, private_token: nil)
     query = <<~QUERY
       query Echo($text: String) {
         echo(text: $text)
       }
     QUERY
 
-    post :execute, query: query, operationName: 'Echo', variables: variables
+    post :execute, query: query, operationName: 'Echo', variables: variables, private_token: private_token
   end
 
   def query_response

spec/controllers/groups_controller_spec.rb

@@ -581,4 +581,24 @@ describe GroupsController do
       end
     end
   end
+
+  context 'token authentication' do
+    it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+      before do
+        default_params.merge!(id: group)
+      end
+    end
+
+    it_behaves_like 'authenticates sessionless user', :issues, :atom, public: true do
+      before do
+        default_params.merge!(id: group, author_id: user.id)
+      end
+    end
+
+    it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics, public: true do
+      before do
+        default_params.merge!(id: group)
+      end
+    end
+  end
 end

spec/controllers/oauth/applications_controller_spec.rb

@@ -23,6 +23,23 @@ describe Oauth::ApplicationsController do
         expect(response).to have_gitlab_http_status(302)
         expect(response).to redirect_to(profile_path)
       end
+
+      context 'redirect_uri' do
+        render_views
+
+        it 'shows an error for a forbidden URI' do
+          invalid_uri_params = {
+            doorkeeper_application: {
+              name: 'foo',
+              redirect_uri: 'javascript://alert()'
+            }
+          }
+
+          post :create, invalid_uri_params
+
+          expect(response.body).to include 'Redirect URI is forbidden by the server'
+        end
+      end
     end
   end
 end

spec/controllers/projects/commits_controller_spec.rb

@@ -5,10 +5,14 @@ describe Projects::CommitsController do
   let(:user) { create(:user) }
 
   before do
-    sign_in(user)
     project.add_maintainer(user)
   end
 
+  context 'signed in' do
+    before do
+      sign_in(user)
+    end
+
     describe "GET commits_root" do
       context "no ref is provided" do
         it 'should redirect to the default branch of the project' do
@@ -91,3 +95,27 @@ describe Projects::CommitsController do
       end
     end
   end
+
+  context 'token authentication' do
+    context 'public project' do
+      it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+        before do
+          public_project = create(:project, :repository, :public)
+
+          default_params.merge!(namespace_id: public_project.namespace, project_id: public_project, id: "master.atom")
+        end
+      end
+    end
+
+    context 'private project' do
+      it_behaves_like 'authenticates sessionless user', :show, :atom, public: false do
+        before do
+          private_project = create(:project, :repository, :private)
+          private_project.add_maintainer(user)
+
+          default_params.merge!(namespace_id: private_project.namespace, project_id: private_project, id: "master.atom")
+        end
+      end
+    end
+  end
+end

spec/controllers/projects/issues_controller_spec.rb

@@ -1049,4 +1049,40 @@ describe Projects::IssuesController do
       end
     end
   end
+
+  context 'private project with token authentication' do
+    let(:private_project) { create(:project, :private) }
+
+    it_behaves_like 'authenticates sessionless user', :index, :atom do
+      before do
+        default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
+
+        private_project.add_maintainer(user)
+      end
+    end
+
+    it_behaves_like 'authenticates sessionless user', :calendar, :ics do
+      before do
+        default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
+
+        private_project.add_maintainer(user)
+      end
+    end
+  end
+
+  context 'public project with token authentication' do
+    let(:public_project) { create(:project, :public) }
+
+    it_behaves_like 'authenticates sessionless user', :index, :atom, public: true do
+      before do
+        default_params.merge!(project_id: public_project, namespace_id: public_project.namespace)
+      end
+    end
+
+    it_behaves_like 'authenticates sessionless user', :calendar, :ics, public: true do
+      before do
+        default_params.merge!(project_id: public_project, namespace_id: public_project.namespace)
+      end
+    end
+  end
 end

spec/controllers/projects/milestones_controller_spec.rb

@@ -143,11 +143,27 @@ describe Projects::MilestonesController do
   end
 
   describe '#promote' do
+    let(:group) { create(:group) }
+
+    before do
+      project.update(namespace: group)
+    end
+
+    context 'when user does not have permission to promote milestone' do
+      before do
+        group.add_guest(user)
+      end
+
+      it 'renders 404' do
+        post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
     context 'promotion succeeds' do
       before do
-        group = create(:group)
         group.add_developer(user)
-        milestone.project.update(namespace: group)
       end
 
       it 'shows group milestone' do
@@ -166,12 +182,17 @@ describe Projects::MilestonesController do
       end
     end
 
-    context 'promotion fails' do
-      it 'shows project milestone' do
+    context 'when user cannot admin group milestones' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'renders 404' do
+        project.update(namespace: user.namespace)
+
         post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
 
-        expect(response).to redirect_to(project_milestone_path(project, milestone))
-        expect(flash[:alert]).to eq('Promotion failed - Project does not belong to a group.')
+        expect(response).to have_gitlab_http_status(404)
       end
     end
   end

spec/controllers/projects/notes_controller_spec.rb

@@ -244,7 +244,7 @@ describe Projects::NotesController do
 
       def post_create(extra_params = {})
         post :create, {
-               note: { note: 'some other note' },
+          note: { note: 'some other note', noteable_id: merge_request.id },
           namespace_id: project.namespace,
           project_id: project,
           target_type: 'merge_request',
@@ -285,6 +285,31 @@ describe Projects::NotesController do
       end
     end
 
+    context 'when target_id and noteable_id do not match' do
+      let(:locked_issue) { create(:issue, project: project) }
+      let(:issue) {create(:issue, project: project)}
+
+      before do
+        locked_issue.update_attribute(:discussion_locked, true)
+        project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
+        project.project_member(user).destroy
+      end
+
+      it 'uses target_id and ignores noteable_id' do
+        request_params = {
+          note: { note: 'some note', noteable_type: 'Issue', noteable_id: locked_issue.id },
+          target_type: 'issue',
+          target_id: issue.id,
+          project_id: project,
+          namespace_id: project.namespace
+        }
+
+        expect { post :create, request_params }.to change { issue.notes.count }.by(1)
+          .and change { locked_issue.notes.count }.by(0)
+        expect(response).to have_gitlab_http_status(302)
+      end
+    end
+
     context 'when the merge request discussion is locked' do
       before do
         project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
@@ -337,6 +362,7 @@ describe Projects::NotesController do
   end
 
   describe 'PUT update' do
+    context "should update the note with a valid issue" do
       let(:request_params) do
         {
           namespace_id: project.namespace,
@@ -358,6 +384,30 @@ describe Projects::NotesController do
         expect { put :update, request_params }.to change { note.reload.note }
       end
     end
+    context "doesnt update the note" do
+      let(:issue)   { create(:issue, :confidential, project: project) }
+      let(:note)    { create(:note, noteable: issue, project: project) }
+
+      before do
+        sign_in(user)
+        project.add_guest(user)
+      end
+
+      it "disallows edits when the issue is confidential and the user has guest permissions" do
+        request_params = {
+          namespace_id: project.namespace,
+          project_id: project,
+          id: note,
+          format: :json,
+          note: {
+            note: "New comment"
+          }
+        }
+        expect { put :update, request_params }.not_to change { note.reload.note }
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
 
   describe 'DELETE destroy' do
     let(:request_params) do

spec/controllers/projects/tags_controller_spec.rb

@@ -35,4 +35,26 @@ describe Projects::TagsController do
       it { is_expected.to respond_with(:not_found) }
     end
   end
+
+  context 'private project with token authentication' do
+    let(:private_project) { create(:project, :repository, :private) }
+
+    it_behaves_like 'authenticates sessionless user', :index, :atom do
+      before do
+        default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
+
+        private_project.add_maintainer(user)
+      end
+    end
+  end
+
+  context 'public project with token authentication' do
+    let(:public_project) { create(:project, :repository, :public) }
+
+    it_behaves_like 'authenticates sessionless user', :index, :atom, public: true do
+      before do
+        default_params.merge!(project_id: public_project, namespace_id: public_project.namespace)
+      end
+    end
+  end
 end

spec/controllers/projects_controller_spec.rb

@@ -882,6 +882,28 @@ describe ProjectsController do
     end
   end
 
+  context 'private project with token authentication' do
+    let(:private_project) { create(:project, :private) }
+
+    it_behaves_like 'authenticates sessionless user', :show, :atom do
+      before do
+        default_params.merge!(id: private_project, namespace_id: private_project.namespace)
+
+        private_project.add_maintainer(user)
+      end
+    end
+  end
+
+  context 'public project with token authentication' do
+    let(:public_project) { create(:project, :public) }
+
+    it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+      before do
+        default_params.merge!(id: public_project, namespace_id: public_project.namespace)
+      end
+    end
+  end
+
   def project_moved_message(redirect_route, project)
     "Project '#{redirect_route.path}' was moved to '#{project.full_path}'. Please update any links and bookmarks that may still have the old path."
   end

spec/controllers/users_controller_spec.rb

@@ -395,6 +395,14 @@ describe UsersController do
     end
   end
 
+  context 'token authentication' do
+    it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+      before do
+        default_params.merge!(username: user.username)
+      end
+    end
+  end
+
   def user_moved_message(redirect_route, user)
     "User '#{redirect_route.path}' was moved to '#{user.full_path}'. Please update any links and bookmarks that may still have the old path."
   end

spec/factories/ci/builds.rb

@@ -257,7 +257,7 @@ FactoryBot.define do
 
     trait :with_runner_session do
       after(:build) do |build|
-        build.build_runner_session(url: 'ws://localhost')
+        build.build_runner_session(url: 'https://localhost')
       end
     end
   end

spec/features/issues/user_comments_on_issue_spec.rb

@@ -40,6 +40,18 @@ describe "User comments on issue", :js do
 
       expect(page.find('pre code').text).to eq code_block_content
     end
+
+    it "does not render html content in mermaid" do
+      html_content = "<img onerror=location=`javascript\\u003aalert\\u0028document.domain\\u0029` src=x>"
+      mermaid_content = "graph LR\n  B-->D(#{html_content});"
+      comment = "```mermaid\n#{mermaid_content}\n```"
+
+      add_note(comment)
+
+      wait_for_requests
+
+      expect(page.find('svg.mermaid')).to have_content html_content
+    end
   end
 
   context "when editing comments" do

spec/features/markdown/mermaid_spec.rb

@@ -18,7 +18,7 @@ describe 'Mermaid rendering', :js do
     visit project_issue_path(project, issue)
 
     %w[A B C D].each do |label|
-      expect(page).to have_selector('svg foreignObject', text: label)
+      expect(page).to have_selector('svg text', text: label)
     end
   end
 end

spec/features/milestones/user_promotes_milestone_spec.rb

@@ -0,0 +1,32 @@
+require 'rails_helper'
+
+describe 'User promotes milestone' do
+  set(:group) { create(:group) }
+  set(:user) { create(:user) }
+  set(:project) { create(:project, namespace: group) }
+  set(:milestone) { create(:milestone, project: project) }
+
+  context 'when user can admin group milestones' do
+    before do
+      group.add_developer(user)
+      sign_in(user)
+      visit(project_milestones_path(project))
+    end
+
+    it "shows milestone promote button" do
+      expect(page).to have_selector('.js-promote-project-milestone-button')
+    end
+  end
+
+  context 'when user cannot admin group milestones' do
+    before do
+      project.add_developer(user)
+      sign_in(user)
+      visit(project_milestones_path(project))
+    end
+
+    it "does not show milestone promote button" do
+      expect(page).not_to have_selector('.js-promote-project-milestone-button')
+    end
+  end
+end

spec/features/projects/commits/user_browses_commits_spec.rb

@@ -4,10 +4,9 @@ describe 'User browses commits' do
   include RepoHelpers
 
   let(:user) { create(:user) }
-  let(:project) { create(:project, :repository, namespace: user.namespace) }
+  let(:project) { create(:project, :public, :repository, namespace: user.namespace) }
 
   before do
-    project.add_maintainer(user)
     sign_in(user)
   end
 
@@ -127,6 +126,26 @@ describe 'User browses commits' do
         .and have_selector('entry summary', text: commit.description[0..10].delete("\r\n"))
     end
 
+    context 'when a commit links to a confidential issue' do
+      let(:confidential_issue) { create(:issue, confidential: true, title: 'Secret issue!', project: project) }
+
+      before do
+        project.repository.create_file(user, 'dummy-file', 'dummy content',
+                                       branch_name: 'feature',
+                                       message: "Linking #{confidential_issue.to_reference}")
+      end
+
+      context 'when the user cannot see confidential issues but was cached with a link', :use_clean_rails_memory_store_fragment_caching do
+        it 'does not render the confidential issue' do
+          visit project_commits_path(project, 'feature')
+          sign_in(create(:user))
+          visit project_commits_path(project, 'feature')
+
+          expect(page).not_to have_link(href: project_issue_path(project, confidential_issue))
+        end
+      end
+    end
+
     context 'master branch' do
       before do
         visit_commits_page

spec/lib/banzai/pipeline/gfm_pipeline_spec.rb

@@ -104,5 +104,17 @@ describe Banzai::Pipeline::GfmPipeline do
 
       expect(output).to include("src=\"test%20image.png\"")
     end
+
+    it 'sanitizes the fixed link' do
+      markdown_xss = "[xss](javascript: alert%28document.domain%29)"
+      output = described_class.to_html(markdown_xss, project: project)
+
+      expect(output).not_to include("javascript")
+
+      markdown_xss = "<invalidtag>\n[xss](javascript:alert%28document.domain%29)"
+      output = described_class.to_html(markdown_xss, project: project)
+
+      expect(output).not_to include("javascript")
+    end
   end
 end

spec/lib/gitlab/auth/request_authenticator_spec.rb

@@ -19,17 +19,17 @@ describe Gitlab::Auth::RequestAuthenticator do
       allow_any_instance_of(described_class).to receive(:find_sessionless_user).and_return(sessionless_user)
       allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user)
 
-      expect(subject.user).to eq sessionless_user
+      expect(subject.user([:api])).to eq sessionless_user
     end
 
     it 'returns session user if no sessionless user found' do
       allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user)
 
-      expect(subject.user).to eq session_user
+      expect(subject.user([:api])).to eq session_user
     end
 
     it 'returns nil if no user found' do
-      expect(subject.user).to be_blank
+      expect(subject.user([:api])).to be_blank
     end
 
     it 'bubbles up exceptions' do
@@ -42,26 +42,26 @@ describe Gitlab::Auth::RequestAuthenticator do
     let!(:feed_token_user) { build(:user) }
 
     it 'returns access_token user first' do
-      allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_return(access_token_user)
+      allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_return(access_token_user)
       allow_any_instance_of(described_class).to receive(:find_user_from_feed_token).and_return(feed_token_user)
 
-      expect(subject.find_sessionless_user).to eq access_token_user
+      expect(subject.find_sessionless_user([:api])).to eq access_token_user
     end
 
     it 'returns feed_token user if no access_token user found' do
       allow_any_instance_of(described_class).to receive(:find_user_from_feed_token).and_return(feed_token_user)
 
-      expect(subject.find_sessionless_user).to eq feed_token_user
+      expect(subject.find_sessionless_user([:api])).to eq feed_token_user
     end
 
     it 'returns nil if no user found' do
-      expect(subject.find_sessionless_user).to be_blank
+      expect(subject.find_sessionless_user([:api])).to be_blank
     end
 
     it 'rescue Gitlab::Auth::AuthenticationError exceptions' do
-      allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_raise(Gitlab::Auth::UnauthorizedError)
+      allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_raise(Gitlab::Auth::UnauthorizedError)
 
-      expect(subject.find_sessionless_user).to be_blank
+      expect(subject.find_sessionless_user([:api])).to be_blank
     end
   end
 end

spec/lib/gitlab/auth/user_auth_finders_spec.rb

@@ -49,6 +49,7 @@ describe Gitlab::Auth::UserAuthFinders do
   describe '#find_user_from_feed_token' do
     context 'when the request format is atom' do
       before do
+        env['SCRIPT_NAME'] = 'url.atom'
         env['HTTP_ACCEPT'] = 'application/atom+xml'
       end
 
@@ -56,17 +57,17 @@ describe Gitlab::Auth::UserAuthFinders do
         it 'returns user if valid feed_token' do
           set_param(:feed_token, user.feed_token)
 
-          expect(find_user_from_feed_token).to eq user
+          expect(find_user_from_feed_token(:rss)).to eq user
         end
 
         it 'returns nil if feed_token is blank' do
-          expect(find_user_from_feed_token).to be_nil
+          expect(find_user_from_feed_token(:rss)).to be_nil
         end
 
         it 'returns exception if invalid feed_token' do
           set_param(:feed_token, 'invalid_token')
 
-          expect { find_user_from_feed_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
+          expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
         end
       end
 
@@ -74,34 +75,38 @@ describe Gitlab::Auth::UserAuthFinders do
         it 'returns user if valid rssd_token' do
           set_param(:rss_token, user.feed_token)
 
-          expect(find_user_from_feed_token).to eq user
+          expect(find_user_from_feed_token(:rss)).to eq user
         end
 
         it 'returns nil if rss_token is blank' do
-          expect(find_user_from_feed_token).to be_nil
+          expect(find_user_from_feed_token(:rss)).to be_nil
         end
 
         it 'returns exception if invalid rss_token' do
           set_param(:rss_token, 'invalid_token')
 
-          expect { find_user_from_feed_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
+          expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
         end
       end
     end
 
     context 'when the request format is not atom' do
       it 'returns nil' do
+        env['SCRIPT_NAME'] = 'json'
+
         set_param(:feed_token, user.feed_token)
 
-        expect(find_user_from_feed_token).to be_nil
+        expect(find_user_from_feed_token(:rss)).to be_nil
       end
     end
 
     context 'when the request format is empty' do
       it 'the method call does not modify the original value' do
+        env['SCRIPT_NAME'] = 'url.atom'
+
         env.delete('action_dispatch.request.formats')
 
-        find_user_from_feed_token
+        find_user_from_feed_token(:rss)
 
         expect(env['action_dispatch.request.formats']).to be_nil
       end
@@ -111,8 +116,12 @@ describe Gitlab::Auth::UserAuthFinders do
   describe '#find_user_from_access_token' do
     let(:personal_access_token) { create(:personal_access_token, user: user) }
 
+    before do
+      env['SCRIPT_NAME'] = 'url.atom'
+    end
+
     it 'returns nil if no access_token present' do
-      expect(find_personal_access_token).to be_nil
+      expect(find_user_from_access_token).to be_nil
     end
 
     context 'when validate_access_token! returns valid' do
@@ -131,9 +140,59 @@ describe Gitlab::Auth::UserAuthFinders do
     end
   end
 
+  describe '#find_user_from_web_access_token' do
+    let(:personal_access_token) { create(:personal_access_token, user: user) }
+
+    before do
+      env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+    end
+
+    it 'returns exception if token has no user' do
+      allow_any_instance_of(PersonalAccessToken).to receive(:user).and_return(nil)
+
+      expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
+    end
+
+    context 'no feed or API requests' do
+      it 'returns nil if the request is not RSS' do
+        expect(find_user_from_web_access_token(:rss)).to be_nil
+      end
+
+      it 'returns nil if the request is not ICS' do
+        expect(find_user_from_web_access_token(:ics)).to be_nil
+      end
+
+      it 'returns nil if the request is not API' do
+        expect(find_user_from_web_access_token(:api)).to be_nil
+      end
+    end
+
+    it 'returns the user for RSS requests' do
+      env['SCRIPT_NAME'] = 'url.atom'
+
+      expect(find_user_from_web_access_token(:rss)).to eq(user)
+    end
+
+    it 'returns the user for ICS requests' do
+      env['SCRIPT_NAME'] = 'url.ics'
+
+      expect(find_user_from_web_access_token(:ics)).to eq(user)
+    end
+
+    it 'returns the user for API requests' do
+      env['SCRIPT_NAME'] = '/api/endpoint'
+
+      expect(find_user_from_web_access_token(:api)).to eq(user)
+    end
+  end
+
   describe '#find_personal_access_token' do
     let(:personal_access_token) { create(:personal_access_token, user: user) }
 
+    before do
+      env['SCRIPT_NAME'] = 'url.atom'
+    end
+
     context 'passed as header' do
       it 'returns token if valid personal_access_token' do
         env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token

spec/lib/gitlab/url_blocker_spec.rb

@@ -10,8 +10,8 @@ describe Gitlab::UrlBlocker do
       expect(described_class.blocked_url?(import_url)).to be false
     end
 
-    it 'allows imports from configured SSH host and port' do
-      import_url = "http://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git"
+    it 'allows mirroring from configured SSH host and port' do
+      import_url = "ssh://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git"
       expect(described_class.blocked_url?(import_url)).to be false
     end
 
@@ -29,24 +29,46 @@ describe Gitlab::UrlBlocker do
       expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['http'])).to be true
     end
 
+    it 'returns true for bad protocol on configured web/SSH host and ports' do
+      web_url = "javascript://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git%0aalert(1)"
+      expect(described_class.blocked_url?(web_url)).to be true
+
+      ssh_url = "javascript://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git%0aalert(1)"
+      expect(described_class.blocked_url?(ssh_url)).to be true
+    end
+
     it 'returns true for localhost IPs' do
+      expect(described_class.blocked_url?('https://[0:0:0:0:0:0:0:0]/foo/foo.git')).to be true
       expect(described_class.blocked_url?('https://0.0.0.0/foo/foo.git')).to be true
-      expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true
-      expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git')).to be true
+      expect(described_class.blocked_url?('https://[::]/foo/foo.git')).to be true
     end
 
     it 'returns true for loopback IP' do
       expect(described_class.blocked_url?('https://127.0.0.2/foo/foo.git')).to be true
+      expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git')).to be true
+      expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true
     end
 
     it 'returns true for alternative version of 127.0.0.1 (0177.1)' do
       expect(described_class.blocked_url?('https://0177.1:65535/foo/foo.git')).to be true
     end
 
+    it 'returns true for alternative version of 127.0.0.1 (017700000001)' do
+      expect(described_class.blocked_url?('https://017700000001:65535/foo/foo.git')).to be true
+    end
+
     it 'returns true for alternative version of 127.0.0.1 (0x7f.1)' do
       expect(described_class.blocked_url?('https://0x7f.1:65535/foo/foo.git')).to be true
     end
 
+    it 'returns true for alternative version of 127.0.0.1 (0x7f.0.0.1)' do
+      expect(described_class.blocked_url?('https://0x7f.0.0.1:65535/foo/foo.git')).to be true
+    end
+
+    it 'returns true for alternative version of 127.0.0.1 (0x7f000001)' do
+      expect(described_class.blocked_url?('https://0x7f000001:65535/foo/foo.git')).to be true
+    end
+
     it 'returns true for alternative version of 127.0.0.1 (2130706433)' do
       expect(described_class.blocked_url?('https://2130706433:65535/foo/foo.git')).to be true
     end
@@ -55,6 +77,27 @@ describe Gitlab::UrlBlocker do
       expect(described_class.blocked_url?('https://127.000.000.001:65535/foo/foo.git')).to be true
     end
 
+    it 'returns true for alternative version of 127.0.0.1 (127.0.1)' do
+      expect(described_class.blocked_url?('https://127.0.1:65535/foo/foo.git')).to be true
+    end
+
+    context 'with ipv6 mapped address' do
+      it 'returns true for localhost IPs' do
+        expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:0.0.0.0]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:0.0.0.0]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:0:0]/foo/foo.git')).to be true
+      end
+
+      it 'returns true for loopback IPs' do
+        expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.1]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:127.0.0.1]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:7f00:1]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.2]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:127.0.0.2]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:7f00:2]/foo/foo.git')).to be true
+      end
+    end
+
     it 'returns true for a non-alphanumeric hostname' do
       stub_resolv
 
@@ -78,7 +121,22 @@ describe Gitlab::UrlBlocker do
     end
 
     context 'when allow_local_network is' do
-      let(:local_ips) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] }
+      let(:local_ips) do
+        [
+          '192.168.1.2',
+          '[0:0:0:0:0:ffff:192.168.1.2]',
+          '[::ffff:c0a8:102]',
+          '10.0.0.2',
+          '[0:0:0:0:0:ffff:10.0.0.2]',
+          '[::ffff:a00:2]',
+          '172.16.0.2',
+          '[0:0:0:0:0:ffff:172.16.0.2]',
+          '[::ffff:ac10:20]',
+          '[feef::1]',
+          '[fee2::]',
+          '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]'
+        ]
+      end
       let(:fake_domain) { 'www.fakedomain.fake' }
 
       context 'true (default)' do
@@ -109,10 +167,14 @@ describe Gitlab::UrlBlocker do
           expect(described_class).not_to be_blocked_url('http://169.254.168.100')
         end
 
-        # This is blocked due to the hostname check: https://gitlab.com/gitlab-org/gitlab-ce/issues/50227
-        it 'blocks IPv6 link-local endpoints' do
-          expect(described_class).to be_blocked_url('http://[::ffff:169.254.169.254]')
-          expect(described_class).to be_blocked_url('http://[::ffff:169.254.168.100]')
+        it 'allows IPv6 link-local endpoints' do
+          expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]')
+          expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.169.254]')
+          expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a9fe]')
+          expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]')
+          expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.168.100]')
+          expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a864]')
+          expect(described_class).not_to be_blocked_url('http://[fe80::c800:eff:fe74:8]')
         end
       end
 
@@ -135,14 +197,20 @@ describe Gitlab::UrlBlocker do
         end
 
         it 'blocks IPv6 link-local endpoints' do
+          expect(described_class).to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]', allow_local_network: false)
           expect(described_class).to be_blocked_url('http://[::ffff:169.254.169.254]', allow_local_network: false)
+          expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a9fe]', allow_local_network: false)
+          expect(described_class).to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]', allow_local_network: false)
           expect(described_class).to be_blocked_url('http://[::ffff:169.254.168.100]', allow_local_network: false)
-          expect(described_class).to be_blocked_url('http://[FE80::C800:EFF:FE74:8]', allow_local_network: false)
+          expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a864]', allow_local_network: false)
+          expect(described_class).to be_blocked_url('http://[fe80::c800:eff:fe74:8]', allow_local_network: false)
         end
       end
 
       def stub_domain_resolv(domain, ip)
-        allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false)])
+        address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false)
+        allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address])
+        allow(address).to receive(:ipv6_v4mapped?).and_return(false)
       end
 
       def unstub_domain_resolv
@@ -183,6 +251,36 @@ describe Gitlab::UrlBlocker do
     end
   end
 
+  describe '#validate_hostname!' do
+    let(:ip_addresses) do
+      [
+        '2001:db8:1f70::999:de8:7648:6e8',
+        'FE80::C800:EFF:FE74:8',
+        '::ffff:127.0.0.1',
+        '::ffff:169.254.168.100',
+        '::ffff:7f00:1',
+        '0:0:0:0:0:ffff:0.0.0.0',
+        'localhost',
+        '127.0.0.1',
+        '127.000.000.001',
+        '0x7f000001',
+        '0x7f.0.0.1',
+        '0x7f.0.0.1',
+        '017700000001',
+        '0177.1',
+        '2130706433',
+        '::',
+        '::1'
+      ]
+    end
+
+    it 'does not raise error for valid Ip addresses' do
+      ip_addresses.each do |ip|
+        expect { described_class.send(:validate_hostname!, ip) }.not_to raise_error
+      end
+    end
+  end
+
   # Resolv does not support resolving UTF-8 domain names
   # See https://bugs.ruby-lang.org/issues/4270
   def stub_resolv

spec/mailers/notify_spec.rb

@@ -522,7 +522,7 @@ describe Notify do
       let(:project_snippet) { create(:project_snippet, project: project) }
       let(:project_snippet_note) { create(:note_on_project_snippet, project: project, noteable: project_snippet) }
 
-      subject { described_class.note_snippet_email(project_snippet_note.author_id, project_snippet_note.id) }
+      subject { described_class.note_project_snippet_email(project_snippet_note.author_id, project_snippet_note.id) }
 
       it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
         let(:model) { project_snippet }

spec/migrations/cleanup_environments_external_url_spec.rb

@@ -0,0 +1,28 @@
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20181108091549_cleanup_environments_external_url.rb')
+
+describe CleanupEnvironmentsExternalUrl, :migration do
+  let(:environments)    { table(:environments) }
+  let(:invalid_entries) { environments.where(environments.arel_table[:external_url].matches('javascript://%')) }
+  let(:namespaces)      { table(:namespaces) }
+  let(:projects)        { table(:projects) }
+
+  before do
+    namespace = namespaces.create(name: 'foo', path: 'foo')
+    project = projects.create!(namespace_id: namespace.id)
+
+    environments.create!(id: 1, project_id: project.id, name: 'poisoned', slug: 'poisoned', external_url: 'javascript://alert("1")')
+  end
+
+  it 'clears every environment with a javascript external_url' do
+    expect do
+      subject.up
+    end.to change { invalid_entries.count }.from(1).to(0)
+  end
+
+  it 'do not removes environments' do
+    expect do
+      subject.up
+    end.not_to change { environments.count }
+  end
+end

spec/migrations/migrate_forbidden_redirect_uris_spec.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20181026091631_migrate_forbidden_redirect_uris.rb')
+
+describe MigrateForbiddenRedirectUris, :migration do
+  let(:oauth_application) { table(:oauth_applications) }
+  let(:oauth_access_grant) { table(:oauth_access_grants) }
+
+  let!(:control_app) { oauth_application.create(random_params) }
+  let!(:control_access_grant) { oauth_application.create(random_params) }
+  let!(:forbidden_js_app) { oauth_application.create(random_params.merge(redirect_uri: 'javascript://alert()')) }
+  let!(:forbidden_vb_app) { oauth_application.create(random_params.merge(redirect_uri: 'VBSCRIPT://alert()')) }
+  let!(:forbidden_access_grant) { oauth_application.create(random_params.merge(redirect_uri: 'vbscript://alert()')) }
+
+  context 'oauth application' do
+    it 'migrates forbidden javascript URI' do
+      expect { migrate! }.to change { forbidden_js_app.reload.redirect_uri }.to('http://forbidden-scheme-has-been-overwritten')
+    end
+
+    it 'migrates forbidden VBScript URI' do
+      expect { migrate! }.to change { forbidden_vb_app.reload.redirect_uri }.to('http://forbidden-scheme-has-been-overwritten')
+    end
+
+    it 'does not migrate a valid URI' do
+      expect { migrate! }.not_to change { control_app.reload.redirect_uri }
+    end
+  end
+
+  context 'access grant' do
+    it 'migrates forbidden VBScript URI' do
+      expect { migrate! }.to change { forbidden_access_grant.reload.redirect_uri }.to('http://forbidden-scheme-has-been-overwritten')
+    end
+
+    it 'does not migrate a valid URI' do
+      expect { migrate! }.not_to change { control_access_grant.reload.redirect_uri }
+    end
+  end
+
+  def random_params
+    {
+      name: 'test',
+      secret: 'test',
+      uid: Doorkeeper::OAuth::Helpers::UniqueToken.generate,
+      redirect_uri: 'http://valid.com'
+    }
+  end
+end

spec/models/note_spec.rb

@@ -517,7 +517,7 @@ describe Note do
 
   describe '#to_ability_name' do
     it 'returns snippet for a project snippet note' do
-      expect(build(:note_on_project_snippet).to_ability_name).to eq('snippet')
+      expect(build(:note_on_project_snippet).to_ability_name).to eq('project_snippet')
     end
 
     it 'returns personal_snippet for a personal snippet note' do

spec/models/project_services/prometheus_service_spec.rb

@@ -11,6 +11,23 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
     it { is_expected.to belong_to :project }
   end
 
+  context 'redirects' do
+    it 'does not follow redirects' do
+      redirect_to = 'https://redirected.example.com'
+      redirect_req_stub = stub_prometheus_request(prometheus_query_url('1'), status: 302, headers: { location: redirect_to })
+      redirected_req_stub = stub_prometheus_request(redirect_to, body: { 'status': 'success' })
+
+      result = service.test
+
+      # result = { success: false, result: error }
+      expect(result[:success]).to be_falsy
+      expect(result[:result]).to be_instance_of(Gitlab::PrometheusClient::Error)
+
+      expect(redirect_req_stub).to have_been_requested
+      expect(redirected_req_stub).not_to have_been_requested
+    end
+  end
+
   describe 'Validations' do
     context 'when manual_configuration is enabled' do
       before do

spec/models/project_spec.rb

@@ -227,6 +227,7 @@ describe Project do
       end
     end
 
+    describe 'import_url' do
       it 'does not allow an invalid URI as import_url' do
         project2 = build(:project, import_url: 'invalid://')
 
@@ -278,6 +279,22 @@ describe Project do
         expect(project2.errors[:import_url].first).to include('Username needs to start with an alphanumeric character')
       end
 
+      include_context 'invalid urls'
+
+      it 'does not allow urls with CR or LF characters' do
+        project = build(:project)
+
+        aggregate_failures do
+          urls_with_CRLF.each do |url|
+            project.import_url = url
+
+            expect(project).not_to be_valid
+            expect(project.errors.full_messages.first).to match(/is blocked: URI is invalid/)
+          end
+        end
+      end
+    end
+
     describe 'project pending deletion' do
       let!(:project_pending_deletion) do
         create(:project,

spec/policies/note_policy_spec.rb

@@ -10,11 +10,50 @@ describe NotePolicy, mdoels: true do
       return @policies if @policies
 
       noteable ||= issue
-      note = create(:note, noteable: noteable, author: user, project: project)
+      note = if noteable.is_a?(Commit)
+               create(:note_on_commit, commit_id: noteable.id, author: user, project: project)
+             else
+               create(:note, noteable: noteable, author: user, project: project)
+             end
 
       @policies = described_class.new(user, note)
     end
 
+    shared_examples_for 'a discussion with a private noteable' do
+      let(:noteable) { issue }
+      let(:policy) { policies(noteable) }
+
+      context 'when the note author can no longer see the noteable' do
+        it 'can not edit nor read the note' do
+          expect(policy).to be_disallowed(:admin_note)
+          expect(policy).to be_disallowed(:resolve_note)
+          expect(policy).to be_disallowed(:read_note)
+        end
+      end
+
+      context 'when the note author can still see the noteable' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'can edit the note' do
+          expect(policy).to be_allowed(:admin_note)
+          expect(policy).to be_allowed(:resolve_note)
+          expect(policy).to be_allowed(:read_note)
+        end
+      end
+    end
+
+    context 'when the project is private' do
+      let(:project) { create(:project, :private, :repository) }
+
+      context 'when the noteable is a commit' do
+        it_behaves_like 'a discussion with a private noteable' do
+          let(:noteable) { project.repository.head_commit }
+        end
+      end
+    end
+
     context 'when the project is public' do
       context 'when the note author is not a project member' do
         it 'can edit a note' do
@@ -24,14 +63,48 @@ describe NotePolicy, mdoels: true do
         end
       end
 
-      context 'when the noteable is a snippet' do
+      context 'when the noteable is a project snippet' do
         it 'can edit note' do
-          policies = policies(create(:project_snippet, project: project))
+          policies = policies(create(:project_snippet, :public, project: project))
 
           expect(policies).to be_allowed(:admin_note)
           expect(policies).to be_allowed(:resolve_note)
           expect(policies).to be_allowed(:read_note)
         end
+
+        context 'when it is private' do
+          it_behaves_like 'a discussion with a private noteable' do
+            let(:noteable) { create(:project_snippet, :private, project: project) }
+          end
+        end
+      end
+
+      context 'when the noteable is a personal snippet' do
+        it 'can edit note' do
+          policies = policies(create(:personal_snippet, :public))
+
+          expect(policies).to be_allowed(:admin_note)
+          expect(policies).to be_allowed(:resolve_note)
+          expect(policies).to be_allowed(:read_note)
+        end
+
+        context 'when it is private' do
+          it 'can not edit nor read the note' do
+            policies = policies(create(:personal_snippet, :private))
+
+            expect(policies).to be_disallowed(:admin_note)
+            expect(policies).to be_disallowed(:resolve_note)
+            expect(policies).to be_disallowed(:read_note)
+          end
+        end
+      end
+
+      context 'when a discussion is confidential' do
+        before do
+          issue.update_attribute(:confidential, true)
+        end
+
+        it_behaves_like 'a discussion with a private noteable'
       end
 
       context 'when a discussion is locked' do

spec/requests/api/applications_spec.rb

@@ -24,7 +24,7 @@ describe API::Applications, :api do
 
       it 'does not allow creating an application with the wrong redirect_uri format' do
         expect do
-          post api('/applications', admin_user), name: 'application_name', redirect_uri: 'wrong_url_format', scopes: ''
+          post api('/applications', admin_user), name: 'application_name', redirect_uri: 'http://', scopes: ''
         end.not_to change { Doorkeeper::Application.count }
 
         expect(response).to have_http_status 400
@@ -32,6 +32,16 @@ describe API::Applications, :api do
         expect(json_response['message']['redirect_uri'][0]).to eq('must be an absolute URI.')
       end
 
+      it 'does not allow creating an application with a forbidden URI format' do
+        expect do
+          post api('/applications', admin_user), name: 'application_name', redirect_uri: 'javascript://alert()', scopes: ''
+        end.not_to change { Doorkeeper::Application.count }
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response).to be_a Hash
+        expect(json_response['message']['redirect_uri'][0]).to eq('is forbidden by the server.')
+      end
+
       it 'does not allow creating an application without a name' do
         expect do
           post api('/applications', admin_user), redirect_uri: 'http://application.url', scopes: ''

spec/support/controllers/sessionless_auth_controller_shared_examples.rb

@@ -0,0 +1,92 @@
+shared_examples 'authenticates sessionless user' do |path, format, params|
+  params ||= {}
+
+  before do
+    stub_authentication_activity_metrics(debug: false)
+  end
+
+  let(:user) { create(:user) }
+  let(:personal_access_token) { create(:personal_access_token, user: user) }
+  let(:default_params) { { format: format }.merge(params.except(:public) || {}) }
+
+  context "when the 'personal_access_token' param is populated with the personal access token" do
+    it 'logs the user in' do
+      expect(authentication_metrics)
+        .to increment(:user_authenticated_counter)
+              .and increment(:user_session_override_counter)
+                     .and increment(:user_sessionless_authentication_counter)
+
+      get path, default_params.merge(private_token: personal_access_token.token)
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(controller.current_user).to eq(user)
+    end
+
+    it 'does not log the user in if page is public', if: params[:public] do
+      get path, default_params
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(controller.current_user).to be_nil
+    end
+  end
+
+  context 'when the personal access token has no api scope', unless: params[:public] do
+    it 'does not log the user in' do
+      expect(authentication_metrics)
+        .to increment(:user_unauthenticated_counter)
+
+      personal_access_token.update(scopes: [:read_user])
+
+      get path, default_params.merge(private_token: personal_access_token.token)
+
+      expect(response).not_to have_gitlab_http_status(200)
+    end
+  end
+
+  context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do
+    it 'logs the user in' do
+      expect(authentication_metrics)
+        .to increment(:user_authenticated_counter)
+              .and increment(:user_session_override_counter)
+                     .and increment(:user_sessionless_authentication_counter)
+
+      @request.headers['PRIVATE-TOKEN'] = personal_access_token.token
+      get path, default_params
+
+      expect(response).to have_gitlab_http_status(200)
+    end
+  end
+
+  context "when the 'feed_token' param is populated with the feed token", if: format == :rss do
+    it "logs the user in" do
+      expect(authentication_metrics)
+        .to increment(:user_authenticated_counter)
+              .and increment(:user_session_override_counter)
+                     .and increment(:user_sessionless_authentication_counter)
+
+      get path, default_params.merge(feed_token: user.feed_token)
+
+      expect(response).to have_gitlab_http_status 200
+    end
+  end
+
+  context "when the 'feed_token' param is populated with an invalid feed token", if: format == :rss, unless: params[:public] do
+    it "logs the user" do
+      expect(authentication_metrics)
+        .to increment(:user_unauthenticated_counter)
+
+      get path, default_params.merge(feed_token: 'token')
+
+      expect(response.status).not_to eq 200
+    end
+  end
+
+  it "doesn't log the user in otherwise", unless: params[:public] do
+    expect(authentication_metrics)
+      .to increment(:user_unauthenticated_counter)
+
+    get path, default_params.merge(private_token: 'token')
+
+    expect(response.status).not_to eq(200)
+  end
+end

spec/support/helpers/prometheus_helpers.rb

@@ -49,11 +49,11 @@ module PrometheusHelpers
     "https://prometheus.example.com/api/v1/series?#{query}"
   end
 
-  def stub_prometheus_request(url, body: {}, status: 200)
+  def stub_prometheus_request(url, body: {}, status: 200, headers: {})
     WebMock.stub_request(:get, url)
       .to_return({
         status: status,
-        headers: { 'Content-Type' => 'application/json' },
+        headers: { 'Content-Type' => 'application/json' }.merge(headers),
         body: body.to_json
       })
   end

spec/support/shared_contexts/url_shared_context.rb

@@ -0,0 +1,17 @@
+shared_context 'invalid urls' do
+  let(:urls_with_CRLF) do
+    ["http://127.0.0.1:333/pa\rth",
+     "http://127.0.0.1:333/pa\nth",
+     "http://127.0a.0.1:333/pa\r\nth",
+     "http://127.0.0.1:333/path?param=foo\r\nbar",
+     "http://127.0.0.1:333/path?param=foo\rbar",
+     "http://127.0.0.1:333/path?param=foo\nbar",
+     "http://127.0.0.1:333/pa%0dth",
+     "http://127.0.0.1:333/pa%0ath",
+     "http://127.0a.0.1:333/pa%0d%0th",
+     "http://127.0.0.1:333/pa%0D%0Ath",
+     "http://127.0.0.1:333/path?param=foo%0Abar",
+     "http://127.0.0.1:333/path?param=foo%0Dbar",
+     "http://127.0.0.1:333/path?param=foo%0D%0Abar"]
+  end
+end

spec/validators/url_validator_spec.rb

@@ -6,6 +6,30 @@ describe UrlValidator do
 
   include_examples 'url validator examples', described_class::DEFAULT_PROTOCOLS
 
+  describe 'validations' do
+    include_context 'invalid urls'
+
+    let(:validator) { described_class.new(attributes: [:link_url]) }
+
+    it 'returns error when url is nil' do
+      expect(validator.validate_each(badge, :link_url, nil)).to be_nil
+      expect(badge.errors.first[1]).to eq 'must be a valid URL'
+    end
+
+    it 'returns error when url is empty' do
+      expect(validator.validate_each(badge, :link_url, '')).to be_nil
+      expect(badge.errors.first[1]).to eq 'must be a valid URL'
+    end
+
+    it 'does not allow urls with CR or LF characters' do
+      aggregate_failures do
+        urls_with_CRLF.each do |url|
+          expect(validator.validate_each(badge, :link_url, url)[0]).to eq 'is blocked: URI is invalid'
+        end
+      end
+    end
+  end
+
   context 'by default' do
     let(:validator) { described_class.new(attributes: [:link_url]) }