diff options
43 files changed, 235 insertions, 118 deletions
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index dc79de19d48..c8ad88e9351 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -384,7 +384,7 @@ ul.notes { top: 0; .note-action-button { - margin-left: 10px; + margin-left: 8px; } } @@ -400,8 +400,7 @@ ul.notes { } .note-action-button { - display: inline-block; - margin-left: 0; + display: inline; line-height: 20px; @media (min-width: $screen-sm-min) { @@ -540,7 +539,6 @@ ul.notes { } .line-resolve-btn { - display: inline-block; position: relative; top: 2px; padding: 0; @@ -563,8 +561,9 @@ ul.notes { } svg { - position: relative; fill: $gray-darkest; + height: 15px; + width: 15px; } } diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 7ffde71c3b1..24504685e48 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -29,11 +29,7 @@ class Admin::UsersController < Admin::ApplicationController end def impersonate - if user.blocked? - flash[:alert] = "You cannot impersonate a blocked user" - - redirect_to admin_user_path(user) - else + if can?(user, :log_in) session[:impersonator_id] = current_user.id warden.set_user(user, scope: :user) @@ -43,6 +39,17 @@ class Admin::UsersController < Admin::ApplicationController flash[:alert] = "You are now impersonating #{user.username}" redirect_to root_path + else + flash[:alert] = + if user.blocked? + "You cannot impersonate a blocked user" + elsif user.internal? + "You cannot impersonate an internal user" + else + "You cannot impersonate a user who cannot log in" + end + + redirect_to admin_user_path(user) end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1c66c530cd2..b7ce081a5cd 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence user = User.find_by_authentication_token(token_string) || User.find_by_personal_access_token(token_string) - if user + if user && can?(user, :log_in) # Notice we are passing store false, so the user is not # actually stored in the session and a token is needed # for every request. If you want the token to work as a @@ -90,7 +90,7 @@ class ApplicationController < ActionController::Base current_application_settings.after_sign_out_path.presence || new_user_session_path end - def can?(object, action, subject) + def can?(object, action, subject = :global) Ability.allowed?(object, action, subject) end diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 4c497711fc0..ea441b1736b 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -23,7 +23,7 @@ module AuthenticatesWithTwoFactor # # Returns nil def prompt_for_two_factor(user) - return locked_user_redirect(user) if user.access_locked? + return locked_user_redirect(user) unless user.can?(:log_in) session[:otp_user_id] = user.id setup_u2f_authentication(user) @@ -37,10 +37,9 @@ module AuthenticatesWithTwoFactor def authenticate_with_two_factor user = self.resource = find_user + return locked_user_redirect(user) unless user.can?(:log_in) - if user.access_locked? - locked_user_redirect(user) - elsif user_params[:otp_attempt].present? && session[:otp_user_id] + if user_params[:otp_attempt].present? && session[:otp_user_id] authenticate_with_two_factor_via_otp(user) elsif user_params[:device_response].present? && session[:otp_user_id] authenticate_with_two_factor_via_u2f(user) diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 4663b6e7fc6..05f9ee1ee90 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -118,7 +118,7 @@ class GroupsController < Groups::ApplicationController end def authorize_create_group! - unless can?(current_user, :create_group, nil) + unless can?(current_user, :create_group) return render_404 end end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index c2b399041c6..aad83731b87 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -1,4 +1,6 @@ module IssuablesHelper + include GitlabRoutingHelper + def sidebar_gutter_toggle_icon sidebar_gutter_collapsed? ? icon('angle-double-left', { 'aria-hidden': 'true' }) : icon('angle-double-right', { 'aria-hidden': 'true' }) end @@ -95,8 +97,23 @@ module IssuablesHelper h(milestone_title.presence || default_label) end + def to_url_reference(issuable) + case issuable + when Issue + link_to issuable.to_reference, issue_url(issuable) + when MergeRequest + link_to issuable.to_reference, merge_request_url(issuable) + else + issuable.to_reference + end + end + def issuable_meta(issuable, project, text) - output = content_tag :strong, "#{text} #{issuable.to_reference}", class: "identifier" + output = content_tag(:strong, class: "identifier") do + concat("#{text} ") + concat(to_url_reference(issuable)) + end + output << " opened #{time_ago_with_tooltip(issuable.created_at)} by ".html_safe output << content_tag(:strong) do author_output = link_to_member(project, issuable.author, size: 24, mobile_classes: "hidden-xs", tooltip: true) diff --git a/app/models/ability.rb b/app/models/ability.rb index ad6c588202e..f3692a5a067 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -56,15 +56,16 @@ class Ability end end - def allowed?(user, action, subject) + def allowed?(user, action, subject = :global) allowed(user, subject).include?(action) end - def allowed(user, subject) + def allowed(user, subject = :global) + return BasePolicy::RuleSet.none if subject.nil? return uncached_allowed(user, subject) unless RequestStore.active? user_key = user ? user.id : 'anonymous' - subject_key = subject ? "#{subject.class.name}/#{subject.id}" : 'global' + subject_key = subject == :global ? 'global' : "#{subject.class.name}/#{subject.id}" key = "/ability/#{user_key}/#{subject_key}" RequestStore[key] ||= uncached_allowed(user, subject).freeze end diff --git a/app/models/guest.rb b/app/models/guest.rb index 01285ca1264..df287c277a7 100644 --- a/app/models/guest.rb +++ b/app/models/guest.rb @@ -1,6 +1,6 @@ class Guest class << self - def can?(action, subject) + def can?(action, subject = :global) Ability.allowed?(nil, action, subject) end end diff --git a/app/models/user.rb b/app/models/user.rb index 76fb4cd470e..39c1281179b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -126,7 +126,6 @@ class User < ActiveRecord::Base validate :unique_email, if: ->(user) { user.email_changed? } validate :owns_notification_email, if: ->(user) { user.notification_email_changed? } validate :owns_public_email, if: ->(user) { user.public_email_changed? } - validate :ghost_users_must_be_blocked validates :avatar, file_size: { maximum: 200.kilobytes.to_i } before_validation :generate_password, on: :create @@ -350,12 +349,27 @@ class User < ActiveRecord::Base def ghost unique_internal(where(ghost: true), 'ghost', 'ghost%s@example.com') do |u| u.bio = 'This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.' - u.state = :blocked u.name = 'Ghost User' end end end + def self.internal_attributes + [:ghost] + end + + def internal? + self.class.internal_attributes.any? { |a| self[a] } + end + + def self.internal + where(Hash[internal_attributes.zip([true] * internal_attributes.size)]) + end + + def self.non_internal + where(Hash[internal_attributes.zip([[false, nil]] * internal_attributes.size)]) + end + # # Instance methods # @@ -452,12 +466,6 @@ class User < ActiveRecord::Base errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email) end - def ghost_users_must_be_blocked - if ghost? && !blocked? - errors.add(:ghost, 'cannot be enabled for a user who is not blocked.') - end - end - def update_emails_with_primary_email primary_email_record = emails.find_by(email: email) if primary_email_record @@ -563,14 +571,14 @@ class User < ActiveRecord::Base end def can_create_group? - can?(:create_group, nil) + can?(:create_group) end def can_select_namespace? several_namespaces? || admin end - def can?(action, subject) + def can?(action, subject = :global) Ability.allowed?(self, action, subject) end @@ -955,6 +963,14 @@ class User < ActiveRecord::Base self.admin = (new_level == 'admin') end + protected + + # override, from Devise::Validatable + def password_required? + return false if internal? + super + end + private def ci_projects_union @@ -1055,7 +1071,6 @@ class User < ActiveRecord::Base scope.create( username: username, - password: Devise.friendly_token, email: email, &creation_block ) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index e07b144355a..8890409d056 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -12,6 +12,10 @@ class BasePolicy new(Set.new, Set.new) end + def self.none + empty.freeze + end + def can?(ability) @can_set.include?(ability) && !@cannot_set.include?(ability) end @@ -49,7 +53,8 @@ class BasePolicy end def self.class_for(subject) - return GlobalPolicy if subject.nil? + return GlobalPolicy if subject == :global + raise ArgumentError, 'no policy for nil' if subject.nil? if subject.class.try(:presenter?) subject = subject.subject @@ -79,7 +84,7 @@ class BasePolicy end def abilities - return RuleSet.empty if @user && @user.blocked? + return RuleSet.none if @user && @user.blocked? return anonymous_abilities if @user.nil? collect_rules { rules } end diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 3c2fbe6b56b..cb72c2b4590 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -4,5 +4,12 @@ class GlobalPolicy < BasePolicy can! :create_group if @user.can_create_group can! :read_users_list + + unless @user.blocked? || @user.internal? + can! :log_in unless @user.access_locked? + can! :access_api + can! :access_git + can! :receive_notifications + end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index fbad85d310e..d12692ecc90 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -465,7 +465,7 @@ class NotificationService end users = users.to_a.compact.uniq - users = users.reject(&:blocked?) + users = users.select { |u| u.can?(:receive_notifications) } users.reject do |user| global_notification_setting = user.global_notification_setting diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml index d20be373564..be41c33b853 100644 --- a/app/views/admin/users/_head.html.haml +++ b/app/views/admin/users/_head.html.haml @@ -2,11 +2,13 @@ = @user.name - if @user.blocked? %span.cred (Blocked) + - if @user.internal? + %span.cred (Internal) - if @user.admin %span.cred (Admin) .pull-right - - unless @user == current_user || @user.blocked? + - if @user != current_user && @user.can?(:log_in) = link_to 'Impersonate', impersonate_admin_user_path(@user), method: :post, class: "btn btn-nr btn-grouped btn-info" = link_to edit_admin_user_path(@user), class: "btn btn-nr btn-grouped" do %i.fa.fa-pencil-square-o diff --git a/app/views/dashboard/issues.atom.builder b/app/views/dashboard/issues.atom.builder index bdea1064096..06fb531b546 100644 --- a/app/views/dashboard/issues.atom.builder +++ b/app/views/dashboard/issues.atom.builder @@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear xml.link href: url_for(params), rel: "self", type: "application/atom+xml" xml.link href: issues_dashboard_url, rel: "alternate", type: "text/html" xml.id issues_dashboard_url - xml.updated @issues.first.created_at.xmlschema if @issues.reorder(nil).any? + xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? xml << render(partial: 'issues/issue', collection: @issues) if @issues.reorder(nil).any? end diff --git a/app/views/events/_event.atom.builder b/app/views/events/_event.atom.builder index 7890e717aa7..43a52cf3002 100644 --- a/app/views/events/_event.atom.builder +++ b/app/views/events/_event.atom.builder @@ -4,7 +4,7 @@ xml.entry do xml.id "tag:#{request.host},#{event.created_at.strftime("%Y-%m-%d")}:#{event.id}" xml.link href: event_feed_url(event) xml.title truncate(event_feed_title(event), length: 80) - xml.updated event.created_at.xmlschema + xml.updated event.updated_at.xmlschema xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(event.author_email)) xml.author do diff --git a/app/views/groups/issues.atom.builder b/app/views/groups/issues.atom.builder index 0cc6466d34e..469768d83f2 100644 --- a/app/views/groups/issues.atom.builder +++ b/app/views/groups/issues.atom.builder @@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear xml.link href: url_for(params), rel: "self", type: "application/atom+xml" xml.link href: issues_group_url, rel: "alternate", type: "text/html" xml.id issues_group_url - xml.updated @issues.first.created_at.xmlschema if @issues.reorder(nil).any? + xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? xml << render(partial: 'issues/issue', collection: @issues) if @issues.reorder(nil).any? end diff --git a/app/views/issues/_issue.atom.builder b/app/views/issues/_issue.atom.builder index 96831874144..fcd30c8c765 100644 --- a/app/views/issues/_issue.atom.builder +++ b/app/views/issues/_issue.atom.builder @@ -2,7 +2,7 @@ xml.entry do xml.id namespace_project_issue_url(issue.project.namespace, issue.project, issue) xml.link href: namespace_project_issue_url(issue.project.namespace, issue.project, issue) xml.title truncate(issue.title, length: 80) - xml.updated issue.created_at.xmlschema + xml.updated issue.updated_at.xmlschema xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(issue.author_email)) xml.author do diff --git a/app/views/projects/deploy_keys/_index.html.haml b/app/views/projects/deploy_keys/_index.html.haml index 0cbe9b3275a..4cfbd9add00 100644 --- a/app/views/projects/deploy_keys/_index.html.haml +++ b/app/views/projects/deploy_keys/_index.html.haml @@ -3,7 +3,7 @@ %h4.prepend-top-0 Deploy Keys %p - Deploy keys allow read-only access to your repository. Deploy keys can be used for CI, staging or production servers. You can create a deploy key or add an existing one. + Deploy keys allow read-only or read-write (if enabled) access to your repository. Deploy keys can be used for CI, staging or production servers. You can create a deploy key or add an existing one. .col-lg-9 %h5.prepend-top-0 Create a new deploy key for this project diff --git a/app/views/projects/issues/index.atom.builder b/app/views/projects/issues/index.atom.builder index a0df0db77c5..4feec09bb5d 100644 --- a/app/views/projects/issues/index.atom.builder +++ b/app/views/projects/issues/index.atom.builder @@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear xml.link href: url_for(params), rel: "self", type: "application/atom+xml" xml.link href: namespace_project_issues_url(@project.namespace, @project), rel: "alternate", type: "text/html" xml.id namespace_project_issues_url(@project.namespace, @project) - xml.updated @issues.first.created_at.xmlschema if @issues.reorder(nil).any? + xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? xml << render(partial: 'issues/issue', collection: @issues) if @issues.reorder(nil).any? end diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index a7618370a5d..5552086bc50 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -43,18 +43,17 @@ "inline-template" => true, "ref" => "note_#{note.id}" } - .note-action-button + %button.note-action-button.line-resolve-btn{ type: "button", + class: ("is-disabled" unless can_resolve), + ":class" => "{ 'is-active': isResolved }", + ":aria-label" => "buttonText", + "@click" => "resolve", + ":title" => "buttonText", + "v-show" => "!loading", + ":ref" => "'button'" } = icon("spin spinner", "v-show" => "loading") - %button.line-resolve-btn{ type: "button", - class: ("is-disabled" unless can_resolve), - ":class" => "{ 'is-active': isResolved }", - ":aria-label" => "buttonText", - "@click" => "resolve", - ":title" => "buttonText", - "v-show" => "!loading", - ":ref" => "'button'" } - = render "shared/icons/icon_status_success.svg" + = render "shared/icons/icon_status_success.svg" - if current_user - if note.emoji_awardable? diff --git a/changelogs/unreleased/24137-issuable-permalink.yml b/changelogs/unreleased/24137-issuable-permalink.yml new file mode 100644 index 00000000000..bcc6c6957a1 --- /dev/null +++ b/changelogs/unreleased/24137-issuable-permalink.yml @@ -0,0 +1,4 @@ +--- +title: Link issuable reference to itself in meta-header +merge_request: 9641 +author: mhasbini diff --git a/changelogs/unreleased/29189-discussion-button.yml b/changelogs/unreleased/29189-discussion-button.yml new file mode 100644 index 00000000000..eea96362117 --- /dev/null +++ b/changelogs/unreleased/29189-discussion-button.yml @@ -0,0 +1,4 @@ +--- +title: Fix alignment of resolve button +merge_request: +author: diff --git a/changelogs/unreleased/29328-fix-transient-failure-in-model-user-spec.yml b/changelogs/unreleased/29328-fix-transient-failure-in-model-user-spec.yml new file mode 100644 index 00000000000..dabf9968c5b --- /dev/null +++ b/changelogs/unreleased/29328-fix-transient-failure-in-model-user-spec.yml @@ -0,0 +1,4 @@ +--- +title: Add custom attributes in factories +merge_request: 9892 +author: George Andrinopoulos diff --git a/changelogs/unreleased/fix_updated_field_in_issues-atom.yml b/changelogs/unreleased/fix_updated_field_in_issues-atom.yml new file mode 100644 index 00000000000..414facdf779 --- /dev/null +++ b/changelogs/unreleased/fix_updated_field_in_issues-atom.yml @@ -0,0 +1,4 @@ +--- +title: Fix xml.updated field in rss/atom feeds +merge_request: 9889 +author: blackst0ne diff --git a/config/initializers/inflections.rb b/config/initializers/0_inflections.rb index d4197da3fa9..d4197da3fa9 100644 --- a/config/initializers/inflections.rb +++ b/config/initializers/0_inflections.rb diff --git a/config/initializers/fix_local_cache_middleware.rb b/config/initializers/fix_local_cache_middleware.rb new file mode 100644 index 00000000000..cb37f9ed22c --- /dev/null +++ b/config/initializers/fix_local_cache_middleware.rb @@ -0,0 +1,24 @@ +module LocalCacheRegistryCleanupWithEnsure + LocalCacheRegistry = + ActiveSupport::Cache::Strategy::LocalCache::LocalCacheRegistry + LocalStore = + ActiveSupport::Cache::Strategy::LocalCache::LocalStore + + def call(env) + LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new) + response = @app.call(env) + response[2] = ::Rack::BodyProxy.new(response[2]) do + LocalCacheRegistry.set_cache_for(local_cache_key, nil) + end + cleanup_after_response = true # ADDED THIS LINE + response + rescue Rack::Utils::InvalidParameterError + [400, {}, []] + ensure # ADDED ensure CLAUSE to cleanup when something is thrown + LocalCacheRegistry.set_cache_for(local_cache_key, nil) unless + cleanup_after_response + end +end + +ActiveSupport::Cache::Strategy::LocalCache::Middleware + .prepend(LocalCacheRegistryCleanupWithEnsure) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index a9b364da9e1..bd22b82476b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -97,7 +97,7 @@ module API end def authenticate! - unauthorized! unless current_user + unauthorized! unless current_user && can?(current_user, :access_api) end def authenticate_non_get! @@ -116,7 +116,7 @@ module API forbidden! unless current_user.is_admin? end - def authorize!(action, subject = nil) + def authorize!(action, subject = :global) forbidden! unless can?(current_user, action, subject) end @@ -134,7 +134,7 @@ module API end end - def can?(object, action, subject) + def can?(object, action, subject = :global) Ability.allowed?(object, action, subject) end diff --git a/lib/api/users.rb b/lib/api/users.rb index 549003f576a..2d4d5a25221 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -45,7 +45,7 @@ module API use :pagination end get do - unless can?(current_user, :read_users_list, nil) + unless can?(current_user, :read_users_list) render_api_error!("Not authorized.", 403) end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index 2058a58d0ae..b121c37c5d0 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -210,7 +210,7 @@ module Banzai grouped_objects_for_nodes(nodes, Project, 'data-project') end - def can?(user, permission, subject) + def can?(user, permission, subject = :global) Ability.allowed?(user, permission, subject) end diff --git a/lib/gitlab/allowable.rb b/lib/gitlab/allowable.rb index f48abcc86d5..e4f7cad2b79 100644 --- a/lib/gitlab/allowable.rb +++ b/lib/gitlab/allowable.rb @@ -1,6 +1,6 @@ module Gitlab module Allowable - def can?(user, action, subject) + def can?(user, action, subject = :global) Ability.allowed?(user, action, subject) end end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 6ce9b229294..f260c0c535f 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -8,7 +8,7 @@ module Gitlab end def can_do_action?(action) - return false if no_user_or_blocked? + return false unless can_access_git? @permission_cache ||= {} @permission_cache[action] ||= user.can?(action, project) @@ -19,7 +19,7 @@ module Gitlab end def allowed? - return false if no_user_or_blocked? + return false unless can_access_git? if user.requires_ldap_check? && user.try_obtain_ldap_lease return false unless Gitlab::LDAP::Access.allowed?(user) @@ -29,7 +29,7 @@ module Gitlab end def can_push_to_branch?(ref) - return false if no_user_or_blocked? + return false unless can_access_git? if project.protected_branch?(ref) return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user) @@ -44,7 +44,7 @@ module Gitlab end def can_merge_to_branch?(ref) - return false if no_user_or_blocked? + return false unless can_access_git? if project.protected_branch?(ref) access_levels = project.protected_branches.matching(ref).map(&:merge_access_levels).flatten @@ -55,15 +55,15 @@ module Gitlab end def can_read_project? - return false if no_user_or_blocked? + return false unless can_access_git? user.can?(:read_project, project) end private - def no_user_or_blocked? - user.nil? || user.blocked? + def can_access_git? + user && user.can?(:access_git) end end end diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index d4e0ef91856..755992069ff 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe 'New/edit issue', feature: true, js: true do + include GitlabRoutingHelper + let!(:project) { create(:project) } let!(:user) { create(:user)} let!(:user2) { create(:user)} @@ -78,6 +80,14 @@ describe 'New/edit issue', feature: true, js: true do expect(page).to have_content label2.title end end + + page.within '.issuable-meta' do + issue = Issue.find_by(title: 'title') + + expect(page).to have_text("Issue #{issue.to_reference}") + # compare paths because the host differ in test + expect(find_link(issue.to_reference)[:href]).to end_with(issue_path(issue)) + end end it 'correctly updates the dropdown toggle when removing a label' do diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index ae609160e18..f32d1f78b40 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -48,6 +48,18 @@ feature 'Login', feature: true do end end + describe 'with the ghost user' do + it 'disallows login' do + login_with(User.ghost) + + expect(page).to have_content('Invalid Login or password.') + end + + it 'does not update Devise trackable attributes' do + expect { login_with(User.ghost) }.not_to change { User.ghost.reload.sign_in_count } + end + end + describe 'with two-factor authentication' do def enter_code(code) fill_in 'user_otp_attempt', with: code diff --git a/spec/features/merge_requests/form_spec.rb b/spec/features/merge_requests/form_spec.rb index 1ecdb8b5983..f8518f450dc 100644 --- a/spec/features/merge_requests/form_spec.rb +++ b/spec/features/merge_requests/form_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe 'New/edit merge request', feature: true, js: true do + include GitlabRoutingHelper + let!(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } let(:fork_project) { create(:project, forked_from_project: project) } let!(:user) { create(:user)} @@ -84,6 +86,15 @@ describe 'New/edit merge request', feature: true, js: true do expect(page).to have_content label2.title end end + + page.within '.issuable-meta' do + merge_request = MergeRequest.find_by(source_branch: 'fix') + + expect(page).to have_text("Merge Request #{merge_request.to_reference}") + # compare paths because the host differ in test + expect(find_link(merge_request.to_reference)[:href]) + .to end_with(merge_request_path(merge_request)) + end end end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 30f8fdf91b2..92d70cfc64c 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -1,6 +1,12 @@ require 'spec_helper' describe Ability, lib: true do + context 'using a nil subject' do + it 'is always empty' do + expect(Ability.allowed(nil, nil).to_set).to be_empty + end + end + describe '.can_edit_note?' do let(:project) { create(:empty_project) } let(:note) { create(:note_on_issue, project: project) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index adb5b538922..9da4140f3ce 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -210,22 +210,6 @@ describe User, models: true do end end end - - describe 'ghost users' do - it 'does not allow a non-blocked ghost user' do - user = build(:user, :ghost) - user.state = 'active' - - expect(user).to be_invalid - end - - it 'allows a blocked ghost user' do - user = build(:user, :ghost) - user.state = 'blocked' - - expect(user).to be_valid - end - end end describe "scopes" do @@ -713,8 +697,9 @@ describe User, models: true do describe '.search_with_secondary_emails' do delegate :search_with_secondary_emails, to: :described_class - let!(:user) { create(:user) } - let!(:email) { create(:email) } + let!(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'john.doe@example.com' ) } + let!(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'albert.smith@example.com' ) } + let!(:email) { create(:email, user: another_user) } it 'returns users with a matching name' do expect(search_with_secondary_emails(user.name)).to eq([user]) diff --git a/spec/policies/base_policy_spec.rb b/spec/policies/base_policy_spec.rb index 63acc0b68cd..02acdcb36df 100644 --- a/spec/policies/base_policy_spec.rb +++ b/spec/policies/base_policy_spec.rb @@ -1,17 +1,19 @@ require 'spec_helper' describe BasePolicy, models: true do - let(:build) { Ci::Build.new } - describe '.class_for' do it 'detects policy class based on the subject ancestors' do - expect(described_class.class_for(build)).to eq(Ci::BuildPolicy) + expect(described_class.class_for(GenericCommitStatus.new)).to eq(CommitStatusPolicy) end it 'detects policy class for a presented subject' do - presentee = Ci::BuildPresenter.new(build) + presentee = Ci::BuildPresenter.new(Ci::Build.new) expect(described_class.class_for(presentee)).to eq(Ci::BuildPolicy) end + + it 'uses GlobalPolicy when :global is given' do + expect(described_class.class_for(:global)).to eq(GlobalPolicy) + end end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index a89676fec93..988a57a80ea 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -436,7 +436,7 @@ describe API::Helpers, api: true do context 'current_user is present' do before do - expect_any_instance_of(self.class).to receive(:current_user).and_return(true) + expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(User.new) end it 'does not raise an error' do diff --git a/vendor/gitlab-ci-yml/Docker.gitlab-ci.yml b/vendor/gitlab-ci-yml/Docker.gitlab-ci.yml index 8c590579934..40648bcd3de 100644 --- a/vendor/gitlab-ci-yml/Docker.gitlab-ci.yml +++ b/vendor/gitlab-ci-yml/Docker.gitlab-ci.yml @@ -7,7 +7,7 @@ services: build: stage: build script: - - export IMAGE_TAG=$(echo -en $CI_BUILD_REF_NAME | tr -c '[:alnum:]_.-' '-') - - docker login -u "gitlab-ci-token" -p "$CI_BUILD_TOKEN" $CI_REGISTRY + - export IMAGE_TAG=$(echo -en $CI_COMMIT_REF_NAME | tr -c '[:alnum:]_.-' '-') + - docker login -u "gitlab-ci-token" -p "$CI_JOB_TOKEN" $CI_REGISTRY - docker build --pull -t "$CI_REGISTRY_IMAGE:$IMAGE_TAG" . - docker push "$CI_REGISTRY_IMAGE:$IMAGE_TAG" diff --git a/vendor/gitlab-ci-yml/Maven.gitlab-ci.yml b/vendor/gitlab-ci-yml/Maven.gitlab-ci.yml index b75f0665bee..91b096654d1 100644 --- a/vendor/gitlab-ci-yml/Maven.gitlab-ci.yml +++ b/vendor/gitlab-ci-yml/Maven.gitlab-ci.yml @@ -3,9 +3,9 @@ # For docker image tags see https://hub.docker.com/_/maven/ # # For general lifecycle information see https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html -# +# # This template will build and test your projects as well as create the documentation. -# +# # * Caches downloaded dependencies and plugins between invocation. # * Does only verify merge requests but deploy built artifacts of the # master branch. @@ -24,12 +24,12 @@ variables: MAVEN_CLI_OPTS: "--batch-mode --errors --fail-at-end --show-version -DinstallAtEnd=true -DdeployAtEnd=true" # Cache downloaded dependencies and plugins between builds. -# To keep cache across branches add 'key: "$CI_BUILD_REF_NAME"' +# To keep cache across branches add 'key: "$CI_JOB_REF_NAME"' cache: paths: - .m2/repository -# This will only validate and compile stuff and run e.g. maven-enforcer-plugin. +# This will only validate and compile stuff and run e.g. maven-enforcer-plugin. # Because some enforcer rules might check dependency convergence and class duplications # we use `test-compile` here instead of `validate`, so the correct classpath is picked up. .validate: &validate diff --git a/vendor/gitlab-ci-yml/OpenShift.gitlab-ci.yml b/vendor/gitlab-ci-yml/OpenShift.gitlab-ci.yml index 6b6c405a507..d3bb388a1e7 100644 --- a/vendor/gitlab-ci-yml/OpenShift.gitlab-ci.yml +++ b/vendor/gitlab-ci-yml/OpenShift.gitlab-ci.yml @@ -38,11 +38,11 @@ review: <<: *deploy stage: review variables: - APP: $CI_BUILD_REF_NAME - APP_HOST: $CI_PROJECT_NAME-$CI_BUILD_REF_NAME.$OPENSHIFT_DOMAIN + APP: $CI_COMMIT_REF_NAME + APP_HOST: $CI_PROJECT_NAME-$CI_ENVIRONMENT_SLUG.$OPENSHIFT_DOMAIN environment: - name: review/$CI_BUILD_REF_NAME - url: http://$CI_PROJECT_NAME-$CI_BUILD_REF_NAME.$OPENSHIFT_DOMAIN + name: review/$CI_COMMIT_REF_SLUG + url: http://$CI_PROJECT_NAME-$CI_ENVIRONMENT_SLUG.$OPENSHIFT_DOMAIN on_stop: stop-review only: - branches @@ -56,10 +56,10 @@ stop-review: - oc delete all -l "app=$APP" when: manual variables: - APP: $CI_BUILD_REF_NAME + APP: $CI_COMMIT_REF_NAME GIT_STRATEGY: none environment: - name: review/$CI_BUILD_REF_NAME + name: review/$CI_COMMIT_REF_SLUG action: stop only: - branches diff --git a/vendor/gitlab-ci-yml/autodeploy/Kubernetes.gitlab-ci.yml b/vendor/gitlab-ci-yml/autodeploy/Kubernetes.gitlab-ci.yml index 574f9365f14..c644560647f 100644 --- a/vendor/gitlab-ci-yml/autodeploy/Kubernetes.gitlab-ci.yml +++ b/vendor/gitlab-ci-yml/autodeploy/Kubernetes.gitlab-ci.yml @@ -24,12 +24,12 @@ build: production: stage: production variables: - CI_ENVIRONMENT_URL: http://production.$KUBE_DOMAIN + CI_ENVIRONMENT_URL: http://$CI_PROJECT_NAME.$KUBE_DOMAIN script: - command deploy environment: name: production - url: http://production.$KUBE_DOMAIN + url: http://$CI_PROJECT_NAME.$KUBE_DOMAIN when: manual only: - master @@ -37,24 +37,24 @@ production: staging: stage: staging variables: - CI_ENVIRONMENT_URL: http://staging.$KUBE_DOMAIN + CI_ENVIRONMENT_URL: http://$CI_PROJECT_NAME-staging.$KUBE_DOMAIN script: - command deploy environment: name: staging - url: http://staging.$KUBE_DOMAIN + url: http://$CI_PROJECT_NAME-staging.$KUBE_DOMAIN only: - master review: stage: review variables: - CI_ENVIRONMENT_URL: http://$CI_ENVIRONMENT_SLUG.$KUBE_DOMAIN + CI_ENVIRONMENT_URL: http://$CI_PROJECT_NAME-$CI_ENVIRONMENT_SLUG.$KUBE_DOMAIN script: - command deploy environment: - name: review/$CI_BUILD_REF_NAME - url: http://$CI_ENVIRONMENT_SLUG.$KUBE_DOMAIN + name: review/$CI_COMMIT_REF_NAME + url: http://$CI_PROJECT_NAME-$CI_ENVIRONMENT_SLUG.$KUBE_DOMAIN on_stop: stop_review only: - branches @@ -68,7 +68,7 @@ stop_review: script: - command destroy environment: - name: review/$CI_BUILD_REF_NAME + name: review/$CI_COMMIT_REF_NAME action: stop when: manual only: diff --git a/vendor/gitlab-ci-yml/autodeploy/OpenShift.gitlab-ci.yml b/vendor/gitlab-ci-yml/autodeploy/OpenShift.gitlab-ci.yml index 4d6f4e00ebb..27c9107e0d7 100644 --- a/vendor/gitlab-ci-yml/autodeploy/OpenShift.gitlab-ci.yml +++ b/vendor/gitlab-ci-yml/autodeploy/OpenShift.gitlab-ci.yml @@ -1,4 +1,4 @@ -# Explaination on the scripts: +# Explanation on the scripts: # https://gitlab.com/gitlab-examples/openshift-deploy/blob/master/README.md image: registry.gitlab.com/gitlab-examples/openshift-deploy @@ -24,12 +24,12 @@ build: production: stage: production variables: - CI_ENVIRONMENT_URL: http://production.$KUBE_DOMAIN + CI_ENVIRONMENT_URL: http://$CI_PROJECT_NAME.$KUBE_DOMAIN script: - command deploy environment: name: production - url: http://production.$KUBE_DOMAIN + url: http://$CI_PROJECT_NAME.$KUBE_DOMAIN when: manual only: - master @@ -37,24 +37,24 @@ production: staging: stage: staging variables: - CI_ENVIRONMENT_URL: http://staging.$KUBE_DOMAIN + CI_ENVIRONMENT_URL: http://$CI_PROJECT_NAME-staging.$KUBE_DOMAIN script: - command deploy environment: name: staging - url: http://staging.$KUBE_DOMAIN + url: http://$CI_PROJECT_NAME-staging.$KUBE_DOMAIN only: - master review: stage: review variables: - CI_ENVIRONMENT_URL: http://$CI_ENVIRONMENT_SLUG.$KUBE_DOMAIN + CI_ENVIRONMENT_URL: http://$CI_PROJECT_NAME-$CI_ENVIRONMENT_SLUG.$KUBE_DOMAIN script: - command deploy environment: - name: review/$CI_BUILD_REF_NAME - url: http://$CI_ENVIRONMENT_SLUG.$KUBE_DOMAIN + name: review/$CI_COMMIT_REF_NAME + url: http://$CI_PROJECT_NAME-$CI_ENVIRONMENT_SLUG.$KUBE_DOMAIN on_stop: stop_review only: - branches @@ -68,7 +68,7 @@ stop_review: script: - command destroy environment: - name: review/$CI_BUILD_REF_NAME + name: review/$CI_COMMIT_REF_NAME action: stop when: manual only: |