Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce

27 files changed, 160 insertions, 72 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e1252d4b947..70846be1c18 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -21,6 +21,7 @@ v 8.8.0 (unreleased)
   - Bump mail_room to 0.7.0 to fix stuck IDLE connections
   - Remove future dates from contribution calendar graph.
   - Support e-mail notifications for comments on project snippets
+  - Fix API leak of notes of unauthorized issues, snippets and merge requests
   - Use ActionDispatch Remote IP for Akismet checking
   - Fix error when visiting commit builds page before build was updated
   - Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project
@@ -38,6 +39,7 @@ v 8.8.0 (unreleased)
   - Create tags using Rugged for performance reasons. !3745
   - API: Expose Issue#user_notes_count. !3126 (Anton Popov)
   - Don't show forks button when user can't view forks
+  - Fix atom feed links and rendering
   - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718
   - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
   - Add eager load paths to help prevent dependency load issues in Sidekiq workers. !3724
@@ -57,9 +59,13 @@ v 8.8.0 (unreleased)
   - Redesign navigation for profile and group pages
   - Add counter metrics for rails cache
   - Import pull requests from GitHub where the source or target branches were removed
+  - All Grape API helpers are now instrumented
+  - Improve Issue formatting for the Slack Service (Jeroen van Baarsen)
 
 v 8.7.6
   - Fix links on wiki pages for relative url setups. !4131 (Artem Sidorenko)
+  - Fix import from GitLab.com to a private instance failure. !4181
+  - Fix external imports not finding the import data. !4106
 
 v 8.7.5
   - Fix relative links in wiki pages. !4050
@@ -889,7 +895,7 @@ v 8.1.3
   - Use issue editor as cross reference comment author when issue is edited with a new mention
   - Add Facebook authentication
 
-v 8.1.2
+v 8.1.1
   - Fix cloning Wiki repositories via HTTP (Stan Hu)
   - Add migration to remove satellites directory
   - Fix specific runners visibility
diff --git a/app/assets/images/ci/arch.jpg b/app/assets/images/ci/arch.jpg
deleted file mode 100644
index 0e05674e840..00000000000
--- a/app/assets/images/ci/arch.jpg
+++ /dev/null
diff --git a/app/assets/images/ci/favicon.ico b/app/assets/images/ci/favicon.ico
deleted file mode 100644
index 9663d4d00b9..00000000000
--- a/app/assets/images/ci/favicon.ico
+++ /dev/null
diff --git a/app/assets/images/ci/loader.gif b/app/assets/images/ci/loader.gif
deleted file mode 100644
index 2fcb8f2da0d..00000000000
--- a/app/assets/images/ci/loader.gif
+++ /dev/null
diff --git a/app/assets/images/ci/no_avatar.png b/app/assets/images/ci/no_avatar.png
deleted file mode 100644
index 752d26adba7..00000000000
--- a/app/assets/images/ci/no_avatar.png
+++ /dev/null
diff --git a/app/assets/images/ci/rails.png b/app/assets/images/ci/rails.png
deleted file mode 100644
index d5edc04e65f..00000000000
--- a/app/assets/images/ci/rails.png
+++ /dev/null
diff --git a/app/assets/images/ci/service_sample.png b/app/assets/images/ci/service_sample.png
deleted file mode 100644
index 65d29e3fd89..00000000000
--- a/app/assets/images/ci/service_sample.png
+++ /dev/null
diff --git a/app/assets/stylesheets/framework/zen.scss b/app/assets/stylesheets/framework/zen.scss
index f870ea0d87f..ff02ebdd34c 100644
--- a/app/assets/stylesheets/framework/zen.scss
+++ b/app/assets/stylesheets/framework/zen.scss
@@ -32,7 +32,7 @@
   }
 }
 
-.zen-cotrol {
+.zen-control {
   padding: 0;
   color: #555;
   background: none;
diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss
index c20f04653fc..a3690e40e28 100644
--- a/app/assets/stylesheets/pages/projects.scss
+++ b/app/assets/stylesheets/pages/projects.scss
@@ -7,7 +7,7 @@
 }
 .no-ssh-key-message, .project-limit-message {
   background-color: #f28d35;
-  margin-bottom: 16px;
+  margin-bottom: 0;
 }
 .new_project,
 .edit_project {
diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb
index 7756f0f0ed3..a1b84afcd91 100644
--- a/app/controllers/projects/imports_controller.rb
+++ b/app/controllers/projects/imports_controller.rb
@@ -20,6 +20,7 @@ class Projects::ImportsController < Projects::ApplicationController
         @project.import_retry
       else
         @project.import_start
+        @project.add_import_job
       end
     end
 
diff --git a/app/models/project.rb b/app/models/project.rb
index a3c4f1d8e9b..b7ae34c95c6 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -204,7 +204,6 @@ class Project < ActiveRecord::Base
     state :finished
     state :failed
 
-    after_transition any => :started, do: :schedule_add_import_job
     after_transition any => :finished, do: :clear_import_data
   end
 
@@ -349,10 +348,6 @@ class Project < ActiveRecord::Base
     id && persisted?
   end
 
-  def schedule_add_import_job
-    run_after_commit(:add_import_job)
-  end
-
   def add_import_job
     if forked?
       job_id = RepositoryForkWorker.perform_async(self.id, forked_from_project.path_with_namespace, self.namespace.path)
diff --git a/app/models/project_services/slack_service/issue_message.rb b/app/models/project_services/slack_service/issue_message.rb
index 438ff33fdff..88e053ec192 100644
--- a/app/models/project_services/slack_service/issue_message.rb
+++ b/app/models/project_services/slack_service/issue_message.rb
@@ -34,7 +34,12 @@ class SlackService
     private
 
     def message
-      "#{user_name} #{state} #{issue_link} in #{project_link}: *#{title}*"
+      case state
+      when "opened"
+        "[#{project_link}] Issue #{state} by #{user_name}"
+      else
+        "[#{project_link}] Issue #{issue_link} #{state} by #{user_name}"
+      end
     end
 
     def opened_issue?
@@ -42,7 +47,11 @@ class SlackService
     end
 
     def description_message
-      [{ text: format(description), color: attachment_color }]
+      [{
+        title: issue_title,
+        title_link: issue_url,
+        text: format(description),
+        color: "#C95823" }]
     end
 
     def project_link
@@ -50,7 +59,11 @@ class SlackService
     end
 
     def issue_link
-      "[issue ##{issue_iid}](#{issue_url})"
+      "[#{issue_title}](#{issue_url})"
+    end
+
+    def issue_title
+      "##{issue_iid} #{title}"
     end
   end
 end
diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index 501e58c1407..6728fabea1e 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -6,6 +6,7 @@ module Projects
 
     def execute
       forked_from_project_id = params.delete(:forked_from_project_id)
+      import_data = params.delete(:import_data)
 
       @project = Project.new(params)
 
@@ -49,16 +50,14 @@ module Projects
         @project.build_forked_project_link(forked_from_project_id: forked_from_project_id)
       end
 
-      Project.transaction do
-        @project.save
+      save_project_and_import_data(import_data)
 
-        if @project.persisted? && !@project.import?
-          raise 'Failed to create repository' unless @project.create_repository
-        end
-      end
+      @project.import_start if @project.import?
 
       after_create_actions if @project.persisted?
 
+      @project.add_import_job if @project.import?
+
       @project
     rescue => e
       message = "Unable to save project: #{e.message}"
@@ -93,8 +92,16 @@ module Projects
       unless @project.group
         @project.team << [current_user, :master, current_user]
       end
+    end
 
-      @project.import_start if @project.import?
+    def save_project_and_import_data(import_data)
+      Project.transaction do
+        @project.create_or_update_import_data(data: import_data[:data], credentials: import_data[:credentials]) if import_data
+
+        if @project.save && !@project.import?
+          raise 'Failed to create repository' unless @project.create_repository
+        end
+      end
     end
   end
 end
diff --git a/app/views/groups/_activities.html.haml b/app/views/groups/_activities.html.haml
index dc76599b776..71cc4d87b1f 100644
--- a/app/views/groups/_activities.html.haml
+++ b/app/views/groups/_activities.html.haml
@@ -4,7 +4,7 @@
 .nav-block
   - if current_user
     .controls
-      = link_to dashboard_projects_path(:atom, { private_token: current_user.private_token }), class: 'btn rss-btn' do
+      = link_to group_path(@group, format: :atom, private_token: current_user.private_token), class: 'btn rss-btn' do
         %i.fa.fa-rss
   = render 'shared/event_filter'
 
diff --git a/app/views/groups/issues.atom.builder b/app/views/groups/issues.atom.builder
index 486d1d8587a..a6eb9abada6 100644
--- a/app/views/groups/issues.atom.builder
+++ b/app/views/groups/issues.atom.builder
@@ -1,9 +1,9 @@
 xml.instruct!
 xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://search.yahoo.com/mrss/" do
-  xml.title   "#{@user.name} issues"
-  xml.link    href: issues_dashboard_url(format: :atom, private_token: @user.private_token), rel: "self", type: "application/atom+xml"
-  xml.link    href: issues_dashboard_url, rel: "alternate", type: "text/html"
-  xml.id      issues_dashboard_url
+  xml.title   "#{@group.name} issues"
+  xml.link    href: issues_group_url(format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml"
+  xml.link    href: issues_group_url, rel: "alternate", type: "text/html"
+  xml.id      issues_group_url
   xml.updated @issues.first.created_at.xmlschema if @issues.any?
 
   @issues.each do |issue|
diff --git a/app/views/projects/_md_preview.html.haml b/app/views/projects/_md_preview.html.haml
index 8de44a6c914..81afea2c60a 100644
--- a/app/views/projects/_md_preview.html.haml
+++ b/app/views/projects/_md_preview.html.haml
@@ -8,7 +8,7 @@
         %a.js-md-preview-button{ href: "#md-preview-holder", tabindex: -1 }
           Preview
       %li.pull-right
-        %button.zen-cotrol.zen-control-full.js-zen-enter{ type: 'button', tabindex: -1 }
+        %button.zen-control.zen-control-full.js-zen-enter{ type: 'button', tabindex: -1 }
           Go full screen
 
   .md-write-holder
diff --git a/app/views/projects/_zen.html.haml b/app/views/projects/_zen.html.haml
index e1e35013968..413477a2d3a 100644
--- a/app/views/projects/_zen.html.haml
+++ b/app/views/projects/_zen.html.haml
@@ -4,5 +4,5 @@
     = f.text_area attr, class: classes, placeholder: placeholder
   - else
     = text_area_tag attr, nil, class: classes, placeholder: placeholder
-  %a.zen-cotrol.zen-control-leave.js-zen-leave{ href: "#" }
+  %a.zen-control.zen-control-leave.js-zen-leave{ href: "#" }
     = icon('compress')
diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb
index b2d08d87bac..2338916e9da 100644
--- a/config/initializers/metrics.rb
+++ b/config/initializers/metrics.rb
@@ -118,6 +118,8 @@ if Gitlab::Metrics.enabled?
     # Instrument the classes used for checking if somebody has push access.
     config.instrument_instance_methods(Gitlab::GitAccess)
     config.instrument_instance_methods(Gitlab::GitAccessWiki)
+
+    config.instrument_instance_methods(API::Helpers)
   end
 
   GC::Profiler.enable
diff --git a/doc/api/README.md b/doc/api/README.md
index ff039f1886f..27c5962decf 100644
--- a/doc/api/README.md
+++ b/doc/api/README.md
@@ -33,7 +33,7 @@ following locations:
 - [Build triggers](build_triggers.md)
 - [Build Variables](build_variables.md)
 - [Runners](runners.md)
-- [Licenses](licenses.md)
+- [Open source license templates](licenses.md)
 
 ## Authentication
 
diff --git a/doc/ci/quick_start/README.md b/doc/ci/quick_start/README.md
index 6a42a935abd..386b8e29fcf 100644
--- a/doc/ci/quick_start/README.md
+++ b/doc/ci/quick_start/README.md
@@ -212,8 +212,8 @@ If you want to receive e-mail notifications about the result status of the
 builds, you should explicitly enable the **Builds Emails** service under your
 project's settings.
 
-For more information read the [Builds emails service documentation]
-(../../project_services/builds_emails.md).
+For more information read the
+[Builds emails service documentation](../../project_services/builds_emails.md).
 
 ## Builds badge
 
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 71a53e6f0d6..d4fcfd3d4d3 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -19,20 +19,24 @@ module API
         #   GET /projects/:id/issues/:noteable_id/notes
         #   GET /projects/:id/snippets/:noteable_id/notes
         get ":id/#{noteables_str}/:#{noteable_id_str}/notes" do
-          @noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"])
-
-          # We exclude notes that are cross-references and that cannot be viewed
-          # by the current user. By doing this exclusion at this level and not
-          # at the DB query level (which we cannot in that case), the current
-          # page can have less elements than :per_page even if
-          # there's more than one page.
-          notes =
-            # paginate() only works with a relation. This could lead to a
-            # mismatch between the pagination headers info and the actual notes
-            # array returned, but this is really a edge-case.
-            paginate(@noteable.notes).
-            reject { |n| n.cross_reference_not_visible_for?(current_user) }
-          present notes, with: Entities::Note
+          @noteable = user_project.send(noteables_str.to_sym).find(params[noteable_id_str.to_sym])
+
+          if can?(current_user, noteable_read_ability_name(@noteable), @noteable)
+            # We exclude notes that are cross-references and that cannot be viewed
+            # by the current user. By doing this exclusion at this level and not
+            # at the DB query level (which we cannot in that case), the current
+            # page can have less elements than :per_page even if
+            # there's more than one page.
+            notes =
+              # paginate() only works with a relation. This could lead to a
+              # mismatch between the pagination headers info and the actual notes
+              # array returned, but this is really a edge-case.
+              paginate(@noteable.notes).
+              reject { |n| n.cross_reference_not_visible_for?(current_user) }
+            present notes, with: Entities::Note
+          else
+            not_found!("Notes")
+          end
         end
 
         # Get a single +noteable+ note
@@ -45,13 +49,14 @@ module API
         #   GET /projects/:id/issues/:noteable_id/notes/:note_id
         #   GET /projects/:id/snippets/:noteable_id/notes/:note_id
         get ":id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id" do
-          @noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"])
+          @noteable = user_project.send(noteables_str.to_sym).find(params[noteable_id_str.to_sym])
           @note = @noteable.notes.find(params[:note_id])
+          can_read_note = can?(current_user, noteable_read_ability_name(@noteable), @noteable) && !@note.cross_reference_not_visible_for?(current_user)
 
-          if @note.cross_reference_not_visible_for?(current_user)
-            not_found!("Note")
-          else
+          if can_read_note
             present @note, with: Entities::Note
+          else
+            not_found!("Note")
           end
         end
 
@@ -136,5 +141,11 @@ module API
         end
       end
     end
+
+    helpers do
+      def noteable_read_ability_name(noteable)
+        "read_#{noteable.class.to_s.underscore.downcase}".to_sym
+      end
+    end
   end
 end
diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb
index 941f818b847..b90ef0b0fba 100644
--- a/lib/gitlab/bitbucket_import/project_creator.rb
+++ b/lib/gitlab/bitbucket_import/project_creator.rb
@@ -11,7 +11,7 @@ module Gitlab
       end
 
       def execute
-        project = ::Projects::CreateService.new(
+        ::Projects::CreateService.new(
           current_user,
           name: repo["name"],
           path: repo["slug"],
@@ -21,11 +21,8 @@ module Gitlab
           import_type: "bitbucket",
           import_source: "#{repo["owner"]}/#{repo["slug"]}",
           import_url: "ssh://git@bitbucket.org/#{repo["owner"]}/#{repo["slug"]}.git",
+          import_data: { credentials: { bb_session: session_data } }
         ).execute
-
-        project.create_or_update_import_data(credentials: { bb_session: session_data })
-
-        project
       end
     end
   end
diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb
index 3840765db87..1918d5b208d 100644
--- a/lib/gitlab/fogbugz_import/project_creator.rb
+++ b/lib/gitlab/fogbugz_import/project_creator.rb
@@ -12,7 +12,7 @@ module Gitlab
       end
 
       def execute
-        project = ::Projects::CreateService.new(
+        ::Projects::CreateService.new(
           current_user,
           name: repo.safe_name,
           path: repo.path,
@@ -21,12 +21,9 @@ module Gitlab
           visibility_level: Gitlab::VisibilityLevel::INTERNAL,
           import_type: 'fogbugz',
           import_source: repo.name,
-          import_url: Project::UNKNOWN_IMPORT_URL
+          import_url: Project::UNKNOWN_IMPORT_URL,
+          import_data: { data: { 'repo' => repo.raw_data, 'user_map' => user_map }, credentials: { fb_session: fb_session } }
         ).execute
-
-        project.create_or_update_import_data(data: { 'repo' => repo.raw_data, 'user_map' => user_map }, credentials: { fb_session: fb_session })
-
-        project
       end
     end
   end
diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb
index 96717b42bae..3e51c06877e 100644
--- a/lib/gitlab/gitlab_import/importer.rb
+++ b/lib/gitlab/gitlab_import/importer.rb
@@ -5,7 +5,7 @@ module Gitlab
 
       def initialize(project)
         @project = project
-        credentials = import_data
+        credentials = project.import_data
         if credentials && credentials[:password]
           @client = Client.new(credentials[:password])
           @formatter = Gitlab::ImportFormatter.new
diff --git a/lib/gitlab/google_code_import/project_creator.rb b/lib/gitlab/google_code_import/project_creator.rb
index 0abb7a64c17..326cfcaa8af 100644
--- a/lib/gitlab/google_code_import/project_creator.rb
+++ b/lib/gitlab/google_code_import/project_creator.rb
@@ -11,7 +11,7 @@ module Gitlab
       end
 
       def execute
-        project = ::Projects::CreateService.new(
+        ::Projects::CreateService.new(
           current_user,
           name: repo.name,
           path: repo.name,
@@ -21,12 +21,9 @@ module Gitlab
           visibility_level: Gitlab::VisibilityLevel::PUBLIC,
           import_type: "google_code",
           import_source: repo.name,
-          import_url: repo.import_url
+          import_url: repo.import_url,
+          import_data: { data: { 'repo' => repo.raw_data, 'user_map' => user_map } }
         ).execute
-
-        project.create_or_update_import_data(data: { 'repo' => repo.raw_data, 'user_map' => user_map })
-
-        project
       end
     end
   end
diff --git a/spec/models/project_services/slack_service/issue_message_spec.rb b/spec/models/project_services/slack_service/issue_message_spec.rb
index f648cbe2dee..0f8889bdf3c 100644
--- a/spec/models/project_services/slack_service/issue_message_spec.rb
+++ b/spec/models/project_services/slack_service/issue_message_spec.rb
@@ -25,7 +25,7 @@ describe SlackService::IssueMessage, models: true do
     }
   end
 
-  let(:color) { '#345' }
+  let(:color) { '#C95823' }
 
   context '#initialize' do
     before do
@@ -40,10 +40,11 @@ describe SlackService::IssueMessage, models: true do
   context 'open' do
     it 'returns a message regarding opening of issues' do
       expect(subject.pretext).to eq(
-        'Test User opened <url|issue #100> in <somewhere.com|project_name>: '\
-        '*Issue title*')
+        '<somewhere.com|[project_name>] Issue opened by Test User')
       expect(subject.attachments).to eq([
         {
+          title: "#100 Issue title",
+          title_link: "url",
           text: "issue description",
           color: color,
         }
@@ -56,10 +57,10 @@ describe SlackService::IssueMessage, models: true do
       args[:object_attributes][:action] = 'close'
       args[:object_attributes][:state] = 'closed'
     end
+
     it 'returns a message regarding closing of issues' do
       expect(subject.pretext). to eq(
-        'Test User closed <url|issue #100> in <somewhere.com|project_name>: '\
-        '*Issue title*')
+        '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by Test User')
       expect(subject.attachments).to be_empty
     end
   end
diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb
index 49091fc0f49..ed1ed5aeb95 100644
--- a/spec/requests/api/notes_spec.rb
+++ b/spec/requests/api/notes_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe API::API, api: true  do
   include ApiHelpers
   let(:user) { create(:user) }
-  let!(:project) { create(:project, namespace: user.namespace) }
+  let!(:project) { create(:project, :public, namespace: user.namespace) }
   let!(:issue) { create(:issue, project: project, author: user) }
   let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) }
   let!(:snippet) { create(:project_snippet, project: project, author: user) }
@@ -39,6 +39,7 @@ describe API::API, api: true  do
     context "when noteable is an Issue" do
       it "should return an array of issue notes" do
         get api("/projects/#{project.id}/issues/#{issue.id}/notes", user)
+
         expect(response.status).to eq(200)
         expect(json_response).to be_an Array
         expect(json_response.first['body']).to eq(issue_note.note)
@@ -46,20 +47,33 @@ describe API::API, api: true  do
 
       it "should return a 404 error when issue id not found" do
         get api("/projects/#{project.id}/issues/12345/notes", user)
+
         expect(response.status).to eq(404)
       end
 
-      context "that references a private issue" do
+      context "and current user cannot view the notes" do
         it "should return an empty array" do
           get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user)
+
           expect(response.status).to eq(200)
           expect(json_response).to be_an Array
           expect(json_response).to be_empty
         end
 
+        context "and issue is confidential" do
+          before { ext_issue.update_attributes(confidential: true) }
+
+          it "returns 404" do
+            get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user)
+
+            expect(response.status).to eq(404)
+          end
+        end
+
         context "and current user can view the note" do
           it "should return an empty array" do
             get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", private_user)
+
             expect(response.status).to eq(200)
             expect(json_response).to be_an Array
             expect(json_response.first['body']).to eq(cross_reference_note.note)
@@ -71,6 +85,7 @@ describe API::API, api: true  do
     context "when noteable is a Snippet" do
       it "should return an array of snippet notes" do
         get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user)
+
         expect(response.status).to eq(200)
         expect(json_response).to be_an Array
         expect(json_response.first['body']).to eq(snippet_note.note)
@@ -78,6 +93,13 @@ describe API::API, api: true  do
 
       it "should return a 404 error when snippet id not found" do
         get api("/projects/#{project.id}/snippets/42/notes", user)
+
+        expect(response.status).to eq(404)
+      end
+
+      it "returns 404 when not authorized" do
+        get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", private_user)
+
         expect(response.status).to eq(404)
       end
     end
@@ -85,6 +107,7 @@ describe API::API, api: true  do
     context "when noteable is a Merge Request" do
       it "should return an array of merge_requests notes" do
         get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/notes", user)
+
         expect(response.status).to eq(200)
         expect(json_response).to be_an Array
         expect(json_response.first['body']).to eq(merge_request_note.note)
@@ -92,6 +115,13 @@ describe API::API, api: true  do
 
       it "should return a 404 error if merge request id not found" do
         get api("/projects/#{project.id}/merge_requests/4444/notes", user)
+
+        expect(response.status).to eq(404)
+      end
+
+      it "returns 404 when not authorized" do
+        get api("/projects/#{project.id}/merge_requests/4444/notes", private_user)
+
         expect(response.status).to eq(404)
       end
     end
@@ -101,24 +131,39 @@ describe API::API, api: true  do
     context "when noteable is an Issue" do
       it "should return an issue note by id" do
         get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{issue_note.id}", user)
+
         expect(response.status).to eq(200)
         expect(json_response['body']).to eq(issue_note.note)
       end
 
       it "should return a 404 error if issue note not found" do
         get api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user)
+
         expect(response.status).to eq(404)
       end
 
-      context "that references a private issue" do
+      context "and current user cannot view the note" do
         it "should return a 404 error" do
           get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", user)
+
           expect(response.status).to eq(404)
         end
 
+        context "when issue is confidential" do
+          before { issue.update_attributes(confidential: true) }
+
+          it "returns 404" do
+            get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{issue_note.id}", private_user)
+
+            expect(response.status).to eq(404)
+          end
+        end
+
+
         context "and current user can view the note" do
           it "should return an issue note by id" do
             get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", private_user)
+
             expect(response.status).to eq(200)
             expect(json_response['body']).to eq(cross_reference_note.note)
           end
@@ -129,12 +174,14 @@ describe API::API, api: true  do
     context "when noteable is a Snippet" do
       it "should return a snippet note by id" do
         get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/#{snippet_note.id}", user)
+
         expect(response.status).to eq(200)
         expect(json_response['body']).to eq(snippet_note.note)
       end
 
       it "should return a 404 error if snippet note not found" do
         get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/12345", user)
+
         expect(response.status).to eq(404)
       end
     end
@@ -144,6 +191,7 @@ describe API::API, api: true  do
     context "when noteable is an Issue" do
       it "should create a new issue note" do
         post api("/projects/#{project.id}/issues/#{issue.id}/notes", user), body: 'hi!'
+
         expect(response.status).to eq(201)
         expect(json_response['body']).to eq('hi!')
         expect(json_response['author']['username']).to eq(user.username)
@@ -151,11 +199,13 @@ describe API::API, api: true  do
 
       it "should return a 400 bad request error if body not given" do
         post api("/projects/#{project.id}/issues/#{issue.id}/notes", user)
+
         expect(response.status).to eq(400)
       end
 
       it "should return a 401 unauthorized error if user not authenticated" do
         post api("/projects/#{project.id}/issues/#{issue.id}/notes"), body: 'hi!'
+
         expect(response.status).to eq(401)
       end
 
@@ -164,6 +214,7 @@ describe API::API, api: true  do
           creation_time = 2.weeks.ago
           post api("/projects/#{project.id}/issues/#{issue.id}/notes", user),
             body: 'hi!', created_at: creation_time
+
           expect(response.status).to eq(201)
           expect(json_response['body']).to eq('hi!')
           expect(json_response['author']['username']).to eq(user.username)
@@ -176,6 +227,7 @@ describe API::API, api: true  do
     context "when noteable is a Snippet" do
       it "should create a new snippet note" do
         post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user), body: 'hi!'
+
         expect(response.status).to eq(201)
         expect(json_response['body']).to eq('hi!')
         expect(json_response['author']['username']).to eq(user.username)
@@ -183,11 +235,13 @@ describe API::API, api: true  do
 
       it "should return a 400 bad request error if body not given" do
         post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user)
+
         expect(response.status).to eq(400)
       end
 
       it "should return a 401 unauthorized error if user not authenticated" do
         post api("/projects/#{project.id}/snippets/#{snippet.id}/notes"), body: 'hi!'
+
         expect(response.status).to eq(401)
       end
     end
@@ -227,6 +281,7 @@ describe API::API, api: true  do
       it 'should return modified note' do
         put api("/projects/#{project.id}/issues/#{issue.id}/"\
                   "notes/#{issue_note.id}", user), body: 'Hello!'
+
         expect(response.status).to eq(200)
         expect(json_response['body']).to eq('Hello!')
       end
@@ -234,12 +289,14 @@ describe API::API, api: true  do
       it 'should return a 404 error when note id not found' do
         put api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user),
                 body: 'Hello!'
+
         expect(response.status).to eq(404)
       end
 
       it 'should return a 400 bad request error if body not given' do
         put api("/projects/#{project.id}/issues/#{issue.id}/"\
                   "notes/#{issue_note.id}", user)
+
         expect(response.status).to eq(400)
       end
     end
@@ -248,6 +305,7 @@ describe API::API, api: true  do
       it 'should return modified note' do
         put api("/projects/#{project.id}/snippets/#{snippet.id}/"\
                   "notes/#{snippet_note.id}", user), body: 'Hello!'
+
         expect(response.status).to eq(200)
         expect(json_response['body']).to eq('Hello!')
       end
@@ -255,6 +313,7 @@ describe API::API, api: true  do
       it 'should return a 404 error when note id not found' do
         put api("/projects/#{project.id}/snippets/#{snippet.id}/"\
                   "notes/12345", user), body: "Hello!"
+
         expect(response.status).to eq(404)
       end
     end
@@ -263,6 +322,7 @@ describe API::API, api: true  do
       it 'should return modified note' do
         put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/"\
                   "notes/#{merge_request_note.id}", user), body: 'Hello!'
+
         expect(response.status).to eq(200)
         expect(json_response['body']).to eq('Hello!')
       end
@@ -270,6 +330,7 @@ describe API::API, api: true  do
       it 'should return a 404 error when note id not found' do
         put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/"\
                   "notes/12345", user), body: "Hello!"
+
         expect(response.status).to eq(404)
       end
     end