diff options
73 files changed, 1394 insertions, 240 deletions
diff --git a/CHANGELOG b/CHANGELOG index 615743e2a7c..358f1d7c836 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ 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) 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/assets/stylesheets/pages/branches.scss b/app/assets/stylesheets/pages/branches.scss new file mode 100644 index 00000000000..abae5c3d0a5 --- /dev/null +++ b/app/assets/stylesheets/pages/branches.scss @@ -0,0 +1,3 @@ +.branch-name{ + font-weight: 600; +} diff --git a/app/assets/stylesheets/pages/commit.scss b/app/assets/stylesheets/pages/commit.scss index 17245d3be7b..05d2f306d0f 100644 --- a/app/assets/stylesheets/pages/commit.scss +++ b/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; diff --git a/app/assets/stylesheets/pages/groups.scss b/app/assets/stylesheets/pages/groups.scss index 263993f59a5..3404c2631e1 100644 --- a/app/assets/stylesheets/pages/groups.scss +++ b/app/assets/stylesheets/pages/groups.scss @@ -11,3 +11,8 @@ height: 42px; } } + +.content-list .group-name { + font-weight: 600; + color: #4c4e54; +} diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 1e1af662850..ad92cc22815 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/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 { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 82effde0bf3..efd33df2e99 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/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 { diff --git a/app/assets/stylesheets/pages/tags.scss b/app/assets/stylesheets/pages/tags.scss new file mode 100644 index 00000000000..e9cd6dc6c5e --- /dev/null +++ b/app/assets/stylesheets/pages/tags.scss @@ -0,0 +1,3 @@ +.tag-name{ + font-weight: 600; +} 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/ci/variable.rb b/app/models/ci/variable.rb index 7f6f497f325..e786bd7dd93 100644 --- a/app/models/ci/variable.rb +++ b/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 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)} - - = 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 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' - = 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/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index a234536723e..d276e5932d1 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/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 diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 012825f0fdb..4d4b410ee29 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/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 ... 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/app/views/projects/tags/_tag.html.haml b/app/views/projects/tags/_tag.html.haml index 28b706c5c7e..56a7ced1236 100644 --- a/app/views/projects/tags/_tag.html.haml +++ b/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? diff --git a/app/views/projects/tags/show.html.haml b/app/views/projects/tags/show.html.haml index b594d4f1f27..dbb20347860 100644 --- a/app/views/projects/tags/show.html.haml +++ b/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 diff --git a/app/views/shared/groups/_group.html.haml b/app/views/shared/groups/_group.html.haml index a54c5fa8c33..f4cfa29ae56 100644 --- a/app/views/shared/groups/_group.html.haml +++ b/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 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/README.md b/doc/api/README.md index 43ee24ddb63..a9c4e47b35d 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -24,6 +24,7 @@ - [Settings](settings.md) - [Keys](keys.md) - [Builds](builds.md) +- [Build Variables](build_variables.md) ## Clients diff --git a/doc/api/build_variables.md b/doc/api/build_variables.md new file mode 100644 index 00000000000..b96f1bdac8a --- /dev/null +++ b/doc/api/build_variables.md @@ -0,0 +1,128 @@ +# 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" +} +``` 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/api.rb b/lib/api/api.rb index 1a1340c428c..7efe0a0262f 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -55,5 +55,6 @@ module API mount Tags mount Triggers mount Builds + mount Variables end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index b403d3c5032..cce46886672 100644 --- a/lib/api/entities.rb +++ b/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 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/api/variables.rb b/lib/api/variables.rb new file mode 100644 index 00000000000..d9a055f6c92 --- /dev/null +++ b/lib/api/variables.rb @@ -0,0 +1,95 @@ +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 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/factories/ci/variables.rb b/spec/factories/ci/variables.rb new file mode 100644 index 00000000000..8f62d64411b --- /dev/null +++ b/spec/factories/ci/variables.rb @@ -0,0 +1,22 @@ +# == 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 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/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb new file mode 100644 index 00000000000..9744729ba0c --- /dev/null +++ b/spec/requests/api/variables_spec.rb @@ -0,0 +1,182 @@ +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 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 |