Merge branch 'master' into ci/api-triggers

* master: (32 commits)
  Fix specs and rubocop warnings
  fixed LDAP activation on login to use new ldap_blocked state
  Fix Admin/Users view to position buttons without spacing magic
  Update to Go 1.5.3
  Fix the undefinded variable error in Project's safe_import_url method
  Fix misaligned edit button in milestone collection partial
  Update button styles for Milestones#show
  Ensure the API doesn't return notes that the current user shouldn't see
  Add spec for Note#cross_reference_not_visible_for?
  Remove (invalid) timestamp formatting
  Move `BroadcastMessage#status` to a helper since it's presentational
  Update CHANGELOG
  Broadcast Messages can now be edited
  Update Broadcast Message features
  Update BroadcastMessage model
  Update broadcast_message helper
  Simplify BroadcastMessage factory
  Simplify broadcast message JS
  Remove alert_type attribute from BroadcastMessage
  Move broadcast message form to a partial
  ...

CHANGELOG

@@ -42,8 +42,10 @@ v 8.4.0 (unreleased)
   - Ajax filter by message for commits page
   - API: Add support for deleting a tag via the API (Robert Schilling)
   - Allow subsequent validations in CI Linter
+  - Show referenced MRs & Issues only when the current viewer can access them
   - Fix Encoding::CompatibilityError bug when markdown content has some complex URL (Jason Lee)
   - Add API support for managing project's build triggers
+  - Allow broadcast messages to be edited
 
 v 8.3.4
   - Use gitlab-workhorse 0.5.4 (fixes API routing bug)

app/assets/javascripts/admin.js.coffee

@@ -10,19 +10,19 @@ class @Admin
 
     $('body').on 'click', '.js-toggle-colors-link', (e) ->
       e.preventDefault()
-      $('.js-toggle-colors-link').hide()
-      $('.js-toggle-colors-container').show()
+      $('.js-toggle-colors-container').toggle()
 
     $('input#broadcast_message_color').on 'input', ->
-      previewColor = $('input#broadcast_message_color').val()
+      previewColor = $(@).val()
       $('div.broadcast-message-preview').css('background-color', previewColor)
 
     $('input#broadcast_message_font').on 'input', ->
-      previewColor = $('input#broadcast_message_font').val()
+      previewColor = $(@).val()
       $('div.broadcast-message-preview').css('color', previewColor)
 
     $('textarea#broadcast_message_message').on 'input', ->
-      previewMessage = $('textarea#broadcast_message_message').val()
+      previewMessage = $(@).val()
+      previewMessage = "Your message here" if previewMessage.trim() == ''
       $('div.broadcast-message-preview span').text(previewMessage)
 
     $('.log-tabs a').click (e) ->

app/assets/stylesheets/framework/buttons.scss

@@ -131,6 +131,12 @@
     &:last-child {
       margin-right: 0px;
     }
+    &.btn-xs {
+      margin-right: 3px;
+    }
+  }
+  &.disabled {
+    pointer-events: auto !important;
   }
 }
 

app/assets/stylesheets/framework/forms.scss

@@ -78,6 +78,10 @@ label {
   padding: 8px $gl-padding;
 }
 
+.form-control-inline {
+  display: inline;
+}
+
 .wiki-content {
   margin-top: 35px;
 }

app/controllers/admin/broadcast_messages_controller.rb

 class Admin::BroadcastMessagesController < Admin::ApplicationController
-  before_action :broadcast_messages
+  before_action :finder, only: [:edit, :update, :destroy]
 
   def index
-    @broadcast_message = BroadcastMessage.new
+    @broadcast_messages = BroadcastMessage.reorder("starts_at ASC").page(params[:page])
+    @broadcast_message  = BroadcastMessage.new
+  end
+
+  def edit
   end
 
   def create
@@ -15,8 +19,16 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
     end
   end
 
+  def update
+    if @broadcast_message.update(broadcast_message_params)
+      redirect_to admin_broadcast_messages_path, notice: 'Broadcast Message was successfully updated.'
+    else
+      render :edit
+    end
+  end
+
   def destroy
-    BroadcastMessage.find(params[:id]).destroy
+    @broadcast_message.destroy
 
     respond_to do |format|
       format.html { redirect_back_or_default(default: { action: 'index' }) }
@@ -26,14 +38,17 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
 
   protected
 
-  def broadcast_messages
-    @broadcast_messages ||= BroadcastMessage.order("starts_at DESC").page(params[:page])
+  def finder
+    @broadcast_message = BroadcastMessage.find(params[:id])
   end
 
   def broadcast_message_params
-    params.require(:broadcast_message).permit(
-      :alert_type, :color, :ends_at, :font,
-      :message, :starts_at
-    )
+    params.require(:broadcast_message).permit(%i(
+      color
+      ends_at
+      font
+      message
+      starts_at
+    ))
   end
 end

app/controllers/admin/identities_controller.rb

@@ -26,6 +26,7 @@ class Admin::IdentitiesController < Admin::ApplicationController
 
   def update
     if @identity.update_attributes(identity_params)
+      RepairLdapBlockedUserService.new(@user).execute
       redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.'
     else
       render :edit
@@ -34,6 +35,7 @@ class Admin::IdentitiesController < Admin::ApplicationController
 
   def destroy
     if @identity.destroy
+      RepairLdapBlockedUserService.new(@user).execute
       redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully removed.'
     else
       redirect_to admin_user_identities_path(@user), alert: 'Failed to remove user identity.'

app/controllers/admin/users_controller.rb

@@ -40,7 +40,9 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def unblock
-    if user.activate
+    if user.ldap_blocked?
+      redirect_back_or_admin_user(alert: "This user cannot be unlocked manually from GitLab")
+    elsif user.activate
       redirect_back_or_admin_user(notice: "Successfully unblocked")
     else
       redirect_back_or_admin_user(alert: "Error occurred. User was not unblocked")

app/controllers/projects/issues_controller.rb

@@ -61,7 +61,7 @@ class Projects::IssuesController < Projects::ApplicationController
     @note = @project.notes.new(noteable: @issue)
     @notes = @issue.notes.nonawards.with_associations.fresh
     @noteable = @issue
-    @merge_requests = @issue.referenced_merge_requests
+    @merge_requests = @issue.referenced_merge_requests(current_user)
 
     respond_with(@issue)
   end

app/helpers/application_helper.rb

@@ -181,10 +181,6 @@ module ApplicationHelper
     end
   end
 
-  def broadcast_message
-    BroadcastMessage.current
-  end
-
   # Render a `time` element with Javascript-based relative date and tooltip
   #
   # time       - Time object

app/helpers/broadcast_messages_helper.rb

 module BroadcastMessagesHelper
-  def broadcast_styling(broadcast_message)
-    styling = ''
+  def broadcast_message(message = BroadcastMessage.current)
+    return unless message.present?
+
+    content_tag :div, class: 'broadcast-message', style: broadcast_message_style(message) do
+      icon('bullhorn') << ' ' << message.message
+    end
+  end
+
+  def broadcast_message_style(broadcast_message)
+    style = ''
 
     if broadcast_message.color.present?
-      styling << "background-color: #{broadcast_message.color}"
-      styling << '; ' if broadcast_message.font.present?
+      style << "background-color: #{broadcast_message.color}"
+      style << '; ' if broadcast_message.font.present?
     end
 
     if broadcast_message.font.present?
-      styling << "color: #{broadcast_message.font}"
+      style << "color: #{broadcast_message.font}"
     end
 
-    styling
+    style
+  end
+
+  def broadcast_message_status(broadcast_message)
+    if broadcast_message.active?
+      'Active'
+    elsif broadcast_message.ended?
+      'Expired'
+    else
+      'Pending'
+    end
   end
 end

app/models/broadcast_message.rb

@@ -6,7 +6,6 @@
 #  message    :text             not null
 #  starts_at  :datetime
 #  ends_at    :datetime
-#  alert_type :integer
 #  created_at :datetime
 #  updated_at :datetime
 #  color      :string(255)
@@ -23,7 +22,22 @@ class BroadcastMessage < ActiveRecord::Base
   validates :color, allow_blank: true, color: true
   validates :font,  allow_blank: true, color: true
 
+  default_value_for :color, '#E75E40'
+  default_value_for :font,  '#FFFFFF'
+
   def self.current
-    where("ends_at > :now AND starts_at < :now", now: Time.zone.now).last
+    where("ends_at > :now AND starts_at <= :now", now: Time.zone.now).last
+  end
+
+  def active?
+    started? && !ended?
+  end
+
+  def started?
+    Time.zone.now >= starts_at
+  end
+
+  def ended?
+    ends_at < Time.zone.now
   end
 end

app/models/identity.rb

@@ -18,4 +18,8 @@ class Identity < ActiveRecord::Base
   validates :provider, presence: true
   validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
   validates :user_id, uniqueness: { scope: :provider }
+
+  def ldap?
+    provider.starts_with?('ldap')
+  end
 end

app/models/issue.rb

@@ -85,10 +85,10 @@ class Issue < ActiveRecord::Base
     reference
   end
 
-  def referenced_merge_requests
+  def referenced_merge_requests(current_user = nil)
     Gitlab::ReferenceExtractor.lazily do
       [self, *notes].flat_map do |note|
-        note.all_references.merge_requests
+        note.all_references(current_user).merge_requests
       end
     end.sort_by(&:iid)
   end

app/models/note.rb

@@ -358,6 +358,10 @@ class Note < ActiveRecord::Base
     !system? && !is_award
   end
 
+  def cross_reference_not_visible_for?(user)
+    cross_reference? && referenced_mentionables(user).empty?
+  end
+
   # Checks if note is an award added as a comment
   #
   # If note is an award, this method sets is_award to true

app/models/project.rb

@@ -397,7 +397,7 @@ class Project < ActiveRecord::Base
     result.password = '*****' unless result.password.nil?
     result.to_s
   rescue
-    original_url
+    self.import_url
   end
 
   def check_limit

app/models/user.rb

@@ -196,10 +196,22 @@ class User < ActiveRecord::Base
   state_machine :state, initial: :active do
     event :block do
       transition active: :blocked
+      transition ldap_blocked: :blocked
+    end
+
+    event :ldap_block do
+      transition active: :ldap_blocked
     end
 
     event :activate do
       transition blocked: :active
+      transition ldap_blocked: :active
+    end
+
+    state :blocked, :ldap_blocked do
+      def blocked?
+        true
+      end
     end
   end
 
@@ -207,7 +219,7 @@ class User < ActiveRecord::Base
 
   # Scopes
   scope :admins, -> { where(admin: true) }
-  scope :blocked, -> { with_state(:blocked) }
+  scope :blocked, -> { with_states(:blocked, :ldap_blocked) }
   scope :active, -> { with_state(:active) }
   scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all }
   scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members)') }

app/services/repair_ldap_blocked_user_service.rb

+class RepairLdapBlockedUserService
+  attr_accessor :user
+
+  def initialize(user)
+    @user = user
+  end
+
+  def execute
+    user.block if ldap_hard_blocked?
+  end
+
+  private
+
+  def ldap_hard_blocked?
+    user.ldap_blocked? && !user.ldap_user?
+  end
+end

app/views/admin/broadcast_messages/_form.html.haml

+.broadcast-message-preview{ style: broadcast_message_style(@broadcast_message) }
+  = icon('bullhorn')
+  %span= @broadcast_message.message || "Your message here"
+
+= form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-requires-input'} do |f|
+  -if @broadcast_message.errors.any?
+    .alert.alert-danger
+      - @broadcast_message.errors.full_messages.each do |msg|
+        %p= msg
+  .form-group
+    = f.label :message, class: 'control-label'
+    .col-sm-10
+      = f.text_area :message, class: "form-control js-quick-submit", rows: 2, required: true
+  .form-group.js-toggle-colors-container
+    .col-sm-10.col-sm-offset-2
+      = link_to 'Customize colors', '#', class: 'js-toggle-colors-link'
+  .form-group.js-toggle-colors-container.hide
+    = f.label :color, "Background Color", class: 'control-label'
+    .col-sm-10
+      = f.color_field :color, class: "form-control"
+  .form-group.js-toggle-colors-container.hide
+    = f.label :font, "Font Color", class: 'control-label'
+    .col-sm-10
+      = f.color_field :font, class: "form-control"
+  .form-group
+    = f.label :starts_at, class: 'control-label'
+    .col-sm-10.datetime-controls
+      = f.datetime_select :starts_at, {}, class: 'form-control form-control-inline'
+  .form-group
+    = f.label :ends_at, class: 'control-label'
+    .col-sm-10.datetime-controls
+      = f.datetime_select :ends_at, {}, class: 'form-control form-control-inline'
+  .form-actions
+    - if @broadcast_message.persisted?
+      = f.submit "Update broadcast message", class: "btn btn-create"
+    - else
+      = f.submit "Add broadcast message", class: "btn btn-create"

app/views/admin/broadcast_messages/edit.html.haml

+- page_title "Broadcast Messages"
+
+= render 'form'

app/views/admin/broadcast_messages/index.html.haml

 - page_title "Broadcast Messages"
+
 %h3.page-title
   Broadcast Messages
 %p.light
-  Broadcast messages are displayed for every user and can be used to notify users about scheduled maintenance, recent upgrades and more.
-.broadcast-message-preview
-  %i.fa.fa-bullhorn
-  %span Your message here
-
-= form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal'} do |f|
-  -if @broadcast_message.errors.any?
-    .alert.alert-danger
-      - @broadcast_message.errors.full_messages.each do |msg|
-        %p= msg
-  .form-group
-    = f.label :message, class: 'control-label'
-    .col-sm-10
-      = f.text_area :message, class: "form-control", rows: 2, required: true
-      %div
-        = link_to '#', class: 'js-toggle-colors-link' do
-          Customize colors
-  .form-group.js-toggle-colors-container.hide
-    = f.label :color, "Background Color", class: 'control-label'
-    .col-sm-10
-      = f.color_field :color, value: "#eb9532", class: "form-control"
-  .form-group.js-toggle-colors-container.hide
-    = f.label :font, "Font Color", class: 'control-label'
-    .col-sm-10
-      = f.color_field :font, value: "#FFFFFF", class: "form-control"
-  .form-group
-    = f.label :starts_at, class: 'control-label'
-    .col-sm-10.datetime-controls
-      = f.datetime_select :starts_at
-  .form-group
-    = f.label :ends_at, class: 'control-label'
-    .col-sm-10.datetime-controls
-      = f.datetime_select :ends_at
-  .form-actions
-    = f.submit "Add broadcast message", class: "btn btn-create"
+  Broadcast messages are displayed for every user and can be used to notify
+  users about scheduled maintenance, recent upgrades and more.
 
--if @broadcast_messages.any?
-  %ul.bordered-list.broadcast-messages
-    - @broadcast_messages.each do |broadcast_message|
-      %li
-        .pull-right
-          - if broadcast_message.starts_at
-            %strong
-              #{broadcast_message.starts_at.to_s(:short)}
-            \...
-          - if broadcast_message.ends_at
-            %strong
-              #{broadcast_message.ends_at.to_s(:short)}
-           
-          = link_to [:admin, broadcast_message], method: :delete, remote: true, class: 'remove-row btn btn-xs' do
-            %i.fa.fa-times.cred
+= render 'form'
 
-        .message= broadcast_message.message
+%br.clearfix
 
+-if @broadcast_messages.any?
+  %table.table
+    %thead
+      %tr
+        %th Status
+        %th Preview
+        %th Starts
+        %th Ends
+        %th  
+    %tbody
+      - @broadcast_messages.each do |message|
+        %tr
+          %td
+            = broadcast_message_status(message)
+          %td
+            = broadcast_message(message)
+          %td
+            = message.starts_at
+          %td
+            = message.ends_at
+          %td
+            = link_to icon('pencil-square-o'), edit_admin_broadcast_message_path(message), title: 'Edit', class: 'btn btn-xs'
+            = link_to icon('times'), admin_broadcast_message_path(message), method: :delete, remote: true, title: 'Remove', class: 'js-remove-tr btn btn-xs btn-danger'
 
   = paginate @broadcast_messages

app/views/admin/users/index.html.haml

@@ -88,14 +88,19 @@
                 %i.fa.fa-envelope
                 = mail_to user.email, user.email, class: 'light'
                
-              = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: "btn btn-xs"
-              - unless user == current_user
-                - if user.blocked?
-                  = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success"
-                - else
-                  = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs btn-warning"
-                - if user.access_locked?
-                  = link_to 'Unlock', unlock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success", data: { confirm: 'Are you sure?' }
-                - if user.can_be_removed?
-                  = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All issues, merge requests and groups linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: "btn btn-xs btn-remove"
+              .pull-right
+                = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: 'btn-grouped btn btn-xs'
+                - unless user == current_user
+                  - if user.ldap_blocked?
+                    = link_to '#', title: 'Cannot unblock LDAP blocked users', data: {toggle: 'tooltip'}, class: 'btn-grouped btn btn-xs btn-success disabled' do
+                      %i.fa.fa-lock
+                      Unblock
+                  - elsif user.blocked?
+                    = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: 'btn-grouped btn btn-xs btn-success'
+                  - else
+                    = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: 'btn-grouped btn btn-xs btn-warning'
+                  - if user.access_locked?
+                    = link_to 'Unlock', unlock_admin_user_path(user), method: :put, class: 'btn-grouped btn btn-xs btn-success', data: { confirm: 'Are you sure?' }
+                  - if user.can_be_removed?
+                    = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All issues, merge requests and groups linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: 'btn-grouped btn btn-xs btn-remove'
     = paginate @users, theme: "gitlab"

app/views/layouts/_broadcast.html.haml

-- if broadcast_message.present?
-  .broadcast-message{ style: broadcast_styling(broadcast_message) }
-    %i.fa.fa-bullhorn
-    = broadcast_message.message
+= broadcast_message

app/views/projects/milestones/_milestone.html.haml

@@ -21,10 +21,11 @@
       = render 'shared/milestone_expired', milestone: milestone
     .col-sm-6
       - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
-        = link_to edit_namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone), class: "btn btn-xs edit-milestone-link btn-grouped" do
-          %i.fa.fa-pencil-square-o
+        = link_to edit_namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone), class: "btn btn-xs" do
+          = icon('pencil-square-o')
           Edit
+        \
         = link_to 'Close Milestone', namespace_project_milestone_path(@project.namespace, @project, milestone, milestone: {state_event: :close }), method: :put, remote: true, class: "btn btn-xs btn-close"
         = link_to namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-xs btn-remove" do
-          %i.fa.fa-trash-o
+          = icon('trash-o')
           Delete

app/views/projects/milestones/show.html.haml

@@ -20,16 +20,16 @@
   .pull-right
     - if can?(current_user, :admin_milestone, @project)
       - if @milestone.active?
-        = link_to 'Close Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-grouped"
+        = link_to 'Close Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-nr btn-grouped"
       - else
-        = link_to 'Reopen Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-grouped"
+        = link_to 'Reopen Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-nr btn-grouped"
 
-      = link_to namespace_project_milestone_path(@project.namespace, @project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-remove" do
-        %i.fa.fa-trash-o
+      = link_to namespace_project_milestone_path(@project.namespace, @project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-nr btn-remove" do
+        = icon('trash-o')
         Delete
 
-      = link_to edit_namespace_project_milestone_path(@project.namespace, @project, @milestone), class: "btn btn-grouped" do
-        %i.fa.fa-pencil-square-o
+      = link_to edit_namespace_project_milestone_path(@project.namespace, @project, @milestone), class: "btn btn-grouped btn-nr" do
+        = icon('pencil-square-o')
         Edit
 
 .detail-page-description.gray-content-block.second-block

app/views/projects/notes/_notes.html.haml

@@ -2,10 +2,14 @@
   - @discussions.each do |discussion_notes|
     - note = discussion_notes.first
     - if note_for_main_target?(note)
+      - next if note.cross_reference_not_visible_for?(current_user)
+
       = render discussion_notes
     - else
       = render 'projects/notes/discussion', discussion_notes: discussion_notes
 - else
   - @notes.each do |note|
     - next unless note.author
+    - next if note.cross_reference_not_visible_for?(current_user)
+
     = render note

config/routes.rb

@@ -219,7 +219,7 @@ Rails.application.routes.draw do
       get :test
     end
 
-    resources :broadcast_messages, only: [:index, :create, :destroy]
+    resources :broadcast_messages, only: [:index, :edit, :create, :update, :destroy]
     resource :logs, only: [:show]
     resource :background_jobs, controller: 'background_jobs', only: [:show]
 

db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb

+class RemoveAlertTypeFromBroadcastMessages < ActiveRecord::Migration
+  def change
+    remove_column :broadcast_messages, :alert_type, :integer
+  end
+end

db/schema.rb

@@ -82,7 +82,6 @@ ActiveRecord::Schema.define(version: 20160113111034) do
     t.text     "message",    null: false
     t.datetime "starts_at"
     t.datetime "ends_at"
-    t.integer  "alert_type"
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "color"

doc/api/users.md

@@ -558,7 +558,8 @@ Parameters:
 
 - `uid` (required) - id of specified user
 
-Will return `200 OK` on success, or `404 User Not Found` is user cannot be found.
+Will return `200 OK` on success, `404 User Not Found` is user cannot be found or 
+`403 Forbidden` when trying to block an already blocked user by LDAP synchronization.
 
 ## Unblock user
 
@@ -572,4 +573,5 @@ Parameters:
 
 - `uid` (required) - id of specified user
 
-Will return `200 OK` on success, or `404 User Not Found` is user cannot be found.
+Will return `200 OK` on success, `404 User Not Found` is user cannot be found or
+`403 Forbidden` when trying to unblock a user blocked by LDAP synchronization.

doc/install/installation.md

@@ -135,11 +135,11 @@ gitlab-workhorse we need a Go compiler. The instructions below assume you
 use 64-bit Linux. You can find downloads for other platforms at the [Go download
 page](https://golang.org/dl).
 
-    curl -O --progress https://storage.googleapis.com/golang/go1.5.1.linux-amd64.tar.gz
-    echo '46eecd290d8803887dec718c691cc243f2175fe0  go1.5.1.linux-amd64.tar.gz' | shasum -c - && \
-      sudo tar -C /usr/local -xzf go1.5.1.linux-amd64.tar.gz
+    curl -O --progress https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
+    echo '43afe0c5017e502630b1aea4d44b8a7f059bf60d7f29dfd58db454d4e4e0ae53  go1.5.3.linux-amd64.tar.gz' | shasum -c - && \
+      sudo tar -C /usr/local -xzf go1.5.3.linux-amd64.tar.gz
     sudo ln -sf /usr/local/go/bin/{go,godoc,gofmt} /usr/local/bin/
-    rm go1.5.1.linux-amd64.tar.gz
+    rm go1.5.3.linux-amd64.tar.gz
 
 ## 4. System Users
 

features/admin/broadcast_messages.feature

@@ -2,16 +2,11 @@
 Feature: Admin Broadcast Messages
   Background:
     Given I sign in as an admin
-    And application already has admin messages
+    And application already has a broadcast message
     And I visit admin messages page
 
   Scenario: See broadcast messages list
-    Then I should be all broadcast messages
-
-  Scenario: Create a broadcast message
-    When submit form with new broadcast message
-    Then I should be redirected to admin messages page
-    And I should see newly created broadcast message
+    Then I should see all broadcast messages
 
   Scenario: Create a customized broadcast message
     When submit form with new customized broadcast message
@@ -19,3 +14,14 @@ Feature: Admin Broadcast Messages
     And I should see newly created broadcast message
     Then I visit dashboard page
     And I should see a customized broadcast message
+
+  Scenario: Edit an existing broadcast message
+    When I edit an existing broadcast message
+    And I change the broadcast message text
+    Then I should be redirected to admin messages page
+    And I should see the updated broadcast message
+
+  Scenario: Remove an existing broadcast message
+    When I remove an existing broadcast message
+    Then I should be redirected to admin messages page
+    And I should not see the removed broadcast message

features/project/issues/references.feature

+@project_issues
+Feature: Project Issues References
+  Background:
+    Given I sign in as "John Doe"
+    And public project "Community"
+    And "John Doe" owns public project "Community"
+    And project "Community" has "Community issue" open issue
+    And I logout
+    And I sign in as "Mary Jane"
+    And private project "Enterprise"
+    And "Mary Jane" owns private project "Enterprise"
+    And project "Enterprise" has "Enterprise issue" open issue
+    And project "Enterprise" has "Enterprise fix" open merge request
+    And I visit issue page "Enterprise issue"
+    And I leave a comment referencing issue "Community issue"
+    And I visit merge request page "Enterprise fix"
+    And I leave a comment referencing issue "Community issue"
+    And I logout
+
+  @javascript
+  Scenario: Viewing the public issue as a "John Doe"
+    Given I sign in as "John Doe"
+    When I visit issue page "Community issue"
+    Then I should not see any related merge requests
+    And I should see no notes at all
+
+  @javascript
+  Scenario: Viewing the public issue as "Mary Jane"
+    Given I sign in as "Mary Jane"
+    When I visit issue page "Community issue"
+    Then I should see the "Enterprise fix" related merge request
+    And I should see a note linking to "Enterprise fix" merge request
+    And I should see a note linking to "Enterprise issue" issue

features/project/merge_requests/references.feature

+@project_merge_requests
+Feature: Project Merge Requests References
+  Background:
+    Given I sign in as "John Doe"
+    And public project "Community"
+    And "John Doe" owns public project "Community"
+    And project "Community" has "Community fix" open merge request
+    And I logout
+    And I sign in as "Mary Jane"
+    And private project "Enterprise"
+    And "Mary Jane" owns private project "Enterprise"
+    And project "Enterprise" has "Enterprise issue" open issue
+    And project "Enterprise" has "Enterprise fix" open merge request
+    And I visit issue page "Enterprise issue"
+    And I leave a comment referencing issue "Community fix"
+    And I visit merge request page "Enterprise fix"
+    And I leave a comment referencing issue "Community fix"
+    And I logout
+
+  @javascript
+  Scenario: Viewing the public issue as a "John Doe"
+    Given I sign in as "John Doe"
+    When I visit issue page "Community fix"
+    Then I should see no notes at all
+
+  @javascript
+  Scenario: Viewing the public issue as "Mary Jane"
+    Given I sign in as "Mary Jane"
+    When I visit issue page "Community fix"
+    And I should see a note linking to "Enterprise fix" merge request
+    And I should see a note linking to "Enterprise issue" issue

features/steps/admin/broadcast_messages.rb

 class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps
   include SharedAuthentication
   include SharedPaths
-  include SharedAdmin
 
-  step 'application already has admin messages' do
-    FactoryGirl.create(:broadcast_message, message: "Migration to new server")
+  step 'application already has a broadcast message' do
+    FactoryGirl.create(:broadcast_message, :expired, message: "Migration to new server")
   end
 
-  step 'I should be all broadcast messages' do
+  step 'I should see all broadcast messages' do
     expect(page).to have_content "Migration to new server"
   end
 
-  step 'submit form with new broadcast message' do
-    fill_in 'broadcast_message_message', with: 'Application update from 4:00 CST to 5:00 CST'
-    select '2018', from: "broadcast_message_ends_at_1i"
-    click_button "Add broadcast message"
-  end
-
   step 'I should be redirected to admin messages page' do
     expect(current_path).to eq admin_broadcast_messages_path
   end
@@ -27,10 +20,9 @@ class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps
 
   step 'submit form with new customized broadcast message' do
     fill_in 'broadcast_message_message', with: 'Application update from 4:00 CST to 5:00 CST'
-    click_link "Customize colors"
     fill_in 'broadcast_message_color', with: '#f2dede'
     fill_in 'broadcast_message_font', with: '#b94a48'
-    select '2018', from: "broadcast_message_ends_at_1i"
+    select Date.today.next_year.year, from: "broadcast_message_ends_at_1i"
     click_button "Add broadcast message"
   end
 
@@ -38,4 +30,25 @@ class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps
     expect(page).to have_content 'Application update from 4:00 CST to 5:00 CST'
     expect(page).to have_selector %(div[style="background-color: #f2dede; color: #b94a48"])
   end
+
+  step 'I edit an existing broadcast message' do
+    click_link 'Edit'
+  end
+
+  step 'I change the broadcast message text' do
+    fill_in 'broadcast_message_message', with: 'Application update RIGHT NOW'
+    click_button 'Update broadcast message'
+  end
+
+  step 'I should see the updated broadcast message' do
+    expect(page).to have_content "Application update RIGHT NOW"
+  end
+
+  step 'I remove an existing broadcast message' do
+    click_link 'Remove'
+  end
+
+  step 'I should not see the removed broadcast message' do
+    expect(page).not_to have_content 'Migration to new server'
+  end
 end

features/steps/project/issues/references.rb

+class Spinach::Features::ProjectIssuesReferences < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedIssuable
+  include SharedNote
+  include SharedProject
+  include SharedUser
+end

features/steps/project/merge_requests/references.rb

+class Spinach::Features::ProjectMergeRequestsReferences < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedIssuable
+  include SharedNote
+  include SharedProject
+  include SharedUser
+end

features/steps/shared/issuable.rb

@@ -5,6 +5,99 @@ module SharedIssuable
     find(:css, '.issuable-edit').click
   end
 
+  step 'project "Community" has "Community issue" open issue' do
+    create_issuable_for_project(
+      project_name: 'Community',
+      title: 'Community issue'
+    )
+  end
+
+  step 'project "Community" has "Community fix" open merge request' do
+    create_issuable_for_project(
+      project_name: 'Community',
+      type: :merge_request,
+      title: 'Community fix'
+    )
+  end
+
+  step 'project "Enterprise" has "Enterprise issue" open issue' do
+    create_issuable_for_project(
+      project_name: 'Enterprise',
+      title: 'Enterprise issue'
+    )
+  end
+
+  step 'project "Enterprise" has "Enterprise fix" open merge request' do
+    create_issuable_for_project(
+      project_name: 'Enterprise',
+      type: :merge_request,
+      title: 'Enterprise fix'
+    )
+  end
+
+  step 'I leave a comment referencing issue "Community issue"' do
+    leave_reference_comment(
+      issuable: Issue.find_by(title: 'Community issue'),
+      from_project_name: 'Enterprise'
+    )
+  end
+
+  step 'I leave a comment referencing issue "Community fix"' do
+    leave_reference_comment(
+      issuable: MergeRequest.find_by(title: 'Community fix'),
+      from_project_name: 'Enterprise'
+    )
+  end
+
+  step 'I visit issue page "Enterprise issue"' do
+    issue = Issue.find_by(title: 'Enterprise issue')
+    visit namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+  end
+
+  step 'I visit merge request page "Enterprise fix"' do
+    mr = MergeRequest.find_by(title: 'Enterprise fix')
+    visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr)
+  end
+
+  step 'I visit issue page "Community issue"' do
+    issue = Issue.find_by(title: 'Community issue')
+    visit namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+  end
+
+  step 'I visit issue page "Community fix"' do
+    mr = MergeRequest.find_by(title: 'Community fix')
+    visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr)
+  end
+
+  step 'I should not see any related merge requests' do
+    page.within '.issue-details' do
+      expect(page).not_to have_content('.merge-requests')
+    end
+  end
+
+  step 'I should see the "Enterprise fix" related merge request' do
+    page.within '.merge-requests' do
+      expect(page).to have_content('1 Related Merge Request')
+      expect(page).to have_content('Enterprise fix')
+    end
+  end
+
+  step 'I should see a note linking to "Enterprise fix" merge request' do
+    visible_note(
+      issuable: MergeRequest.find_by(title: 'Enterprise fix'),
+      from_project_name: 'Community',
+      user_name: 'Mary Jane'
+    )
+  end
+
+  step 'I should see a note linking to "Enterprise issue" issue' do
+    visible_note(
+      issuable: Issue.find_by(title: 'Enterprise issue'),
+      from_project_name: 'Community',
+      user_name: 'Mary Jane'
+    )
+  end
+
   step 'I click link "Edit" for the merge request' do
     edit_issuable
   end
@@ -12,4 +105,45 @@ module SharedIssuable
   step 'I click link "Edit" for the issue' do
     edit_issuable
   end
+
+  def create_issuable_for_project(project_name:, title:, type: :issue)
+    project = Project.find_by(name: project_name)
+
+    attrs = {
+      title: title,
+      author: project.users.first,
+      description: '# Description header'
+    }
+
+    case type
+    when :issue
+      attrs.merge!(project: project)
+    when :merge_request
+      attrs.merge!(
+        source_project: project,
+        target_project: project,
+        source_branch: 'fix',
+        target_branch: 'master'
+      )
+    end
+
+    create(type, attrs)
+  end
+
+  def leave_reference_comment(issuable:, from_project_name:)
+    project = Project.find_by(name: from_project_name)
+
+    page.within('.js-main-target-form') do
+      fill_in 'note[note]', with: "##{issuable.to_reference(project)}"
+      click_button 'Add Comment'
+    end
+  end
+
+  def visible_note(issuable:, from_project_name:, user_name:)
+    project = Project.find_by(name: from_project_name)
+
+    expect(page).to have_content(user_name)
+    expect(page).to have_content("mentioned in #{issuable.class.to_s.titleize.downcase} #{issuable.to_reference(project)}")
+  end
+
 end

features/steps/shared/note.rb

@@ -106,6 +106,10 @@ module SharedNote
     end
   end
 
+  step 'I should see no notes at all' do
+    expect(page).to_not have_css('.note')
+  end
+
   # Markdown
 
   step 'I leave a comment with a header containing "Comment with a header"' do

features/steps/shared/project.rb

@@ -161,24 +161,33 @@ module SharedProject
   end
 
   step '"John Doe" owns private project "Enterprise"' do
-    user = user_exists("John Doe", username: "john_doe")
-    project = Project.find_by(name: "Enterprise")
-    project ||= create(:empty_project, name: "Enterprise", namespace: user.namespace)
-    project.team << [user, :master]
+    user_owns_project(
+      user_name: 'John Doe',
+      project_name: 'Enterprise'
+    )
+  end
+
+  step '"Mary Jane" owns private project "Enterprise"' do
+    user_owns_project(
+      user_name: 'Mary Jane',
+      project_name: 'Enterprise'
+    )
   end
 
   step '"John Doe" owns internal project "Internal"' do
-    user = user_exists("John Doe", username: "john_doe")
-    project = Project.find_by(name: "Internal")
-    project ||= create :empty_project, :internal, name: 'Internal', namespace: user.namespace
-    project.team << [user, :master]
+    user_owns_project(
+      user_name: 'John Doe',
+      project_name: 'Internal',
+      visibility: :internal
+    )
   end
 
   step '"John Doe" owns public project "Community"' do
-    user = user_exists("John Doe", username: "john_doe")
-    project = Project.find_by(name: "Community")
-    project ||= create :empty_project, :public, name: 'Community', namespace: user.namespace
-    project.team << [user, :master]
+    user_owns_project(
+      user_name: 'John Doe',
+      project_name: 'Community',
+      visibility: :public
+    )
   end
 
   step 'public empty project "Empty Public Project"' do
@@ -213,4 +222,12 @@ module SharedProject
       expect(page).to have_content("skipped")
     end
   end
+
+  def user_owns_project(user_name:, project_name:, visibility: :private)
+    user = user_exists(user_name, username: user_name.gsub(/\s/, '').underscore)
+    project = Project.find_by(name: project_name)
+    project ||= create(:empty_project, visibility, name: project_name, namespace: user.namespace)
+    project.team << [user, :master]
+  end
+
 end

lib/api/notes.rb

@@ -20,7 +20,19 @@ module API
         #   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}"])
-          present paginate(@noteable.notes), with: Entities::Note
+
+          # 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
         end
 
         # Get a single +noteable+ note
@@ -35,7 +47,12 @@ module API
         get ":id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id" do
           @noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"])
           @note = @noteable.notes.find(params[:note_id])
-          present @note, with: Entities::Note
+
+          if @note.cross_reference_not_visible_for?(current_user)
+            not_found!("Note")
+          else
+            present @note, with: Entities::Note
+          end
         end
 
         # Create a new +noteable+ note

lib/api/users.rb

@@ -284,10 +284,12 @@ module API
         authenticated_as_admin!
         user = User.find_by(id: params[:id])
 
-        if user
+        if !user
+          not_found!('User')
+        elsif !user.ldap_blocked?
           user.block
         else
-          not_found!('User')
+          forbidden!('LDAP blocked users cannot be modified by the API')
         end
       end
 
@@ -299,10 +301,12 @@ module API
         authenticated_as_admin!
         user = User.find_by(id: params[:id])
 
-        if user
-          user.activate
-        else
+        if !user
           not_found!('User')
+        elsif user.ldap_blocked?
+          forbidden!('LDAP blocked users cannot be unblocked by the API')
+        else
+          user.activate
         end
       end
     end

lib/gitlab/ldap/access.rb

@@ -37,15 +37,15 @@ module Gitlab
 
           # Block user in GitLab if he/she was blocked in AD
           if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter)
-            user.block
+            user.ldap_block
             false
           else
-            user.activate if user.blocked? && !ldap_config.block_auto_created_users
+            user.activate if user.ldap_blocked?
             true
           end
         else
           # Block the user if they no longer exist in LDAP/AD
-          user.block 
+          user.ldap_block
           false
         end
       rescue

spec/controllers/admin/identities_controller_spec.rb

+require 'spec_helper'
+
+describe Admin::IdentitiesController do
+  let(:admin) { create(:admin) }
+  before { sign_in(admin) }
+
+  describe 'UPDATE identity' do
+    let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') }
+
+    it 'repairs ldap blocks' do
+      expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute)
+
+      put :update, user_id: user.username, id: user.ldap_identity.id, identity: { provider: 'twitter' }
+    end
+  end
+
+  describe 'DELETE identity' do
+    let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') }
+
+    it 'repairs ldap blocks' do
+      expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute)
+
+      delete :destroy, user_id: user.username, id: user.ldap_identity.id
+    end
+  end
+end

spec/controllers/admin/users_controller_spec.rb

@@ -34,17 +34,34 @@ describe Admin::UsersController do
   end
 
   describe 'PUT unblock/:id' do
-    let(:user) { create(:user) }
-
-    before do
-      user.block
+    context 'ldap blocked users' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain') }
+
+      before do
+        user.ldap_block
+      end
+
+      it 'will not unblock user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_truthy
+        expect(flash[:alert]).to eq 'This user cannot be unlocked manually from GitLab'
+      end
     end
 
-    it 'unblocks user' do
-      put :unblock, id: user.username
-      user.reload
-      expect(user.blocked?).to be_falsey
-      expect(flash[:notice]).to eq 'Successfully unblocked'
+    context 'manually blocked users' do
+      let(:user) { create(:user) }
+
+      before do
+        user.block
+      end
+
+      it 'unblocks user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_falsey
+        expect(flash[:notice]).to eq 'Successfully unblocked'
+      end
     end
   end
 

spec/factories/broadcast_messages.rb

@@ -6,7 +6,6 @@
 #  message    :text             not null
 #  starts_at  :datetime
 #  ends_at    :datetime
-#  alert_type :integer
 #  created_at :datetime
 #  updated_at :datetime
 #  color      :string(255)
@@ -18,10 +17,17 @@
 FactoryGirl.define do
   factory :broadcast_message do
     message "MyText"
-    starts_at "2013-11-12 13:43:25"
-    ends_at "2013-11-12 13:43:25"
-    alert_type 1
-    color "#555555"
-    font "#BBBBBB"
+    starts_at Date.today
+    ends_at Date.tomorrow
+
+    trait :expired do
+      starts_at 5.days.ago
+      ends_at 3.days.ago
+    end
+
+    trait :future do
+      starts_at 5.days.from_now
+      ends_at 6.days.from_now
+    end
   end
 end

spec/helpers/broadcast_messages_helper_spec.rb

 require 'spec_helper'
 
 describe BroadcastMessagesHelper do
-  describe 'broadcast_styling' do
-    let(:broadcast_message) { double(color: '', font: '') }
+  describe 'broadcast_message' do
+    it 'returns nil when no current message' do
+      expect(helper.broadcast_message(nil)).to be_nil
+    end
+
+    it 'includes the current message' do
+      current = double(message: 'Current Message')
+
+      allow(helper).to receive(:broadcast_message_style).and_return(nil)
+
+      expect(helper.broadcast_message(current)).to include 'Current Message'
+    end
+
+    it 'includes custom style' do
+      current = double(message: 'Current Message')
+
+      allow(helper).to receive(:broadcast_message_style).and_return('foo')
+
+      expect(helper.broadcast_message(current)).to include 'style="foo"'
+    end
+  end
+
+  describe 'broadcast_message_style' do
+    it 'defaults to no style' do
+      broadcast_message = spy
+
+      expect(helper.broadcast_message_style(broadcast_message)).to eq ''
+    end
+
+    it 'allows custom style' do
+      broadcast_message = double(color: '#f2dede', font: '#b94a48')
+
+      expect(helper.broadcast_message_style(broadcast_message)).
+        to match('background-color: #f2dede; color: #b94a48')
+    end
+  end
+
+  describe 'broadcast_message_status' do
+    it 'returns Active' do
+      message = build(:broadcast_message)
+
+      expect(helper.broadcast_message_status(message)).to eq 'Active'
+    end
+
+    it 'returns Expired' do
+      message = build(:broadcast_message, :expired)
 
-    context "default style" do
-      it "should have no style" do
-        expect(broadcast_styling(broadcast_message)).to eq ''
-      end
+      expect(helper.broadcast_message_status(message)).to eq 'Expired'
     end
 
-    context "customized style" do
-      let(:broadcast_message) { double(color: "#f2dede", font: '#b94a48') }
+    it 'returns Pending' do
+      message = build(:broadcast_message, :future)
 
-      it "should have a customized style" do
-        expect(broadcast_styling(broadcast_message)).
-          to match('background-color: #f2dede; color: #b94a48')
-      end
+      expect(helper.broadcast_message_status(message)).to eq 'Pending'
     end
   end
 end

spec/lib/gitlab/ldap/access_spec.rb

@@ -13,64 +13,58 @@ describe Gitlab::LDAP::Access, lib: true do
       end
 
       it { is_expected.to be_falsey }
-      
+
       it 'should block user in GitLab' do
         access.allowed?
         expect(user).to be_blocked
+        expect(user).to be_ldap_blocked
       end
     end
 
     context 'when the user is found' do
       before do
-        allow(Gitlab::LDAP::Person).
-          to receive(:find_by_dn).and_return(:ldap_user)
+        allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user)
       end
 
       context 'and the user is disabled via active directory' do
         before do
-          allow(Gitlab::LDAP::Person).
-            to receive(:disabled_via_active_directory?).and_return(true)
+          allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true)
         end
 
         it { is_expected.to be_falsey }
 
-        it "should block user in GitLab" do
+        it 'should block user in GitLab' do
           access.allowed?
           expect(user).to be_blocked
+          expect(user).to be_ldap_blocked
         end
       end
 
       context 'and has no disabled flag in active diretory' do
         before do
-          user.block
-
-          allow(Gitlab::LDAP::Person).
-            to receive(:disabled_via_active_directory?).and_return(false)
+          allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false)
         end
 
         it { is_expected.to be_truthy }
 
         context 'when auto-created users are blocked' do
-
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).
-              to receive(:block_auto_created_users).and_return(true)
+            user.block
           end
 
-          it "does not unblock user in GitLab" do
+          it 'does not unblock user in GitLab' do
             access.allowed?
             expect(user).to be_blocked
+            expect(user).not_to be_ldap_blocked # this block is handled by omniauth not by our internal logic
           end
         end
 
-        context "when auto-created users are not blocked" do
-
+        context 'when auto-created users are not blocked' do
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).
-              to receive(:block_auto_created_users).and_return(false)
+            user.ldap_block
           end
 
-          it "should unblock user in GitLab" do
+          it 'should unblock user in GitLab' do
             access.allowed?
             expect(user).not_to be_blocked
           end
@@ -80,8 +74,7 @@ describe Gitlab::LDAP::Access, lib: true do
       context 'without ActiveDirectory enabled' do
         before do
           allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
-          allow_any_instance_of(Gitlab::LDAP::Config).
-            to receive(:active_directory).and_return(false)
+          allow_any_instance_of(Gitlab::LDAP::Config).to receive(:active_directory).and_return(false)
         end
 
         it { is_expected.to be_truthy }

spec/models/broadcast_message_spec.rb

@@ -6,7 +6,6 @@
 #  message    :text             not null
 #  starts_at  :datetime
 #  ends_at    :datetime
-#  alert_type :integer
 #  created_at :datetime
 #  updated_at :datetime
 #  color      :string(255)
@@ -16,6 +15,8 @@
 require 'spec_helper'
 
 describe BroadcastMessage, models: true do
+  include ActiveSupport::Testing::TimeHelpers
+
   subject { create(:broadcast_message) }
 
   it { is_expected.to be_valid }
@@ -35,20 +36,79 @@ describe BroadcastMessage, models: true do
     it { is_expected.not_to allow_value('000').for(:font) }
   end
 
-  describe :current do
+  describe '.current' do
     it "should return last message if time match" do
-      broadcast_message = create(:broadcast_message, starts_at: Time.now.yesterday, ends_at: Time.now.tomorrow)
-      expect(BroadcastMessage.current).to eq(broadcast_message)
+      message = create(:broadcast_message)
+
+      expect(BroadcastMessage.current).to eq message
     end
 
     it "should return nil if time not come" do
-      create(:broadcast_message, starts_at: Time.now.tomorrow, ends_at: Time.now + 2.days)
+      create(:broadcast_message, :future)
+
       expect(BroadcastMessage.current).to be_nil
     end
 
     it "should return nil if time has passed" do
-      create(:broadcast_message, starts_at: Time.now - 2.days, ends_at: Time.now.yesterday)
+      create(:broadcast_message, :expired)
+
       expect(BroadcastMessage.current).to be_nil
     end
   end
+
+  describe '#active?' do
+    it 'is truthy when started and not ended' do
+      message = build(:broadcast_message)
+
+      expect(message).to be_active
+    end
+
+    it 'is falsey when ended' do
+      message = build(:broadcast_message, :expired)
+
+      expect(message).not_to be_active
+    end
+
+    it 'is falsey when not started' do
+      message = build(:broadcast_message, :future)
+
+      expect(message).not_to be_active
+    end
+  end
+
+  describe '#started?' do
+    it 'is truthy when starts_at has passed' do
+      message = build(:broadcast_message)
+
+      travel_to(3.days.from_now) do
+        expect(message).to be_started
+      end
+    end
+
+    it 'is falsey when starts_at is in the future' do
+      message = build(:broadcast_message)
+
+      travel_to(3.days.ago) do
+        expect(message).not_to be_started
+      end
+    end
+  end
+
+  describe '#ended?' do
+    it 'is truthy when ends_at has passed' do
+      message = build(:broadcast_message)
+
+      travel_to(3.days.from_now) do
+        expect(message).to be_ended
+      end
+    end
+
+    it 'is falsey when ends_at is in the future' do
+      message = build(:broadcast_message)
+
+      travel_to(3.days.ago) do
+        expect(message).not_to be_ended
+      end
+    end
+  end
 end

spec/models/identity_spec.rb

+# == Schema Information
+#
+# Table name: identities
+#
+#  id         :integer          not null, primary key
+#  extern_uid :string(255)
+#  provider   :string(255)
+#  user_id    :integer
+#  created_at :datetime
+#  updated_at :datetime
+#
+
+require 'spec_helper'
+
+RSpec.describe Identity, models: true do
+
+  describe 'relations' do
+    it { is_expected.to belong_to(:user) }
+  end
+
+  describe 'fields' do
+    it { is_expected.to respond_to(:provider) }
+    it { is_expected.to respond_to(:extern_uid) }
+  end
+
+  describe '#is_ldap?' do
+    let(:ldap_identity) { create(:identity, provider: 'ldapmain') }
+    let(:other_identity) { create(:identity, provider: 'twitter') }
+
+    it 'returns true if it is a ldap identity' do
+      expect(ldap_identity.ldap?).to be_truthy
+    end
+
+    it 'returns false if it is not a ldap identity' do
+      expect(other_identity.ldap?).to be_falsey
+    end
+  end
+end

spec/models/note_spec.rb

@@ -178,6 +178,30 @@ describe Note, models: true do
     end
   end
 
+  describe "cross_reference_not_visible_for?" do
+    let(:private_user)    { create(:user) }
+    let(:private_project) { create(:project, namespace: private_user.namespace).tap { |p| p.team << [private_user, :master] } }
+    let(:private_issue)   { create(:issue, project: private_project) }
+
+    let(:ext_proj)  { create(:project, :public) }
+    let(:ext_issue) { create(:issue, project: ext_proj) }
+
+    let(:note) do
+      create :note,
+        noteable: ext_issue, project: ext_proj,
+        note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
+        system: true
+    end
+
+    it "returns true" do
+      expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
+    end
+
+    it "returns false" do
+      expect(note.cross_reference_not_visible_for?(private_user)).to be_falsy
+    end
+  end
+
   describe "set_award!" do
     let(:issue) { create :issue }
 

spec/models/user_spec.rb

@@ -569,27 +569,39 @@ describe User, models: true do
     end
   end
 
-  describe :ldap_user? do
-    it "is true if provider name starts with ldap" do
-      user = create(:omniauth_user, provider: 'ldapmain')
-      expect( user.ldap_user? ).to be_truthy
-    end
+  context 'ldap synchronized user' do
+    describe :ldap_user? do
+      it 'is true if provider name starts with ldap' do
+        user = create(:omniauth_user, provider: 'ldapmain')
+        expect(user.ldap_user?).to be_truthy
+      end
 
-    it "is false for other providers" do
-      user = create(:omniauth_user, provider: 'other-provider')
-      expect( user.ldap_user? ).to be_falsey
+      it 'is false for other providers' do
+        user = create(:omniauth_user, provider: 'other-provider')
+        expect(user.ldap_user?).to be_falsey
+      end
+
+      it 'is false if no extern_uid is provided' do
+        user = create(:omniauth_user, extern_uid: nil)
+        expect(user.ldap_user?).to be_falsey
+      end
     end
 
-    it "is false if no extern_uid is provided" do
-      user = create(:omniauth_user, extern_uid: nil)
-      expect( user.ldap_user? ).to be_falsey
+    describe :ldap_identity do
+      it 'returns ldap identity' do
+        user = create :omniauth_user
+        expect(user.ldap_identity.provider).not_to be_empty
+      end
     end
-  end
 
-  describe :ldap_identity do
-    it "returns ldap identity" do
-      user = create :omniauth_user
-      expect(user.ldap_identity.provider).not_to be_empty
+    describe '#ldap_block' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain', name: 'John Smith') }
+
+      it 'blocks user flaging the action caming from ldap' do
+        user.ldap_block
+        expect(user.blocked?).to be_truthy
+        expect(user.ldap_blocked?).to be_truthy
+      end
     end
   end
 

spec/requests/api/notes_spec.rb

@@ -10,6 +10,25 @@ describe API::API, api: true  do
   let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) }
   let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) }
   let!(:snippet_note) { create(:note, noteable: snippet, project: project, author: user) }
+
+  # For testing the cross-reference of a private issue in a public issue
+  let(:private_user)    { create(:user) }
+  let(:private_project) do
+    create(:project, namespace: private_user.namespace).
+    tap { |p| p.team << [private_user, :master] }
+  end
+  let(:private_issue)    { create(:issue, project: private_project) }
+
+  let(:ext_proj)  { create(:project, :public) }
+  let(:ext_issue) { create(:issue, project: ext_proj) }
+
+  let!(:cross_reference_note) do
+    create :note,
+    noteable: ext_issue, project: ext_proj,
+    note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
+    system: true
+  end
+
   before { project.team << [user, :reporter] }
 
   describe "GET /projects/:id/noteable/:noteable_id/notes" do
@@ -25,6 +44,24 @@ describe API::API, api: true  do
         get api("/projects/#{project.id}/issues/123/notes", user)
         expect(response.status).to eq(404)
       end
+
+      context "that references a private issue" 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 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)
+          end
+        end
+      end
     end
 
     context "when noteable is a Snippet" do
@@ -68,6 +105,21 @@ describe API::API, api: true  do
         get api("/projects/#{project.id}/issues/#{issue.id}/notes/123", user)
         expect(response.status).to eq(404)
       end
+
+      context "that references a private issue" 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 "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
+        end
+      end
     end
 
     context "when noteable is a Snippet" do

spec/requests/api/users_spec.rb

@@ -8,6 +8,8 @@ describe API::API, api: true  do
   let(:key)   { create(:key, user: user) }
   let(:email)   { create(:email, user: user) }
   let(:omniauth_user) { create(:omniauth_user) }
+  let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') }
+  let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
 
   describe "GET /users" do
     context "when unauthenticated" do
@@ -783,6 +785,12 @@ describe API::API, api: true  do
       expect(user.reload.state).to eq('blocked')
     end
 
+    it 'should not re-block ldap blocked users' do
+      put api("/users/#{ldap_blocked_user.id}/block", admin)
+      expect(response.status).to eq(403)
+      expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
+    end
+
     it 'should not be available for non admin users' do
       put api("/users/#{user.id}/block", user)
       expect(response.status).to eq(403)
@@ -797,7 +805,9 @@ describe API::API, api: true  do
   end
 
   describe 'PUT /user/:id/unblock' do
+    let(:blocked_user)  { create(:user, state: 'blocked') }
     before { admin }
+
     it 'should unblock existing user' do
       put api("/users/#{user.id}/unblock", admin)
       expect(response.status).to eq(200)
@@ -805,12 +815,15 @@ describe API::API, api: true  do
     end
 
     it 'should unblock a blocked user' do
-      put api("/users/#{user.id}/block", admin)
-      expect(response.status).to eq(200)
-      expect(user.reload.state).to eq('blocked')
-      put api("/users/#{user.id}/unblock", admin)
+      put api("/users/#{blocked_user.id}/unblock", admin)
       expect(response.status).to eq(200)
-      expect(user.reload.state).to eq('active')
+      expect(blocked_user.reload.state).to eq('active')
+    end
+
+    it 'should not unblock ldap blocked users' do
+      put api("/users/#{ldap_blocked_user.id}/unblock", admin)
+      expect(response.status).to eq(403)
+      expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
     end
 
     it 'should not be available for non admin users' do

spec/services/repair_ldap_blocked_user_service_spec.rb

+require 'spec_helper'
+
+describe RepairLdapBlockedUserService, services: true do
+  let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
+  let(:identity) { user.ldap_identity }
+  subject(:service) { RepairLdapBlockedUserService.new(user) }
+
+  describe '#execute' do
+    it 'change to normal block after destroying last ldap identity' do
+      identity.destroy
+      service.execute
+
+      expect(user.reload).not_to be_ldap_blocked
+    end
+
+    it 'change to normal block after changing last ldap identity to another provider' do
+      identity.update_attribute(:provider, 'twitter')
+      service.execute
+
+      expect(user.reload).not_to be_ldap_blocked
+    end
+  end
+end