Merge branch '10-0-stable-prepare-rc4' into '10-0-stable'

Prepare 10.0 RC4 release

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

22 files changed, 279 insertions, 31 deletions

diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss
index 5ffa67a1220..2f7717760ec 100644
--- a/app/assets/stylesheets/framework/nav.scss
+++ b/app/assets/stylesheets/framework/nav.scss
@@ -328,7 +328,7 @@
   border-bottom: 1px solid $border-color;
   transition: padding $sidebar-transition-duration;
   text-align: center;
-  margin-top: $header-height;
+  margin-top: $new-navbar-height;
 
   .container-fluid {
     position: relative;
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 871c76fbad3..ae85fdb720a 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -453,6 +453,10 @@ module Ci
         .fabricate!
     end
 
+    def latest_builds_with_artifacts
+      @latest_builds_with_artifacts ||= builds.latest.with_artifacts
+    end
+
     private
 
     def ci_yaml_from_repo
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 9b05f8b1cd5..44e39e21442 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -6,7 +6,10 @@ class Environment < ActiveRecord::Base
 
   belongs_to :project, required: true, validate: true
 
-  has_many :deployments, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  has_many :deployments,
+    -> (env) { where(project_id: env.project_id) },
+    dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+
   has_one :last_deployment, -> { order('deployments.id DESC') }, class_name: 'Deployment'
 
   before_validation :nullify_external_url
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 4a9a23fea1f..e279d8dd8c5 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -231,6 +231,13 @@ class Namespace < ActiveRecord::Base
   end
 
   def force_share_with_group_lock_on_descendants
-    descendants.update_all(share_with_group_lock: true)
+    return unless Group.supports_nested_groups?
+
+    # We can't use `descendants.update_all` since Rails will throw away the WITH
+    # RECURSIVE statement. We also can't use WHERE EXISTS since we can't use
+    # different table aliases, hence we're just using WHERE IN. Since we have a
+    # maximum of 20 nested groups this should be fine.
+    Namespace.where(id: descendants.select(:id))
+      .update_all(share_with_group_lock: true)
   end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index ff5638dd155..add272cc5ce 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1163,6 +1163,23 @@ class Project < ActiveRecord::Base
     pipelines.order(id: :desc).find_by(sha: sha, ref: ref)
   end
 
+  def latest_successful_pipeline_for_default_branch
+    if defined?(@latest_successful_pipeline_for_default_branch)
+      return @latest_successful_pipeline_for_default_branch
+    end
+
+    @latest_successful_pipeline_for_default_branch =
+      pipelines.latest_successful_for(default_branch)
+  end
+
+  def latest_successful_pipeline_for(ref = nil)
+    if ref && ref != default_branch
+      pipelines.latest_successful_for(ref)
+    else
+      latest_successful_pipeline_for_default_branch
+    end
+  end
+
   def enable_ci
     project_feature.update_attribute(:builds_access_level, ProjectFeature::ENABLED)
   end
diff --git a/app/views/projects/buttons/_download.html.haml b/app/views/projects/buttons/_download.html.haml
index 883922dbf04..9d85e027ac9 100644
--- a/app/views/projects/buttons/_download.html.haml
+++ b/app/views/projects/buttons/_download.html.haml
@@ -1,4 +1,4 @@
-- pipeline = local_assigns.fetch(:pipeline) { project.pipelines.latest_successful_for(ref) }
+- pipeline = local_assigns.fetch(:pipeline) { project.latest_successful_pipeline_for(ref) }
 
 - if !project.empty_repo? && can?(current_user, :download_code, project)
   .project-action-button.dropdown.inline>
@@ -26,18 +26,16 @@
           %i.fa.fa-download
           %span= _('Download tar')
 
-      - if pipeline
-        - artifacts = pipeline.builds.latest.with_artifacts
-        - if artifacts.any?
-          %li.dropdown-header Artifacts
-          - unless pipeline.latest?
-            - latest_pipeline = project.pipeline_for(ref)
-            %li
-              .unclickable= ci_status_for_statuseable(latest_pipeline)
-            %li.dropdown-header Previous Artifacts
-          - artifacts.each do |job|
-            %li
-              = link_to latest_succeeded_project_artifacts_path(project, "#{ref}/download", job: job.name), rel: 'nofollow', download: '' do
-                %i.fa.fa-download
-                %span
-                  #{ s_('DownloadArtifacts|Download') } '#{job.name}'
+      - if pipeline && pipeline.latest_builds_with_artifacts.any?
+        %li.dropdown-header Artifacts
+        - unless pipeline.latest?
+          - latest_pipeline = project.pipeline_for(ref)
+          %li
+            .unclickable= ci_status_for_statuseable(latest_pipeline)
+          %li.dropdown-header Previous Artifacts
+        - pipeline.latest_builds_with_artifacts.each do |job|
+          %li
+            = link_to latest_succeeded_project_artifacts_path(project, "#{ref}/download", job: job.name), rel: 'nofollow', download: '' do
+              %i.fa.fa-download
+              %span
+                #{s_('DownloadArtifacts|Download')} '#{job.name}'
diff --git a/app/views/projects/runners/_form.html.haml b/app/views/projects/runners/_form.html.haml
index ac8e15a48b2..ceae53abfff 100644
--- a/app/views/projects/runners/_form.html.haml
+++ b/app/views/projects/runners/_form.html.haml
@@ -11,7 +11,7 @@
     .col-sm-10
       .checkbox
         = f.check_box :access_level, {}, 'ref_protected', 'not_protected'
-        %span.light This runner will only run on pipelines trigged on protected branches
+        %span.light This runner will only run on pipelines triggered on protected branches
   .form-group
     = label :run_untagged, 'Run untagged jobs', class: 'control-label'
     .col-sm-10
diff --git a/changelogs/unreleased/ci-environment-status-performance.yml b/changelogs/unreleased/ci-environment-status-performance.yml
new file mode 100644
index 00000000000..8812733b5a7
--- /dev/null
+++ b/changelogs/unreleased/ci-environment-status-performance.yml
@@ -0,0 +1,5 @@
+---
+title: Constrain environment deployments to project IDs
+merge_request:
+author:
+type: other
diff --git a/changelogs/unreleased/disallow-null-values-for-environments-project-id.yml b/changelogs/unreleased/disallow-null-values-for-environments-project-id.yml
new file mode 100644
index 00000000000..f4a956e6724
--- /dev/null
+++ b/changelogs/unreleased/disallow-null-values-for-environments-project-id.yml
@@ -0,0 +1,5 @@
+---
+title: "Disallow NULL values for environments.project_id"
+merge_request:
+author:
+type: other
diff --git a/changelogs/unreleased/memoize-the-latest-builds-of-a-pipeline.yml b/changelogs/unreleased/memoize-the-latest-builds-of-a-pipeline.yml
new file mode 100644
index 00000000000..5a7cd42b888
--- /dev/null
+++ b/changelogs/unreleased/memoize-the-latest-builds-of-a-pipeline.yml
@@ -0,0 +1,5 @@
+---
+title: "Memoize the latest builds of a pipeline on a project's homepage"
+merge_request:
+author:
+type: other
diff --git a/changelogs/unreleased/projects-controller-show.yml b/changelogs/unreleased/projects-controller-show.yml
new file mode 100644
index 00000000000..25f4a72710b
--- /dev/null
+++ b/changelogs/unreleased/projects-controller-show.yml
@@ -0,0 +1,5 @@
+---
+title: Memoize pipelines for project download buttons
+merge_request:
+author:
+type: other
diff --git a/db/migrate/20170913131410_environments_project_id_not_null.rb b/db/migrate/20170913131410_environments_project_id_not_null.rb
new file mode 100644
index 00000000000..d5404f8ede9
--- /dev/null
+++ b/db/migrate/20170913131410_environments_project_id_not_null.rb
@@ -0,0 +1,16 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class EnvironmentsProjectIdNotNull < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    change_column_null :environments, :project_id, false
+  end
+
+  def down
+    change_column_null :environments, :project_id, true
+  end
+end
diff --git a/db/migrate/20170914135630_add_index_for_recent_push_events.rb b/db/migrate/20170914135630_add_index_for_recent_push_events.rb
new file mode 100644
index 00000000000..99f593b0465
--- /dev/null
+++ b/db/migrate/20170914135630_add_index_for_recent_push_events.rb
@@ -0,0 +1,40 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddIndexForRecentPushEvents < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index_if_not_present(
+      :merge_requests,
+      [:source_project_id, :source_branch]
+    )
+
+    remove_concurrent_index_if_present(:merge_requests, :source_project_id)
+  end
+
+  def down
+    add_concurrent_index_if_not_present(:merge_requests, :source_project_id)
+
+    remove_concurrent_index_if_present(
+      :merge_requests,
+      [:source_project_id, :source_branch]
+    )
+  end
+
+  def add_concurrent_index_if_not_present(table, columns)
+    return if index_exists?(table, columns)
+
+    add_concurrent_index(table, columns)
+  end
+
+  def remove_concurrent_index_if_present(table, columns)
+    return unless index_exists?(table, columns)
+
+    remove_concurrent_index(table, columns)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 2149f5ad23d..2d8c33591f0 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170905112933) do
+ActiveRecord::Schema.define(version: 20170914135630) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -520,7 +520,7 @@ ActiveRecord::Schema.define(version: 20170905112933) do
   add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree
 
   create_table "environments", force: :cascade do |t|
-    t.integer "project_id"
+    t.integer "project_id", null: false
     t.string "name", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
@@ -892,7 +892,7 @@ ActiveRecord::Schema.define(version: 20170905112933) do
   add_index "merge_requests", ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id", using: :btree
   add_index "merge_requests", ["milestone_id"], name: "index_merge_requests_on_milestone_id", using: :btree
   add_index "merge_requests", ["source_branch"], name: "index_merge_requests_on_source_branch", using: :btree
-  add_index "merge_requests", ["source_project_id"], name: "index_merge_requests_on_source_project_id", using: :btree
+  add_index "merge_requests", ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_id_and_source_branch", using: :btree
   add_index "merge_requests", ["target_branch"], name: "index_merge_requests_on_target_branch", using: :btree
   add_index "merge_requests", ["target_project_id", "iid"], name: "index_merge_requests_on_target_project_id_and_iid", unique: true, using: :btree
   add_index "merge_requests", ["title"], name: "index_merge_requests_on_title", using: :btree
diff --git a/doc/administration/reply_by_email.md b/doc/administration/reply_by_email.md
index e99a7ee29cc..1304476e678 100644
--- a/doc/administration/reply_by_email.md
+++ b/doc/administration/reply_by_email.md
@@ -77,6 +77,33 @@ and use [an application password](https://support.google.com/mail/answer/185833)
 To set up a basic Postfix mail server with IMAP access on Ubuntu, follow the
 [Postfix setup documentation](reply_by_email_postfix_setup.md).
 
+### Security Concerns
+
+**WARNING:** Be careful when choosing the domain used for receiving incoming
+email.
+
+For the sake of example, suppose your top-level company domain is `hooli.com`.
+All employees in your company have an email address at that domain via Google
+Apps, and your company's private Slack instance requires a valid `@hooli.com`
+email address in order to sign up.
+
+If you also host a public-facing GitLab instance at `hooli.com` and set your
+incoming email domain to `hooli.com`, an attacker could abuse the "Create new
+issue by email" feature by using a project's unique address as the email when
+signing up for Slack, which would send a confirmation email, which would create
+a new issue on the project owned by the attacker, allowing them to click the
+confirmation link and validate their account on your company's private Slack
+instance.
+
+We recommend receiving incoming email on a subdomain, such as
+`incoming.hooli.com`, and ensuring that you do not employ any services that
+authenticate solely based on access to an email domain such as `*.hooli.com.`
+Alternatively, use a dedicated domain for GitLab email communications such as
+`hooli-gitlab.com`.
+
+See GitLab issue [#30366](https://gitlab.com/gitlab-org/gitlab-ce/issues/30366)
+for a real-world example of this exploit.
+
 ### Omnibus package installations
 
 1. Find the `incoming_email` section in `/etc/gitlab/gitlab.rb`, enable the
@@ -141,7 +168,7 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow the
     # The IDLE command timeout.
     gitlab_rails['incoming_email_idle_timeout'] = 60
     ```
-    
+
     ```ruby
     # Configuration for Microsoft Exchange mail server w/ IMAP enabled, assumes mailbox incoming@exchange.example.com
     gitlab_rails['incoming_email_enabled'] = true
@@ -253,7 +280,7 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow the
       # The IDLE command timeout.
       idle_timeout: 60
     ```
-    
+
     ```yaml
     # Configuration for Microsoft Exchange mail server w/ IMAP enabled, assumes mailbox incoming@exchange.example.com
     incoming_email:
diff --git a/doc/user/search/index.md b/doc/user/search/index.md
index bcc3625f908..2b23c494dc4 100644
--- a/doc/user/search/index.md
+++ b/doc/user/search/index.md
@@ -31,8 +31,8 @@ on the search field on the top-right of your screen:
 
 If you want to search for issues present in a specific project, navigate to
 a project's **Issues** tab, and click on the field **Search or filter results...**. It will
-display a dropdown menu, from which you can add filters per author, assignee, milestone, label,
-and weight. When done, press **Enter** on your keyboard to filter the issues.
+display a dropdown menu, from which you can add filters per author, assignee, milestone, 
+label, weight, and 'my-reaction' (based on your emoji votes). When done, press **Enter** on your keyboard to filter the issues.
 
 ![filter issues in a project](img/issue_search_filter.png)
 
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 1825c90a23b..bdebda58d3f 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -88,7 +88,7 @@ module API
         user = User.find_by(id: params[:id])
         not_found!('User') unless user && can?(current_user, :read_user, user)
 
-        opts = current_user&.admin? ? { with: Entities::UserWithAdmin } : {}
+        opts = current_user&.admin? ? { with: Entities::UserWithAdmin } : { with: Entities::User }
         present user, opts
       end
 
diff --git a/lib/gitlab/database/read_only_relation.rb b/lib/gitlab/database/read_only_relation.rb
new file mode 100644
index 00000000000..4571ad122ce
--- /dev/null
+++ b/lib/gitlab/database/read_only_relation.rb
@@ -0,0 +1,16 @@
+module Gitlab
+  module Database
+    # Module that can be injected into a ActiveRecord::Relation to make it
+    # read-only.
+    module ReadOnlyRelation
+      [:delete, :delete_all, :update, :update_all].each do |method|
+        define_method(method) do |*args|
+          raise(
+            ActiveRecord::ReadOnlyRecord,
+            "This relation is marked as read-only"
+          )
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/group_hierarchy.rb b/lib/gitlab/group_hierarchy.rb
index 5a31e56cb30..635f52131f9 100644
--- a/lib/gitlab/group_hierarchy.rb
+++ b/lib/gitlab/group_hierarchy.rb
@@ -22,7 +22,7 @@ module Gitlab
     def base_and_ancestors
       return ancestors_base unless Group.supports_nested_groups?
 
-      base_and_ancestors_cte.apply_to(model.all)
+      read_only(base_and_ancestors_cte.apply_to(model.all))
     end
 
     # Returns a relation that includes the descendants_base set of groups
@@ -30,7 +30,7 @@ module Gitlab
     def base_and_descendants
       return descendants_base unless Group.supports_nested_groups?
 
-      base_and_descendants_cte.apply_to(model.all)
+      read_only(base_and_descendants_cte.apply_to(model.all))
     end
 
     # Returns a relation that includes the base groups, their ancestors,
@@ -67,11 +67,13 @@ module Gitlab
       union = SQL::Union.new([model.unscoped.from(ancestors_table),
                               model.unscoped.from(descendants_table)])
 
-      model
+      relation = model
         .unscoped
         .with
         .recursive(ancestors.to_arel, descendants.to_arel)
         .from("(#{union.to_sql}) #{model.table_name}")
+
+      read_only(relation)
     end
 
     private
@@ -107,5 +109,12 @@ module Gitlab
     def groups_table
       model.arel_table
     end
+
+    def read_only(relation)
+      # relations using a CTE are not safe to use with update_all as it will
+      # throw away the CTE, hence we mark them as read-only.
+      relation.extend(Gitlab::Database::ReadOnlyRelation)
+      relation
+    end
   end
 end
diff --git a/spec/lib/gitlab/group_hierarchy_spec.rb b/spec/lib/gitlab/group_hierarchy_spec.rb
index 08010c2d0e2..8dc83a6db7f 100644
--- a/spec/lib/gitlab/group_hierarchy_spec.rb
+++ b/spec/lib/gitlab/group_hierarchy_spec.rb
@@ -23,6 +23,11 @@ describe Gitlab::GroupHierarchy, :postgresql do
 
       expect(relation).to include(parent, child1, child2)
     end
+
+    it 'does not allow the use of #update_all' do
+      expect { relation.update_all(share_with_group_lock: false) }
+        .to raise_error(ActiveRecord::ReadOnlyRecord)
+    end
   end
 
   describe '#base_and_descendants' do
@@ -43,6 +48,11 @@ describe Gitlab::GroupHierarchy, :postgresql do
 
       expect(relation).to include(parent, child1, child2)
     end
+
+    it 'does not allow the use of #update_all' do
+      expect { relation.update_all(share_with_group_lock: false) }
+        .to raise_error(ActiveRecord::ReadOnlyRecord)
+    end
   end
 
   describe '#all_groups' do
@@ -73,5 +83,10 @@ describe Gitlab::GroupHierarchy, :postgresql do
 
       expect(relation).to include(child2)
     end
+
+    it 'does not allow the use of #update_all' do
+      expect { relation.update_all(share_with_group_lock: false) }
+        .to raise_error(ActiveRecord::ReadOnlyRecord)
+    end
   end
 end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 95da97b7bc5..77f0be6b120 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1439,4 +1439,24 @@ describe Ci::Pipeline, :mailer do
       it_behaves_like 'not sending any notification'
     end
   end
+
+  describe '#latest_builds_with_artifacts' do
+    let!(:pipeline) { create(:ci_pipeline, :success) }
+
+    let!(:build) do
+      create(:ci_build, :success, :artifacts, pipeline: pipeline)
+    end
+
+    it 'returns the latest builds' do
+      expect(pipeline.latest_builds_with_artifacts).to eq([build])
+    end
+
+    it 'memoizes the returned relation' do
+      query_count = ActiveRecord::QueryRecorder
+        .new { 2.times { pipeline.latest_builds_with_artifacts.to_a } }
+        .count
+
+      expect(query_count).to eq(1)
+    end
+  end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 48fc77423ff..78226c6c3fa 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2682,4 +2682,60 @@ describe Project do
       end
     end
   end
+
+  describe '#latest_successful_builds_for' do
+    let(:project) { build(:project) }
+
+    before do
+      allow(project).to receive(:default_branch).and_return('master')
+    end
+
+    context 'without a ref' do
+      it 'returns a pipeline for the default branch' do
+        expect(project)
+          .to receive(:latest_successful_pipeline_for_default_branch)
+
+        project.latest_successful_pipeline_for
+      end
+    end
+
+    context 'with the ref set to the default branch' do
+      it 'returns a pipeline for the default branch' do
+        expect(project)
+          .to receive(:latest_successful_pipeline_for_default_branch)
+
+        project.latest_successful_pipeline_for(project.default_branch)
+      end
+    end
+
+    context 'with a ref that is not the default branch' do
+      it 'returns the latest successful pipeline for the given ref' do
+        expect(project.pipelines).to receive(:latest_successful_for).with('foo')
+
+        project.latest_successful_pipeline_for('foo')
+      end
+    end
+  end
+
+  describe '#latest_successful_pipeline_for_default_branch' do
+    let(:project) { build(:project) }
+
+    before do
+      allow(project).to receive(:default_branch).and_return('master')
+    end
+
+    it 'memoizes and returns the latest successful pipeline for the default branch' do
+      pipeline = double(:pipeline)
+
+      expect(project.pipelines).to receive(:latest_successful_for)
+        .with(project.default_branch)
+        .and_return(pipeline)
+        .once
+
+      2.times do
+        expect(project.latest_successful_pipeline_for_default_branch)
+          .to eq(pipeline)
+      end
+    end
+  end
 end