Merge branch 'master' into ci/api-builds

* master: (51 commits)
  Fix version
  Fix specs and rubocop warnings
  Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages
  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
  Update CHANGELOG [ci skip]
  Add some cosmetic changes to variables API documentation [ci skip]
  Fix misaligned edit button in milestone collection partial
  Update button styles for Milestones#show
  Modify builds API documentation style [ci skip]
  Modify :ci_variable factory
  Ensure the API doesn't return notes that the current user shouldn't see
  Add 'Build' prefix to Variables entry name in API docs index
  Fix some typos
  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
  ...

Conflicts:
	doc/api/README.md
	lib/api/api.rb
	lib/api/entities.rb

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.4.0 (unreleased)
+  - Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages
   - Autocomplete data is now always loaded, instead of when focusing a comment text area (Yorick Peterse)
   - Improved performance of finding issues for an entire group (Yorick Peterse)
   - Added custom application performance measuring system powered by InfluxDB (Yorick Peterse)
@@ -42,8 +43,11 @@ 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 builds of a project
+  - Add API support for managing build variables of project
+  - 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/assets/stylesheets/pages/branches.scss

+.branch-name{
+  font-weight: 600;
+}

app/assets/stylesheets/pages/commit.scss

@@ -2,6 +2,10 @@
   display: block;
 }
 
+.commit-row-title .commit-title {
+  font-weight: 600;
+}
+
 .commit-author, .commit-committer{
   display: block;
   color: #999;

app/assets/stylesheets/pages/groups.scss

@@ -11,3 +11,8 @@
     height: 42px;
   }
 }
+
+.content-list .group-name {
+  font-weight: 600;
+  color: #4c4e54;
+}

app/assets/stylesheets/pages/issues.scss

@@ -6,7 +6,7 @@
     .issue-title {
       margin-bottom: 5px;
       font-size: $list-font-size;
-      font-weight: bold;
+      font-weight: 600;
     }
 
     .issue-info {

app/assets/stylesheets/pages/merge_requests.scss

@@ -150,7 +150,7 @@
     .merge-request-title {
       margin-bottom: 5px;
       font-size: $list-font-size;
-      font-weight: bold;
+      font-weight: 600;
     }
 
     .merge-request-info {

app/assets/stylesheets/pages/tags.scss

+.tag-name{
+  font-weight: 600;
+}

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_messages = BroadcastMessage.reorder("starts_at ASC").page(params[:page])
     @broadcast_message  = BroadcastMessage.new
   end
 
+  def edit
+  end
+
   def create
     @broadcast_message = BroadcastMessage.new(broadcast_message_params)
 
@@ -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/ci/variable.rb

@@ -18,8 +18,12 @@ module Ci
     
     belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id
 
-    validates_presence_of :key
     validates_uniqueness_of :key, scope: :gl_project_id
+    validates :key,
+      presence: true,
+      length: { within: 0..255 },
+      format: { with: /\A[a-zA-Z0-9_]+\z/,
+                message: "can contain only letters, digits and '_'." }
 
     attr_encrypted :value, mode: :per_attribute_iv_and_salt, key: Gitlab::Application.secrets.db_key_base
   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"
+              .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.blocked?
-                  = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success"
+                  - 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 btn-xs btn-warning"
+                    = 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 btn-xs btn-success", data: { confirm: 'Are you sure?' }
+                    = 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 btn-xs btn-remove"
+                    = 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/branches/_branch.html.haml

@@ -6,7 +6,7 @@
 %li(class="js-branch-#{branch.name}")
   %div
     = link_to namespace_project_tree_path(@project.namespace, @project, branch.name) do
-      %strong.str-truncated= branch.name
+      .branch-name.str-truncated= branch.name
      
     - if branch.name == @repository.root_ref
       %span.label.label-primary default

app/views/projects/commits/_commit.html.haml

@@ -11,7 +11,7 @@
 = cache(cache_key) do
   %li.commit.js-toggle-container{ id: "commit-#{commit.short_id}" }
     .commit-row-title
-      %strong.str-truncated
+      .commit-title.str-truncated
         = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-row-message"
         - if commit.description?
           %a.text-expander.js-toggle-button ...

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

app/views/projects/tags/_tag.html.haml

@@ -3,7 +3,7 @@
 %li
   %div
     = link_to namespace_project_tag_path(@project.namespace, @project, tag.name) do
-      %strong
+      .tag-name
         = icon('tag')
         = tag.name
     - if tag.message.present?

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

@@ -17,8 +17,8 @@
       .pull-right
         = link_to namespace_project_tag_path(@project.namespace, @project, @tag.name), class: 'btn btn-remove remove-row grouped has_tooltip', title: "Delete tag", method: :delete, data: { confirm: "Deleting the '#{@tag.name}' tag cannot be undone. Are you sure?" } do
           %i.fa.fa-trash-o
-  .title
-    %strong= @tag.name
+  .tag-name.title
+    = @tag.name
     - if @tag.message.present?
       %span.light
          

app/views/shared/groups/_group.html.haml

@@ -10,8 +10,7 @@
         %i.fa.fa-sign-out
 
   = image_tag group_icon(group), class: "avatar s46 hidden-xs"
-  = link_to group, class: 'group-name' do
-    %strong= group.name
+  = link_to group.name, group, class: 'group-name'
 
   - if group_member
     as

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/README.md

@@ -24,6 +24,7 @@
 - [Settings](settings.md)
 - [Keys](keys.md)
 - [Builds](builds.md)
+- [Build Variables](build_variables.md)
 
 ## Clients
 

doc/api/build_variables.md

+# Build Variables
+
+## List project variables
+
+Get list of a project's build variables.
+
+```
+GET /projects/:id/variables
+```
+
+| Attribute | Type    | required | Description         |
+|-----------|---------|----------|---------------------|
+| `id`      | integer | yes      | The ID of a project |
+
+```
+curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables"
+```
+
+```json
+[
+    {
+        "key": "TEST_VARIABLE_1",
+        "value": "TEST_1"
+    },
+    {
+        "key": "TEST_VARIABLE_2",
+        "value": "TEST_2"
+    }
+]
+```
+
+## Show variable details
+
+Get the details of a project's specific build variable.
+
+```
+GET /projects/:id/variables/:key
+```
+
+| Attribute | Type    | required | Description           |
+|-----------|---------|----------|-----------------------|
+| `id`      | integer | yes      | The ID of a project   |
+| `key`     | string  | yes      | The `key` of a variable |
+
+```
+curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/TEST_VARIABLE_1"
+```
+
+```json
+{
+    "key": "TEST_VARIABLE_1",
+    "value": "TEST_1"
+}
+```
+
+## Create variable
+
+Create a new build variable.
+
+```
+POST /projects/:id/variables
+```
+
+| Attribute | Type    | required | Description           |
+|-----------|---------|----------|-----------------------|
+| `id`      | integer | yes      | The ID of a project   |
+| `key`     | string  | yes      | The `key` of a variable; must have no more than 255 characters; only `A-Z`, `a-z`, `0-9`, and `_` are allowed |
+| `value`   | string  | yes      | The `value` of a variable |
+
+```
+curl -X POST -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables" -F "key=NEW_VARIABLE" -F "value=new value"
+```
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "new value"
+}
+```
+
+## Update variable
+
+Update a project's build variable.
+
+```
+PUT /projects/:id/variables/:key
+```
+
+| Attribute | Type    | required | Description             |
+|-----------|---------|----------|-------------------------|
+| `id`      | integer | yes      | The ID of a project     |
+| `key`     | string  | yes      | The `key` of a variable   |
+| `value`   | string  | yes      | The `value` of a variable |
+
+```
+curl -X PUT -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/NEW_VARIABLE" -F "value=updated value"
+```
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "updated value"
+}
+```
+
+## Remove variable
+
+Remove a project's build variable.
+
+```
+DELETE /projects/:id/variables/:key
+```
+
+| Attribute | Type    | required | Description             |
+|-----------|---------|----------|-------------------------|
+| `id`      | integer | yes      | The ID of a project     |
+| `key`     | string  | yes      | The `key` of a variable |
+
+```
+curl -X DELETE -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/VARIABLE_1"
+```
+
+```json
+{
+    "key": "VARIABLE_1",
+    "value": "VALUE_1"
+}
+```

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/api.rb

@@ -55,5 +55,6 @@ module API
     mount Tags
     mount Triggers
     mount Builds
+    mount Variables
   end
 end

lib/api/entities.rb

@@ -393,5 +393,9 @@ module API
       end
       expose :runner, with: Runner
     end
+
+    class Variable < Grape::Entity
+      expose :key, :value
+    end
   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,8 +47,13 @@ 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])
+
+          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/api/variables.rb

+module API
+  # Projects variables API
+  class Variables < Grape::API
+    before { authenticate! }
+    before { authorize_admin_project }
+
+    resource :projects do
+      # Get project variables
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   page (optional) - The page number for pagination
+      #   per_page (optional) - The value of items per page to show
+      # Example Request:
+      #   GET /projects/:id/variables
+      get ':id/variables' do
+        variables = user_project.variables
+        present paginate(variables), with: Entities::Variable
+      end
+
+      # Get specific variable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (required) - The `key` of variable
+      # Example Request:
+      #   GET /projects/:id/variables/:key
+      get ':id/variables/:key' do
+        key = params[:key]
+        variable = user_project.variables.find_by(key: key.to_s)
+
+        return not_found!('Variable') unless variable
+
+        present variable, with: Entities::Variable
+      end
+
+      # Create a new variable in project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (required) - The key of variable
+      #   value (required) - The value of variable
+      # Example Request:
+      #   POST /projects/:id/variables
+      post ':id/variables' do
+        required_attributes! [:key, :value]
+
+        variable = user_project.variables.create(key: params[:key], value: params[:value])
+
+        if variable.valid?
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
+        end
+      end
+
+      # Update existing variable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (optional) - The `key` of variable
+      #   value (optional) - New value for `value` field of variable
+      # Example Request:
+      #   PUT /projects/:id/variables/:key
+      put ':id/variables/:key' do
+        variable = user_project.variables.find_by(key: params[:key].to_s)
+
+        return not_found!('Variable') unless variable
+
+        attrs = attributes_for_keys [:value]
+        if variable.update(attrs)
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
+        end
+      end
+
+      # Delete existing variable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (required) - The ID of a variable
+      # Example Request:
+      #   DELETE /projects/:id/variables/:key
+      delete ':id/variables/:key' do
+        variable = user_project.variables.find_by(key: params[:key].to_s)
+
+        return not_found!('Variable') unless variable
+        variable.destroy
+
+        present variable, with: Entities::Variable
+      end
+    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,6 +34,22 @@ describe Admin::UsersController do
   end
 
   describe 'PUT unblock/:id' do
+    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
+
+    context 'manually blocked users' do
       let(:user) { create(:user) }
 
       before do
@@ -47,6 +63,7 @@ describe Admin::UsersController do
         expect(flash[:notice]).to eq 'Successfully unblocked'
       end
     end
+  end
 
   describe 'PUT unlock/:id' do
     let(:user) { create(:user) }

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/factories/ci/variables.rb

+# == Schema Information
+#
+# Table name: ci_variables
+#
+#  id                   :integer          not null, primary key
+#  project_id           :integer          not null
+#  key                  :string(255)
+#  value                :text
+#  encrypted_value      :text
+#  encrypted_value_salt :string(255)
+#  encrypted_value_iv   :string(255)
+#  gl_project_id        :integer
+#
+
+# Read about factories at https://github.com/thoughtbot/factory_girl
+
+FactoryGirl.define do
+  factory :ci_variable, class: Ci::Variable do
+    sequence(:key) { |n| "VARIABLE_#{n}" }
+    value 'VARIABLE_VALUE'
+  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
 
-    context "default style" do
-      it "should have no style" do
-        expect(broadcast_styling(broadcast_message)).to eq ''
+    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
 
-    context "customized style" do
-      let(:broadcast_message) { double(color: "#f2dede", font: '#b94a48') }
+    it 'allows custom style' do
+      broadcast_message = double(color: '#f2dede', font: '#b94a48')
 
-      it "should have a customized style" do
-        expect(broadcast_styling(broadcast_message)).
+      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)
+
+      expect(helper.broadcast_message_status(message)).to eq 'Expired'
+    end
+
+    it 'returns Pending' do
+      message = build(:broadcast_message, :future)
+
+      expect(helper.broadcast_message_status(message)).to eq 'Pending'
+    end
   end
 end

spec/lib/gitlab/ldap/access_spec.rb

@@ -17,60 +17,54 @@ describe Gitlab::LDAP::Access, lib: true do
       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,30 +569,42 @@ describe User, models: true do
     end
   end
 
+  context 'ldap synchronized user' do
     describe :ldap_user? do
-    it "is true if provider name starts with ldap" do
+      it 'is true if provider name starts with ldap' do
         user = create(:omniauth_user, provider: 'ldapmain')
-      expect( user.ldap_user? ).to be_truthy
+        expect(user.ldap_user?).to be_truthy
       end
 
-    it "is false for other providers" do
+      it 'is false for other providers' do
         user = create(:omniauth_user, provider: 'other-provider')
-      expect( user.ldap_user? ).to be_falsey
+        expect(user.ldap_user?).to be_falsey
       end
 
-    it "is false if no extern_uid is provided" do
+      it 'is false if no extern_uid is provided' do
         user = create(:omniauth_user, extern_uid: nil)
-      expect( user.ldap_user? ).to be_falsey
+        expect(user.ldap_user?).to be_falsey
       end
     end
 
     describe :ldap_identity do
-    it "returns ldap identity" do
+      it 'returns ldap identity' do
         user = create :omniauth_user
         expect(user.ldap_identity.provider).not_to be_empty
       end
     end
 
+    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
+
   describe '#full_website_url' do
     let(:user) { create(:user) }
 

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/requests/api/variables_spec.rb

+require 'spec_helper'
+
+describe API::API, api: true do
+  include ApiHelpers
+
+  let(:user) { create(:user) }
+  let(:user2) { create(:user) }
+  let!(:project) { create(:project, creator_id: user.id) }
+  let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) }
+  let!(:developer) { create(:project_member, user: user2, project: project, access_level: ProjectMember::DEVELOPER) }
+  let!(:variable) { create(:ci_variable, project: project) }
+
+  describe 'GET /projects/:id/variables' do
+    context 'authorized user with proper permissions' do
+      it 'should return project variables' do
+        get api("/projects/#{project.id}/variables", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response).to be_a(Array)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not return project variables' do
+        get api("/projects/#{project.id}/variables", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not return project variables' do
+        get api("/projects/#{project.id}/variables")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe 'GET /projects/:id/variables/:key' do
+    context 'authorized user with proper permissions' do
+      it 'should return project variable details' do
+        get api("/projects/#{project.id}/variables/#{variable.key}", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response['value']).to eq(variable.value)
+      end
+
+      it 'should respond with 404 Not Found if requesting non-existing variable' do
+        get api("/projects/#{project.id}/variables/non_existing_variable", user)
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not return project variable details' do
+        get api("/projects/#{project.id}/variables/#{variable.key}", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not return project variable details' do
+        get api("/projects/#{project.id}/variables/#{variable.key}")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe 'POST /projects/:id/variables' do
+    context 'authorized user with proper permissions' do
+      it 'should create variable' do
+        expect do
+          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2'
+        end.to change{project.variables.count}.by(1)
+
+        expect(response.status).to eq(201)
+        expect(json_response['key']).to eq('TEST_VARIABLE_2')
+        expect(json_response['value']).to eq('VALUE_2')
+      end
+
+      it 'should not allow to duplicate variable key' do
+        expect do
+          post api("/projects/#{project.id}/variables", user), key: variable.key, value: 'VALUE_2'
+        end.to change{project.variables.count}.by(0)
+
+        expect(response.status).to eq(400)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not create variable' do
+        post api("/projects/#{project.id}/variables", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not create variable' do
+        post api("/projects/#{project.id}/variables")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe 'PUT /projects/:id/variables/:key' do
+    context 'authorized user with proper permissions' do
+      it 'should update variable data' do
+        initial_variable = project.variables.first
+        value_before = initial_variable.value
+
+        put api("/projects/#{project.id}/variables/#{variable.key}", user), value: 'VALUE_1_UP'
+
+        updated_variable = project.variables.first
+
+        expect(response.status).to eq(200)
+        expect(value_before).to eq(variable.value)
+        expect(updated_variable.value).to eq('VALUE_1_UP')
+      end
+
+      it 'should responde with 404 Not Found if requesting non-existing variable' do
+        put api("/projects/#{project.id}/variables/non_existing_variable", user)
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not update variable' do
+        put api("/projects/#{project.id}/variables/#{variable.key}", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not update variable' do
+        put api("/projects/#{project.id}/variables/#{variable.key}")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe 'DELETE /projects/:id/variables/:key' do
+    context 'authorized user with proper permissions' do
+      it 'should delete variable' do
+        expect do
+          delete api("/projects/#{project.id}/variables/#{variable.key}", user)
+        end.to change{project.variables.count}.by(-1)
+        expect(response.status).to eq(200)
+      end
+
+      it 'should responde with 404 Not Found if requesting non-existing variable' do
+        delete api("/projects/#{project.id}/variables/non_existing_variable", user)
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not delete variable' do
+        delete api("/projects/#{project.id}/variables/#{variable.key}", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not delete variable' do
+        delete api("/projects/#{project.id}/variables/#{variable.key}")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+end

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