Merge branch 'master' into fix-git-hooks-when-creating-file

* master: (1031 commits)
  Add changelog entry for renaming API param [ci skip]
  Add missing milestone parameter
  Refactor issues filter in API
  Fix project hooks params
  Gitlab::LDAP::Person uses LDAP attributes configuration
  Don't delete files from spec/fixtures
  Copy, don't move uploaded avatar files
  Minor improvements to changelog docs
  Rename logo, apply for Slack too
  Fix Gemfile.lock for the octokit update
  Fix cross-project references copy to include the project reference
  Add logo in public files
  Use stable icon for Mattermost integration
  rewrite the item.respond_to?(:x?) && item.x? to item.try(:x?)
  API: extern_uid is a string
  Increases pipeline graph drowdown width in order to prevent strange position on chrome on ubuntu
  Removed bottom padding from merge manually from CLI because of repositioning award emoji's
  Make haml_lint happy
  Improve spec
  Add feature tests for Cycle Analytics
  ...

20 files changed, 251 insertions, 105 deletions

diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb
new file mode 100644
index 00000000000..ddaaed90e5b
--- /dev/null
+++ b/app/services/access_token_validation_service.rb
@@ -0,0 +1,32 @@
+AccessTokenValidationService = Struct.new(:token) do
+  # Results:
+  VALID = :valid
+  EXPIRED = :expired
+  REVOKED = :revoked
+  INSUFFICIENT_SCOPE = :insufficient_scope
+
+  def validate(scopes: [])
+    if token.expired?
+      return EXPIRED
+
+    elsif token.revoked?
+      return REVOKED
+
+    elsif !self.include_any_scope?(scopes)
+      return INSUFFICIENT_SCOPE
+
+    else
+      return VALID
+    end
+  end
+
+  # True if the token's scope contains any of the passed scopes.
+  def include_any_scope?(scopes)
+    if scopes.blank?
+      true
+    else
+      # Check whether the token is allowed access to any of the required scopes.
+      Set.new(scopes).intersection(Set.new(token.scopes)).present?
+    end
+  end
+end
diff --git a/app/services/ci/create_pipeline_builds_service.rb b/app/services/ci/create_pipeline_builds_service.rb
index 005014fa1de..b7da3f8e7eb 100644
--- a/app/services/ci/create_pipeline_builds_service.rb
+++ b/app/services/ci/create_pipeline_builds_service.rb
@@ -10,18 +10,29 @@ module Ci
       end
     end
 
+    def project
+      pipeline.project
+    end
+
     private
 
     def create_build(build_attributes)
       build_attributes = build_attributes.merge(
         pipeline: pipeline,
-        project: pipeline.project,
+        project: project,
         ref: pipeline.ref,
         tag: pipeline.tag,
         user: current_user,
         trigger_request: trigger_request
       )
-      pipeline.builds.create(build_attributes)
+      build = pipeline.builds.create(build_attributes)
+
+      # Create the environment before the build starts. This sets its slug and
+      # makes it available as an environment variable
+      project.environments.find_or_create_by(name: build.expanded_environment_name) if
+        build.has_environment?
+
+      build
     end
 
     def new_builds
diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb
index 75d847d5bee..240ddabec36 100644
--- a/app/services/ci/image_for_build_service.rb
+++ b/app/services/ci/image_for_build_service.rb
@@ -1,13 +1,13 @@
 module Ci
   class ImageForBuildService
     def execute(project, opts)
-      sha = opts[:sha] || ref_sha(project, opts[:ref])
-
+      ref = opts[:ref]
+      sha = opts[:sha] || ref_sha(project, ref)
       pipelines = project.pipelines.where(sha: sha)
-      pipelines = pipelines.where(ref: opts[:ref]) if opts[:ref]
-      image_name = image_for_status(pipelines.status)
 
+      image_name = image_for_status(pipelines.latest_status(ref))
       image_path = Rails.root.join('public/ci', image_name)
+
       OpenStruct.new(path: image_path, name: image_name)
     end
 
diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index 9c630f5bbf1..9b241aa8b04 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -43,7 +43,7 @@ module Commits
         success
       else
         error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically.
-                     It may have already been #{action.to_s.dasherize}, or a more recent commit may have updated some of its content."
+                     A #{action.to_s.dasherize} may have already been performed with this #{@commit.change_type_title(current_user)}, or a more recent commit may have updated some of its content."
         raise ChangeError, error_msg
       end
     end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 185556c12cc..dbe2fda27b5 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -3,6 +3,9 @@ class GitPushService < BaseService
   include Gitlab::CurrentSettings
   include Gitlab::Access
 
+  # The N most recent commits to process in a single push payload.
+  PROCESS_COMMIT_LIMIT = 100
+
   # This method will be called after each git update
   # and only if the provided user and project are present in GitLab.
   #
@@ -74,7 +77,17 @@ class GitPushService < BaseService
       types = []
     end
 
-    ProjectCacheWorker.perform_async(@project.id, types)
+    ProjectCacheWorker.perform_async(@project.id, types, [:commit_count, :repository_size])
+  end
+
+  # Schedules processing of commit messages.
+  def process_commit_messages
+    default = is_default_branch?
+
+    push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit|
+      ProcessCommitWorker.
+        perform_async(project.id, current_user.id, commit.to_hash, default)
+    end
   end
 
   protected
@@ -128,17 +141,6 @@ class GitPushService < BaseService
     end
   end
 
-  # Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched,
-  # close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables.
-  def process_commit_messages
-    default = is_default_branch?
-
-    @push_commits.each do |commit|
-      ProcessCommitWorker.
-        perform_async(project.id, current_user.id, commit.to_hash, default)
-    end
-  end
-
   def build_push_data
     @push_data ||= Gitlab::DataBuilder::Push.build(
       @project,
diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb
index 20a4445bddf..96432837481 100644
--- a/app/services/git_tag_push_service.rb
+++ b/app/services/git_tag_push_service.rb
@@ -12,7 +12,7 @@ class GitTagPushService < BaseService
     project.execute_hooks(@push_data.dup, :tag_push_hooks)
     project.execute_services(@push_data.dup, :tag_push_hooks)
     Ci::CreatePipelineService.new(project, current_user, @push_data).execute
-    ProjectCacheWorker.perform_async(project.id)
+    ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size])
 
     true
   end
diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb
index 2bccd584dde..febeb661fb5 100644
--- a/app/services/groups/create_service.rb
+++ b/app/services/groups/create_service.rb
@@ -12,6 +12,13 @@ module Groups
         return @group
       end
 
+      if @group.parent && !can?(current_user, :admin_group, @group.parent)
+        @group.parent = nil
+        @group.errors.add(:parent_id, 'manage access required to create subgroup')
+
+        return @group
+      end
+
       @group.name ||= @group.path.dup
       @group.save
       @group.add_owner(current_user)
diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb
index 99ad12b1003..4e878ec556a 100644
--- a/app/services/groups/update_service.rb
+++ b/app/services/groups/update_service.rb
@@ -5,7 +5,7 @@ module Groups
       new_visibility = params[:visibility_level]
       if new_visibility && new_visibility.to_i != group.visibility_level
         unless can?(current_user, :change_visibility_level, group) &&
-          Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
+            Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
 
           deny_visibility_level(group, new_visibility)
           return group
@@ -14,7 +14,13 @@ module Groups
 
       group.assign_attributes(params)
 
-      group.save
+      begin
+        group.save
+      rescue Gitlab::UpdatePathError => e
+        group.errors.add(:base, e.message)
+
+        false
+      end
     end
   end
 end
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index b5f63cc5a1a..4ce5fd993d9 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -36,14 +36,10 @@ class IssuableBaseService < BaseService
     end
   end
 
-  def filter_params(issuable_ability_name = :issue)
-    filter_assignee
-    filter_milestone
-    filter_labels
+  def filter_params(issuable)
+    ability_name = :"admin_#{issuable.to_ability_name}"
 
-    ability = :"admin_#{issuable_ability_name}"
-
-    unless can?(current_user, ability, project)
+    unless can?(current_user, ability_name, project)
       params.delete(:milestone_id)
       params.delete(:labels)
       params.delete(:add_label_ids)
@@ -52,14 +48,35 @@ class IssuableBaseService < BaseService
       params.delete(:assignee_id)
       params.delete(:due_date)
     end
+
+    filter_assignee(issuable)
+    filter_milestone
+    filter_labels
   end
 
-  def filter_assignee
-    if params[:assignee_id] == IssuableFinder::NONE
-      params[:assignee_id] = ''
+  def filter_assignee(issuable)
+    return unless params[:assignee_id].present?
+
+    assignee_id = params[:assignee_id]
+
+    if assignee_id.to_s == IssuableFinder::NONE
+      params[:assignee_id] = ""
+    else
+      params.delete(:assignee_id) unless assignee_can_read?(issuable, assignee_id)
     end
   end
 
+  def assignee_can_read?(issuable, assignee_id)
+    new_assignee = User.find_by_id(assignee_id)
+
+    return false unless new_assignee.present?
+
+    ability_name = :"read_#{issuable.to_ability_name}"
+    resource     = issuable.persisted? ? issuable : project
+
+    can?(new_assignee, ability_name, resource)
+  end
+
   def filter_milestone
     milestone_id = params[:milestone_id]
     return unless milestone_id
@@ -138,7 +155,7 @@ class IssuableBaseService < BaseService
 
   def create(issuable)
     merge_slash_commands_into_params!(issuable)
-    filter_params
+    filter_params(issuable)
 
     params.delete(:state_event)
     params[:author] ||= current_user
@@ -180,11 +197,12 @@ class IssuableBaseService < BaseService
     change_state(issuable)
     change_subscription(issuable)
     change_todo(issuable)
-    filter_params
+    filter_params(issuable)
     old_labels = issuable.labels.to_a
     old_mentioned_users = issuable.mentioned_users.to_a
 
-    params[:label_ids] = process_label_ids(params, existing_label_ids: issuable.label_ids)
+    label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids)
+    params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids)
 
     if params.present? && update_issuable(issuable, params)
       # We do not touch as it will affect a update on updated_at field
@@ -201,6 +219,10 @@ class IssuableBaseService < BaseService
     issuable
   end
 
+  def labels_changing?(old_label_ids, new_label_ids)
+    old_label_ids.sort != new_label_ids.sort
+  end
+
   def change_state(issuable)
     case params.delete(:state_event)
     when 'reopen'
diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb
index 742e834df97..35af867a098 100644
--- a/app/services/issues/base_service.rb
+++ b/app/services/issues/base_service.rb
@@ -17,10 +17,6 @@ module Issues
 
     private
 
-    def filter_params
-      super(:issue)
-    end
-
     def execute_hooks(issue, action = 'open')
       issue_data  = hook_data(issue, action)
       hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index a2111b3806b..78cbf94ec69 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -10,7 +10,7 @@ module Issues
       end
 
       if issue.previous_changes.include?('title') ||
-         issue.previous_changes.include?('description')
+          issue.previous_changes.include?('description')
         todo_service.update_issue(issue, current_user)
       end
 
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 800fd39c424..70e25956dc7 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -38,10 +38,6 @@ module MergeRequests
 
     private
 
-    def filter_params
-      super(:merge_request)
-    end
-
     def merge_requests_for(branch)
       origin_merge_requests = @project.origin_merge_requests
         .opened.where(source_branch: branch).to_a
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index a52a94c5ffa..548c7b9baf4 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -43,7 +43,7 @@ module MergeRequests
       end
 
       if merge_request.source_project == merge_request.target_project &&
-         merge_request.target_branch == merge_request.source_branch
+          merge_request.target_branch == merge_request.source_branch
 
         messages << 'You must select different branches'
       end
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index fda0da19d87..ad16ef8c70f 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -25,7 +25,7 @@ module MergeRequests
       end
 
       if merge_request.previous_changes.include?('title') ||
-         merge_request.previous_changes.include?('description')
+          merge_request.previous_changes.include?('description')
         todo_service.update_merge_request(merge_request, current_user)
       end
 
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index d75592e31f3..1beca9f4109 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -41,7 +41,7 @@ module Notes
         # We must add the error after we call #save because errors are reset
         # when #save is called
         if only_commands
-          note.errors.add(:commands_only, 'Your commands have been executed!')
+          note.errors.add(:commands_only, 'Commands applied')
         end
 
         note.commands_changes = command_params.keys
diff --git a/app/services/oauth2/access_token_validation_service.rb b/app/services/oauth2/access_token_validation_service.rb
deleted file mode 100644
index 264fdccde8f..00000000000
--- a/app/services/oauth2/access_token_validation_service.rb
+++ /dev/null
@@ -1,42 +0,0 @@
-module Oauth2::AccessTokenValidationService
-  # Results:
-  VALID = :valid
-  EXPIRED = :expired
-  REVOKED = :revoked
-  INSUFFICIENT_SCOPE = :insufficient_scope
-
-  class << self
-    def validate(token, scopes: [])
-      if token.expired?
-        return EXPIRED
-
-      elsif token.revoked?
-        return REVOKED
-
-      elsif !self.sufficient_scope?(token, scopes)
-        return INSUFFICIENT_SCOPE
-
-      else
-        return VALID
-      end
-    end
-
-    protected
-
-    # True if the token's scope is a superset of required scopes,
-    # or the required scopes is empty.
-    def sufficient_scope?(token, scopes)
-      if scopes.blank?
-        # if no any scopes required, the scopes of token is sufficient.
-        return true
-      else
-        # If there are scopes required, then check whether
-        # the set of authorized scopes is a superset of the set of required scopes
-        required_scopes = Set.new(scopes)
-        authorized_scopes = Set.new(token.scopes)
-
-        return authorized_scopes >= required_scopes
-      end
-    end
-  end
-end
diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb
index d7221fe993c..cd230528743 100644
--- a/app/services/projects/import_service.rb
+++ b/app/services/projects/import_service.rb
@@ -4,15 +4,6 @@ module Projects
 
     class Error < StandardError; end
 
-    ALLOWED_TYPES = [
-      'bitbucket',
-      'fogbugz',
-      'gitlab',
-      'github',
-      'google_code',
-      'gitlab_project'
-    ]
-
     def execute
       add_repository_to_project unless project.gitlab_project_import?
 
@@ -64,14 +55,11 @@ module Projects
     end
 
     def has_importer?
-      ALLOWED_TYPES.include?(project.import_type)
+      Gitlab::ImportSources.importer_names.include?(project.import_type)
     end
 
     def importer
-      return Gitlab::ImportExport::Importer.new(project) if @project.gitlab_project_import?
-
-      class_name = "Gitlab::#{project.import_type.camelize}Import::Importer"
-      class_name.constantize.new(project)
+      Gitlab::ImportSources.importer(project.import_type).new(project)
     end
 
     def unknown_url?
diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb
index 921ca6748d3..8a6af8d8ada 100644
--- a/app/services/projects/update_service.rb
+++ b/app/services/projects/update_service.rb
@@ -6,7 +6,7 @@ module Projects
 
       if new_visibility && new_visibility.to_i != project.visibility_level
         unless can?(current_user, :change_visibility_level, project) &&
-          Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
+            Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
 
           deny_visibility_level(project, new_visibility)
           return project
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 8b48d90f60b..7613ecd5021 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -146,7 +146,7 @@ module SystemNoteService
   end
 
   def remove_merge_request_wip(noteable, project, author)
-    body = 'unmarked as a Work In Progress'
+    body = 'unmarked as a **Work In Progress**'
 
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb
new file mode 100644
index 00000000000..8559908e0c3
--- /dev/null
+++ b/app/services/users/refresh_authorized_projects_service.rb
@@ -0,0 +1,128 @@
+module Users
+  # Service for refreshing the authorized projects of a user.
+  #
+  # This particular service class can not be used to update data for the same
+  # user concurrently. Doing so could lead to an incorrect state. To ensure this
+  # doesn't happen a caller must synchronize access (e.g. using
+  # `Gitlab::ExclusiveLease`).
+  #
+  # Usage:
+  #
+  #     user = User.find_by(username: 'alice')
+  #     service = Users::RefreshAuthorizedProjectsService.new(some_user)
+  #     service.execute
+  class RefreshAuthorizedProjectsService
+    attr_reader :user
+
+    LEASE_TIMEOUT = 1.minute.to_i
+
+    # user - The User for which to refresh the authorized projects.
+    def initialize(user)
+      @user = user
+
+      # We need an up to date User object that has access to all relations that
+      # may have been created earlier. The only way to ensure this is to reload
+      # the User object.
+      user.reload
+    end
+
+    # This method returns the updated User object.
+    def execute
+      current = current_authorizations_per_project
+      fresh = fresh_access_levels_per_project
+
+      remove = current.each_with_object([]) do |(project_id, row), array|
+        # rows not in the new list or with a different access level should be
+        # removed.
+        if !fresh[project_id] || fresh[project_id] != row.access_level
+          array << row.id
+        end
+      end
+
+      add = fresh.each_with_object([]) do |(project_id, level), array|
+        # rows not in the old list or with a different access level should be
+        # added.
+        if !current[project_id] || current[project_id].access_level != level
+          array << [user.id, project_id, level]
+        end
+      end
+
+      update_with_lease(remove, add)
+    end
+
+    # Updates the list of authorizations using an exclusive lease.
+    def update_with_lease(remove = [], add = [])
+      lease_key = "refresh_authorized_projects:#{user.id}"
+      lease = Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT)
+
+      until uuid = lease.try_obtain
+        # Keep trying until we obtain the lease. If we don't do so we may end up
+        # not updating the list of authorized projects properly. To prevent
+        # hammering Redis too much we'll wait for a bit between retries.
+        sleep(1)
+      end
+
+      begin
+        update_authorizations(remove, add)
+      ensure
+        Gitlab::ExclusiveLease.cancel(lease_key, uuid)
+      end
+    end
+
+    # Updates the list of authorizations for the current user.
+    #
+    # remove - The IDs of the authorization rows to remove.
+    # add - Rows to insert in the form `[user id, project id, access level]`
+    def update_authorizations(remove = [], add = [])
+      return if remove.empty? && add.empty? && user.authorized_projects_populated
+
+      User.transaction do
+        user.remove_project_authorizations(remove) unless remove.empty?
+        ProjectAuthorization.insert_authorizations(add) unless add.empty?
+        user.set_authorized_projects_column
+      end
+
+      # Since we batch insert authorization rows, Rails' associations may get
+      # out of sync. As such we force a reload of the User object.
+      user.reload
+    end
+
+    def fresh_access_levels_per_project
+      fresh_authorizations.each_with_object({}) do |row, hash|
+        hash[row.project_id] = row.access_level
+      end
+    end
+
+    def current_authorizations_per_project
+      current_authorizations.each_with_object({}) do |row, hash|
+        hash[row.project_id] = row
+      end
+    end
+
+    def current_authorizations
+      user.project_authorizations.select(:id, :project_id, :access_level)
+    end
+
+    def fresh_authorizations
+      ProjectAuthorization.
+        unscoped.
+        select('project_id, MAX(access_level) AS access_level').
+        from("(#{project_authorizations_union.to_sql}) #{ProjectAuthorization.table_name}").
+        group(:project_id)
+    end
+
+    private
+
+    # Returns a union query of projects that the user is authorized to access
+    def project_authorizations_union
+      relations = [
+        user.personal_projects.select("#{user.id} AS user_id, projects.id AS project_id, #{Gitlab::Access::MASTER} AS access_level"),
+        user.groups_projects.select_for_project_authorization,
+        user.projects.select_for_project_authorization,
+        user.groups.joins(:shared_projects).select_for_project_authorization
+      ]
+
+      Gitlab::SQL::Union.new(relations)
+    end
+  end
+end