Merge branch 'merge-dev-to-master' into 'master'

Merge dev.gitlab.org master into GitLab.com master

Closes #2794, #2814, #2806, #2805, #2798, #2795, #2788, and #2787

See merge request gitlab-org/gitlab-ce!25746

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,7 +53,6 @@ class GroupPolicy < BasePolicy
   rule { admin }.enable :read_group
 
   rule { has_projects }.policy do
-    enable :read_group
     enable :read_list
     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

+# frozen_string_literal: true
+
+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 }