summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTomasz Maczukin <tomasz@maczukin.pl>2016-01-14 12:24:20 +0100
committerTomasz Maczukin <tomasz@maczukin.pl>2016-01-14 12:24:20 +0100
commit8eed187677d656e2a11eaa2484618e50973c0a67 (patch)
tree3bf5dd1511d57ea539256efa6765860d2fddd62b
parentc5b429f099d8b6b71225a96f111d65c97f15d2a8 (diff)
parent4d64a32c88dd5f87621d391c0f10f6acef094073 (diff)
downloadgitlab-ce-8eed187677d656e2a11eaa2484618e50973c0a67.tar.gz
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 ...
-rw-r--r--CHANGELOG2
-rw-r--r--app/assets/javascripts/admin.js.coffee10
-rw-r--r--app/assets/stylesheets/framework/buttons.scss6
-rw-r--r--app/assets/stylesheets/framework/forms.scss4
-rw-r--r--app/controllers/admin/broadcast_messages_controller.rb33
-rw-r--r--app/controllers/admin/identities_controller.rb2
-rw-r--r--app/controllers/admin/users_controller.rb4
-rw-r--r--app/controllers/projects/issues_controller.rb2
-rw-r--r--app/helpers/application_helper.rb4
-rw-r--r--app/helpers/broadcast_messages_helper.rb30
-rw-r--r--app/models/broadcast_message.rb18
-rw-r--r--app/models/identity.rb4
-rw-r--r--app/models/issue.rb4
-rw-r--r--app/models/note.rb4
-rw-r--r--app/models/project.rb2
-rw-r--r--app/models/user.rb14
-rw-r--r--app/services/repair_ldap_blocked_user_service.rb17
-rw-r--r--app/views/admin/broadcast_messages/_form.html.haml37
-rw-r--r--app/views/admin/broadcast_messages/edit.html.haml3
-rw-r--r--app/views/admin/broadcast_messages/index.html.haml79
-rw-r--r--app/views/admin/users/index.html.haml25
-rw-r--r--app/views/layouts/_broadcast.html.haml5
-rw-r--r--app/views/projects/milestones/_milestone.html.haml7
-rw-r--r--app/views/projects/milestones/show.html.haml12
-rw-r--r--app/views/projects/notes/_notes.html.haml4
-rw-r--r--config/routes.rb2
-rw-r--r--db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb5
-rw-r--r--db/schema.rb1
-rw-r--r--doc/api/users.md6
-rw-r--r--doc/install/installation.md8
-rw-r--r--features/admin/broadcast_messages.feature20
-rw-r--r--features/project/issues/references.feature33
-rw-r--r--features/project/merge_requests/references.feature31
-rw-r--r--features/steps/admin/broadcast_messages.rb37
-rw-r--r--features/steps/project/issues/references.rb7
-rw-r--r--features/steps/project/merge_requests/references.rb7
-rw-r--r--features/steps/shared/issuable.rb134
-rw-r--r--features/steps/shared/note.rb4
-rw-r--r--features/steps/shared/project.rb41
-rw-r--r--lib/api/notes.rb21
-rw-r--r--lib/api/users.rb14
-rw-r--r--lib/gitlab/ldap/access.rb6
-rw-r--r--spec/controllers/admin/identities_controller_spec.rb26
-rw-r--r--spec/controllers/admin/users_controller_spec.rb35
-rw-r--r--spec/factories/broadcast_messages.rb18
-rw-r--r--spec/helpers/broadcast_messages_helper_spec.rb62
-rw-r--r--spec/lib/gitlab/ldap/access_spec.rb35
-rw-r--r--spec/models/broadcast_message_spec.rb72
-rw-r--r--spec/models/identity_spec.rb38
-rw-r--r--spec/models/note_spec.rb24
-rw-r--r--spec/models/user_spec.rb44
-rw-r--r--spec/requests/api/notes_spec.rb52
-rw-r--r--spec/requests/api/users_spec.rb23
-rw-r--r--spec/services/repair_ldap_blocked_user_service_spec.rb23
54 files changed, 931 insertions, 230 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 593d3e1c0cd..c33727c76f6 100644
--- a/CHANGELOG
+++ b/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)
diff --git a/app/assets/javascripts/admin.js.coffee b/app/assets/javascripts/admin.js.coffee
index bcb2e6df7c0..eb951f71711 100644
--- a/app/assets/javascripts/admin.js.coffee
+++ b/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) ->
diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss
index 97a94638847..bb29829b7a1 100644
--- a/app/assets/stylesheets/framework/buttons.scss
+++ b/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;
}
}
diff --git a/app/assets/stylesheets/framework/forms.scss b/app/assets/stylesheets/framework/forms.scss
index 032d343df44..d0cb91a5b64 100644
--- a/app/assets/stylesheets/framework/forms.scss
+++ b/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;
}
diff --git a/app/controllers/admin/broadcast_messages_controller.rb b/app/controllers/admin/broadcast_messages_controller.rb
index 497c34f8f49..4735b27c65d 100644
--- a/app/controllers/admin/broadcast_messages_controller.rb
+++ b/app/controllers/admin/broadcast_messages_controller.rb
@@ -1,8 +1,12 @@
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
diff --git a/app/controllers/admin/identities_controller.rb b/app/controllers/admin/identities_controller.rb
index e383fe38ea6..79a53556f0a 100644
--- a/app/controllers/admin/identities_controller.rb
+++ b/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.'
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index d7c927d444c..87f4fb455b8 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/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")
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index b59b52291fb..f476afb2d92 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/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
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 436fbcd4138..f3a2723ee0d 100644
--- a/app/helpers/application_helper.rb
+++ b/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
diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index 6484dca6b55..1ed8c710f77 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -1,16 +1,34 @@
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
diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index ad514706160..61119633717 100644
--- a/app/models/broadcast_message.rb
+++ b/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
diff --git a/app/models/identity.rb b/app/models/identity.rb
index 8bcdc194953..e1915b079d4 100644
--- a/app/models/identity.rb
+++ b/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
diff --git a/app/models/issue.rb b/app/models/issue.rb
index f52e47f3e62..7beba984608 100644
--- a/app/models/issue.rb
+++ b/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
diff --git a/app/models/note.rb b/app/models/note.rb
index 3d5b663c99f..3e1375e5ad6 100644
--- a/app/models/note.rb
+++ b/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
diff --git a/app/models/project.rb b/app/models/project.rb
index 31990485f7d..7e131151513 100644
--- a/app/models/project.rb
+++ b/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
diff --git a/app/models/user.rb b/app/models/user.rb
index 46b36c605b0..592468933ed 100644
--- a/app/models/user.rb
+++ b/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)') }
diff --git a/app/services/repair_ldap_blocked_user_service.rb b/app/services/repair_ldap_blocked_user_service.rb
new file mode 100644
index 00000000000..863cef7ff61
--- /dev/null
+++ b/app/services/repair_ldap_blocked_user_service.rb
@@ -0,0 +1,17 @@
+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
diff --git a/app/views/admin/broadcast_messages/_form.html.haml b/app/views/admin/broadcast_messages/_form.html.haml
new file mode 100644
index 00000000000..953b8b69368
--- /dev/null
+++ b/app/views/admin/broadcast_messages/_form.html.haml
@@ -0,0 +1,37 @@
+.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"
diff --git a/app/views/admin/broadcast_messages/edit.html.haml b/app/views/admin/broadcast_messages/edit.html.haml
new file mode 100644
index 00000000000..45e053eb31d
--- /dev/null
+++ b/app/views/admin/broadcast_messages/edit.html.haml
@@ -0,0 +1,3 @@
+- page_title "Broadcast Messages"
+
+= render 'form'
diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 17dffebd360..49e33698b63 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -1,60 +1,37 @@
- 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)}
- &nbsp;
- = 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 &nbsp;
+ %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
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index a92c9c152b9..8312642b6c3 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -88,14 +88,19 @@
%i.fa.fa-envelope
= mail_to user.email, user.email, class: 'light'
&nbsp;
- = 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"
diff --git a/app/views/layouts/_broadcast.html.haml b/app/views/layouts/_broadcast.html.haml
index e7d477c225e..3a7e0929c16 100644
--- a/app/views/layouts/_broadcast.html.haml
+++ b/app/views/layouts/_broadcast.html.haml
@@ -1,4 +1 @@
-- if broadcast_message.present?
- .broadcast-message{ style: broadcast_styling(broadcast_message) }
- %i.fa.fa-bullhorn
- = broadcast_message.message
+= broadcast_message
diff --git a/app/views/projects/milestones/_milestone.html.haml b/app/views/projects/milestones/_milestone.html.haml
index d6a44c9f0a1..67d95ab0364 100644
--- a/app/views/projects/milestones/_milestone.html.haml
+++ b/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
diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index 1670ea8741a..d1db0f64f88 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/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
diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml
index ca60dd239b2..62db86fb181 100644
--- a/app/views/projects/notes/_notes.html.haml
+++ b/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
diff --git a/config/routes.rb b/config/routes.rb
index 3d5c70987c8..05d6ff1e884 100644
--- a/config/routes.rb
+++ b/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]
diff --git a/db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb b/db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb
new file mode 100644
index 00000000000..78fdfeaf5cf
--- /dev/null
+++ b/db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb
@@ -0,0 +1,5 @@
+class RemoveAlertTypeFromBroadcastMessages < ActiveRecord::Migration
+ def change
+ remove_column :broadcast_messages, :alert_type, :integer
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ecbe575bf83..42c3e79f9d7 100644
--- a/db/schema.rb
+++ b/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"
diff --git a/doc/api/users.md b/doc/api/users.md
index 773fe36d277..b7fc903825e 100644
--- a/doc/api/users.md
+++ b/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.
diff --git a/doc/install/installation.md b/doc/install/installation.md
index e645445df2a..00030729a4b 100644
--- a/doc/install/installation.md
+++ b/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
diff --git a/features/admin/broadcast_messages.feature b/features/admin/broadcast_messages.feature
index b2c3112320a..fd3bac77f86 100644
--- a/features/admin/broadcast_messages.feature
+++ b/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
diff --git a/features/project/issues/references.feature b/features/project/issues/references.feature
new file mode 100644
index 00000000000..4ae2d653337
--- /dev/null
+++ b/features/project/issues/references.feature
@@ -0,0 +1,33 @@
+@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
diff --git a/features/project/merge_requests/references.feature b/features/project/merge_requests/references.feature
new file mode 100644
index 00000000000..571612261a9
--- /dev/null
+++ b/features/project/merge_requests/references.feature
@@ -0,0 +1,31 @@
+@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
diff --git a/features/steps/admin/broadcast_messages.rb b/features/steps/admin/broadcast_messages.rb
index f6daf852977..6cacdf4764c 100644
--- a/features/steps/admin/broadcast_messages.rb
+++ b/features/steps/admin/broadcast_messages.rb
@@ -1,22 +1,15 @@
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
diff --git a/features/steps/project/issues/references.rb b/features/steps/project/issues/references.rb
new file mode 100644
index 00000000000..69e8b5cbde5
--- /dev/null
+++ b/features/steps/project/issues/references.rb
@@ -0,0 +1,7 @@
+class Spinach::Features::ProjectIssuesReferences < Spinach::FeatureSteps
+ include SharedAuthentication
+ include SharedIssuable
+ include SharedNote
+ include SharedProject
+ include SharedUser
+end
diff --git a/features/steps/project/merge_requests/references.rb b/features/steps/project/merge_requests/references.rb
new file mode 100644
index 00000000000..ab2ae6847a2
--- /dev/null
+++ b/features/steps/project/merge_requests/references.rb
@@ -0,0 +1,7 @@
+class Spinach::Features::ProjectMergeRequestsReferences < Spinach::FeatureSteps
+ include SharedAuthentication
+ include SharedIssuable
+ include SharedNote
+ include SharedProject
+ include SharedUser
+end
diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb
index e6d1b8b8efc..4c5f7488efb 100644
--- a/features/steps/shared/issuable.rb
+++ b/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
diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb
index f6aabfefeff..444d6726f99 100644
--- a/features/steps/shared/note.rb
+++ b/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
diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb
index da643bf3ba9..d3501b5f5cb 100644
--- a/features/steps/shared/project.rb
+++ b/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
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 3efdfe2d46e..174473f5371 100644
--- a/lib/api/notes.rb
+++ b/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
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 0d7813428e2..fd2128bd179 100644
--- a/lib/api/users.rb
+++ b/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
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index b2bdbc10d7f..da4435c7308 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/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
diff --git a/spec/controllers/admin/identities_controller_spec.rb b/spec/controllers/admin/identities_controller_spec.rb
new file mode 100644
index 00000000000..c131d22a30a
--- /dev/null
+++ b/spec/controllers/admin/identities_controller_spec.rb
@@ -0,0 +1,26 @@
+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
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 8b7af4d3a0a..5b1f65d7aff 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/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
diff --git a/spec/factories/broadcast_messages.rb b/spec/factories/broadcast_messages.rb
index ea0039d39e6..978a7d4cecb 100644
--- a/spec/factories/broadcast_messages.rb
+++ b/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
diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb
index c7c6f45d144..157cc4665a2 100644
--- a/spec/helpers/broadcast_messages_helper_spec.rb
+++ b/spec/helpers/broadcast_messages_helper_spec.rb
@@ -1,22 +1,60 @@
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
diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb
index a628d0c0157..32a19bf344b 100644
--- a/spec/lib/gitlab/ldap/access_spec.rb
+++ b/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 }
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index e4cac105110..f6f84db57e6 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/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
diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
new file mode 100644
index 00000000000..5afe042e154
--- /dev/null
+++ b/spec/models/identity_spec.rb
@@ -0,0 +1,38 @@
+# == 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
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 151a29e974b..9182b42661d 100644
--- a/spec/models/note_spec.rb
+++ b/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 }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 3cd63b2b0e8..0bef68e2885 100644
--- a/spec/models/user_spec.rb
+++ b/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
diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb
index 8b177af4689..d8bbd107269 100644
--- a/spec/requests/api/notes_spec.rb
+++ b/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
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 4f278551d07..b82c5c7685f 100644
--- a/spec/requests/api/users_spec.rb
+++ b/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
diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb
new file mode 100644
index 00000000000..ce7d1455975
--- /dev/null
+++ b/spec/services/repair_ldap_blocked_user_service_spec.rb
@@ -0,0 +1,23 @@
+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