Merge dev master into GitLab.com master

CHANGELOG.md

@@ -486,6 +486,33 @@ entry.
 - Update url placeholder for the sentry configuration page. !24338
 
 
+## 11.6.10 (2019-02-28)
+
+### Security (21 changes)
+
+- Stop linking to unrecognized package sources. !55518
+- Check snippet attached file to be moved is within designated directory.
+- Fix potential Addressable::URI::InvalidURIError.
+- Do not display impersonated sessions under active sessions and remove ability to revoke session.
+- Display only information visible to current user on the Milestone page.
+- Show only merge requests visible to user on milestone detail page.
+- Disable issue boards API when issues are disabled.
+- Don't show new issue link after move when a user does not have permissions.
+- Fix git clone revealing private repo's presence.
+- Fix blind SSRF in Prometheus integration by checking URL before querying.
+- Check if desired milestone for an issue is available.
+- Don't allow non-members to see private related MRs.
+- Fix arbitrary file read via diffs during import.
+- Display the correct number of MRs a user has access to.
+- Forbid creating discussions for users with restricted access.
+- Do not disclose milestone titles for unauthorized users.
+- Validate session key when authorizing with GCP to create a cluster.
+- Block local URLs for Kubernetes integration.
+- Limit mermaid rendering to 5K characters.
+- Remove the possibility to share a project with a group that a user is not a member of.
+- Fix leaking private repository information in API.
+
+
 ## 11.6.8 (2019-01-30)
 
 - No changes.

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

 import flash from '~/flash';
+import { sprintf, __ } from '../../locale';
 
 // Renders diagrams and flowcharts from text using Mermaid in any element with the
 // `js-render-mermaid` class.
@@ -14,6 +15,9 @@ import flash from '~/flash';
 // </pre>
 //
 
+// This is an arbitary number; Can be iterated upon when suitable.
+const MAX_CHAR_LIMIT = 5000;
+
 export default function renderMermaid($els) {
   if (!$els.length) return;
 
@@ -34,6 +38,21 @@ export default function renderMermaid($els) {
       $els.each((i, el) => {
         const source = el.textContent;
 
+        /**
+         * Restrict the rendering to a certain amount of character to
+         * prevent mermaidjs from hanging up the entire thread and
+         * causing a DoS.
+         */
+        if (source && source.length > MAX_CHAR_LIMIT) {
+          el.textContent = sprintf(
+            __(
+              'Cannot render the image. Maximum character count (%{charLimit}) has been exceeded.',
+            ),
+            { charLimit: MAX_CHAR_LIMIT },
+          );
+          return;
+        }
+
         // Remove any extra spans added by the backend syntax highlighting.
         Object.assign(el, { textContent: source });
 

app/controllers/concerns/milestone_actions.rb

@@ -8,7 +8,7 @@ module MilestoneActions
       format.html { redirect_to milestone_redirect_path }
       format.json do
         render json: tabs_json("shared/milestones/_merge_requests_tab", {
-          merge_requests: @milestone.sorted_merge_requests, # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          merge_requests: @milestone.sorted_merge_requests(current_user), # rubocop:disable Gitlab/ModuleWithInstanceVariables
           show_project_name: true
         })
       end

app/controllers/google_api/authorizations_controller.rb

@@ -2,6 +2,10 @@
 
 module GoogleApi
   class AuthorizationsController < ApplicationController
+    include Gitlab::Utils::StrongMemoize
+
+    before_action :validate_session_key!
+
     def callback
       token, expires_at = GoogleApi::CloudPlatform::Client
         .new(nil, callback_google_api_auth_url)
@@ -11,21 +15,27 @@ module GoogleApi
       session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] =
         expires_at.to_s
 
-      state_redirect_uri = redirect_uri_from_session_key(params[:state])
+      redirect_to redirect_uri_from_session
+    end
+
+    private
+
+    def validate_session_key!
+      access_denied! unless redirect_uri_from_session.present?
+    end
 
-      if state_redirect_uri
-        redirect_to state_redirect_uri
+    def redirect_uri_from_session
+      strong_memoize(:redirect_uri_from_session) do
+        if params[:state].present?
+          session[session_key_for_redirect_uri(params[:state])]
         else
-        redirect_to root_path
+          nil
+        end
       end
     end
 
-    private
-
-    def redirect_uri_from_session_key(state)
-      key = GoogleApi::CloudPlatform::Client
-        .session_key_for_redirect_uri(params[:state])
-      session[key] if key
+    def session_key_for_redirect_uri(state)
+      GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(state)
     end
   end
 end

app/controllers/profiles/active_sessions_controller.rb

@@ -2,15 +2,6 @@
 
 class Profiles::ActiveSessionsController < Profiles::ApplicationController
   def index
-    @sessions = ActiveSession.list(current_user)
-  end
-
-  def destroy
-    ActiveSession.destroy(current_user, params[:id])
-
-    respond_to do |format|
-      format.html { redirect_to profile_active_sessions_url, status: :found }
-      format.js { head :ok }
-    end
+    @sessions = ActiveSession.list(current_user).reject(&:is_impersonated)
   end
 end

app/controllers/projects/autocomplete_sources_controller.rb

 # frozen_string_literal: true
 
 class Projects::AutocompleteSourcesController < Projects::ApplicationController
+  before_action :authorize_read_milestone!, only: :milestones
+
   def members
     render json: ::Projects::ParticipantsService.new(@project, current_user).execute(target)
   end

app/controllers/projects/commit_controller.rb

@@ -65,7 +65,11 @@ class Projects::CommitController < Projects::ApplicationController
   # rubocop: enable CodeReuse/ActiveRecord
 
   def merge_requests
-    @merge_requests = @commit.merge_requests.map do |mr|
+    @merge_requests = MergeRequestsFinder.new(
+      current_user,
+      project_id: @project.id,
+      commit_sha: @commit.sha
+    ).execute.map do |mr|
       { iid: mr.iid, path: merge_request_path(mr), title: mr.title }
     end
 

app/controllers/projects/group_links_controller.rb

@@ -13,9 +13,10 @@ class Projects::GroupLinksController < Projects::ApplicationController
     group = Group.find(params[:link_group_id]) if params[:link_group_id].present?
 
     if group
-      return render_404 unless can?(current_user, :read_group, group)
+      result = Projects::GroupLinks::CreateService.new(project, current_user, group_link_create_params).execute(group)
+      return render_404 if result[:http_status] == 404
 
-      Projects::GroupLinks::CreateService.new(project, current_user, group_link_create_params).execute(group)
+      flash[:alert] = result[:message] if result[:http_status] == 409
     else
       flash[:alert] = 'Please select a group.'
     end

app/finders/merge_requests_finder.rb

@@ -37,13 +37,20 @@ class MergeRequestsFinder < IssuableFinder
   end
 
   def filter_items(_items)
-    items = by_source_branch(super)
+    items = by_commit(super)
+    items = by_source_branch(items)
     items = by_wip(items)
     by_target_branch(items)
   end
 
   private
 
+  def by_commit(items)
+    return items unless params[:commit_sha].presence
+
+    items.by_commit_sha(params[:commit_sha])
+  end
+
   def source_branch
     @source_branch ||= params[:source_branch].presence
   end

app/graphql/types/project_type.rb

@@ -16,7 +16,6 @@ module Types
 
     field :description, GraphQL::STRING_TYPE, null: true
 
-    field :default_branch, GraphQL::STRING_TYPE, null: true
     field :tag_list, GraphQL::STRING_TYPE, null: true
 
     field :ssh_url_to_repo, GraphQL::STRING_TYPE, null: true
@@ -59,7 +58,6 @@ module Types
     end
 
     field :import_status, GraphQL::STRING_TYPE, null: true
-    field :ci_config_path, GraphQL::STRING_TYPE, null: true
 
     field :only_allow_merge_if_pipeline_succeeds, GraphQL::BOOLEAN_TYPE, null: true
     field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true

app/mailers/emails/issues.rb

@@ -74,6 +74,7 @@ module Emails
 
       @new_issue = new_issue
       @new_project = new_issue.project
+      @can_access_project = recipient.can?(:read_project, @new_project)
       mail_answer_thread(issue, issue_thread_options(updated_by_user.id, recipient.id, reason))
     end
 

app/models/active_session.rb

@@ -5,7 +5,8 @@ class ActiveSession
 
   attr_accessor :created_at, :updated_at,
     :session_id, :ip_address,
-    :browser, :os, :device_name, :device_type
+    :browser, :os, :device_name, :device_type,
+    :is_impersonated
 
   def current?(session)
     return false if session_id.nil? || session.id.nil?
@@ -31,7 +32,8 @@ class ActiveSession
         device_type: client.device_type,
         created_at: user.current_sign_in_at || timestamp,
         updated_at: timestamp,
-        session_id: session_id
+        session_id: session_id,
+        is_impersonated: request.session[:impersonator_id].present?
       )
 
       redis.pipelined do

app/models/clusters/platforms/kubernetes.rb

@@ -41,7 +41,7 @@ module Clusters
       validate :no_namespace, unless: :allow_user_defined_namespace?
 
       # We expect to be `active?` only when enabled and cluster is created (the api_url is assigned)
-      validates :api_url, url: true, presence: true
+      validates :api_url, public_url: true, presence: true
       validates :token, presence: true
       validates :ca_cert, certificate: true, allow_blank: true, if: :ca_cert_changed?
 

app/models/concerns/issuable.rb

@@ -75,6 +75,7 @@ module Issuable
 
     validates :author, presence: true
     validates :title, presence: true, length: { maximum: 255 }
+    validate :milestone_is_valid
 
     scope :authored, ->(user) { where(author_id: user) }
     scope :recent, -> { reorder(id: :desc) }
@@ -118,6 +119,16 @@ module Issuable
     def has_multiple_assignees?
       assignees.count > 1
     end
+
+    def milestone_available?
+      project_id == milestone&.project_id || project.ancestors_upto.compact.include?(milestone&.group)
+    end
+
+    private
+
+    def milestone_is_valid
+      errors.add(:milestone_id, message: "is invalid") if milestone_id.present? && !milestone_available?
+    end
   end
 
   class_methods do

app/models/concerns/milestoneish.rb

@@ -46,12 +46,19 @@ module Milestoneish
     end
   end
 
+  def merge_requests_visible_to_user(user)
+    memoize_per_user(user, :merge_requests_visible_to_user) do
+      MergeRequestsFinder.new(user, {})
+        .execute.where(milestone_id: milestoneish_id)
+    end
+  end
+
   def sorted_issues(user)
     issues_visible_to_user(user).preload_associations.sort_by_attribute('label_priority')
   end
 
-  def sorted_merge_requests
-    merge_requests.sort_by_attribute('label_priority')
+  def sorted_merge_requests(user)
+    merge_requests_visible_to_user(user).sort_by_attribute('label_priority')
   end
 
   def upcoming?

app/models/merge_request.rb

@@ -71,7 +71,7 @@ class MergeRequest < ActiveRecord::Base
 
   serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
 
-  after_create :ensure_merge_request_diff, unless: :importing?
+  after_create :ensure_merge_request_diff
   after_update :clear_memoized_shas
   after_update :reload_diff_if_branch_changed
   after_save :ensure_metrics
@@ -189,6 +189,9 @@ class MergeRequest < ActiveRecord::Base
 
   after_save :keep_around_commit
 
+  alias_attribute :project, :target_project
+  alias_attribute :project_id, :target_project_id
+
   def self.reference_prefix
     '!'
   end
@@ -847,10 +850,6 @@ class MergeRequest < ActiveRecord::Base
     target_project != source_project
   end
 
-  def project
-    target_project
-  end
-
   # If the merge request closes any issues, save this information in the
   # `MergeRequestsClosingIssues` model. This is a performance optimization.
   # Calculating this information for a number of merge requests requires

app/models/merge_request_diff.rb

@@ -22,6 +22,8 @@ class MergeRequestDiff < ActiveRecord::Base
 
   has_many :merge_request_diff_commits, -> { order(:merge_request_diff_id, :relative_order) }
 
+  validates :base_commit_sha, :head_commit_sha, :start_commit_sha, sha: true
+
   state_machine :state, initial: :empty do
     event :clean do
       transition any => :without_files

app/models/project_services/prometheus_service.rb

@@ -71,7 +71,7 @@ class PrometheusService < MonitoringService
   end
 
   def prometheus_client
-    RestClient::Resource.new(api_url, max_redirects: 0) if api_url && manual_configuration? && active?
+    RestClient::Resource.new(api_url, max_redirects: 0) if should_return_client?
   end
 
   def prometheus_available?
@@ -83,6 +83,10 @@ class PrometheusService < MonitoringService
 
   private
 
+  def should_return_client?
+    api_url && manual_configuration? && active? && valid?
+  end
+
   def synchronize_service_state
     self.active = prometheus_available? || manual_configuration?
 

app/policies/group_policy.rb

@@ -53,8 +53,11 @@ class GroupPolicy < BasePolicy
   rule { admin }.enable :read_group
 
   rule { has_projects }.policy do
+<<<<<<< HEAD
     enable :read_group
     enable :read_list
+=======
+>>>>>>> dev/master
     enable :read_label
   end
 

app/policies/project_policy.rb

@@ -300,6 +300,8 @@ class ProjectPolicy < BasePolicy
 
   rule { issues_disabled }.policy do
     prevent(*create_read_update_admin_destroy(:issue))
+    prevent(*create_read_update_admin_destroy(:board))
+    prevent(*create_read_update_admin_destroy(:list))
   end
 
   rule { merge_requests_disabled | repository_disabled }.policy do

app/services/issuable_base_service.rb

@@ -387,4 +387,10 @@ class IssuableBaseService < BaseService
   def parent
     project
   end
+
+  # we need to check this because milestone from milestone_id param is displayed on "new" page
+  # where private project milestone could leak without this check
+  def ensure_milestone_available(issuable)
+    issuable.milestone_id = nil unless issuable.milestone_available?
+  end
 end

app/services/issues/build_service.rb

@@ -6,7 +6,9 @@ module Issues
 
     def execute
       filter_resolve_discussion_params
-      @issue = project.issues.new(issue_params)
+      @issue = project.issues.new(issue_params).tap do |issue|
+        ensure_milestone_available(issue)
+      end
     end
 
     def issue_params_with_info_from_discussions

app/services/merge_requests/build_service.rb

@@ -19,6 +19,7 @@ module MergeRequests
       merge_request.target_project  = find_target_project
       merge_request.target_branch   = find_target_branch
       merge_request.can_be_created  = projects_and_branches_valid?
+      ensure_milestone_available(merge_request)
 
       # compare branches only if branches are valid, otherwise
       # compare_branches may raise an error

app/services/projects/group_links/create_service.rb

@@ -4,13 +4,19 @@ module Projects
   module GroupLinks
     class CreateService < BaseService
       def execute(group)
-        return false unless group
+        return error('Not Found', 404) unless group && can?(current_user, :read_namespace, group)
 
-        project.project_group_links.create(
+        link = project.project_group_links.new(
           group: group,
           group_access: params[:link_group_access],
           expires_at: params[:expires_at]
         )
+
+        if link.save
+          success(link: link)
+        else
+          error(link.errors.full_messages.to_sentence, 409)
+        end
       end
     end
   end

app/uploaders/file_mover.rb

@@ -11,6 +11,8 @@ class FileMover
   end
 
   def execute
+    return unless valid?
+
     move
 
     if update_markdown
@@ -21,6 +23,12 @@ class FileMover
 
   private
 
+  def valid?
+    Pathname.new(temp_file_path).realpath.to_path.start_with?(
+      (Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path
+    )
+  end
+
   def move
     FileUtils.mkdir_p(File.dirname(file_path))
     FileUtils.move(temp_file_path, file_path)

app/validators/sha_validator.rb

+# frozen_string_literal: true
+
+class ShaValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    return if value.blank? || value.match(/\A\h{40}\z/)
+
+    record.errors.add(attribute, 'is not a valid SHA')
+  end
+end

app/views/notify/issue_moved_email.html.haml

 %p
   Issue was moved to another project.
-%p
+- if @can_access_project
+  %p
     New issue:
     = link_to project_issue_url(@new_project, @new_issue) do
       = @new_issue.title
+- else
+  You don't have access to the project.

app/views/notify/issue_moved_email.text.erb

 Issue was moved to another project.
 
+<% if @can_access_project %>
 New issue location:
 <%= project_issue_url(@new_project, @new_issue) %>
+<% else %>
+You don't have access to the project.
+<% end %>

app/views/profiles/active_sessions/_active_session.html.haml

@@ -23,9 +23,3 @@
       %strong Signed in
       on
       = l(active_session.created_at, format: :short)
-
-  - unless is_current_session
-    .float-right
-      = link_to profile_active_session_path(active_session.session_id), data: { confirm: 'Are you sure? The device will be signed out of GitLab.' }, method: :delete, class: "btn btn-danger prepend-left-10" do
-        %span.sr-only Revoke
-        Revoke

app/views/projects/blob/viewers/_dependency_manager.html.haml

@@ -3,9 +3,4 @@
   This project manages its dependencies using
   %strong= viewer.manager_name
 
-  - if viewer.package_name
-    and defines a #{viewer.package_type} named
-    %strong<
-      = link_to_if viewer.package_url.present?, viewer.package_name, viewer.package_url, target: '_blank', rel: 'noopener noreferrer'
-
 = link_to 'Learn more', viewer.manager_url, target: '_blank', rel: 'noopener noreferrer'

changelogs/unreleased/2802-security-add-public-internal-groups-as-members-to-your-project-idor.yml

+---
+title: Remove the possibility to share a project with a group that a user is not a member
+  of
+merge_request:
+author:
+type: security

changelogs/unreleased/51971-milestones-visibility.yml

+---
+title: Check if desired milestone for an issue is available
+merge_request:
+author:
+type: security

changelogs/unreleased/57534_filter_impersonated_sessions.yml

+---
+title: Do not display impersonated sessions under active sessions and remove ability
+  to revoke session
+merge_request:
+author:
+type: security

changelogs/unreleased/security-2797-milestone-mrs.yml

+---
+title: Show only merge requests visible to user on milestone detail page
+merge_request:
+author:
+type: security

changelogs/unreleased/security-2798-fix-boards-policy.yml

+---
+title: Disable issue boards API when issues are disabled
+merge_request:
+author:
+type: security

changelogs/unreleased/security-2799-emails.yml

+---
+title: Don't show new issue link after move when a user does not have permissions
+merge_request:
+author:
+type: security

changelogs/unreleased/security-50334.yml

+---
+title: Fix git clone revealing private repo's presence
+merge_request:
+author:
+type: security

changelogs/unreleased/security-55468-check-validity-before-querying.yml

+---
+title: Fix blind SSRF in Prometheus integration by checking URL before querying
+merge_request:
+author:
+type: security

changelogs/unreleased/security-56348.yml

+---
+title: Check snippet attached file to be moved is within designated directory
+merge_request:
+author:
+type: security

changelogs/unreleased/security-commit-private-related-mr.yml

+---
+title: Don't allow non-members to see private related MRs.
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fj-diff-import-file-read-fix.yml

+---
+title: Fix arbitrary file read via diffs during import
+merge_request:
+author:
+type: security

changelogs/unreleased/security-id-restricted-access-to-private-repo.yml

+---
+title: Forbid creating discussions for users with restricted access
+merge_request:
+author:
+type: security

changelogs/unreleased/security-issue_54789_2.yml

+---
+title: Do not disclose milestone titles for unauthorized users
+merge_request:
+author:
+type: security

changelogs/unreleased/security-kubernetes-google-login-csrf.yml

+---
+title: Validate session key when authorizing with GCP to create a cluster
+merge_request:
+author:
+type: security

changelogs/unreleased/security-kubernetes-local-ssrf.yml

+---
+title: Block local URLs for Kubernetes integration
+merge_request:
+author:
+type: security

changelogs/unreleased/security-mermaid.yml

+---
+title: Limit mermaid rendering to 5K characters
+merge_request:
+author:
+type: security

changelogs/unreleased/security-osw-stop-linking-to-packages.yml

+---
+title: Stop linking to unrecognized package sources
+merge_request: 55518
+author:
+type: security

changelogs/unreleased/security-protect-private-repo-information.yml

+---
+title: Fix leaking private repository information in API
+merge_request:
+author:
+type: security

changelogs/unreleased/security-shared-project-private-group.yml

+---
+title: Fixed ability to see private groups by users not belonging to given group
+merge_request:
+author:
+type: security

changelogs/unreleased/security-tags-oracle.yml

+---
+title: Prevent releases links API to leak tag existance
+merge_request:
+author:
+type: security

config/routes/git_http.rb

@@ -40,7 +40,7 @@ scope(path: '*namespace_id/:project_id',
     # /info/refs?service=git-receive-pack, but nothing else.
     #
     git_http_handshake = lambda do |request|
-      ::Constraints::ProjectUrlConstrainer.new.matches?(request) &&
+      ::Constraints::ProjectUrlConstrainer.new.matches?(request, existence_check: false) &&
         (request.query_string.blank? ||
          request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/))
     end

doc/user/profile/active_sessions.md

@@ -4,7 +4,7 @@
 >   in GitLab 10.8.
 
 GitLab lists all devices that have logged into your account. This allows you to
-review the sessions and revoke any of it that you don't recognize.
+review the sessions.
 
 ## Listing all active sessions
 
@@ -12,9 +12,3 @@ review the sessions and revoke any of it that you don't recognize.
 1. Navigate to the **Active Sessions** tab.
 
 ![Active sessions list](img/active_sessions_list.png)
-
-## Revoking a session
-
-1. Navigate to your [profile's](#profile-settings) **Settings > Active Sessions**.
-1. Click on **Revoke** besides a session. The current session cannot be
-   revoked, as this would sign you out of GitLab.

lib/api/commits.rb

@@ -318,10 +318,18 @@ module API
         use :pagination
       end
       get ':id/repository/commits/:sha/merge_requests', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do
+        authorize! :read_merge_request, user_project
+
         commit = user_project.commit(params[:sha])
         not_found! 'Commit' unless commit
 
-        present paginate(commit.merge_requests), with: Entities::MergeRequestBasic
+        commit_merge_requests = MergeRequestsFinder.new(
+          current_user,
+          project_id: user_project.id,
+          commit_sha: commit.sha
+        ).execute
+
+        present paginate(commit_merge_requests), with: Entities::MergeRequestBasic
       end
 
       desc "Get a commit's GPG signature" do

lib/api/entities.rb

@@ -156,7 +156,7 @@ module API
     class BasicProjectDetails < ProjectIdentity
       include ::API::ProjectsRelationBuilder
 
-      expose :default_branch
+      expose :default_branch, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) }
       # Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770
       expose :tag_list do |project|
         # project.tags.order(:name).pluck(:name) is the most suitable option
@@ -261,7 +261,7 @@ module API
       expose :open_issues_count, if: lambda { |project, options| project.feature_available?(:issues, options[:current_user]) }
       expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] }
       expose :public_builds, as: :public_jobs
-      expose :ci_config_path
+      expose :ci_config_path, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) }
       expose :shared_with_groups do |project, options|
         SharedGroup.represent(project.project_group_links, options)
       end
@@ -270,8 +270,9 @@ module API
       expose :only_allow_merge_if_all_discussions_are_resolved
       expose :printing_merge_request_link_enabled
       expose :merge_method
-
-      expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics
+      expose :statistics, using: 'API::Entities::ProjectStatistics', if: -> (project, options) {
+        options[:statistics] && Ability.allowed?(options[:current_user], :download_code, project)
+      }
 
       # rubocop: disable CodeReuse/ActiveRecord
       def self.preload_relation(projects_relation, options = {})

lib/api/environments.rb

@@ -22,7 +22,7 @@ module API
       get ':id/environments' do
         authorize! :read_environment, user_project
 
-        present paginate(user_project.environments), with: Entities::Environment
+        present paginate(user_project.environments), with: Entities::Environment, current_user: current_user
       end
 
       desc 'Creates a new environment' do
@@ -40,7 +40,7 @@ module API
         environment = user_project.environments.create(declared_params)
 
         if environment.persisted?
-          present environment, with: Entities::Environment
+          present environment, with: Entities::Environment, current_user: current_user
         else
           render_validation_error!(environment)
         end
@@ -63,7 +63,7 @@ module API
 
         update_params = declared_params(include_missing: false).extract!(:name, :external_url)
         if environment.update(update_params)
-          present environment, with: Entities::Environment
+          present environment, with: Entities::Environment, current_user: current_user
         else
           render_validation_error!(environment)
         end
@@ -99,7 +99,7 @@ module API
         environment.stop_with_action!(current_user)
 
         status 200
-        present environment, with: Entities::Environment
+        present environment, with: Entities::Environment, current_user: current_user
       end
     end
   end

lib/api/helpers/notes_helpers.rb

@@ -70,14 +70,7 @@ module API
       def find_noteable(parent, noteables_str, noteable_id)
         noteable = public_send("find_#{parent}_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
 
-        readable =
-          if noteable.is_a?(Commit)
-            # for commits there is not :read_commit policy, check if user
-            # has :read_note permission on the commit's project
-            can?(current_user, :read_note, user_project)
-          else
-            can?(current_user, noteable_read_ability_name(noteable), noteable)
-          end
+        readable = can?(current_user, noteable_read_ability_name(noteable), noteable)
 
         return not_found!(noteables_str) unless readable
 
@@ -89,12 +82,11 @@ module API
       end
 
       def create_note(noteable, opts)
-        policy_object = noteable.is_a?(Commit) ? user_project : noteable
-        authorize!(:create_note, policy_object)
+        authorize!(:create_note, noteable)
 
         parent = noteable_parent(noteable)
 
-        opts.delete(:created_at) unless current_user.can?(:set_note_created_at, policy_object)
+        opts.delete(:created_at) unless current_user.can?(:set_note_created_at, noteable)
 
         opts[:updated_at] = opts[:created_at] if opts[:created_at]
 

lib/api/projects.rb

@@ -184,7 +184,8 @@ module API
 
         if project.saved?
           present project, with: Entities::Project,
-                           user_can_admin_project: can?(current_user, :admin_project, project)
+                           user_can_admin_project: can?(current_user, :admin_project, project),
+                           current_user: current_user
         else
           if project.errors[:limit_reached].present?
             error!(project.errors[:limit_reached], 403)
@@ -217,7 +218,8 @@ module API
 
         if project.saved?
           present project, with: Entities::Project,
-                           user_can_admin_project: can?(current_user, :admin_project, project)
+                           user_can_admin_project: can?(current_user, :admin_project, project),
+                           current_user: current_user
         else
           render_validation_error!(project)
         end
@@ -281,7 +283,8 @@ module API
           conflict!(forked_project.errors.messages)
         else
           present forked_project, with: Entities::Project,
-                                  user_can_admin_project: can?(current_user, :admin_project, forked_project)
+                                  user_can_admin_project: can?(current_user, :admin_project, forked_project),
+                                  current_user: current_user
         end
       end
 
@@ -330,7 +333,8 @@ module API
 
         if result[:status] == :success
           present user_project, with: Entities::Project,
-                                user_can_admin_project: can?(current_user, :admin_project, user_project)
+                                user_can_admin_project: can?(current_user, :admin_project, user_project),
+                                current_user: current_user
         else
           render_validation_error!(user_project)
         end
@@ -344,7 +348,7 @@ module API
 
         ::Projects::UpdateService.new(user_project, current_user, archived: true).execute
 
-        present user_project, with: Entities::Project
+        present user_project, with: Entities::Project, current_user: current_user
       end
 
       desc 'Unarchive a project' do
@@ -355,7 +359,7 @@ module API
 
         ::Projects::UpdateService.new(@project, current_user, archived: false).execute
 
-        present user_project, with: Entities::Project
+        present user_project, with: Entities::Project, current_user: current_user
       end
 
       desc 'Star a project' do
@@ -368,7 +372,7 @@ module API
           current_user.toggle_star(user_project)
           user_project.reload
 
-          present user_project, with: Entities::Project
+          present user_project, with: Entities::Project, current_user: current_user
         end
       end
 
@@ -380,7 +384,7 @@ module API
           current_user.toggle_star(user_project)
           user_project.reload
 
-          present user_project, with: Entities::Project
+          present user_project, with: Entities::Project, current_user: current_user
         else
           not_modified!
         end
@@ -420,7 +424,7 @@ module API
         result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project)
 
         if result
-          present user_project.reload, with: Entities::Project
+          present user_project.reload, with: Entities::Project, current_user: current_user
         else
           render_api_error!("Project already forked", 409) if user_project.forked?
         end
@@ -442,27 +446,24 @@ module API
       end
       params do
         requires :group_id, type: Integer, desc: 'The ID of a group'
-        requires :group_access, type: Integer, values: Gitlab::Access.values, desc: 'The group access level'
+        requires :group_access, type: Integer, values: Gitlab::Access.values, as: :link_group_access, desc: 'The group access level'
         optional :expires_at, type: Date, desc: 'Share expiration date'
       end
       post ":id/share" do
         authorize! :admin_project, user_project
         group = Group.find_by_id(params[:group_id])
 
-        unless group && can?(current_user, :read_group, group)
-          not_found!('Group')
-        end
-
         unless user_project.allowed_to_share_with_group?
           break render_api_error!("The project sharing with group is disabled", 400)
         end
 
-        link = user_project.project_group_links.new(declared_params(include_missing: false))
+        result = ::Projects::GroupLinks::CreateService.new(user_project, current_user, declared_params(include_missing: false))
+          .execute(group)
 
-        if link.save
-          present link, with: Entities::ProjectGroupLink
+        if result[:status] == :success
+          present result[:link], with: Entities::ProjectGroupLink
         else
-          render_api_error!(link.errors.full_messages.first, 409)
+          render_api_error!(result[:message], result[:http_status])
         end
       end
 
@@ -526,7 +527,7 @@ module API
         result = ::Projects::TransferService.new(user_project, current_user).execute(namespace)
 
         if result
-          present user_project, with: Entities::Project
+          present user_project, with: Entities::Project, current_user: current_user
         else
           render_api_error!("Failed to transfer project #{user_project.errors.messages}", 400)
         end

lib/api/release/links.rb

@@ -8,6 +8,8 @@ module API
       RELEASE_ENDPOINT_REQUIREMETS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS
         .merge(tag_name: API::NO_SLASH_URL_PART_REGEX)
 
+      before { authorize! :read_release, user_project }
+
       params do
         requires :id, type: String, desc: 'The ID of a project'
       end

lib/constraints/project_url_constrainer.rb

@@ -2,12 +2,13 @@
 
 module Constraints
   class ProjectUrlConstrainer
-    def matches?(request)
+    def matches?(request, existence_check: true)
       namespace_path = request.params[:namespace_id]
       project_path = request.params[:project_id] || request.params[:id]
       full_path = [namespace_path, project_path].join('/')
 
       return false unless ProjectPathValidator.valid_path?(full_path)
+      return true unless existence_check
 
       # We intentionally allow SELECT(*) here so result of this query can be used
       # as cache for further Project.find_by_full_path calls within request

lib/gitlab/auth/omniauth_identity_linker_base.rb

@@ -12,7 +12,7 @@ module Gitlab
       end
 
       def link
-        save if identity.new_record?
+        save if unlinked?
       end
 
       def changed?
@@ -35,6 +35,10 @@ module Gitlab
         @changed = identity.save
       end
 
+      def unlinked?
+        identity.new_record?
+      end
+
       # rubocop: disable CodeReuse/ActiveRecord
       def identity
         @identity ||= current_user.identities

lib/gitlab/dependency_linker/base_linker.rb

@@ -4,6 +4,7 @@ module Gitlab
   module DependencyLinker
     class BaseLinker
       URL_REGEX = %r{https?://[^'" ]+}.freeze
+      GIT_INVALID_URL_REGEX = /^git\+#{URL_REGEX}/.freeze
       REPO_REGEX = %r{[^/'" ]+/[^/'" ]+}.freeze
 
       class_attribute :file_type
@@ -29,8 +30,25 @@ module Gitlab
         highlighted_lines.join.html_safe
       end
 
+      def external_url(name, external_ref)
+        return if external_ref =~ GIT_INVALID_URL_REGEX
+
+        case external_ref
+        when /\A#{URL_REGEX}\z/
+          external_ref
+        when /\A#{REPO_REGEX}\z/
+          github_url(external_ref)
+        else
+          package_url(name)
+        end
+      end
+
       private
 
+      def package_url(_name)
+        raise NotImplementedError
+      end
+
       def link_dependencies
         raise NotImplementedError
       end

lib/gitlab/dependency_linker/composer_json_linker.rb

@@ -8,8 +8,8 @@ module Gitlab
       private
 
       def link_packages
-        link_packages_at_key("require", &method(:package_url))
-        link_packages_at_key("require-dev", &method(:package_url))
+        link_packages_at_key("require")
+        link_packages_at_key("require-dev")
       end
 
       def package_url(name)

lib/gitlab/dependency_linker/gemfile_linker.rb

@@ -3,8 +3,14 @@
 module Gitlab
   module DependencyLinker
     class GemfileLinker < MethodLinker
+      class_attribute :package_keyword
+
+      self.package_keyword = :gem
       self.file_type = :gemfile
 
+      GITHUB_REGEX = /(github:|:github\s*=>)\s*['"](?<name>[^'"]+)['"]/.freeze
+      GIT_REGEX = /(git:|:git\s*=>)\s*['"](?<name>#{URL_REGEX})['"]/.freeze
+
       private
 
       def link_dependencies
@@ -14,20 +20,34 @@ module Gitlab
 
       def link_urls
         # Link `github: "user/repo"` to https://github.com/user/repo
-        link_regex(/(github:|:github\s*=>)\s*['"](?<name>[^'"]+)['"]/, &method(:github_url))
+        link_regex(GITHUB_REGEX, &method(:github_url))
 
         # Link `git: "https://gitlab.example.com/user/repo"` to https://gitlab.example.com/user/repo
-        link_regex(/(git:|:git\s*=>)\s*['"](?<name>#{URL_REGEX})['"]/, &:itself)
+        link_regex(GIT_REGEX, &:itself)
 
         # Link `source "https://rubygems.org"` to https://rubygems.org
         link_method_call('source', URL_REGEX, &:itself)
       end
 
       def link_packages
-        # Link `gem "package_name"` to https://rubygems.org/gems/package_name
-        link_method_call('gem') do |name|
+        packages = parse_packages
+
+        return if packages.blank?
+
+        packages.each do |package|
+          link_method_call('gem', package.name) do
+            external_url(package.name, package.external_ref)
+          end
+        end
+      end
+
+      def package_url(name)
         "https://rubygems.org/gems/#{name}"
       end
+
+      def parse_packages
+        parser = Gitlab::DependencyLinker::Parser::Gemfile.new(plain_text)
+        parser.parse(keyword: self.class.package_keyword)
       end
     end
   end

lib/gitlab/dependency_linker/gemspec_linker.rb

@@ -11,7 +11,7 @@ module Gitlab
         link_method_call('homepage', URL_REGEX, &:itself)
         link_method_call('license', &method(:license_url))
 
-        link_method_call(%w[name add_dependency add_runtime_dependency add_development_dependency]) do |name|
+        link_method_call(%w[add_dependency add_runtime_dependency add_development_dependency]) do |name|
           "https://rubygems.org/gems/#{name}"
         end
       end

lib/gitlab/dependency_linker/method_linker.rb

@@ -23,18 +23,22 @@ module Gitlab
       #   link_method_call('name')
       #   # Will link `package` in `self.name = "package"`
       def link_method_call(method_name, value = nil, &url_proc)
+        regex = method_call_regex(method_name, value)
+
+        link_regex(regex, &url_proc)
+      end
+
+      def method_call_regex(method_name, value = nil)
         method_name = regexp_for_value(method_name)
         value = regexp_for_value(value)
 
-        regex = %r{
+        %r{
           #{method_name}            # Method name
           \s*                       # Whitespace
           [(=]?                     # Opening brace or equals sign
           \s*                       # Whitespace
           ['"](?<name>#{value})['"] # Package name in quotes
         }x
-
-        link_regex(regex, &url_proc)
       end
     end
   end

lib/gitlab/dependency_linker/package.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DependencyLinker
+    class Package
+      attr_reader :name, :git_ref, :github_ref
+
+      def initialize(name, git_ref, github_ref)
+        @name = name
+        @git_ref = git_ref
+        @github_ref = github_ref
+      end
+
+      def external_ref
+        @git_ref || @github_ref
+      end
+    end
+  end
+end

lib/gitlab/dependency_linker/package_json_linker.rb

@@ -8,7 +8,6 @@ module Gitlab
       private
 
       def link_dependencies
-        link_json('name', json["name"], &method(:package_url))
         link_json('license', &method(:license_url))
         link_json(%w[homepage url], URL_REGEX, &:itself)
 
@@ -16,25 +15,19 @@ module Gitlab
       end
 
       def link_packages
-        link_packages_at_key("dependencies", &method(:package_url))
-        link_packages_at_key("devDependencies", &method(:package_url))
+        link_packages_at_key("dependencies")
+        link_packages_at_key("devDependencies")
       end
 
-      def link_packages_at_key(key, &url_proc)
+      def link_packages_at_key(key)
         dependencies = json[key]
         return unless dependencies
 
         dependencies.each do |name, version|
-          link_json(name, version, link: :key, &url_proc)
-
-          link_json(name) do |value|
-            case value
-            when /\A#{URL_REGEX}\z/
-              value
-            when /\A#{REPO_REGEX}\z/
-              github_url(value)
-            end
-          end
+          external_url = external_url(name, version)
+
+          link_json(name, version, link: :key) { external_url }
+          link_json(name) { external_url }
         end
       end
 

lib/gitlab/dependency_linker/parser/gemfile.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DependencyLinker
+    module Parser
+      class Gemfile < MethodLinker
+        GIT_REGEX = Gitlab::DependencyLinker::GemfileLinker::GIT_REGEX
+        GITHUB_REGEX = Gitlab::DependencyLinker::GemfileLinker::GITHUB_REGEX
+
+        def initialize(plain_text)
+          @plain_text = plain_text
+        end
+
+        # Returns a list of Gitlab::DependencyLinker::Package
+        #
+        # keyword - The package definition keyword, e.g. `:gem` for
+        # Gemfile parsing, `:pod` for Podfile.
+        def parse(keyword:)
+          plain_lines.each_with_object([]) do |line, packages|
+            name = fetch(line, method_call_regex(keyword))
+
+            next unless name
+
+            git_ref = fetch(line, GIT_REGEX)
+            github_ref = fetch(line, GITHUB_REGEX)
+
+            packages << Gitlab::DependencyLinker::Package.new(name, git_ref, github_ref)
+          end
+        end
+
+        private
+
+        def fetch(line, regex, group: :name)
+          match = line.match(regex)
+          match[group] if match
+        end
+      end
+    end
+  end
+end

lib/gitlab/dependency_linker/podfile_linker.rb

@@ -5,12 +5,21 @@ module Gitlab
     class PodfileLinker < GemfileLinker
       include Cocoapods
 
+      self.package_keyword = :pod
       self.file_type = :podfile
 
       private
 
       def link_packages
-        link_method_call('pod', &method(:package_url))
+        packages = parse_packages
+
+        return unless packages
+
+        packages.each do |package|
+          link_method_call('pod', package.name) do
+            external_url(package.name, package.external_ref)
+          end
+        end
       end
     end
   end

lib/gitlab/dependency_linker/podspec_linker.rb

@@ -19,7 +19,7 @@ module Gitlab
         link_method_call('license', &method(:license_url))
         link_regex(/license\s*=\s*\{\s*(type:|:type\s*=>)\s*#{STRING_REGEX}/, &method(:license_url))
 
-        link_method_call(%w[name dependency], &method(:package_url))
+        link_method_call('dependency', &method(:package_url))
       end
     end
   end

lib/gitlab/import_export/merge_request_parser.rb

@@ -20,6 +20,17 @@ module Gitlab
           create_target_branch unless branch_exists?(@merge_request.target_branch)
         end
 
+        # The merge_request_diff associated with the current @merge_request might
+        # be invalid. Than means, when the @merge_request object is saved, the
+        # @merge_request.merge_request_diff won't. This can leave the merge request
+        # in an invalid state, because a merge request must have an associated
+        # merge request diff.
+        # In this change, if the associated merge request diff is invalid, we set
+        # it to nil. This change, in association with the after callback
+        # :ensure_merge_request_diff in the MergeRequest class, makes that
+        # when the merge request is going to be created and it doesn't have
+        # one, a default one will be generated.
+        @merge_request.merge_request_diff = nil unless @merge_request.merge_request_diff&.valid?
         @merge_request
       end
 

lib/gitlab/kubernetes/kube_client.rb

@@ -82,6 +82,8 @@ module Gitlab
       def initialize(api_prefix, **kubeclient_options)
         @api_prefix = api_prefix
         @kubeclient_options = kubeclient_options.merge(http_max_redirects: 0)
+
+        validate_url!
       end
 
       def create_or_update_cluster_role_binding(resource)
@@ -118,6 +120,12 @@ module Gitlab
 
       private
 
+      def validate_url!
+        return if Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
+
+        Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false)
+      end
+
       def cluster_role_binding_exists?(resource)
         get_cluster_role_binding(resource.metadata.name)
       rescue ::Kubeclient::ResourceNotFoundError

locale/gitlab.pot

@@ -1341,6 +1341,9 @@ msgstr ""
 msgid "Cannot modify managed Kubernetes cluster"
 msgstr ""
 
+msgid "Cannot render the image. Maximum character count (%{charLimit}) has been exceeded."
+msgstr ""
+
 msgid "Certificate"
 msgstr ""
 

spec/controllers/dashboard/milestones_controller_spec.rb

@@ -13,7 +13,7 @@ describe Dashboard::MilestonesController do
     )
   end
   let(:issue) { create(:issue, project: project, milestone: project_milestone) }
-  let(:group_issue) { create(:issue, milestone: group_milestone) }
+  let(:group_issue) { create(:issue, milestone: group_milestone, project: create(:project, group: group)) }
 
   let!(:label) { create(:label, project: project, title: 'Issue Label', issues: [issue]) }
   let!(:group_label) { create(:group_label, group: group, title: 'Group Issue Label', issues: [group_issue]) }

spec/controllers/google_api/authorizations_controller_spec.rb

@@ -6,7 +6,7 @@ describe GoogleApi::AuthorizationsController do
     let(:token) { 'token' }
     let(:expires_at) { 1.hour.since.strftime('%s') }
 
-    subject { get :callback, params: { code: 'xxx', state: @state } }
+    subject { get :callback, params: { code: 'xxx', state: state } }
 
     before do
       sign_in(user)
@@ -15,24 +15,33 @@ describe GoogleApi::AuthorizationsController do
         .to receive(:get_token).and_return([token, expires_at])
     end
 
-    it 'sets token and expires_at in session' do
+    shared_examples_for 'access denied' do
+      it 'returns a 404' do
         subject
 
-      expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token])
-        .to eq(token)
-      expect(session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at])
-        .to eq(expires_at)
+        expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token]).to be_nil
+        expect(response).to have_http_status(:not_found)
+      end
     end
 
-    context 'when redirect uri key is stored in state' do
-      set(:project) { create(:project) }
-      let(:redirect_uri) { project_clusters_url(project).to_s }
+    context 'session key is present' do
+      let(:session_key) { 'session-key' }
+      let(:redirect_uri) { 'example.com' }
 
       before do
-        @state = GoogleApi::CloudPlatform::Client
-          .new_session_key_for_redirect_uri do |key|
-          session[key] = redirect_uri
+        session[GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(session_key)] = redirect_uri
       end
+
+      context 'session key matches state param' do
+        let(:state) { session_key }
+
+        it 'sets token and expires_at in session' do
+          subject
+
+          expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token])
+            .to eq(token)
+          expect(session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at])
+            .to eq(expires_at)
         end
 
         it 'redirects to the URL stored in state param' do
@@ -40,10 +49,23 @@ describe GoogleApi::AuthorizationsController do
         end
       end
 
-    context 'when redirection url is not stored in state' do
-      it 'redirects to root_path' do
-        expect(subject).to redirect_to(root_path)
+      context 'session key does not match state param' do
+        let(:state) { 'bad-key' }
+
+        it_behaves_like 'access denied'
       end
+
+      context 'state param is blank' do
+        let(:state) { '' }
+
+        it_behaves_like 'access denied'
+      end
+    end
+
+    context 'state param is present, but session key is blank' do
+      let(:state) { 'session-key' }
+
+      it_behaves_like 'access denied'
     end
   end
 end

spec/controllers/groups/shared_projects_controller_spec.rb

@@ -6,6 +6,8 @@ describe Groups::SharedProjectsController do
   end
 
   def share_project(project)
+    group.add_developer(user)
+
     Projects::GroupLinks::CreateService.new(
       project,
       user,

spec/controllers/omniauth_callbacks_controller_spec.rb

@@ -193,7 +193,7 @@ describe OmniauthCallbacksController, type: :controller do
     before do
       stub_omniauth_saml_config({ enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'],
                                   providers: [saml_config] })
-      mock_auth_hash('saml', 'my-uid', user.email, mock_saml_response)
+      mock_auth_hash_with_saml_xml('saml', 'my-uid', user.email, mock_saml_response)
       request.env["devise.mapping"] = Devise.mappings[:user]
       request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth']
       post :saml, params: { SAMLResponse: mock_saml_response }

spec/controllers/projects/autocomplete_sources_controller_spec.rb

@@ -35,4 +35,35 @@ describe Projects::AutocompleteSourcesController do
                                                  avatar_url: user.avatar_url)
     end
   end
+
+  describe 'GET milestones' do
+    let(:group) { create(:group, :public) }
+    let(:project) { create(:project, :public, namespace: group) }
+    let!(:project_milestone) { create(:milestone, project: project) }
+    let!(:group_milestone) { create(:milestone, group: group) }
+
+    before do
+      sign_in(user)
+    end
+
+    it 'lists milestones' do
+      group.add_owner(user)
+
+      get :milestones, format: :json, params: { namespace_id: group.path, project_id: project.path }
+
+      milestone_titles = json_response.map { |milestone| milestone["title"] }
+      expect(milestone_titles).to match_array([project_milestone.title, group_milestone.title])
+    end
+
+    context 'when user cannot read project issues and merge requests' do
+      it 'renders 404' do
+        project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
+        project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+
+        get :milestones, format: :json, params: { namespace_id: group.path, project_id: project.path }
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
 end

spec/controllers/projects/group_links_controller_spec.rb

@@ -65,8 +65,24 @@ describe Projects::GroupLinksController do
       end
     end
 
+    context 'when user does not have access to the public group' do
+      let(:group) { create(:group, :public) }
+
+      include_context 'link project to group'
+
+      it 'renders 404' do
+        expect(response.status).to eq 404
+      end
+
+      it 'does not share project with that group' do
+        expect(group.shared_projects).not_to include project
+      end
+    end
+
     context 'when project group id equal link group id' do
       before do
+        group2.add_developer(user)
+
         post(:create, params: {
                         namespace_id: project.namespace,
                         project_id: project,
@@ -102,5 +118,26 @@ describe Projects::GroupLinksController do
         expect(flash[:alert]).to eq('Please select a group.')
       end
     end
+
+    context 'when link is not persisted in the database' do
+      before do
+        allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute)
+          .and_return({ status: :error, http_status: 409, message: 'error' })
+
+        post(:create, params: {
+                        namespace_id: project.namespace,
+                        project_id: project,
+                        link_group_id: group.id,
+                        link_group_access: ProjectGroupLink.default_access
+                      })
+      end
+
+      it 'redirects to project group links page' do
+        expect(response).to redirect_to(
+          project_project_members_path(project)
+        )
+        expect(flash[:alert]).to eq('error')
+      end
+    end
   end
 end

spec/controllers/snippets_controller_spec.rb

@@ -205,6 +205,8 @@ describe SnippetsController do
     end
 
     context 'when the snippet description contains a file' do
+      include FileMoverHelpers
+
       let(:picture_file) { '/-/system/temp/secret56/picture.jpg' }
       let(:text_file) { '/-/system/temp/secret78/text.txt' }
       let(:description) do
@@ -215,6 +217,8 @@ describe SnippetsController do
       before do
         allow(FileUtils).to receive(:mkdir_p)
         allow(FileUtils).to receive(:move)
+        stub_file_mover(text_file)
+        stub_file_mover(picture_file)
       end
 
       subject { create_snippet({ description: description }, { files: [picture_file, text_file] }) }

spec/features/issues_spec.rb

@@ -233,8 +233,8 @@ describe 'Issues' do
                          created_at: Time.now - (index * 60))
         end
       end
-      let(:newer_due_milestone) { create(:milestone, due_date: '2013-12-11') }
-      let(:later_due_milestone) { create(:milestone, due_date: '2013-12-12') }
+      let(:newer_due_milestone) { create(:milestone, project: project, due_date: '2013-12-11') }
+      let(:later_due_milestone) { create(:milestone, project: project, due_date: '2013-12-12') }
 
       it 'sorts by newest' do
         visit project_issues_path(project, sort: sort_value_created_date)

spec/features/merge_request/user_sees_versions_spec.rb

 require 'rails_helper'
 
 describe 'Merge request > User sees versions', :js do
-  let(:merge_request) { create(:merge_request, importing: true) }
+  let(:merge_request) do
+    create(:merge_request).tap do |mr|
+      mr.merge_request_diff.destroy
+    end
+  end
   let(:project) { merge_request.source_project }
   let(:user) { project.creator }
   let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }

spec/features/merge_requests/user_lists_merge_requests_spec.rb

@@ -13,7 +13,7 @@ describe 'Merge requests > User lists merge requests' do
                   source_project: project,
                   source_branch: 'fix',
                   assignee: user,
-                  milestone: create(:milestone, due_date: '2013-12-11'),
+                  milestone: create(:milestone, project: project, due_date: '2013-12-11'),
                   created_at: 1.minute.ago,
                   updated_at: 1.minute.ago)
     create(:merge_request,
@@ -21,7 +21,7 @@ describe 'Merge requests > User lists merge requests' do
            source_project: project,
            source_branch: 'markdown',
            assignee: user,
-           milestone: create(:milestone, due_date: '2013-12-12'),
+           milestone: create(:milestone, project: project, due_date: '2013-12-12'),
            created_at: 2.minutes.ago,
            updated_at: 2.minutes.ago)
     create(:merge_request,

spec/features/profiles/active_sessions_spec.rb

@@ -7,6 +7,8 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do
     end
   end
 
+  let(:admin) { create(:admin) }
+
   around do |example|
     Timecop.freeze(Time.zone.parse('2018-03-12 09:06')) do
       example.run
@@ -16,6 +18,7 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do
   it 'User sees their active sessions' do
     Capybara::Session.new(:session1)
     Capybara::Session.new(:session2)
+    Capybara::Session.new(:session3)
 
     # note: headers can only be set on the non-js (aka. rack-test) driver
     using_session :session1 do
@@ -37,9 +40,27 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do
       gitlab_sign_in(user)
     end
 
+    # set an admin session impersonating the user
+    using_session :session3 do
+      Capybara.page.driver.header(
+        'User-Agent',
+        'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36'
+      )
+
+      gitlab_sign_in(admin)
+
+      visit admin_user_path(user)
+
+      click_link 'Impersonate'
+    end
+
     using_session :session1 do
       visit profile_active_sessions_path
 
+      expect(page).to(
+        have_selector('ul.list-group li.list-group-item', { text: 'Signed in on',
+                                                            count: 2 }))
+
       expect(page).to have_content(
         '127.0.0.1 ' \
         'This is your current session ' \
@@ -57,33 +78,8 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do
       )
 
       expect(page).to have_selector '[title="Smartphone"]', count: 1
-    end
-  end
-
-  it 'User can revoke a session', :js, :redis_session_store do
-    Capybara::Session.new(:session1)
-    Capybara::Session.new(:session2)
-
-    # set an additional session in another browser
-    using_session :session2 do
-      gitlab_sign_in(user)
-    end
-
-    using_session :session1 do
-      gitlab_sign_in(user)
-      visit profile_active_sessions_path
-
-      expect(page).to have_link('Revoke', count: 1)
-
-      accept_confirm { click_on 'Revoke' }
-
-      expect(page).not_to have_link('Revoke')
-    end
-
-    using_session :session2 do
-      visit profile_active_sessions_path
 
-      expect(page).to have_content('You need to sign in or sign up before continuing.')
+      expect(page).not_to have_content('Chrome on Windows')
     end
   end
 end

spec/features/projects/blobs/blob_show_spec.rb

@@ -548,10 +548,7 @@ describe 'File blob', :js do
     it 'displays an auxiliary viewer' do
       aggregate_failures do
         # shows names of dependency manager and package
-        expect(page).to have_content('This project manages its dependencies using RubyGems and defines a gem named activerecord.')
-
-        # shows a link to the gem
-        expect(page).to have_link('activerecord', href: 'https://rubygems.org/gems/activerecord')
+        expect(page).to have_content('This project manages its dependencies using RubyGems.')
 
         # shows a learn more link
         expect(page).to have_link('Learn more', href: 'https://rubygems.org/')

spec/features/projects/members/invite_group_spec.rb

@@ -27,6 +27,7 @@ describe 'Project > Members > Invite group', :js do
 
       before do
         project.add_maintainer(maintainer)
+        group_to_share_with.add_guest(maintainer)
         sign_in(maintainer)
       end
 
@@ -112,6 +113,7 @@ describe 'Project > Members > Invite group', :js do
 
     before do
       project.add_maintainer(maintainer)
+      group.add_guest(maintainer)
       sign_in(maintainer)
 
       visit project_settings_members_path(project)

spec/features/projects/settings/user_manages_group_links_spec.rb

@@ -10,6 +10,7 @@ describe 'Projects > Settings > User manages group links' do
 
   before do
     project.add_maintainer(user)
+    group_market.add_guest(user)
     sign_in(user)
 
     share_link = project.project_group_links.new(group_access: Gitlab::Access::MAINTAINER)

spec/features/security/group/private_access_spec.rb

@@ -27,7 +27,7 @@ describe 'Private Group access' do
     it { is_expected.to be_allowed_for(:developer).of(group) }
     it { is_expected.to be_allowed_for(:reporter).of(group) }
     it { is_expected.to be_allowed_for(:guest).of(group) }
-    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(project_guest) }
     it { is_expected.to be_denied_for(:user) }
     it { is_expected.to be_denied_for(:external) }
     it { is_expected.to be_denied_for(:visitor) }
@@ -42,7 +42,7 @@ describe 'Private Group access' do
     it { is_expected.to be_allowed_for(:developer).of(group) }
     it { is_expected.to be_allowed_for(:reporter).of(group) }
     it { is_expected.to be_allowed_for(:guest).of(group) }
-    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(project_guest) }
     it { is_expected.to be_denied_for(:user) }
     it { is_expected.to be_denied_for(:external) }
     it { is_expected.to be_denied_for(:visitor) }
@@ -58,7 +58,7 @@ describe 'Private Group access' do
     it { is_expected.to be_allowed_for(:developer).of(group) }
     it { is_expected.to be_allowed_for(:reporter).of(group) }
     it { is_expected.to be_allowed_for(:guest).of(group) }
-    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(project_guest) }
     it { is_expected.to be_denied_for(:user) }
     it { is_expected.to be_denied_for(:external) }
     it { is_expected.to be_denied_for(:visitor) }
@@ -73,7 +73,7 @@ describe 'Private Group access' do
     it { is_expected.to be_allowed_for(:developer).of(group) }
     it { is_expected.to be_allowed_for(:reporter).of(group) }
     it { is_expected.to be_allowed_for(:guest).of(group) }
-    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(project_guest) }
     it { is_expected.to be_denied_for(:user) }
     it { is_expected.to be_denied_for(:external) }
     it { is_expected.to be_denied_for(:visitor) }
@@ -93,4 +93,28 @@ describe 'Private Group access' do
     it { is_expected.to be_denied_for(:visitor) }
     it { is_expected.to be_denied_for(:external) }
   end
+
+  describe 'GET /groups/:path for shared projects' do
+    let(:project) { create(:project, :public) }
+    before do
+      Projects::GroupLinks::CreateService.new(
+        project,
+        create(:user),
+        link_group_access: ProjectGroupLink::DEVELOPER
+      ).execute(group)
+    end
+
+    subject { group_path(group) }
+
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:maintainer).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_denied_for(project_guest) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
+  end
 end

spec/finders/merge_requests_finder_spec.rb

@@ -31,7 +31,7 @@ describe MergeRequestsFinder do
       p
     end
   end
-  let(:project4) { create_project_without_n_plus_1(group: subgroup) }
+  let(:project4) { create_project_without_n_plus_1(:repository, group: subgroup) }
   let(:project5) { create_project_without_n_plus_1(group: subgroup) }
   let(:project6) { create_project_without_n_plus_1(group: subgroup) }
 
@@ -68,6 +68,15 @@ describe MergeRequestsFinder do
       expect(merge_requests.size).to eq(2)
     end
 
+    it 'filters by commit sha' do
+      merge_requests = described_class.new(
+        user,
+        commit_sha: merge_request5.merge_request_diff.last_commit_sha
+      ).execute
+
+      expect(merge_requests).to contain_exactly(merge_request5)
+    end
+
     context 'filtering by group' do
       it 'includes all merge requests when user has access' do
         params = { group_id: group.id }
@@ -269,6 +278,21 @@ describe MergeRequestsFinder do
         expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request)
       end
     end
+
+    context 'when project restricts merge requests' do
+      let(:non_member) { create(:user) }
+      let(:project) { create(:project, :repository, :public, :merge_requests_private) }
+      let!(:merge_request) { create(:merge_request, source_project: project) }
+
+      it "returns nothing to to non members" do
+        merge_requests = described_class.new(
+          non_member,
+          project_id: project.id
+        ).execute
+
+        expect(merge_requests).to be_empty
+      end
+    end
   end
 
   describe '#row_count', :request_store do

spec/lib/constraints/project_url_constrainer_spec.rb

@@ -16,6 +16,10 @@ describe Constraints::ProjectUrlConstrainer do
         let(:request) { build_request('foo', 'bar') }
 
         it { expect(subject.matches?(request)).to be_falsey }
+
+        context 'existence_check is false' do
+          it { expect(subject.matches?(request, existence_check: false)).to be_truthy }
+        end
       end
 
       context "project id ending with .git" do

spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb

@@ -50,8 +50,8 @@ describe Gitlab::DependencyLinker::ComposerJsonLinker do
       %{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>}
     end
 
-    it 'links the module name' do
-      expect(subject).to include(link('laravel/laravel', 'https://packagist.org/packages/laravel/laravel'))
+    it 'does not link the module name' do
+      expect(subject).not_to include(link('laravel/laravel', 'https://packagist.org/packages/laravel/laravel'))
     end
 
     it 'links the homepage' do

spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb

@@ -41,13 +41,16 @@ describe Gitlab::DependencyLinker::GemfileLinker do
     end
 
     it 'links dependencies' do
-      expect(subject).to include(link('rails', 'https://rubygems.org/gems/rails'))
       expect(subject).to include(link('rails-deprecated_sanitizer', 'https://rubygems.org/gems/rails-deprecated_sanitizer'))
-      expect(subject).to include(link('responders', 'https://rubygems.org/gems/responders'))
-      expect(subject).to include(link('sprockets', 'https://rubygems.org/gems/sprockets'))
       expect(subject).to include(link('default_value_for', 'https://rubygems.org/gems/default_value_for'))
     end
 
+    it 'links to external dependencies' do
+      expect(subject).to include(link('rails', 'https://github.com/rails/rails'))
+      expect(subject).to include(link('responders', 'https://github.com/rails/responders'))
+      expect(subject).to include(link('sprockets', 'https://gitlab.example.com/gems/sprockets'))
+    end
+
     it 'links GitHub repos' do
       expect(subject).to include(link('rails/rails', 'https://github.com/rails/rails'))
       expect(subject).to include(link('rails/responders', 'https://github.com/rails/responders'))

spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb

@@ -43,8 +43,8 @@ describe Gitlab::DependencyLinker::GemspecLinker do
       %{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>}
     end
 
-    it 'links the gem name' do
-      expect(subject).to include(link('gitlab_git', 'https://rubygems.org/gems/gitlab_git'))
+    it 'does not link the gem name' do
+      expect(subject).not_to include(link('gitlab_git', 'https://rubygems.org/gems/gitlab_git'))
     end
 
     it 'links the license' do

spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb

@@ -33,7 +33,8 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do
             "express": "4.2.x",
             "bigpipe": "bigpipe/pagelet",
             "plates": "https://github.com/flatiron/plates/tarball/master",
-            "karma": "^1.4.1"
+            "karma": "^1.4.1",
+            "random": "git+https://EdOverflow@github.com/example/example.git"
           },
           "devDependencies": {
             "vows": "^0.7.0",
@@ -51,8 +52,8 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do
       %{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>}
     end
 
-    it 'links the module name' do
-      expect(subject).to include(link('module-name', 'https://npmjs.com/package/module-name'))
+    it 'does not link the module name' do
+      expect(subject).not_to include(link('module-name', 'https://npmjs.com/package/module-name'))
     end
 
     it 'links the homepage' do
@@ -71,14 +72,21 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do
       expect(subject).to include(link('primus', 'https://npmjs.com/package/primus'))
       expect(subject).to include(link('async', 'https://npmjs.com/package/async'))
       expect(subject).to include(link('express', 'https://npmjs.com/package/express'))
-      expect(subject).to include(link('bigpipe', 'https://npmjs.com/package/bigpipe'))
-      expect(subject).to include(link('plates', 'https://npmjs.com/package/plates'))
       expect(subject).to include(link('karma', 'https://npmjs.com/package/karma'))
       expect(subject).to include(link('vows', 'https://npmjs.com/package/vows'))
       expect(subject).to include(link('assume', 'https://npmjs.com/package/assume'))
       expect(subject).to include(link('pre-commit', 'https://npmjs.com/package/pre-commit'))
     end
 
+    it 'links dependencies to URL detected on value' do
+      expect(subject).to include(link('bigpipe', 'https://github.com/bigpipe/pagelet'))
+      expect(subject).to include(link('plates', 'https://github.com/flatiron/plates/tarball/master'))
+    end
+
+    it 'does not link to NPM when invalid git URL' do
+      expect(subject).not_to include(link('random', 'https://npmjs.com/package/random'))
+    end
+
     it 'links GitHub repos' do
       expect(subject).to include(link('bigpipe/pagelet', 'https://github.com/bigpipe/pagelet'))
     end

spec/lib/gitlab/dependency_linker/parser/gemfile_spec.rb

+require 'rails_helper'
+
+describe Gitlab::DependencyLinker::Parser::Gemfile do
+  describe '#parse' do
+    let(:file_content) do
+      <<-CONTENT.strip_heredoc
+        source 'https://rubygems.org'
+
+        gem "rails", '4.2.6', github: "rails/rails"
+        gem 'rails-deprecated_sanitizer', '~> 1.0.3'
+        gem 'responders', '~> 2.0', :github => 'rails/responders'
+        gem 'sprockets', '~> 3.6.0', git: 'https://gitlab.example.com/gems/sprockets'
+        gem 'default_value_for', '~> 3.0.0'
+      CONTENT
+    end
+
+    subject { described_class.new(file_content).parse(keyword: 'gem') }
+
+    def fetch_package(name)
+      subject.find { |package| package.name == name }
+    end
+
+    it 'returns parsed packages' do
+      expect(subject.size).to eq(5)
+      expect(subject).to all(be_a(Gitlab::DependencyLinker::Package))
+    end
+
+    it 'packages respond to name and external_ref accordingly' do
+      expect(fetch_package('rails')).to have_attributes(name: 'rails',
+                                                        github_ref: 'rails/rails',
+                                                        git_ref: nil)
+
+      expect(fetch_package('sprockets')).to have_attributes(name: 'sprockets',
+                                                            github_ref: nil,
+                                                            git_ref: 'https://gitlab.example.com/gems/sprockets')
+
+      expect(fetch_package('default_value_for')).to have_attributes(name: 'default_value_for',
+                                                                    github_ref: nil,
+                                                                    git_ref: nil)
+    end
+  end
+end

spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb

@@ -43,7 +43,10 @@ describe Gitlab::DependencyLinker::PodfileLinker do
 
     it 'links packages' do
       expect(subject).to include(link('AFNetworking', 'https://cocoapods.org/pods/AFNetworking'))
-      expect(subject).to include(link('Interstellar/Core', 'https://cocoapods.org/pods/Interstellar'))
+    end
+
+    it 'links external packages' do
+      expect(subject).to include(link('Interstellar/Core', 'https://github.com/ashfurrow/Interstellar.git'))
     end
 
     it 'links Git repos' do

spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb

@@ -42,8 +42,8 @@ describe Gitlab::DependencyLinker::PodspecLinker do
       %{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>}
     end
 
-    it 'links the gem name' do
-      expect(subject).to include(link('Reachability', 'https://cocoapods.org/pods/Reachability'))
+    it 'does not link the pod name' do
+      expect(subject).not_to include(link('Reachability', 'https://cocoapods.org/pods/Reachability'))
     end
 
     it 'links the license' do

spec/lib/gitlab/import_export/merge_request_parser_spec.rb

@@ -41,4 +41,20 @@ describe Gitlab::ImportExport::MergeRequestParser do
 
     expect(parsed_merge_request).to eq(merge_request)
   end
+
+  context 'when the merge request has diffs' do
+    let(:merge_request) do
+      build(:merge_request, source_project: forked_project, target_project: project)
+    end
+
+    context 'when the diff is invalid' do
+      let(:merge_request_diff) { build(:merge_request_diff, merge_request: merge_request, base_commit_sha: 'foobar') }
+
+      it 'sets the diff to nil' do
+        expect(merge_request_diff).to be_invalid
+        expect(merge_request_diff.merge_request).to eq merge_request
+        expect(parsed_merge_request.merge_request_diff).to be_nil
+      end
+    end
+  end
 end

spec/lib/gitlab/kubernetes/kube_client_spec.rb

@@ -50,6 +50,36 @@ describe Gitlab::Kubernetes::KubeClient do
     end
   end
 
+  describe '#initialize' do
+    shared_examples 'local address' do
+      it 'blocks local addresses' do
+        expect { client }.to raise_error(Gitlab::UrlBlocker::BlockedUrlError)
+      end
+
+      context 'when local requests are allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: true)
+        end
+
+        it 'allows local addresses' do
+          expect { client }.not_to raise_error
+        end
+      end
+    end
+
+    context 'localhost address' do
+      let(:api_url) { 'http://localhost:22' }
+
+      it_behaves_like 'local address'
+    end
+
+    context 'private network address' do
+      let(:api_url) { 'http://192.168.1.2:3003' }
+
+      it_behaves_like 'local address'
+    end
+  end
+
   describe '#core_client' do
     subject { client.core_client }
 

spec/mailers/notify_spec.rb

@@ -208,6 +208,7 @@ describe Notify do
         let(:new_issue) { create(:issue) }
         subject { described_class.issue_moved_email(recipient, issue, new_issue, current_user) }
 
+<<<<<<< HEAD
         it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
           let(:model) { issue }
         end
@@ -215,6 +216,17 @@ describe Notify do
         it_behaves_like 'an unsubscribeable thread'
         it_behaves_like 'appearance header and footer enabled'
         it_behaves_like 'appearance header and footer not enabled'
+=======
+        context 'when a user has permissions to access the new issue' do
+          before do
+            new_issue.project.add_developer(recipient)
+          end
+
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { issue }
+          end
+          it_behaves_like 'it should show Gmail Actions View Issue link'
+          it_behaves_like 'an unsubscribeable thread'
 
           it 'contains description about action taken' do
             is_expected.to have_body_text 'Issue was moved to another project'
@@ -222,6 +234,7 @@ describe Notify do
 
           it 'has the correct subject and body' do
             new_issue_url = project_issue_path(new_issue.project, new_issue)
+>>>>>>> dev/master
 
             aggregate_failures do
               is_expected.to have_referable_subject(issue, reply: true)
@@ -229,6 +242,31 @@ describe Notify do
               is_expected.to have_body_text(project_issue_path(project, issue))
             end
           end
+
+          it 'contains the issue title' do
+            is_expected.to have_body_text new_issue.title
+          end
+        end
+
+        context 'when a user does not permissions to access the new issue' do
+          it 'has the correct subject and body' do
+            new_issue_url = project_issue_path(new_issue.project, new_issue)
+
+            aggregate_failures do
+              is_expected.to have_referable_subject(issue, reply: true)
+              is_expected.not_to have_body_text(new_issue_url)
+              is_expected.to have_body_text(project_issue_path(project, issue))
+            end
+          end
+
+          it 'does not contain the issue title' do
+            is_expected.not_to have_body_text new_issue.title
+          end
+
+          it 'contains information about missing permissions' do
+            is_expected.to have_body_text "You don't have access to the project."
+          end
+        end
       end
     end
 

spec/models/active_session_spec.rb

@@ -7,7 +7,10 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
     end
   end
 
-  let(:session) { double(:session, id: '6919a6f1bb119dd7396fadc38fd18d0d') }
+  let(:session) do
+    double(:session, { id: '6919a6f1bb119dd7396fadc38fd18d0d',
+                       '[]': {} })
+  end
 
   let(:request) do
     double(:request, {

spec/models/clusters/platforms/kubernetes_spec.rb

@@ -98,6 +98,22 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
 
         it { expect(kubernetes.save).to be_truthy }
       end
+
+      context 'when api_url is localhost' do
+        let(:api_url) { 'http://localhost:22' }
+
+        it { expect(kubernetes.save).to be_falsey }
+
+        context 'Application settings allows local requests' do
+          before do
+            allow(ApplicationSetting)
+              .to receive(:current)
+              .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true))
+          end
+
+          it { expect(kubernetes.save).to be_truthy }
+        end
+      end
     end
 
     context 'when validates token' do

spec/models/concerns/issuable_spec.rb

@@ -32,6 +32,7 @@ describe Issuable do
   end
 
   describe "Validation" do
+    context 'general validations' do
       subject { build(:issue) }
 
       before do
@@ -45,6 +46,44 @@ describe Issuable do
       it { is_expected.to validate_length_of(:title).is_at_most(255) }
     end
 
+    describe 'milestone' do
+      let(:project) { create(:project) }
+      let(:milestone_id) { create(:milestone, project: project).id }
+      let(:params) do
+        {
+          title: 'something',
+          project: project,
+          author: build(:user),
+          milestone_id: milestone_id
+        }
+      end
+
+      subject { issuable_class.new(params) }
+
+      context 'with correct params' do
+        it { is_expected.to be_valid }
+      end
+
+      context 'with empty string milestone' do
+        let(:milestone_id) { '' }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'with nil milestone id' do
+        let(:milestone_id) { nil }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'with a milestone id from another project' do
+        let(:milestone_id) { create(:milestone).id }
+
+        it { is_expected.to be_invalid }
+      end
+    end
+  end
+
   describe "Scope" do
     subject { build(:issue) }
 
@@ -66,6 +105,48 @@ describe Issuable do
     end
   end
 
+  describe '#milestone_available?' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project, group: group) }
+    let(:issue) { create(:issue, project: project) }
+
+    def build_issuable(milestone_id)
+      issuable_class.new(project: project, milestone_id: milestone_id)
+    end
+
+    it 'returns true with a milestone from the issue project' do
+      milestone = create(:milestone, project: project)
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+    end
+
+    it 'returns true with a milestone from the issue project group' do
+      milestone = create(:milestone, group: group)
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+    end
+
+    it 'returns true with a milestone from the the parent of the issue project group', :nested_groups do
+      parent = create(:group)
+      group.update(parent: parent)
+      milestone = create(:milestone, group: parent)
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_truthy
+    end
+
+    it 'returns false with a milestone from another project' do
+      milestone = create(:milestone)
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_falsey
+    end
+
+    it 'returns false with a milestone from another group' do
+      milestone = create(:milestone, group: create(:group))
+
+      expect(build_issuable(milestone.id).milestone_available?).to be_falsey
+    end
+  end
+
   describe ".search" do
     let!(:searchable_issue) { create(:issue, title: "Searchable awesome issue") }
     let!(:searchable_issue2) { create(:issue, title: 'Aw') }

spec/models/concerns/milestoneish_spec.rb

@@ -48,7 +48,7 @@ describe Milestone, 'Milestoneish' do
       merge_request_2 = create(:labeled_merge_request, labels: [label_1], source_project: project, source_branch: 'branch_2', milestone: milestone)
       merge_request_3 = create(:labeled_merge_request, labels: [label_3], source_project: project, source_branch: 'branch_3', milestone: milestone)
 
-      merge_requests = milestone.sorted_merge_requests
+      merge_requests = milestone.sorted_merge_requests(member)
 
       expect(merge_requests.first).to eq(merge_request_2)
       expect(merge_requests.second).to eq(merge_request_1)
@@ -56,6 +56,51 @@ describe Milestone, 'Milestoneish' do
     end
   end
 
+  describe '#merge_requests_visible_to_user' do
+    let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
+
+    context 'when project is private' do
+      before do
+        project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+      end
+
+      it 'does not return any merge request for a non member' do
+        merge_requests = milestone.merge_requests_visible_to_user(non_member)
+        expect(merge_requests).to be_empty
+      end
+
+      it 'returns milestone merge requests for a member' do
+        merge_requests = milestone.merge_requests_visible_to_user(member)
+        expect(merge_requests).to contain_exactly(merge_request)
+      end
+    end
+
+    context 'when project is public' do
+      context 'when merge requests are available to anyone' do
+        it 'returns milestone merge requests for a non member' do
+          merge_requests = milestone.merge_requests_visible_to_user(non_member)
+          expect(merge_requests).to contain_exactly(merge_request)
+        end
+      end
+
+      context 'when merge requests are available to project members' do
+        before do
+          project.project_feature.update(merge_requests_access_level: ProjectFeature::PRIVATE)
+        end
+
+        it 'does not return any merge request for a non member' do
+          merge_requests = milestone.merge_requests_visible_to_user(non_member)
+          expect(merge_requests).to be_empty
+        end
+
+        it 'returns milestone merge requests for a member' do
+          merge_requests = milestone.merge_requests_visible_to_user(member)
+          expect(merge_requests).to contain_exactly(merge_request)
+        end
+      end
+    end
+  end
+
   describe '#closed_items_count' do
     it 'does not count confidential issues for non project members' do
       expect(milestone.closed_items_count(non_member)).to eq 2

spec/models/issue/metrics_spec.rb

@@ -9,7 +9,7 @@ describe Issue::Metrics do
     context "milestones" do
       it "records the first time an issue is associated with a milestone" do
         time = Time.now
-        Timecop.freeze(time) { subject.update(milestone: create(:milestone)) }
+        Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) }
         metrics = subject.metrics
 
         expect(metrics).to be_present
@@ -18,9 +18,9 @@ describe Issue::Metrics do
 
       it "does not record the second time an issue is associated with a milestone" do
         time = Time.now
-        Timecop.freeze(time) { subject.update(milestone: create(:milestone)) }
+        Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) }
         Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) }
-        Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone)) }
+        Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone, project: project)) }
         metrics = subject.metrics
 
         expect(metrics).to be_present

spec/models/merge_request_diff_spec.rb

@@ -3,6 +3,18 @@ require 'spec_helper'
 describe MergeRequestDiff do
   let(:diff_with_commits) { create(:merge_request).merge_request_diff }
 
+  describe 'validations' do
+    subject { diff_with_commits }
+
+    it 'checks sha format of base_commit_sha, head_commit_sha and start_commit_sha' do
+      subject.base_commit_sha = subject.head_commit_sha = subject.start_commit_sha = 'foobar'
+
+      expect(subject.valid?).to be false
+      expect(subject.errors.count).to eq 3
+      expect(subject.errors).to all(include('is not a valid SHA'))
+    end
+  end
+
   describe 'create new record' do
     subject { diff_with_commits }
 
@@ -78,7 +90,7 @@ describe MergeRequestDiff do
       it 'returns persisted diffs if cannot compare with diff refs' do
         expect(diff).to receive(:load_diffs).and_call_original
 
-        diff.update!(head_commit_sha: 'invalid-sha')
+        diff.update!(head_commit_sha: Digest::SHA1.hexdigest(SecureRandom.hex))
 
         diff.diffs.diff_files
       end

spec/models/milestone_spec.rb

@@ -182,7 +182,7 @@ describe Milestone do
   describe '#total_items_count' do
     before do
       create :closed_issue, milestone: milestone, project: project
-      create :merge_request, milestone: milestone
+      create :merge_request, milestone: milestone, source_project: project
     end
 
     it 'returns total count of issues and merge requests assigned to milestone' do
@@ -192,10 +192,10 @@ describe Milestone do
 
   describe '#can_be_closed?' do
     before do
-      milestone = create :milestone
-      create :closed_issue, milestone: milestone
+      milestone = create :milestone, project: project
+      create :closed_issue, milestone: milestone, project: project
 
-      create :issue
+      create :issue, project: project
     end
 
     it 'returns true if milestone active and all nested issues closed' do

spec/models/project_services/prometheus_service_spec.rb

@@ -33,18 +33,38 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
   describe 'Validations' do
     context 'when manual_configuration is enabled' do
       before do
-        subject.manual_configuration = true
+        service.manual_configuration = true
       end
 
-      it { is_expected.to validate_presence_of(:api_url) }
+      it 'validates presence of api_url' do
+        expect(service).to validate_presence_of(:api_url)
+      end
     end
 
     context 'when manual configuration is disabled' do
       before do
-        subject.manual_configuration = false
+        service.manual_configuration = false
+      end
+
+      it 'does not validate presence of api_url' do
+        expect(service).not_to validate_presence_of(:api_url)
+      end
     end
 
-      it { is_expected.not_to validate_presence_of(:api_url) }
+    context 'when the api_url domain points to localhost or local network' do
+      let(:domain) { Addressable::URI.parse(service.api_url).hostname }
+
+      it 'cannot query' do
+        expect(service.can_query?).to be true
+
+        aggregate_failures do
+          ['127.0.0.1', '192.168.2.3'].each do |url|
+            allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([Addrinfo.tcp(url, 80)])
+
+            expect(service.can_query?).to be false
+          end
+        end
+      end
     end
   end
 
@@ -74,30 +94,35 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
   end
 
   describe '#prometheus_client' do
-    context 'manual configuration is enabled' do
     let(:api_url) { 'http://some_url' }
 
     before do
-        subject.active = true
-        subject.manual_configuration = true
-        subject.api_url = api_url
+      service.active = true
+      service.api_url = api_url
+      service.manual_configuration = manual_configuration
     end
 
+    context 'manual configuration is enabled' do
+      let(:manual_configuration) { true }
+
       it 'returns rest client from api_url' do
-        expect(subject.prometheus_client.url).to eq(api_url)
-      end
+        expect(service.prometheus_client.url).to eq(api_url)
       end
 
-    context 'manual configuration is disabled' do
-      let(:api_url) { 'http://some_url' }
+      it 'calls valid?' do
+        allow(service).to receive(:valid?).and_call_original
 
-      before do
-        subject.manual_configuration = false
-        subject.api_url = api_url
+        expect(service.prometheus_client).not_to be_nil
+
+        expect(service).to have_received(:valid?)
+      end
     end
 
+    context 'manual configuration is disabled' do
+      let(:manual_configuration) { false }
+
       it 'no client provided' do
-        expect(subject.prometheus_client).to be_nil
+        expect(service.prometheus_client).to be_nil
       end
     end
   end

spec/policies/commit_policy_spec.rb

+require 'spec_helper'
+
+describe CommitPolicy do
+  describe '#rules' do
+    let(:user) { create(:user) }
+    let(:commit) { project.repository.head_commit }
+    let(:policy) { described_class.new(user, commit) }
+
+    context 'when project is public' do
+      let(:project) { create(:project, :public, :repository) }
+
+      it 'can read commit and create a note' do
+        expect(policy).to be_allowed(:read_commit)
+      end
+
+      context 'when repository access level is private' do
+        let(:project) { create(:project, :public, :repository, :repository_private) }
+
+        it 'can not read commit and create a note' do
+          expect(policy).to be_disallowed(:read_commit)
+        end
+
+        context 'when the user is a project member' do
+          before do
+            project.add_developer(user)
+          end
+
+          it 'can read commit and create a note' do
+            expect(policy).to be_allowed(:read_commit)
+          end
+        end
+      end
+    end
+
+    context 'when project is private' do
+      let(:project) { create(:project, :private, :repository) }
+
+      it 'can not read commit and create a note' do
+        expect(policy).to be_disallowed(:read_commit)
+      end
+
+      context 'when the user is a project member' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'can read commit and create a note' do
+          expect(policy).to be_allowed(:read_commit)
+        end
+      end
+    end
+  end
+end

spec/policies/group_policy_spec.rb

@@ -74,6 +74,38 @@ describe GroupPolicy do
     end
   end
 
+  context 'with no user and public project' do
+    let(:project) { create(:project, :public) }
+    let(:user) { create(:user) }
+    let(:current_user) { nil }
+
+    before do
+      Projects::GroupLinks::CreateService.new(
+        project,
+        user,
+        link_group_access: ProjectGroupLink::DEVELOPER
+      ).execute(group)
+    end
+
+    it { expect_disallowed(:read_group) }
+  end
+
+  context 'with foreign user and public project' do
+    let(:project) { create(:project, :public) }
+    let(:user) { create(:user) }
+    let(:current_user) { create(:user) }
+
+    before do
+      Projects::GroupLinks::CreateService.new(
+        project,
+        user,
+        link_group_access: ProjectGroupLink::DEVELOPER
+      ).execute(group)
+    end
+
+    it { expect_disallowed(:read_group) }
+  end
+
   context 'has projects' do
     let(:current_user) { create(:user) }
     let(:project) { create(:project, namespace: group) }
@@ -82,17 +114,25 @@ describe GroupPolicy do
       project.add_developer(current_user)
     end
 
+<<<<<<< HEAD
     it do
       expect_allowed(:read_group, :read_list, :read_label)
     end
+=======
+    it { expect_allowed(:read_label) }
+>>>>>>> dev/master
 
     context 'in subgroups', :nested_groups do
       let(:subgroup) { create(:group, :private, parent: group) }
       let(:project) { create(:project, namespace: subgroup) }
 
+<<<<<<< HEAD
       it do
         expect_allowed(:read_group, :read_list, :read_label)
       end
+=======
+      it { expect_allowed(:read_label) }
+>>>>>>> dev/master
     end
   end
 

spec/policies/note_policy_spec.rb

 require 'spec_helper'
 
-describe NotePolicy, mdoels: true do
+describe NotePolicy do
   describe '#rules' do
     let(:user) { create(:user) }
     let(:project) { create(:project, :public) }
     let(:issue) { create(:issue, project: project) }
-
-    def policies(noteable = nil)
-      return @policies if @policies
-
-      noteable ||= issue
-      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) }
+    let(:policy) { described_class.new(user, note) }
+    let(:note) { create(:note, noteable: noteable, author: user, project: project) }
 
+    shared_examples_for 'a discussion with a private noteable' do
       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)
@@ -46,12 +33,21 @@ describe NotePolicy, mdoels: true do
       end
     end
 
+    context 'when the noteable is a commit' do
+      let(:commit) { project.repository.head_commit }
+      let(:note) { create(:note_on_commit, commit_id: commit.id, author: user, project: project) }
+
       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 }
+        it_behaves_like 'a discussion with a private noteable'
+      end
+
+      context 'when the project is public' do
+        context 'when repository access level is private' do
+          let(:project) { create(:project, :public, :repository, :repository_private) }
+
+          it_behaves_like 'a discussion with a private noteable'
         end
       end
     end
@@ -59,44 +55,44 @@ describe NotePolicy, mdoels: true do
     context 'when the project is public' do
       context 'when the note author is not a project member' do
         it 'can edit a note' do
-          expect(policies).to be_allowed(:admin_note)
-          expect(policies).to be_allowed(:resolve_note)
-          expect(policies).to be_allowed(:read_note)
+          expect(policy).to be_allowed(:admin_note)
+          expect(policy).to be_allowed(:resolve_note)
+          expect(policy).to be_allowed(:read_note)
         end
       end
 
       context 'when the noteable is a project snippet' do
-        it 'can edit note' do
-          policies = policies(create(:project_snippet, :public, project: project))
+        let(:noteable) { 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)
+        it 'can edit note' do
+          expect(policy).to be_allowed(:admin_note)
+          expect(policy).to be_allowed(:resolve_note)
+          expect(policy).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
+
+          it_behaves_like 'a discussion with a private noteable'
         end
       end
 
       context 'when the noteable is a personal snippet' do
-        it 'can edit note' do
-          policies = policies(create(:personal_snippet, :public))
+        let(:noteable) { 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)
+        it 'can edit note' do
+          expect(policy).to be_allowed(:admin_note)
+          expect(policy).to be_allowed(:resolve_note)
+          expect(policy).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))
+          let(:noteable) { 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)
+          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
       end
@@ -120,20 +116,20 @@ describe NotePolicy, mdoels: true do
           end
 
           it 'can edit a note' do
-            expect(policies).to be_allowed(:admin_note)
-            expect(policies).to be_allowed(:resolve_note)
-            expect(policies).to be_allowed(:read_note)
+            expect(policy).to be_allowed(:admin_note)
+            expect(policy).to be_allowed(:resolve_note)
+            expect(policy).to be_allowed(:read_note)
           end
         end
 
         context 'when the note author is not a project member' do
           it 'can not edit a note' do
-            expect(policies).to be_disallowed(:admin_note)
-            expect(policies).to be_disallowed(:resolve_note)
+            expect(policy).to be_disallowed(:admin_note)
+            expect(policy).to be_disallowed(:resolve_note)
           end
 
           it 'can read a note' do
-            expect(policies).to be_allowed(:read_note)
+            expect(policy).to be_allowed(:read_note)
           end
         end
       end

spec/policies/project_policy_spec.rb

@@ -131,25 +131,29 @@ describe ProjectPolicy do
     subject { described_class.new(owner, project) }
 
     context 'when the feature is disabled' do
-      it 'does not include the issues permissions' do
+      before do
         project.issues_enabled = false
         project.save!
+      end
 
+      it 'does not include the issues permissions' do
         expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue
       end
+
+      it 'disables boards and lists permissions' do
+        expect_disallowed :read_board, :create_board, :update_board, :admin_board
+        expect_disallowed :read_list, :create_list, :update_list, :admin_list
       end
 
-    context 'when the feature is disabled and external tracker configured' do
+      context 'when external tracker configured' do
         it 'does not include the issues permissions' do
           create(:jira_service, project: project)
 
-        project.issues_enabled = false
-        project.save!
-
           expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue
         end
       end
     end
+  end
 
   context 'merge requests feature' do
     subject { described_class.new(owner, project) }

spec/requests/api/commits_spec.rb

@@ -1430,8 +1430,8 @@ describe API::Commits do
   end
 
   describe 'GET /projects/:id/repository/commits/:sha/merge_requests' do
-    let!(:project) { create(:project, :repository, :private) }
-    let!(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') }
+    let(:project) { create(:project, :repository, :private) }
+    let(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') }
     let(:commit) { merged_mr.merge_request_diff.commits.last }
 
     it 'returns the correct merge request' do
@@ -1456,6 +1456,17 @@ describe API::Commits do
 
       expect(response).to have_gitlab_http_status(404)
     end
+
+    context 'public project' do
+      let(:project) { create(:project, :repository, :public, :merge_requests_private) }
+      let(:non_member) { create(:user) }
+
+      it 'responds 403 when only members are allowed to read merge requests' do
+        get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", non_member)
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
   end
 
   describe 'GET /projects/:id/repository/commits/:sha/signature' do

spec/requests/api/issues_spec.rb

@@ -49,7 +49,7 @@ describe API::Issues do
     create(:label, title: 'label', color: '#FFAABB', project: project)
   end
   let!(:label_link) { create(:label_link, label: label, target: issue) }
-  set(:milestone) { create(:milestone, title: '1.0.0', project: project) }
+  let(:milestone) { create(:milestone, title: '1.0.0', project: project) }
   set(:empty_milestone) do
     create(:milestone, title: '2.0.0', project: project)
   end

spec/requests/api/projects_spec.rb

@@ -136,6 +136,7 @@ describe API::Projects do
     end
 
     let!(:public_project) { create(:project, :public, name: 'public_project') }
+
     before do
       project
       project2
@@ -968,8 +969,16 @@ describe API::Projects do
 
   describe 'GET /projects/:id' do
     context 'when unauthenticated' do
-      it 'returns the public projects' do
-        public_project = create(:project, :public)
+      it 'does not return private projects' do
+        private_project = create(:project, :private)
+
+        get api("/projects/#{private_project.id}")
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+
+      it 'returns public projects' do
+        public_project = create(:project, :repository, :public)
 
         get api("/projects/#{public_project.id}")
 
@@ -977,8 +986,34 @@ describe API::Projects do
         expect(json_response['id']).to eq(public_project.id)
         expect(json_response['description']).to eq(public_project.description)
         expect(json_response['default_branch']).to eq(public_project.default_branch)
+        expect(json_response['ci_config_path']).to eq(public_project.ci_config_path)
         expect(json_response.keys).not_to include('permissions')
       end
+
+      context 'and the project has a private repository' do
+        let(:project) { create(:project, :repository, :public, :repository_private) }
+        let(:protected_attributes) { %w(default_branch ci_config_path) }
+
+        it 'hides protected attributes of private repositories if user is not a member' do
+          get api("/projects/#{project.id}", user)
+
+          expect(response).to have_gitlab_http_status(200)
+          protected_attributes.each do |attribute|
+            expect(json_response.keys).not_to include(attribute)
+          end
+        end
+
+        it 'exposes protected attributes of private repositories if user is a member' do
+          project.add_developer(user)
+
+          get api("/projects/#{project.id}", user)
+
+          expect(response).to have_gitlab_http_status(200)
+          protected_attributes.each do |attribute|
+            expect(json_response.keys).to include(attribute)
+          end
+        end
+      end
     end
 
     context 'when authenticated' do
@@ -1130,6 +1165,26 @@ describe API::Projects do
         expect(json_response).to include 'statistics'
       end
 
+      context "and the project has a private repository" do
+        let(:project) { create(:project, :public, :repository, :repository_private) }
+
+        it "does not include statistics if user is not a member" do
+          get api("/projects/#{project.id}", user), params: { statistics: true }
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).not_to include 'statistics'
+        end
+
+        it "includes statistics if user is a member" do
+          project.add_developer(user)
+
+          get api("/projects/#{project.id}", user), params: { statistics: true }
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to include 'statistics'
+        end
+      end
+
       it "includes import_error if user can admin project" do
         get api("/projects/#{project.id}", user)
 
@@ -1510,6 +1565,9 @@ describe API::Projects do
 
   describe "POST /projects/:id/share" do
     let(:group) { create(:group) }
+    before do
+      group.add_developer(user)
+    end
 
     it "shares project with group" do
       expires_at = 10.days.from_now.to_date
@@ -1560,6 +1618,15 @@ describe API::Projects do
       expect(response).to have_gitlab_http_status(400)
       expect(json_response['error']).to eq 'group_access does not have a valid value'
     end
+
+    it "returns a 409 error when link is not saved" do
+      allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute)
+        .and_return({ status: :error, http_status: 409, message: 'error' })
+
+      post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: Gitlab::Access::DEVELOPER }
+
+      expect(response).to have_gitlab_http_status(409)
+    end
   end
 
   describe 'DELETE /projects/:id/share/:group_id' do

spec/requests/api/release/links_spec.rb

@@ -73,6 +73,22 @@ describe API::Release::Links do
           expect(response).to have_gitlab_http_status(:ok)
         end
       end
+
+      context 'when project is public and the repository is private' do
+        let(:project) { create(:project, :repository, :public, :repository_private) }
+
+        it_behaves_like '403 response' do
+          let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) }
+        end
+
+        context 'when the release does not exists' do
+          let!(:release) { }
+
+          it_behaves_like '403 response' do
+            let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) }
+          end
+        end
+      end
     end
   end
 

spec/requests/git_http_spec.rb

@@ -104,6 +104,70 @@ describe 'Git HTTP requests' do
     end
   end
 
+  shared_examples_for 'project path without .git suffix' do
+    context "GET info/refs" do
+      let(:path) { "/#{project_path}/info/refs" }
+
+      context "when no params are added" do
+        before do
+          get path
+        end
+
+        it "redirects to the .git suffix version" do
+          expect(response).to redirect_to("/#{project_path}.git/info/refs")
+        end
+      end
+
+      context "when the upload-pack service is requested" do
+        let(:params) { { service: 'git-upload-pack' } }
+
+        before do
+          get path, params: params
+        end
+
+        it "redirects to the .git suffix version" do
+          expect(response).to redirect_to("/#{project_path}.git/info/refs?service=#{params[:service]}")
+        end
+      end
+
+      context "when the receive-pack service is requested" do
+        let(:params) { { service: 'git-receive-pack' } }
+
+        before do
+          get path, params: params
+        end
+
+        it "redirects to the .git suffix version" do
+          expect(response).to redirect_to("/#{project_path}.git/info/refs?service=#{params[:service]}")
+        end
+      end
+
+      context "when the params are anything else" do
+        let(:params) { { service: 'git-implode-pack' } }
+
+        before do
+          get path, params: params
+        end
+
+        it "redirects to the sign-in page" do
+          expect(response).to redirect_to(new_user_session_path)
+        end
+      end
+    end
+
+    context "POST git-upload-pack" do
+      it "fails to find a route" do
+        expect { clone_post(project_path) }.to raise_error(ActionController::RoutingError)
+      end
+    end
+
+    context "POST git-receive-pack" do
+      it "fails to find a route" do
+        expect { push_post(project_path) }.to raise_error(ActionController::RoutingError)
+      end
+    end
+  end
+
   describe "User with no identities" do
     let(:user) { create(:user) }
 
@@ -143,6 +207,10 @@ describe 'Git HTTP requests' do
             expect(response).to have_gitlab_http_status(:unprocessable_entity)
           end
         end
+
+        it_behaves_like 'project path without .git suffix' do
+          let(:project_path) { "#{user.namespace.path}/project.git-project" }
+        end
       end
     end
 
@@ -706,70 +774,8 @@ describe 'Git HTTP requests' do
         end
       end
 
-      context "when the project path doesn't end in .git" do
-        let(:project) { create(:project, :repository, :public, path: 'project.git-project') }
-
-        context "GET info/refs" do
-          let(:path) { "/#{project.full_path}/info/refs" }
-
-          context "when no params are added" do
-            before do
-              get path
-            end
-
-            it "redirects to the .git suffix version" do
-              expect(response).to redirect_to("/#{project.full_path}.git/info/refs")
-            end
-          end
-
-          context "when the upload-pack service is requested" do
-            let(:params) { { service: 'git-upload-pack' } }
-
-            before do
-              get path, params: params
-            end
-
-            it "redirects to the .git suffix version" do
-              expect(response).to redirect_to("/#{project.full_path}.git/info/refs?service=#{params[:service]}")
-            end
-          end
-
-          context "when the receive-pack service is requested" do
-            let(:params) { { service: 'git-receive-pack' } }
-
-            before do
-              get path, params: params
-            end
-
-            it "redirects to the .git suffix version" do
-              expect(response).to redirect_to("/#{project.full_path}.git/info/refs?service=#{params[:service]}")
-            end
-          end
-
-          context "when the params are anything else" do
-            let(:params) { { service: 'git-implode-pack' } }
-
-            before do
-              get path, params: params
-            end
-
-            it "redirects to the sign-in page" do
-              expect(response).to redirect_to(new_user_session_path)
-            end
-          end
-        end
-
-        context "POST git-upload-pack" do
-          it "fails to find a route" do
-            expect { clone_post(project.full_path) }.to raise_error(ActionController::RoutingError)
-          end
-        end
-
-        context "POST git-receive-pack" do
-          it "fails to find a route" do
-            expect { push_post(project.full_path) }.to raise_error(ActionController::RoutingError)
-          end
-        end
+      it_behaves_like 'project path without .git suffix' do
+        let(:project_path) { create(:project, :repository, :public, path: 'project.git-project').full_path }
       end
 
       context "retrieving an info/refs file" do

spec/services/issuable/common_system_notes_service_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Issuable::CommonSystemNotesService do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
-  let(:issuable) { create(:issue) }
+  let(:issuable) { create(:issue, project: project) }
 
   context 'on issuable update' do
     it_behaves_like 'system note creation', { title: 'New title' }, 'changed title'
@@ -70,7 +70,7 @@ describe Issuable::CommonSystemNotesService do
   end
 
   context 'on issuable create' do
-    let(:issuable) { build(:issue) }
+    let(:issuable) { build(:issue, project: project) }
 
     subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) }
 

spec/services/issues/build_service_spec.rb

@@ -8,29 +8,29 @@ describe Issues::BuildService do
     project.add_developer(user)
   end
 
+  def build_issue(issue_params = {})
+    described_class.new(project, user, issue_params).execute
+  end
+
   context 'for a single discussion' do
     describe '#execute' do
       let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) }
       let(:discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "Almost done").to_discussion }
-      let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) }
 
-      it 'references the noteable title in the issue title' do
-        issue = service.execute
+      subject { build_issue(merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) }
 
-        expect(issue.title).to include('Hello world')
+      it 'references the noteable title in the issue title' do
+        expect(subject.title).to include('Hello world')
       end
 
       it 'adds the note content to the description' do
-        issue = service.execute
-
-        expect(issue.description).to include('Almost done')
+        expect(subject.description).to include('Almost done')
       end
     end
   end
 
   context 'for discussions in a merge request' do
     let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
-    let(:issue) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute }
 
     describe '#items_for_discussions' do
       it 'has an item for each discussion' do
@@ -66,28 +66,30 @@ describe Issues::BuildService do
     end
 
     describe '#execute' do
+      let(:base_params) { { merge_request_to_resolve_discussions_of: merge_request.iid } }
+
+      context 'without additional params' do
+        subject { build_issue(base_params) }
+
         it 'has the merge request reference in the title' do
-        expect(issue.title).to include(merge_request.title)
+          expect(subject.title).to include(merge_request.title)
         end
 
         it 'has the reference of the merge request in the description' do
-        expect(issue.description).to include(merge_request.to_reference)
+          expect(subject.description).to include(merge_request.to_reference)
+        end
       end
 
-      it 'does not assign title when a title was given' do
-        issue = described_class.new(project, user,
-                                    merge_request_to_resolve_discussions_of: merge_request,
-                                    title: 'What an issue').execute
+      it 'uses provided title if title param given' do
+        issue = build_issue(base_params.merge(title: 'What an issue'))
 
         expect(issue.title).to eq('What an issue')
       end
 
-      it 'does not assign description when a description was given' do
-        issue = described_class.new(project, user,
-                                    merge_request_to_resolve_discussions_of: merge_request,
-                                    description: 'Fix at your earliest conveignance').execute
+      it 'uses provided description if description param given' do
+        issue = build_issue(base_params.merge(description: 'Fix at your earliest convenience'))
 
-        expect(issue.description).to eq('Fix at your earliest conveignance')
+        expect(issue.description).to eq('Fix at your earliest convenience')
       end
 
       describe 'with multiple discussions' do
@@ -96,20 +98,20 @@ describe Issues::BuildService do
         it 'mentions all the authors in the description' do
           authors = merge_request.resolvable_discussions.map(&:author)
 
-          expect(issue.description).to include(*authors.map(&:to_reference))
+          expect(build_issue(base_params).description).to include(*authors.map(&:to_reference))
         end
 
         it 'has a link for each unresolved discussion in the description' do
           notes = merge_request.resolvable_discussions.map(&:first_note)
           links = notes.map { |note| Gitlab::UrlBuilder.build(note) }
 
-          expect(issue.description).to include(*links)
+          expect(build_issue(base_params).description).to include(*links)
         end
 
         it 'mentions additional notes' do
           create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, in_reply_to: diff_note)
 
-          expect(issue.description).to include('(+2 comments)')
+          expect(build_issue(base_params).description).to include('(+2 comments)')
         end
       end
     end
@@ -120,7 +122,7 @@ describe Issues::BuildService do
 
     describe '#execute' do
       it 'mentions the merge request in the description' do
-        issue = described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute
+        issue = build_issue(merge_request_to_resolve_discussions_of: merge_request.iid)
 
         expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}")
       end
@@ -128,20 +130,18 @@ describe Issues::BuildService do
   end
 
   describe '#execute' do
-    let(:milestone) { create(:milestone, project: project) }
-
     it 'builds a new issues with given params' do
-      issue = described_class.new(
-        project,
-        user,
-        title: 'Issue #1',
-        description: 'Issue description',
-        milestone_id: milestone.id
-      ).execute
-
-      expect(issue.title).to eq('Issue #1')
-      expect(issue.description).to eq('Issue description')
+      milestone = create(:milestone, project: project)
+      issue = build_issue(milestone_id: milestone.id)
+
       expect(issue.milestone).to eq(milestone)
     end
+
+    it 'sets milestone to nil if it is not available for the project' do
+      milestone = create(:milestone, project: create(:project))
+      issue = build_issue(milestone_id: milestone.id)
+
+      expect(issue.milestone).to be_nil
+    end
   end
 end

spec/services/issues/update_service_spec.rb

@@ -356,7 +356,7 @@ describe Issues::UpdateService, :mailer do
         it_behaves_like 'system notes for milestones'
 
         it 'sends notifications for subscribers of changed milestone' do
-          issue.milestone = create(:milestone)
+          issue.milestone = create(:milestone, project: project)
 
           issue.save
 
@@ -380,7 +380,7 @@ describe Issues::UpdateService, :mailer do
         end
 
         it 'marks todos as done' do
-          update_issue(milestone: create(:milestone))
+          update_issue(milestone: create(:milestone, project: project))
 
           expect(todo.reload.done?).to eq true
         end
@@ -389,7 +389,7 @@ describe Issues::UpdateService, :mailer do
 
         it 'sends notifications for subscribers of changed milestone' do
           perform_enqueued_jobs do
-            update_issue(milestone: create(:milestone))
+            update_issue(milestone: create(:milestone, project: project))
           end
 
           should_email(subscriber)

spec/services/merge_requests/build_service_spec.rb

@@ -229,6 +229,15 @@ describe MergeRequests::BuildService do
             end
           end
         end
+
+        context 'when a milestone is from another project' do
+          let(:milestone) { create(:milestone, project: create(:project)) }
+          let(:milestone_id) { milestone.id }
+
+          it 'sets milestone to nil' do
+            expect(merge_request.milestone).to be_nil
+          end
+        end
       end
     end
 

spec/services/merge_requests/update_service_spec.rb

@@ -328,7 +328,7 @@ describe MergeRequests::UpdateService, :mailer do
         it_behaves_like 'system notes for milestones'
 
         it 'sends notifications for subscribers of changed milestone' do
-          merge_request.milestone = create(:milestone)
+          merge_request.milestone = create(:milestone, project: project)
 
           merge_request.save
 
@@ -352,7 +352,7 @@ describe MergeRequests::UpdateService, :mailer do
         end
 
         it 'marks pending todos as done' do
-          update_merge_request({ milestone: create(:milestone) })
+          update_merge_request({ milestone: create(:milestone, project: project) })
 
           expect(pending_todo.reload).to be_done
         end
@@ -361,7 +361,7 @@ describe MergeRequests::UpdateService, :mailer do
 
         it 'sends notifications for subscribers of changed milestone' do
           perform_enqueued_jobs do
-            update_merge_request(milestone: create(:milestone))
+            update_merge_request(milestone: create(:milestone, project: project))
           end
 
           should_email(subscriber)

spec/services/projects/group_links/create_service_spec.rb

@@ -12,6 +12,10 @@ describe Projects::GroupLinks::CreateService, '#execute' do
   end
   let(:subject) { described_class.new(project, user, opts) }
 
+  before do
+    group.add_developer(user)
+  end
+
   it 'adds group to project' do
     expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1)
   end
@@ -19,4 +23,8 @@ describe Projects::GroupLinks::CreateService, '#execute' do
   it 'returns false if group is blank' do
     expect { subject.execute(nil) }.not_to change { project.project_group_links.count }
   end
+
+  it 'returns error if user is not allowed to share with a group' do
+    expect { subject.execute(create :group) }.not_to change { project.project_group_links.count }
+  end
 end

spec/support/helpers/file_mover_helpers.rb

+# frozen_string_literal: true
+
+module FileMoverHelpers
+  def stub_file_mover(file_path, stub_real_path: nil)
+    file_name = File.basename(file_path)
+    allow(Pathname).to receive(:new).and_call_original
+
+    expect_next_instance_of(Pathname, a_string_including(file_name)) do |pathname|
+      allow(pathname).to receive(:realpath) { stub_real_path || pathname.cleanpath }
+    end
+  end
+end

spec/support/helpers/login_helpers.rb

@@ -47,7 +47,7 @@ module LoginHelpers
   end
 
   def gitlab_sign_in_via(provider, user, uid, saml_response = nil)
-    mock_auth_hash(provider, uid, user.email, saml_response)
+    mock_auth_hash_with_saml_xml(provider, uid, user.email, saml_response)
     visit new_user_session_path
     click_link provider
   end
@@ -87,7 +87,12 @@ module LoginHelpers
     click_link "oauth-login-#{provider}"
   end
 
-  def mock_auth_hash(provider, uid, email, saml_response = nil)
+  def mock_auth_hash_with_saml_xml(provider, uid, email, saml_response)
+    response_object = { document: saml_xml(saml_response) }
+    mock_auth_hash(provider, uid, email, response_object: response_object)
+  end
+
+  def mock_auth_hash(provider, uid, email, response_object: nil)
     # The mock_auth configuration allows you to set per-provider (or default)
     # authentication hashes to return during integration testing.
     OmniAuth.config.mock_auth[provider.to_sym] = OmniAuth::AuthHash.new({
@@ -110,9 +115,7 @@ module LoginHelpers
             image: 'mock_user_thumbnail_url'
           }
         },
-        response_object: {
-          document: saml_xml(saml_response)
-        }
+        response_object: response_object
       }
     })
     Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym]

spec/support/shared_examples/issuable_shared_examples.rb

@@ -31,7 +31,7 @@ shared_examples 'system notes for milestones' do
   context 'project milestones' do
     it 'creates a system note' do
       expect do
-        update_issuable(milestone: create(:milestone))
+        update_issuable(milestone: create(:milestone, project: project))
       end.to change { Note.system.count }.by(1)
     end
   end

spec/support/shared_examples/requests/api/discussions.rb

@@ -86,6 +86,37 @@ shared_examples 'discussions API' do |parent_type, noteable_type, id_name|
         expect(response).to have_gitlab_http_status(404)
       end
     end
+
+    context 'when a project is public with private repo access' do
+      let!(:parent) { create(:project, :public, :repository, :repository_private, :snippets_private) }
+      let!(:user_without_access) { create(:user) }
+
+      context 'when user is not a team member of private repo' do
+        before do
+          project.team.truncate
+        end
+
+        context "creating a new note" do
+          before do
+            post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user_without_access), params: { body: 'hi!' }
+          end
+
+          it 'raises 404 error' do
+            expect(response).to have_gitlab_http_status(404)
+          end
+        end
+
+        context "fetching a discussion" do
+          before do
+            get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{note.discussion_id}", user_without_access)
+          end
+
+          it 'raises 404 error' do
+            expect(response).to have_gitlab_http_status(404)
+          end
+        end
+      end
+    end
   end
 
   describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do

spec/uploaders/file_mover_spec.rb

 require 'spec_helper'
 
 describe FileMover do
+  include FileMoverHelpers
+
   let(:filename) { 'banana_sample.gif' }
-  let(:file) { fixture_file_upload(File.join('spec', 'fixtures', filename)) }
   let(:temp_file_path) { File.join('uploads/-/system/temp', 'secret55', filename) }
 
   let(:temp_description) do
@@ -12,7 +13,7 @@ describe FileMover do
   let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, 'secret55', filename) }
   let(:snippet) { create(:personal_snippet, description: temp_description) }
 
-  subject { described_class.new(file_path, snippet).execute }
+  subject { described_class.new(temp_file_path, snippet).execute }
 
   describe '#execute' do
     before do
@@ -20,6 +21,8 @@ describe FileMover do
       expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path))
       allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true)
       allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10)
+
+      stub_file_mover(temp_file_path)
     end
 
     context 'when move and field update successful' do
@@ -66,4 +69,30 @@ describe FileMover do
       end
     end
   end
+
+  context 'security' do
+    context 'when relative path is involved' do
+      let(:temp_file_path) { File.join('uploads/-/system/temp', '..', 'another_subdir_of_temp') }
+
+      it 'does not trigger move if path is outside designated directory' do
+        stub_file_mover('uploads/-/system/another_subdir_of_temp')
+        expect(FileUtils).not_to receive(:move)
+
+        subject
+
+        expect(snippet.reload.description).to eq(temp_description)
+      end
+    end
+
+    context 'when symlink is involved' do
+      it 'does not trigger move if path is outside designated directory' do
+        stub_file_mover(temp_file_path, stub_real_path: Pathname('/etc'))
+        expect(FileUtils).not_to receive(:move)
+
+        subject
+
+        expect(snippet.reload.description).to eq(temp_description)
+      end
+    end
+  end
 end

spec/validators/sha_validator_spec.rb

+require 'spec_helper'
+
+describe ShaValidator do
+  let(:validator) { described_class.new(attributes: [:base_commit_sha]) }
+  let(:merge_diff) { build(:merge_request_diff) }
+
+  subject { validator.validate_each(merge_diff, :base_commit_sha, value) }
+
+  context 'with empty value' do
+    let(:value) { nil }
+
+    it 'does not add any error if value is empty' do
+      subject
+
+      expect(merge_diff.errors).to be_empty
+    end
+  end
+
+  context 'with valid sha' do
+    let(:value) { Digest::SHA1.hexdigest(SecureRandom.hex) }
+
+    it 'does not add any error if value is empty' do
+      subject
+
+      expect(merge_diff.errors).to be_empty
+    end
+  end
+
+  context 'with invalid sha' do
+    let(:value) { 'foo' }
+
+    it 'adds error to the record' do
+      expect(merge_diff.errors).to be_empty
+
+      subject
+
+      expect(merge_diff.errors).not_to be_empty
+    end
+  end
+end

spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb

@@ -18,7 +18,7 @@ describe UpdateHeadPipelineForMergeRequestWorker do
 
       context 'when merge request sha does not equal pipeline sha' do
         before do
-          merge_request.merge_request_diff.update(head_commit_sha: 'different_sha')
+          merge_request.merge_request_diff.update(head_commit_sha: Digest::SHA1.hexdigest(SecureRandom.hex))
         end
 
         it 'does not update head pipeline' do