Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq

13 files changed, 85 insertions, 42 deletions

diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js
index c14eb936930..8178821be3d 100644
--- a/app/assets/javascripts/gfm_auto_complete.js
+++ b/app/assets/javascripts/gfm_auto_complete.js
@@ -256,7 +256,7 @@ class GfmAutoComplete {
       displayTpl(value) {
         let tmpl = GfmAutoComplete.Loading.template;
         if (value.title != null) {
-          tmpl = GfmAutoComplete.Milestones.template;
+          tmpl = GfmAutoComplete.Milestones.templateFunction(value.title);
         }
         return tmpl;
       },
@@ -323,7 +323,7 @@ class GfmAutoComplete {
       searchKey: 'search',
       data: GfmAutoComplete.defaultLoadingData,
       displayTpl(value) {
-        let tmpl = GfmAutoComplete.Labels.template;
+        let tmpl = GfmAutoComplete.Labels.templateFunction(value.color, value.title);
         if (GfmAutoComplete.isLoading(value)) {
           tmpl = GfmAutoComplete.Loading.template;
         }
@@ -588,9 +588,11 @@ GfmAutoComplete.Members = {
   },
 };
 GfmAutoComplete.Labels = {
-  template:
-    // eslint-disable-next-line no-template-curly-in-string
-    '<li><span class="dropdown-label-box" style="background: ${color}"></span> ${title}</li>',
+  templateFunction(color, title) {
+    return `<li><span class="dropdown-label-box" style="background: ${_.escape(
+      color,
+    )}"></span> ${_.escape(title)}</li>`;
+  },
 };
 // Issues, MergeRequests and Snippets
 GfmAutoComplete.Issues = {
@@ -600,8 +602,9 @@ GfmAutoComplete.Issues = {
 };
 // Milestones
 GfmAutoComplete.Milestones = {
-  // eslint-disable-next-line no-template-curly-in-string
-  template: '<li>${title}</li>',
+  templateFunction(title) {
+    return `<li>${_.escape(title)}</li>`;
+  },
 };
 GfmAutoComplete.Loading = {
   template:
diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb
index c1dcc463de7..f476f428fdb 100644
--- a/app/controllers/groups/settings/ci_cd_controller.rb
+++ b/app/controllers/groups/settings/ci_cd_controller.rb
@@ -4,7 +4,7 @@ module Groups
   module Settings
     class CiCdController < Groups::ApplicationController
       skip_cross_project_access_check :show
-      before_action :authorize_admin_pipeline!
+      before_action :authorize_admin_group!
 
       def show
         define_ci_variables
@@ -26,8 +26,8 @@ module Groups
           .map { |variable| variable.present(current_user: current_user) }
       end
 
-      def authorize_admin_pipeline!
-        return render_404 unless can?(current_user, :admin_pipeline, group)
+      def authorize_admin_group!
+        return render_404 unless can?(current_user, :admin_group, group)
       end
     end
   end
diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb
index a44acb12bdf..255f1f3569a 100644
--- a/app/controllers/projects/snippets_controller.rb
+++ b/app/controllers/projects/snippets_controller.rb
@@ -75,7 +75,14 @@ class Projects::SnippetsController < Projects::ApplicationController
       format.json do
         render_blob_json(blob)
       end
-      format.js { render 'shared/snippets/show'}
+
+      format.js do
+        if @snippet.embeddable?
+          render 'shared/snippets/show'
+        else
+          head :not_found
+        end
+      end
     end
   end
 
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 8bf93bfd68d..878816475b2 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -19,6 +19,7 @@ class ProjectsController < Projects::ApplicationController
   before_action :lfs_blob_ids, only: [:show], if: [:repo_exists?, :project_view_files?]
   before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
   before_action :present_project, only: [:edit]
+  before_action :authorize_download_code!, only: [:refs]
 
   # Authorize
   before_action :authorize_admin_project!, only: [:edit, :update, :housekeeping, :download_export, :export, :remove_export, :generate_new_export]
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index dd9bf17cf0c..8ea5450b4e8 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -80,7 +80,13 @@ class SnippetsController < ApplicationController
         render_blob_json(blob)
       end
 
-      format.js { render 'shared/snippets/show' }
+      format.js do
+        if @snippet.embeddable?
+          render 'shared/snippets/show'
+        else
+          head :not_found
+        end
+      end
     end
   end
 
diff --git a/app/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb
index 8fded0efe4d..ecb2b2d707b 100644
--- a/app/helpers/snippets_helper.rb
+++ b/app/helpers/snippets_helper.rb
@@ -130,12 +130,4 @@ module SnippetsHelper
 
     link_to external_snippet_icon('download'), download_url, class: 'btn', target: '_blank', title: 'Download', rel: 'noopener noreferrer'
   end
-
-  def public_snippet?
-    if @snippet.project_id?
-      can?(nil, :read_project_snippet, @snippet)
-    else
-      can?(nil, :read_personal_snippet, @snippet)
-    end
-  end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index 09e2a6114fe..eab19fe4506 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -324,10 +324,9 @@ class Project < ActiveRecord::Base
 
   validates :namespace, presence: true
   validates :name, uniqueness: { scope: :namespace_id }
-  validates :import_url, url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
-                                ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
-                                allow_localhost: false,
-                                enforce_user: true }, if: [:external_import?, :import_url_changed?]
+  validates :import_url, public_url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
+                                       ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
+                                       enforce_user: true }, if: [:external_import?, :import_url_changed?]
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
   validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb
index 5a6895aefab..a3fa67c72bf 100644
--- a/app/models/remote_mirror.rb
+++ b/app/models/remote_mirror.rb
@@ -17,7 +17,7 @@ class RemoteMirror < ActiveRecord::Base
 
   belongs_to :project, inverse_of: :remote_mirrors
 
-  validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true }
+  validates :url, presence: true, public_url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true }
 
   before_save :set_new_remote_name, if: :mirror_url_changed?
 
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 11856b55902..f9b23bbbf6c 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -175,6 +175,12 @@ class Snippet < ActiveRecord::Base
     :visibility_level
   end
 
+  def embeddable?
+    ability = project_id? ? :read_project_snippet : :read_personal_snippet
+
+    Ability.allowed?(nil, ability, self)
+  end
+
   def notes_with_associations
     notes.includes(:author)
   end
diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb
index 6d8b575102e..ecb2797d1d9 100644
--- a/app/policies/issuable_policy.rb
+++ b/app/policies/issuable_policy.rb
@@ -11,7 +11,7 @@ class IssuablePolicy < BasePolicy
     @user && @subject.assignee_or_author?(@user)
   end
 
-  rule { assignee_or_author }.policy do
+  rule { can?(:guest_access) & assignee_or_author }.policy do
     enable :read_issue
     enable :update_issue
     enable :reopen_issue
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 36767621d74..48419da98ad 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -18,7 +18,7 @@ module MergeRequests
       merge_request.source_project  = find_source_project
       merge_request.target_project  = find_target_project
       merge_request.target_branch   = find_target_branch
-      merge_request.can_be_created  = branches_valid?
+      merge_request.can_be_created  = projects_and_branches_valid?
 
       # compare branches only if branches are valid, otherwise
       # compare_branches may raise an error
@@ -49,15 +49,19 @@ module MergeRequests
              to: :merge_request
 
     def find_source_project
-      return source_project if source_project.present? && can?(current_user, :read_project, source_project)
+      return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project)
 
       project
     end
 
     def find_target_project
-      return target_project if target_project.present? && can?(current_user, :read_project, target_project)
+      return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project)
 
-      project.default_merge_request_target
+      target_project = project.default_merge_request_target
+
+      return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project)
+
+      project
     end
 
     def find_target_branch
@@ -72,10 +76,11 @@ module MergeRequests
       params[:target_branch].present?
     end
 
-    def branches_valid?
+    def projects_and_branches_valid?
+      return false if source_project.nil? || target_project.nil?
       return false unless source_branch_specified? || target_branch_specified?
 
-      validate_branches
+      validate_projects_and_branches
       errors.blank?
     end
 
@@ -94,7 +99,12 @@ module MergeRequests
       end
     end
 
-    def validate_branches
+    def validate_projects_and_branches
+      merge_request.validate_target_project
+      merge_request.validate_fork
+
+      return if errors.any?
+
       add_error('You must select source and target branch') unless branches_present?
       add_error('You must select different branches') if same_source_and_target?
       add_error("Source branch \"#{source_branch}\" does not exist") unless source_branch_exists?
diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb
index f9b9781ad5f..b5128443435 100644
--- a/app/services/projects/lfs_pointers/lfs_download_service.rb
+++ b/app/services/projects/lfs_pointers/lfs_download_service.rb
@@ -12,28 +12,43 @@ module Projects
 
         return if LfsObject.exists?(oid: oid)
 
-        sanitized_uri = Gitlab::UrlSanitizer.new(url)
-        Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, protocols: VALID_PROTOCOLS)
+        sanitized_uri = sanitize_url!(url)
 
         with_tmp_file(oid) do |file|
-          size = download_and_save_file(file, sanitized_uri)
-          lfs_object = LfsObject.new(oid: oid, size: size, file: file)
+          download_and_save_file(file, sanitized_uri)
+          lfs_object = LfsObject.new(oid: oid, size: file.size, file: file)
 
           project.all_lfs_objects << lfs_object
         end
+      rescue Gitlab::UrlBlocker::BlockedUrlError => e
+        Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded: #{e.message}")
       rescue StandardError => e
-        Rails.logger.error("LFS file with oid #{oid} could't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}")
+        Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}")
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
       private
 
+      def sanitize_url!(url)
+        Gitlab::UrlSanitizer.new(url).tap do |sanitized_uri|
+          # Just validate that HTTP/HTTPS protocols are used. The
+          # subsequent Gitlab::HTTP.get call will do network checks
+          # based on the settings.
+          Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url,
+                                       protocols: VALID_PROTOCOLS)
+        end
+      end
+
       def download_and_save_file(file, sanitized_uri)
-        IO.copy_stream(open(sanitized_uri.sanitized_url, headers(sanitized_uri)), file) # rubocop:disable Security/Open
+        response = Gitlab::HTTP.get(sanitized_uri.sanitized_url, headers(sanitized_uri)) do |fragment|
+          file.write(fragment)
+        end
+
+        raise StandardError, "Received error code #{response.code}" unless response.success?
       end
 
       def headers(sanitized_uri)
-        {}.tap do |headers|
+        query_options.tap do |headers|
           credentials = sanitized_uri.credentials
 
           if credentials[:user].present? || credentials[:password].present?
@@ -43,10 +58,14 @@ module Projects
         end
       end
 
+      def query_options
+        { stream_body: true }
+      end
+
       def with_tmp_file(oid)
         create_tmp_storage_dir
 
-        File.open(File.join(tmp_storage_dir, oid), 'w') { |file| yield file }
+        File.open(File.join(tmp_storage_dir, oid), 'wb') { |file| yield file }
       end
 
       def create_tmp_storage_dir
diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml
index 10bfc30492a..a43296aa806 100644
--- a/app/views/shared/snippets/_header.html.haml
+++ b/app/views/shared/snippets/_header.html.haml
@@ -30,7 +30,7 @@
   - if @snippet.updated_at != @snippet.created_at
     = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago', exclude_author: true)
 
-  - if public_snippet?
+  - if @snippet.embeddable?
     .embed-snippet
       .input-group
         .input-group-prepend