diff --git a/CHANGELOG-EE b/CHANGELOG-EE index a864e2df731a4968b81eb73c98b49ce7831d2c29..506ff45c69966e138dec243009d447c70fe1d7b8 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.6.0 (unreleased) - Handle duplicate appearances table creation issue with upgrade from CE to EE + - Add confidential issues - Improve weight filter for issues - Clear "stuck" mirror updates before periodically updating all mirrors. - [Elastic] Add elastic checker to gitlab:check diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e884a286c4358a4e9581666573d3eb636253a89f..bf621a1c89af24f9a4e9765f4c662e82a43b29ee 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -5,7 +5,7 @@ class Projects::IssuesController < Projects::ApplicationController before_action :issue, only: [:edit, :update, :show] # Allow read any issue - before_action :authorize_read_issue! + before_action :authorize_read_issue!, only: [:show] # Allow write(create) issue before_action :authorize_create_issue!, only: [:new, :create] @@ -133,6 +133,10 @@ def issue end alias_method :subscribable_resource, :issue + def authorize_read_issue! + return render_404 unless can?(current_user, :read_issue, @issue) + end + def authorize_update_issue! return render_404 unless can?(current_user, :update_issue, @issue) end @@ -163,7 +167,7 @@ def redirect_old def issue_params params.require(:issue).permit( - :title, :assignee_id, :position, :description, :weight, + :title, :assignee_id, :position, :description, :confidential, :weight, :milestone_id, :state_event, :task_num, label_ids: [] ) end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 89e8ea678786f5d4e2bf776b9c0fc8c53fba8904..fd8580d49e06053659999dfaa6b00c38f2c67d86 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -135,7 +135,7 @@ def destroy def autocomplete_sources note_type = params['type'] note_id = params['type_id'] - autocomplete = ::Projects::AutocompleteService.new(@project) + autocomplete = ::Projects::AutocompleteService.new(@project, current_user) participants = ::Projects::ParticipantsService.new(@project, current_user).execute(note_type, note_id) @suggestions = { diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index 20a2b0ce8f0dbb21d25e8941954dd8b26b0128ca..c2befa5a5b3a4837010ecc1c035f36437c2f56d8 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -19,4 +19,10 @@ class IssuesFinder < IssuableFinder def klass Issue end + + private + + def init_collection + Issue.visible_to_user(current_user) + end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 324fee6c8069d70bb83c3ba231abe68fbd7700e8..2c98cb69ad9454fe5ab54e0cdafca60856cd9f8f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -288,7 +288,7 @@ def state_filters_text_for(entity, project) if project.nil? nil elsif current_controller?(:issues) - project.issues.send(entity).count + project.issues.visible_to_user(current_user).send(entity).count elsif current_controller?(:merge_requests) project.merge_requests.send(entity).count end diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 37a888d9c60bdd3d70ffa1c202e2ae3c53a0f77e..a67a6b208e256bdfb9aa4023569e57b16b071ed4 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -194,7 +194,7 @@ def event_commit_title(message) end def event_to_atom(xml, event) - if event.proper? + if event.proper?(current_user) xml.entry do event_link = event_feed_url(event) event_title = event_feed_title(event) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index ea6f1d5f6aefc43e3e5be9d45251cf984dc1d650..1f0340549468f2af8b8e1832720770ef4cd9dea5 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -98,6 +98,10 @@ def merge_requests_sentence(merge_requests) end.sort.to_sentence(last_word_connector: ', or ') end + def confidential_icon(issue) + icon('eye-slash') if issue.confidential? + end + def emoji_icon(name, unicode = nil, aliases = []) unicode ||= Emoji.emoji_filename(name) rescue "" diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb index e8ac8788d9dcc3af98c0d8f257fbf426847197dd..92ed0891e92bdf2c8fb6ec30ca2cf03a8b7f58fb 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/milestones_helper.rb @@ -38,7 +38,7 @@ def milestone_issues_by_label_count(milestone, label, state:) def milestone_progress_bar(milestone) options = { class: 'progress-bar progress-bar-success', - style: "width: #{milestone.percent_complete}%;" + style: "width: #{milestone.percent_complete(current_user)}%;" } content_tag :div, class: 'progress' do diff --git a/app/models/ability.rb b/app/models/ability.rb index 1941229e973ca10d9a3155af23440c0bd39a575f..357d48168da6c27d1a0a704a3372fea1ca8f6673 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -63,7 +63,6 @@ def anonymous_project_abilities(subject) rules = [ :read_project, :read_wiki, - :read_issue, :read_label, :read_milestone, :read_project_snippet, @@ -77,6 +76,9 @@ def anonymous_project_abilities(subject) # Allow to read builds by anonymous user if guests are allowed rules << :read_build if project.public_builds? + # Allow to read issues by anonymous user if issue is not confidential + rules << :read_issue unless subject.is_a?(Issue) && subject.confidential? + rules - project_disabled_features_rules(project) else [] @@ -343,6 +345,7 @@ def namespace_abilities(user, namespace) end rules += project_abilities(user, subject.project) + rules = filter_confidential_issues_abilities(user, subject, rules) if subject.is_a?(Issue) rules end end @@ -461,5 +464,17 @@ def named_abilities(name) :"admin_#{name}" ] end + + def filter_confidential_issues_abilities(user, issue, rules) + return rules if user.admin? || !issue.confidential? + + unless issue.author == user || issue.assignee == user || issue.project.team.member?(user.id) + rules.delete(:admin_issue) + rules.delete(:read_issue) + rules.delete(:update_issue) + end + + rules + end end end diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index d67df7c1d9c1933592f0ceebb40ebdd6f0af3b61..5b8e3f654ea6be9a667ad6a08557ef0bb245a789 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -1,18 +1,18 @@ module Milestoneish - def closed_items_count - issues.closed.size + merge_requests.closed_and_merged.size + def closed_items_count(user = nil) + issues_visible_to_user(user).closed.size + merge_requests.closed_and_merged.size end - def total_items_count - issues.size + merge_requests.size + def total_items_count(user = nil) + issues_visible_to_user(user).size + merge_requests.size end - def complete? - total_items_count == closed_items_count + def complete?(user = nil) + total_items_count(user) == closed_items_count(user) end - def percent_complete - ((closed_items_count * 100) / total_items_count).abs + def percent_complete(user = nil) + ((closed_items_count(user) * 100) / total_items_count(user)).abs rescue ZeroDivisionError 0 end @@ -22,4 +22,8 @@ def remaining_days (due_date - Date.today).to_i end + + def issues_visible_to_user(user = nil) + issues.visible_to_user(user) + end end diff --git a/app/models/event.rb b/app/models/event.rb index 67b75ea251d9dcb98c2977fe4a6a3a8607f1d3c4..59057983f216dc56a9d2333e9669dad0734487e4 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -79,15 +79,17 @@ def limit_recent(limit = 20, offset = nil) end end - def proper? + def proper?(user = nil) if push? true elsif membership_changed? true elsif created_project? true + elsif issue? + Ability.abilities.allowed?(user, :read_issue, issue) else - ((issue? || merge_request? || note?) && target) || milestone? + ((merge_request? || note?) && target) || milestone? end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 89b4c827e44c640b84b4230e34c19a180dd53232..0cb16f97f2b3656e02de4ff6cb7efb8f31b11268 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -64,6 +64,13 @@ def hook_attrs attributes end + def self.visible_to_user(user) + return where(confidential: false) if user.blank? + return all if user.admin? + + where('issues.confidential = false OR (issues.confidential = true AND (issues.author_id = :user_id OR issues.assignee_id = :user_id OR issues.project_id IN(:project_ids)))', user_id: user.id, project_ids: user.authorized_projects.select(:id)) + end + def self.reference_prefix '#' end diff --git a/app/models/milestone.rb b/app/models/milestone.rb index a15876bde2d62cfdc542ecb53ef0b53dfc7e7490..0ce1654d8f1568043e7384313d05be7c9b855bea 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -122,8 +122,8 @@ def can_be_closed? active? && issues.opened.count.zero? end - def is_empty? - total_items_count.zero? + def is_empty?(user = nil) + total_items_count(user).zero? end def author_id diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 7408e09ed1e9df60df177b33cade2521db187cda..ba50305dbd5fca2831c1534cd4a8a0eec4f94a57 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -1,11 +1,7 @@ module Projects class AutocompleteService < BaseService - def initialize(project) - @project = project - end - def issues - @project.issues.opened.select([:iid, :title]) + @project.issues.visible_to_user(current_user).opened.select([:iid, :title]) end def merge_requests diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb index a515317bb14688ae82017e12ed92ea35acc22694..bc4478b5a44d9189005081c90657047a6b06d2a4 100644 --- a/app/services/search/global_service.rb +++ b/app/services/search/global_service.rb @@ -12,9 +12,9 @@ def execute projects = projects.in_namespace(group.id) if group if Gitlab.config.elasticsearch.enabled - Gitlab::Elastic::SearchResults.new(projects.pluck(:id), params[:search]) + Gitlab::Elastic::SearchResults.new(current_user, projects.pluck(:id), params[:search]) else - Gitlab::SearchResults.new(projects, params[:search]) + Gitlab::SearchResults.new(current_user, projects, params[:search]) end end end diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb index 1ab5339597a63c91e6d877453eee49cce84519fd..0531c046ba0acac92504a38fdf91ddb45922eea5 100644 --- a/app/services/search/project_service.rb +++ b/app/services/search/project_service.rb @@ -8,11 +8,13 @@ def initialize(project, user, params) def execute if Gitlab.config.elasticsearch.enabled - Gitlab::Elastic::ProjectSearchResults.new(project.id, - params[:search], - params[:repository_ref]) + Gitlab::Elastic::ProjectSearchResults.new(current_user, + project.id, + params[:search], + params[:repository_ref]) else - Gitlab::ProjectSearchResults.new(project, + Gitlab::ProjectSearchResults.new(current_user, + project, params[:search], params[:repository_ref]) end diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml index 36fb2d5162955f96ff9cdbc18818400fb22c333a..2d9d9dd634243eb44d8878f0851c065fff0446aa 100644 --- a/app/views/events/_event.html.haml +++ b/app/views/events/_event.html.haml @@ -1,4 +1,4 @@ -- if event.proper? +- if event.proper?(current_user) .event-item{class: "#{event.body? ? "event-block" : "event-inline" }"} .event-item-timestamp #{time_ago_with_tooltip(event.created_at)} diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml index a9152b2ab415136732b9c43163b5e37b82d99478..7a90d4474cf0098e381d9a16256592b39d4407a4 100644 --- a/app/views/layouts/nav/_group.html.haml +++ b/app/views/layouts/nav/_group.html.haml @@ -30,7 +30,7 @@ %span Issues - if current_user - %span.count= number_with_delimiter(Issue.opened.of_group(@group).count) + %span.count= number_with_delimiter(Issue.visible_to_user(current_user).opened.of_group(@group).count) = nav_link(path: 'groups#merge_requests') do = link_to merge_requests_group_path(@group), title: 'Merge Requests' do = icon('tasks fw') diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 0ae83ee01ebc6bd519a6333dbcc446ab6d1d40f0..86b46e8c75ec33cf5f4be6463a0c0546f4d98740 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -67,7 +67,7 @@ %span Issues - if @project.default_issues_tracker? - %span.count.issue_counter= number_with_delimiter(@project.issues.opened.count) + %span.count.issue_counter= number_with_delimiter(@project.issues.visible_to_user(current_user).opened.count) - if project_nav_tab? :merge_requests = nav_link(controller: :merge_requests) do diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 0daf492b652c0820b2d0d67df2e9cfc6be70ed28..3a1e4d13b7c663df12232d96f665f37dfe94b972 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -5,6 +5,7 @@ .issue-title %span.issue-title-text + = confidential_icon(issue) = link_to_gfm issue.title, issue_path(issue), class: "title" %ul.controls.light - if issue.closed? diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 0242276cd84176c78e70b08f07b3b67b5aa9d457..c6c6fe967b4dd5bdc0c5ef0f4e8ac61209d7d603 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -22,6 +22,7 @@ = icon('angle-double-left') .issue-meta + = confidential_icon(@issue) %strong.identifier Issue ##{@issue.iid} %span.creator @@ -50,7 +51,6 @@ = icon('pencil-square-o') Edit - .issue-details.issuable-details .detail-page-description.content-block %h2.title diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index b4597043a27af1ec6a0fc1cc541e056e4da12886..be63875ab34c0ab1f97104adb9fb6df13d8d478c 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -42,7 +42,7 @@ = preserve do = markdown @milestone.description -- if @milestone.complete? && @milestone.active? +- if @milestone.complete?(current_user) && @milestone.active? .alert.alert-success.prepend-top-default %span All issues for this milestone are closed. You may close milestone now. diff --git a/app/views/search/results/_issue.html.haml b/app/views/search/results/_issue.html.haml index 45d700781f3e3d14547268ef9444810112a8e32e..710f5613c817238893531ba540af9573f796150d 100644 --- a/app/views/search/results/_issue.html.haml +++ b/app/views/search/results/_issue.html.haml @@ -1,5 +1,6 @@ .search-result-row %h4 + = confidential_icon(issue) = link_to [issue.project.namespace.becomes(Namespace), issue.project, issue] do %span.term.str-truncated= issue.title .pull-right ##{issue.iid} diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 476d0131747dd1b71efc9eab310b8b927b0fe335..2955ad22a8dec9367cbf06359d0494784b06e865 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -19,6 +19,7 @@ - else Start the title with [WIP] or WIP: to prevent a Work In Progress merge request from being merged before it's ready. + .form-group.detail-page-description = f.label :description, 'Description', class: 'control-label' .col-sm-10 @@ -29,6 +30,15 @@ = render 'projects/notes/hints' .clearfix .error-alert + +- if issuable.is_a?(Issue) && !issuable.project.private? + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :confidential do + = f.check_box :confidential + This issue is confidential and should only be visible to team members + - if can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project) %hr .form-group diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml index f7c6fc14adf196b0ea186f01f6312e8800bb3a2c..85888096722f5120bdeea56791de5c820c554335 100644 --- a/app/views/shared/milestones/_issuable.html.haml +++ b/app/views/shared/milestones/_issuable.html.haml @@ -10,6 +10,8 @@ %strong #{project.name} · - elsif show_full_project_name %strong #{project.name_with_namespace} · + - if issuable.is_a?(Issue) + = confidential_icon(issuable) = link_to_gfm issuable.title, [project.namespace.becomes(Namespace), project, issuable], title: issuable.title %div{class: 'issuable-detail'} = link_to [project.namespace.becomes(Namespace), project, issuable] do diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml index f01138af3f09c20d2eaef18d63366c44d733aa9f..6b25745c55420f400ff8605739b345d8e5547e00 100644 --- a/app/views/shared/milestones/_milestone.html.haml +++ b/app/views/shared/milestones/_milestone.html.haml @@ -6,10 +6,10 @@ .col-sm-6 %strong= link_to_gfm truncate(milestone.title, length: 100), milestone_path .col-sm-6 - .pull-right.light #{milestone.percent_complete}% complete + .pull-right.light #{milestone.percent_complete(current_user)}% complete .row .col-sm-6 - = link_to pluralize(milestone.issues.size, 'Issue'), issues_path + = link_to pluralize(milestone.issues_visible_to_user(current_user).size, 'Issue'), issues_path · = link_to pluralize(milestone.merge_requests.size, 'Merge Request'), merge_requests_path .col-sm-6= milestone_progress_bar(milestone) diff --git a/app/views/shared/milestones/_summary.html.haml b/app/views/shared/milestones/_summary.html.haml index 59d4ae29f791f349d13e0bc8903011c5655f3636..385c65966067485788a8bc4b3c44d164e4b3c083 100644 --- a/app/views/shared/milestones/_summary.html.haml +++ b/app/views/shared/milestones/_summary.html.haml @@ -3,15 +3,15 @@ .context.prepend-top-default .milestone-summary %h4 Progress - %strong= milestone.issues.size + %strong= milestone.issues_visible_to_user(current_user).size issues: %span.milestone-stat - %strong= milestone.issues.opened.size + %strong= milestone.issues_visible_to_user(current_user).opened.size open and - %strong= milestone.issues.closed.size + %strong= milestone.issues_visible_to_user(current_user).closed.size closed %span.milestone-stat - %strong== #{milestone.percent_complete}% + %strong== #{milestone.percent_complete(current_user)}% complete %span.milestone-stat diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml index 57d7ee85a3b3ac27289d39c02e5b5ce21a9c3c31..2b6ce2d7e7a38cf6304802b540d5f8dbb32745c1 100644 --- a/app/views/shared/milestones/_tabs.html.haml +++ b/app/views/shared/milestones/_tabs.html.haml @@ -2,7 +2,7 @@ %li.active = link_to '#tab-issues', 'data-toggle' => 'tab', 'data-show' => '.tab-issues-buttons' do Issues - %span.badge= milestone.issues.size + %span.badge= milestone.issues_visible_to_user(current_user).size %li = link_to '#tab-merge-requests', 'data-toggle' => 'tab', 'data-show' => '.tab-merge-requests-buttons' do Merge Requests @@ -21,7 +21,7 @@ .tab-content.milestone-content .tab-pane.active#tab-issues - = render 'shared/milestones/issues_tab', issues: milestone.issues, show_project_name: show_project_name, show_full_project_name: show_full_project_name + = render 'shared/milestones/issues_tab', issues: milestone.issues_visible_to_user(current_user), show_project_name: show_project_name, show_full_project_name: show_full_project_name .tab-pane#tab-merge-requests = render 'shared/milestones/merge_requests_tab', merge_requests: milestone.merge_requests, show_project_name: show_project_name, show_full_project_name: show_full_project_name .tab-pane#tab-participants diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml index 4cf1d948b5b952d84f08931b556433b79f0b9e4b..cab8743a0772dca5e169fe3d4b3fbc16ab993154 100644 --- a/app/views/shared/milestones/_top.html.haml +++ b/app/views/shared/milestones/_top.html.haml @@ -28,7 +28,7 @@ %h2.title = markdown escape_once(milestone.title), pipeline: :single_line -- if milestone.complete? && milestone.active? +- if milestone.complete?(current_user) && milestone.active? .alert.alert-success.prepend-top-default - close_msg = group ? 'You may close the milestone now.' : 'Navigate to the project to close the milestone.' %span All issues for this milestone are closed. #{close_msg} @@ -47,7 +47,7 @@ - project_name = group ? ms.project.name : ms.project.name_with_namespace = link_to project_name, namespace_project_milestone_path(ms.project.namespace, ms.project, ms) %td - = ms.issues.opened.count + = ms.issues_visible_to_user(current_user).opened.count %td - if ms.closed? Closed diff --git a/db/migrate/20160223192159_add_confidential_to_issues.rb b/db/migrate/20160223192159_add_confidential_to_issues.rb new file mode 100644 index 0000000000000000000000000000000000000000..e9d47fd589aff45f4de9c10db52b066074e05131 --- /dev/null +++ b/db/migrate/20160223192159_add_confidential_to_issues.rb @@ -0,0 +1,6 @@ +class AddConfidentialToIssues < ActiveRecord::Migration + def change + add_column :issues, :confidential, :boolean, default: false + add_index :issues, :confidential + end +end diff --git a/db/schema.rb b/db/schema.rb index 6baa49e538ce32952456129beeff57854fb859c2..cfc31bf383bb80ae7e580fcdb3b68f74682979c7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -486,10 +486,12 @@ t.integer "iid" t.integer "updated_by_id" t.integer "weight" + t.boolean "confidential", default: false end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree add_index "issues", ["author_id"], name: "index_issues_on_author_id", using: :btree + add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree add_index "issues", ["created_at", "id"], name: "index_issues_on_created_at_and_id", using: :btree add_index "issues", ["created_at"], name: "index_issues_on_created_at", using: :btree add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} diff --git a/features/steps/groups.rb b/features/steps/groups.rb index 5df6401cee26254073957c2c268dcf8ea2622b6b..dd9e4dce23df59157c579afe3dc0e45aaa359f5c 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -39,7 +39,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps end step 'I should see projects activity feed' do - expect(page).to have_content 'closed issue' + expect(page).to have_content 'joined project' end step 'I should see issues from group "Owned" assigned to me' do diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 252744515da88088f8a8b3aa8eb49161db9ab1e4..fda6f8414384a7190f8558fb316d1364e39bd870 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -82,7 +82,7 @@ def create_spam_log(project, current_user, attrs) # GET /projects/:id/issues?milestone=1.0.0&state=closed # GET /issues?iid=42 get ":id/issues" do - issues = user_project.issues + issues = user_project.issues.visible_to_user(current_user) issues = filter_issues_state(issues, params[:state]) unless params[:state].nil? issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil? issues = filter_by_iid(issues, params[:iid]) unless params[:iid].nil? @@ -104,6 +104,7 @@ def create_spam_log(project, current_user, attrs) # GET /projects/:id/issues/:issue_id get ":id/issues/:issue_id" do @issue = user_project.issues.find(params[:issue_id]) + not_found! unless can?(current_user, :read_issue, @issue) present @issue, with: Entities::Issue end diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb index 9f08aa36e8b5bfd9cc541ee08bc65452d2f27212..2732e0b51455cadfebabf4b936fbf319e854f19c 100644 --- a/lib/banzai/filter/issue_reference_filter.rb +++ b/lib/banzai/filter/issue_reference_filter.rb @@ -9,6 +9,11 @@ def self.object_class Issue end + def self.user_can_see_reference?(user, node, context) + issue = Issue.find(node.attr('data-issue')) rescue nil + Ability.abilities.allowed?(user, :read_issue, issue) + end + def find_object(project, id) project.get_issue(id) end diff --git a/lib/elastic/application_search.rb b/lib/elastic/application_search.rb index fdebd58b4bfecd41430a35501e073a7d5927b5a2..8ef796f0215fab09b4b221927fd52459c73af084 100644 --- a/lib/elastic/application_search.rb +++ b/lib/elastic/application_search.rb @@ -113,7 +113,9 @@ def iid_query_hash(query_hash, iid) def project_ids_filter(query_hash, project_ids) if project_ids query_hash[:query][:filtered][:filter] = { - and: [ { terms: { project_id: project_ids } } ] + bool: { + must: [ { terms: { project_id: project_ids } } ] + } } end diff --git a/lib/elastic/issues_search.rb b/lib/elastic/issues_search.rb index 90593d4dd91d4af6042cfed7b04d75b14472fda5..55a6ce1eb90f799be4ead41a900558e16679e8d7 100644 --- a/lib/elastic/issues_search.rb +++ b/lib/elastic/issues_search.rb @@ -19,10 +19,13 @@ module IssuesSearch indexes :project_id, type: :integer indexes :author_id, type: :integer + indexes :assignee_id, type: :integer indexes :project, type: :nested indexes :author, type: :nested + indexes :confidential, type: :boolean + indexes :updated_at_sort, type: :date, index: :not_analyzed end @@ -31,7 +34,7 @@ def as_indexed_json(options = {}) # We don't use as_json(only: ...) because it calls all virtual and serialized attributtes # https://gitlab.com/gitlab-org/gitlab-ee/issues/349 - [:id, :iid, :title, :description, :created_at, :updated_at, :state, :project_id, :author_id].each do |attr| + [:id, :iid, :title, :description, :created_at, :updated_at, :state, :project_id, :author_id, :assignee_id, :confidential].each do |attr| data[attr.to_s] = self.send(attr) end @@ -49,9 +52,43 @@ def self.elastic_search(query, options: {}) end query_hash = project_ids_filter(query_hash, options[:project_ids]) + query_hash = confidentiality_filter(query_hash, options[:current_user]) self.__elasticsearch__.search(query_hash) end + + def self.confidentiality_filter(query_hash, current_user) + return query_hash if current_user.present? && current_user.admin? + + filter = if current_user.present? + { + bool: { + should: [ + { term: { confidential: false } }, + { bool: { + must: [ + { term: { confidential: true } }, + { bool: { + should: [ + { term: { author_id: current_user.id } }, + { term: { assignee_id: current_user.id } }, + { terms: { project_id: current_user.authorized_projects.pluck(:id) } } + ] + } + } + ] + } + } + ] + } + } + else + { term: { confidential: false } } + end + + query_hash[:query][:filtered][:filter][:bool][:must] << filter + query_hash + end end end end diff --git a/lib/gitlab/elastic/project_search_results.rb b/lib/gitlab/elastic/project_search_results.rb index 55803e04fe41872e9a188b0fe569259ceebf524f..78e53af685392f2c132944e8ac4306776a7bcf73 100644 --- a/lib/gitlab/elastic/project_search_results.rb +++ b/lib/gitlab/elastic/project_search_results.rb @@ -3,7 +3,8 @@ module Elastic class ProjectSearchResults < SearchResults attr_reader :project, :repository_ref - def initialize(project_id, query, repository_ref = nil) + def initialize(current_user, project_id, query, repository_ref = nil) + @current_user = current_user @project = Project.find(project_id) @repository_ref = if repository_ref.present? diff --git a/lib/gitlab/elastic/search_results.rb b/lib/gitlab/elastic/search_results.rb index ef295d04e17edd8ad455b84106c55d30958dc1a7..37ddd1c13c7621bc25d699cace755957b0429a07 100644 --- a/lib/gitlab/elastic/search_results.rb +++ b/lib/gitlab/elastic/search_results.rb @@ -1,13 +1,14 @@ module Gitlab module Elastic class SearchResults - attr_reader :query + attr_reader :current_user, :query # Limit search results by passed project ids # It allows us to search only for projects user has access to attr_reader :limit_project_ids - def initialize(limit_project_ids, query) + def initialize(current_user, limit_project_ids, query) + @current_user = current_user @limit_project_ids = limit_project_ids || Project.all @query = Shellwords.shellescape(query) if query.present? end @@ -63,7 +64,8 @@ def projects def issues opt = { - project_ids: limit_project_ids + project_ids: limit_project_ids, + current_user: current_user } Issue.elastic_search(query, options: opt) diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 0607a8b95927940941509ff216ad98dd3fd839b0..71c5b6801fb66ef94cba8c8533d530be688a17fb 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -2,7 +2,8 @@ module Gitlab class ProjectSearchResults < SearchResults attr_reader :project, :repository_ref - def initialize(project, query, repository_ref = nil) + def initialize(current_user, project, query, repository_ref = nil) + @current_user = current_user @project = project @repository_ref = if repository_ref.present? repository_ref diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index f13528a2eea549b2a929ea501edc1d6cc7da6747..f8ab2b1f09ec0acafaee12bcb8d7eb3375983c7d 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -1,12 +1,13 @@ module Gitlab class SearchResults - attr_reader :query + attr_reader :current_user, :query # Limit search results by passed projects # It allows us to search only for projects user has access to attr_reader :limit_projects - def initialize(limit_projects, query) + def initialize(current_user, limit_projects, query) + @current_user = current_user @limit_projects = limit_projects || Project.all @query = Shellwords.shellescape(query) if query.present? end @@ -58,7 +59,7 @@ def projects end def issues - issues = Issue.where(project_id: project_ids_relation) + issues = Issue.visible_to_user(current_user).where(project_id: project_ids_relation) if query =~ /#(\d+)\z/ issues = issues.where(iid: $1) diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 76d56bc989d97507a399a8aa60ba96122164f194..2cd81231144c418bcfe9c4357794a9d6c32e6f87 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1,16 +1,16 @@ require('spec_helper') describe Projects::IssuesController do - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:issue) { create(:issue, project: project) } + describe "GET #index" do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } - before do - sign_in(user) - project.team << [user, :developer] - end + before do + sign_in(user) + project.team << [user, :developer] + end - describe "GET #index" do it "returns index" do get :index, namespace_id: project.namespace.path, project_id: project.path @@ -38,6 +38,152 @@ get :index, namespace_id: project.namespace.path, project_id: project.path expect(response.status).to eq(404) end + end + + describe 'Confidential Issues' do + let(:project) { create(:empty_project, :public) } + let(:assignee) { create(:assignee) } + let(:author) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let!(:issue) { create(:issue, project: project) } + let!(:unescaped_parameter_value) { create(:issue, :confidential, project: project, author: author) } + let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignee: assignee) } + + describe 'GET #index' do + it 'should not list confidential issues for guests' do + sign_out(:user) + get_issues + + expect(assigns(:issues)).to eq [issue] + end + + it 'should not list confidential issues for non project members' do + sign_in(non_member) + get_issues + + expect(assigns(:issues)).to eq [issue] + end + + it 'should list confidential issues for author' do + sign_in(author) + get_issues + + expect(assigns(:issues)).to include unescaped_parameter_value + expect(assigns(:issues)).not_to include request_forgery_timing_attack + end + + it 'should list confidential issues for assignee' do + sign_in(assignee) + get_issues + + expect(assigns(:issues)).not_to include unescaped_parameter_value + expect(assigns(:issues)).to include request_forgery_timing_attack + end + + it 'should list confidential issues for project members' do + sign_in(member) + project.team << [member, :developer] + + get_issues + + expect(assigns(:issues)).to include unescaped_parameter_value + expect(assigns(:issues)).to include request_forgery_timing_attack + end + + it 'should list confidential issues for admin' do + sign_in(admin) + get_issues + + expect(assigns(:issues)).to include unescaped_parameter_value + expect(assigns(:issues)).to include request_forgery_timing_attack + end + + def get_issues + get :index, + namespace_id: project.namespace.to_param, + project_id: project.to_param + end + end + shared_examples_for 'restricted action' do |http_status| + it 'returns 404 for guests' do + sign_out :user + go(id: unescaped_parameter_value.to_param) + + expect(response).to have_http_status :not_found + end + + it 'returns 404 for non project members' do + sign_in(non_member) + go(id: unescaped_parameter_value.to_param) + + expect(response).to have_http_status :not_found + end + + it "returns #{http_status[:success]} for author" do + sign_in(author) + go(id: unescaped_parameter_value.to_param) + + expect(response).to have_http_status http_status[:success] + end + + it "returns #{http_status[:success]} for assignee" do + sign_in(assignee) + go(id: request_forgery_timing_attack.to_param) + + expect(response).to have_http_status http_status[:success] + end + + it "returns #{http_status[:success]} for project members" do + sign_in(member) + project.team << [member, :developer] + go(id: unescaped_parameter_value.to_param) + + expect(response).to have_http_status http_status[:success] + end + + it "returns #{http_status[:success]} for admin" do + sign_in(admin) + go(id: unescaped_parameter_value.to_param) + + expect(response).to have_http_status http_status[:success] + end + end + + describe 'GET #show' do + it_behaves_like 'restricted action', success: 200 + + def go(id:) + get :show, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: id + end + end + + describe 'GET #edit' do + it_behaves_like 'restricted action', success: 200 + + def go(id:) + get :edit, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: id + end + end + + describe 'PUT #update' do + it_behaves_like 'restricted action', success: 302 + + def go(id:) + put :update, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: id, + issue: { title: 'New title' } + end + end end end diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index 722095de5905efa1f2937a14862ef391008590c8..e72aa9479b757973db6330557595826e4e17a9c7 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -4,6 +4,10 @@ author project + trait :confidential do + confidential true + end + trait :closed do state :closed end diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb index e9bb388e361b44743f791ad087411799049df26b..9acf6304bcb19fc9b79792f7141f4850f00d0755 100644 --- a/spec/lib/banzai/filter/redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/redactor_filter_spec.rb @@ -44,8 +44,78 @@ def reference_link(data) end end - context "for user references" do + context 'with data-issue' do + context 'for confidential issues' do + it 'removes references for non project members' do + non_member = create(:user) + project = create(:empty_project, :public) + issue = create(:issue, :confidential, project: project) + + link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + doc = filter(link, current_user: non_member) + + expect(doc.css('a').length).to eq 0 + end + + it 'allows references for author' do + author = create(:user) + project = create(:empty_project, :public) + issue = create(:issue, :confidential, project: project, author: author) + + link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + doc = filter(link, current_user: author) + + expect(doc.css('a').length).to eq 1 + end + + it 'allows references for assignee' do + assignee = create(:user) + project = create(:empty_project, :public) + issue = create(:issue, :confidential, project: project, assignee: assignee) + + link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + doc = filter(link, current_user: assignee) + expect(doc.css('a').length).to eq 1 + end + + it 'allows references for project members' do + member = create(:user) + project = create(:empty_project, :public) + project.team << [member, :developer] + issue = create(:issue, :confidential, project: project) + + link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + doc = filter(link, current_user: member) + + expect(doc.css('a').length).to eq 1 + end + + it 'allows references for admin' do + admin = create(:admin) + project = create(:empty_project, :public) + issue = create(:issue, :confidential, project: project) + + link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + doc = filter(link, current_user: admin) + + expect(doc.css('a').length).to eq 1 + end + end + + it 'allows references for non confidential issues' do + user = create(:user) + project = create(:empty_project, :public) + issue = create(:issue, project: project) + + link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + doc = filter(link, current_user: user) + + expect(doc.css('a').length).to eq 1 + end + end + + context "for user references" do context 'with data-group' do it 'removes unpermitted Group references' do user = create(:user) diff --git a/spec/lib/elastic/issue_spec.rb b/spec/lib/elastic/issue_spec.rb index 73ad0b7d29125bc09e61bd6178e5faf81fa53671..372bbd0dcb1e6f15d8f4dcc0eb530f7444aa82ff 100644 --- a/spec/lib/elastic/issue_spec.rb +++ b/spec/lib/elastic/issue_spec.rb @@ -33,7 +33,8 @@ issue = create :issue, project: project expected_hash = issue.attributes.extract!('id', 'iid', 'title', 'description', 'created_at', - 'updated_at', 'state', 'project_id', 'author_id') + 'updated_at', 'state', 'project_id', 'author_id', + 'assignee_id', 'confidential') expected_hash['project'] = { "id" => project.id } expected_hash['author'] = { "id" => issue.author_id } diff --git a/spec/lib/gitlab/closing_issue_extractor_spec.rb b/spec/lib/gitlab/closing_issue_extractor_spec.rb index 04cf11fc6f17525039eaa017a8bbf071f7f99d91..844fd79c991af8b69f1be01d296f51c4e1c302ca 100644 --- a/spec/lib/gitlab/closing_issue_extractor_spec.rb +++ b/spec/lib/gitlab/closing_issue_extractor_spec.rb @@ -11,6 +11,7 @@ subject { described_class.new(project, project.creator) } before do + project.team << [project.creator, :developer] project2.team << [project.creator, :master] end diff --git a/spec/lib/gitlab/elastic/project_search_results_spec.rb b/spec/lib/gitlab/elastic/project_search_results_spec.rb index 32c8d38941c0a98c60cdab4819edcf6e1af5d718..cee317fe21377ca07eecd90accff383244896763 100644 --- a/spec/lib/gitlab/elastic/project_search_results_spec.rb +++ b/spec/lib/gitlab/elastic/project_search_results_spec.rb @@ -1,33 +1,35 @@ require 'spec_helper' describe Gitlab::Elastic::ProjectSearchResults, lib: true do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:query) { 'hello world' } + before do allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(true) Project.__elasticsearch__.create_index! - - @project = create(:project) + Issue.__elasticsearch__.create_index! end after do allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(false) Project.__elasticsearch__.delete_index! + Issue.__elasticsearch__.delete_index! end - let(:query) { 'hello world' } - describe 'initialize with empty ref' do - let(:results) { Gitlab::Elastic::ProjectSearchResults.new(@project.id, query, '') } + subject(:results) { described_class.new(user, project.id, query, '') } - it { expect(results.project).to eq(@project) } + it { expect(results.project).to eq(project) } it { expect(results.repository_ref).to be_nil } it { expect(results.query).to eq('hello world') } end describe 'initialize with ref' do let(:ref) { 'refs/heads/test' } - let(:results) { Gitlab::Elastic::ProjectSearchResults.new(@project.id, query, ref) } + subject(:results) { described_class.new(user, project.id, query, ref) } - it { expect(results.project).to eq(@project) } + it { expect(results.project).to eq(project) } it { expect(results.repository_ref).to eq(ref) } it { expect(results.query).to eq('hello world') } end @@ -39,7 +41,7 @@ project.repository.index_blobs project.repository.index_commits - + # Notes create :note, note: 'bla-bla term', project: project # The note in the project you have no access to @@ -53,13 +55,81 @@ Project.__elasticsearch__.refresh_index! - result = Gitlab::Elastic::ProjectSearchResults.new(project.id, "term") + result = Gitlab::Elastic::ProjectSearchResults.new(user, project.id, "term") expect(result.notes_count).to eq(1) expect(result.wiki_blobs_count).to eq(1) expect(result.blobs_count).to eq(1) - result1 = Gitlab::Elastic::ProjectSearchResults.new(project.id, "initial") + result1 = Gitlab::Elastic::ProjectSearchResults.new(user, project.id, "initial") expect(result1.commits_count).to eq(1) end end + + describe 'confidential issues' do + let(:query) { 'issue' } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let!(:issue) { create(:issue, project: project, title: 'Issue 1') } + let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) } + + before do + Issue.__elasticsearch__.refresh_index! + end + + it 'should not list project confidential issues for non project members' do + results = described_class.new(non_member, project.id, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(results.issues_count).to eq 1 + end + + it 'should list project confidential issues for author' do + results = described_class.new(author, project.id, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(results.issues_count).to eq 2 + end + + it 'should list project confidential issues for assignee' do + results = described_class.new(assignee, project.id, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).to include security_issue_2 + expect(results.issues_count).to eq 2 + end + + it 'should list project confidential issues for project members' do + project.team << [member, :developer] + + results = described_class.new(member, project.id, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).to include security_issue_2 + expect(results.issues_count).to eq 3 + end + + it 'should list all project issues for admin' do + results = described_class.new(admin, project.id, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).to include security_issue_2 + expect(results.issues_count).to eq 3 + end + end end diff --git a/spec/lib/gitlab/elastic/search_results_spec.rb b/spec/lib/gitlab/elastic/search_results_spec.rb index 89b8af6206321b6648ee198c675da87e111a448f..667a466dfdb0c7394d3e4a104618fd8168ce9239 100644 --- a/spec/lib/gitlab/elastic/search_results_spec.rb +++ b/spec/lib/gitlab/elastic/search_results_spec.rb @@ -9,6 +9,7 @@ allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(false) end + let(:user) { create(:user) } let(:project_1) { create(:project) } let(:project_2) { create(:project) } let(:limit_project_ids) { [project_1.id] } @@ -45,7 +46,7 @@ end it 'should list issues that title or description contain the query' do - results = described_class.new(limit_project_ids, 'hello world') + results = described_class.new(user, limit_project_ids, 'hello world') issues = results.objects('issues') expect(issues).to include @issue_1 @@ -55,14 +56,14 @@ end it 'should return empty list when issues title or description does not contain the query' do - results = described_class.new(limit_project_ids, 'security') + results = described_class.new(user, limit_project_ids, 'security') expect(results.objects('issues')).to be_empty expect(results.issues_count).to eq 0 end it 'should list issue when search by a valid iid' do - results = described_class.new(limit_project_ids, '#2') + results = described_class.new(user, limit_project_ids, '#2') issues = results.objects('issues') expect(issues).not_to include @issue_1 @@ -72,13 +73,204 @@ end it 'should return empty list when search by invalid iid' do - results = described_class.new(limit_project_ids, '#222') + results = described_class.new(user, limit_project_ids, '#222') expect(results.objects('issues')).to be_empty expect(results.issues_count).to eq 0 end end + describe 'confidential issues' do + let(:project_3) { create(:empty_project) } + let(:project_4) { create(:empty_project) } + let(:limit_project_ids) { [project_1.id, project_2.id, project_3.id] } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let!(:issue) { create(:issue, project: project_1, title: 'Issue 1', iid: 1) } + let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author, iid: 2) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignee: assignee, iid: 3) } + let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author, iid: 1) } + let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignee: assignee, iid: 1) } + let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5', iid: 1) } + + before do + Issue.__elasticsearch__.refresh_index! + end + + context 'search by term' do + let(:query) { 'issue' } + + it 'should not list confidential issues for guests' do + results = described_class.new(nil, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).not_to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 1 + end + + it 'should not list confidential issues for non project members' do + results = described_class.new(non_member, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).not_to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 1 + end + + it 'should list confidential issues for author' do + results = described_class.new(author, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 3 + end + + it 'should list confidential issues for assignee' do + results = described_class.new(assignee, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).to include security_issue_2 + expect(issues).not_to include security_issue_3 + expect(issues).to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 3 + end + + it 'should list confidential issues for project members' do + project_1.team << [member, :developer] + project_2.team << [member, :developer] + + results = described_class.new(member, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 4 + end + + it 'should list all issues for admin' do + results = described_class.new(admin, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 5 + end + end + + context 'search by iid' do + let(:query) { '#1' } + + it 'should not list confidential issues for guests' do + results = described_class.new(nil, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).not_to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 1 + end + + it 'should not list confidential issues for non project members' do + results = described_class.new(non_member, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).not_to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 1 + end + + it 'should list confidential issues for author' do + results = described_class.new(author, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 2 + end + + it 'should list confidential issues for assignee' do + results = described_class.new(assignee, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).not_to include security_issue_3 + expect(issues).to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 2 + end + + it 'should list confidential issues for project members' do + project_2.team << [member, :developer] + project_3.team << [member, :developer] + + results = described_class.new(member, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 3 + end + + it 'should list all issues for admin' do + results = described_class.new(admin, limit_project_ids, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 3 + end + end + end + describe 'merge requests' do before do MergeRequest.__elasticsearch__.create_index! @@ -91,8 +283,8 @@ iid: 1 ) @merge_request_2 = create( - :merge_request, - :conflict, + :merge_request, + :conflict, source_project: project_1, target_project: project_1, title: 'Merge Request 2', @@ -115,7 +307,7 @@ end it 'should list merge requests that title or description contain the query' do - results = described_class.new(limit_project_ids, 'hello world') + results = described_class.new(user, limit_project_ids, 'hello world') merge_requests = results.objects('merge_requests') expect(merge_requests).to include @merge_request_1 @@ -125,14 +317,14 @@ end it 'should return empty list when merge requests title or description does not contain the query' do - results = described_class.new(limit_project_ids, 'security') + results = described_class.new(user, limit_project_ids, 'security') expect(results.objects('merge_requests')).to be_empty expect(results.merge_requests_count).to eq 0 end it 'should list merge request when search by a valid iid' do - results = described_class.new(limit_project_ids, '#2') + results = described_class.new(user, limit_project_ids, '#2') merge_requests = results.objects('merge_requests') expect(merge_requests).not_to include @merge_request_1 @@ -142,14 +334,13 @@ end it 'should return empty list when search by invalid iid' do - results = described_class.new(limit_project_ids, '#222') + results = described_class.new(user, limit_project_ids, '#222') expect(results.objects('merge_requests')).to be_empty expect(results.merge_requests_count).to eq 0 end end - describe 'project scoping' do before do [Project, MergeRequest, Issue, Milestone].each do |model| @@ -190,7 +381,7 @@ model.__elasticsearch__.refresh_index! end - result = Gitlab::Elastic::SearchResults.new([project.id], 'term') + result = Gitlab::Elastic::SearchResults.new(user, [project.id], 'term') expect(result.issues_count).to eq(2) expect(result.merge_requests_count).to eq(2) diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index 09adbc07dcbbdb1c7f5bc58a5ca9526d3430d472..db0ff95b4f5c6c83be75122b2ad560f05dea14a4 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -1,11 +1,12 @@ require 'spec_helper' describe Gitlab::ProjectSearchResults, lib: true do + let(:user) { create(:user) } let(:project) { create(:project) } let(:query) { 'hello world' } describe 'initialize with empty ref' do - let(:results) { Gitlab::ProjectSearchResults.new(project, query, '') } + let(:results) { Gitlab::ProjectSearchResults.new(user, project, query, '') } it { expect(results.project).to eq(project) } it { expect(results.repository_ref).to be_nil } @@ -14,10 +15,74 @@ describe 'initialize with ref' do let(:ref) { 'refs/heads/test' } - let(:results) { Gitlab::ProjectSearchResults.new(project, query, ref) } + let(:results) { Gitlab::ProjectSearchResults.new(user, project, query, ref) } it { expect(results.project).to eq(project) } it { expect(results.repository_ref).to eq(ref) } it { expect(results.query).to eq('hello world') } end + + describe 'confidential issues' do + let(:query) { 'issue' } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let!(:issue) { create(:issue, project: project, title: 'Issue 1') } + let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) } + + it 'should not list project confidential issues for non project members' do + results = described_class.new(non_member, project, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(results.issues_count).to eq 1 + end + + it 'should list project confidential issues for author' do + results = described_class.new(author, project, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(results.issues_count).to eq 2 + end + + it 'should list project confidential issues for assignee' do + results = described_class.new(assignee, project.id, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).to include security_issue_2 + expect(results.issues_count).to eq 2 + end + + it 'should list project confidential issues for project members' do + project.team << [member, :developer] + + results = described_class.new(member, project, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).to include security_issue_2 + expect(results.issues_count).to eq 3 + end + + it 'should list all project issues for admin' do + results = described_class.new(admin, project, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).to include security_issue_2 + expect(results.issues_count).to eq 3 + end + end end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 7d963795e17b48265d15c55ac92d4b4598716409..65af37e24f14e6139511c8b401357a0d59db7aee 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -2,6 +2,7 @@ describe Gitlab::ReferenceExtractor, lib: true do let(:project) { create(:project) } + subject { Gitlab::ReferenceExtractor.new(project, project.creator) } it 'accesses valid user objects' do @@ -41,6 +42,7 @@ end it 'accesses valid issue objects' do + project.team << [project.creator, :developer] @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index bb18f41785824b1c5e37e9d1ae38f214db8ed333..f4afe597e8d8c415c40b5f3bc48c2e00ac3be71f 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe Gitlab::SearchResults do + let(:user) { create(:user) } let!(:project) { create(:project, name: 'foo') } let!(:issue) { create(:issue, project: project, title: 'foo') } @@ -9,7 +10,7 @@ end let!(:milestone) { create(:milestone, project: project, title: 'foo') } - let(:results) { described_class.new(Project.all, 'foo') } + let(:results) { described_class.new(user, Project.all, 'foo') } describe '#total_count' do it 'returns the total amount of search hits' do @@ -52,4 +53,92 @@ expect(results.empty?).to eq(false) end end + + describe 'confidential issues' do + let(:project_1) { create(:empty_project) } + let(:project_2) { create(:empty_project) } + let(:project_3) { create(:empty_project) } + let(:project_4) { create(:empty_project) } + let(:query) { 'issue' } + let(:limit_projects) { Project.where(id: [project_1.id, project_2.id, project_3.id]) } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') } + let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignee: assignee) } + let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) } + let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignee: assignee) } + let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') } + + it 'should not list confidential issues for non project members' do + results = described_class.new(non_member, limit_projects, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).not_to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 1 + end + + it 'should list confidential issues for author' do + results = described_class.new(author, limit_projects, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).not_to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 3 + end + + it 'should list confidential issues for assignee' do + results = described_class.new(assignee, limit_projects, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).not_to include security_issue_1 + expect(issues).to include security_issue_2 + expect(issues).not_to include security_issue_3 + expect(issues).to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 3 + end + + it 'should list confidential issues for project members' do + project_1.team << [member, :developer] + project_2.team << [member, :developer] + + results = described_class.new(member, limit_projects, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).not_to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 4 + end + + it 'should list all issues for admin' do + results = described_class.new(admin, limit_projects, query) + issues = results.objects('issues') + + expect(issues).to include issue + expect(issues).to include security_issue_1 + expect(issues).to include security_issue_2 + expect(issues).to include security_issue_3 + expect(issues).to include security_issue_4 + expect(issues).not_to include security_issue_5 + expect(results.issues_count).to eq 5 + end + end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 253902512c337b178a752f65b5b108050a6c638a..0e9111c8029332a962f3a7cc9d2815c8cdeb569c 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -86,10 +86,21 @@ let(:issue) { create :issue, project: project } let(:other_project) { create :project, :public } let(:other_issue) { create :issue, project: other_project } + let(:commiter) { create :user } + + before do + project.team << [commiter, :developer] + other_project.team << [commiter, :developer] + end it 'detects issues that this commit is marked as closing' do ext_ref = "#{other_project.path_with_namespace}##{other_issue.iid}" - allow(commit).to receive(:safe_message).and_return("Fixes ##{issue.iid} and #{ext_ref}") + + allow(commit).to receive_messages( + safe_message: "Fixes ##{issue.iid} and #{ext_ref}", + committer_email: commiter.email + ) + expect(commit.closes_issues).to include(issue) expect(commit.closes_issues).to include(other_issue) end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 20f0c561e44b16e3f9970f387ecc0a8d57e14fd8..cb33edde820d225b73a6fa529ef77aafa714a95c 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -48,7 +48,8 @@ def author describe '#create_new_cross_references!' do let(:project) { create(:project) } - let(:issues) { create_list(:issue, 2, project: project) } + let(:author) { create(:author) } + let(:issues) { create_list(:issue, 2, project: project, author: author) } context 'before changes are persisted' do it 'ignores pre-existing references' do @@ -91,7 +92,7 @@ def author end def create_issue(description:) - create(:issue, project: project, description: description) + create(:issue, project: project, description: description, author: author) end end end diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..47c3be673c56815d3828be81793f5c2fe9f87303 --- /dev/null +++ b/spec/models/concerns/milestoneish_spec.rb @@ -0,0 +1,104 @@ +require 'spec_helper' + +describe Milestone, 'Milestoneish' do + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let(:project) { create(:project, :public) } + let(:milestone) { create(:milestone, project: project) } + let!(:issue) { create(:issue, project: project, milestone: milestone) } + let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone) } + let!(:security_issue_2) { create(:issue, :confidential, project: project, assignee: assignee, milestone: milestone) } + let!(:closed_issue_1) { create(:issue, :closed, project: project, milestone: milestone) } + let!(:closed_issue_2) { create(:issue, :closed, project: project, milestone: milestone) } + let!(:closed_security_issue_1) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } + let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) } + let!(:closed_security_issue_3) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } + let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) } + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } + + before do + project.team << [member, :developer] + end + + describe '#closed_items_count' do + it 'should not count confidential issues for non project members' do + expect(milestone.closed_items_count(non_member)).to eq 2 + end + + it 'should count confidential issues for author' do + expect(milestone.closed_items_count(author)).to eq 4 + end + + it 'should count confidential issues for assignee' do + expect(milestone.closed_items_count(assignee)).to eq 4 + end + + it 'should count confidential issues for project members' do + expect(milestone.closed_items_count(member)).to eq 6 + end + + it 'should count all issues for admin' do + expect(milestone.closed_items_count(admin)).to eq 6 + end + end + + describe '#total_items_count' do + it 'should not count confidential issues for non project members' do + expect(milestone.total_items_count(non_member)).to eq 4 + end + + it 'should count confidential issues for author' do + expect(milestone.total_items_count(author)).to eq 7 + end + + it 'should count confidential issues for assignee' do + expect(milestone.total_items_count(assignee)).to eq 7 + end + + it 'should count confidential issues for project members' do + expect(milestone.total_items_count(member)).to eq 10 + end + + it 'should count all issues for admin' do + expect(milestone.total_items_count(admin)).to eq 10 + end + end + + describe '#complete?' do + it 'returns false when has items opened' do + expect(milestone.complete?(non_member)).to eq false + end + + it 'returns true when all items are closed' do + issue.close + merge_request.close + + expect(milestone.complete?(non_member)).to eq true + end + end + + describe '#percent_complete' do + it 'should not count confidential issues for non project members' do + expect(milestone.percent_complete(non_member)).to eq 50 + end + + it 'should count confidential issues for author' do + expect(milestone.percent_complete(author)).to eq 57 + end + + it 'should count confidential issues for assignee' do + expect(milestone.percent_complete(assignee)).to eq 57 + end + + it 'should count confidential issues for project members' do + expect(milestone.percent_complete(member)).to eq 60 + end + + it 'should count confidential issues for admin' do + expect(milestone.percent_complete(admin)).to eq 60 + end + end +end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index ec2a923f91bb4a4c4af968f034afda442e865316..5fe442467385f9f75f0ab24dd669e92ad429ff18 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -65,6 +65,42 @@ it { expect(@event.author).to eq(@user) } end + describe '#proper?' do + context 'issue event' do + let(:project) { create(:empty_project, :public) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:author) { create(:author) } + let(:assignee) { create(:user) } + let(:admin) { create(:admin) } + let(:event) { Event.new(project: project, action: Event::CREATED, target: issue, author_id: author.id) } + + before do + project.team << [member, :developer] + end + + context 'for non confidential issues' do + let(:issue) { create(:issue, project: project, author: author, assignee: assignee) } + + it { expect(event.proper?(non_member)).to eq true } + it { expect(event.proper?(author)).to eq true } + it { expect(event.proper?(assignee)).to eq true } + it { expect(event.proper?(member)).to eq true } + it { expect(event.proper?(admin)).to eq true } + end + + context 'for confidential issues' do + let(:issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + + it { expect(event.proper?(non_member)).to eq false } + it { expect(event.proper?(author)).to eq true } + it { expect(event.proper?(assignee)).to eq true } + it { expect(event.proper?(member)).to eq true } + it { expect(event.proper?(admin)).to eq true } + end + end + end + describe '.limit_recent' do let!(:event1) { create(:closed_issue_event) } let!(:event2) { create(:closed_issue_event) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 879b0d5e717ee11141b8cd66587a1829eb01d240..001063d8f5ef4d1fb5f08a7ea92f4317b647bb7e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -150,6 +150,7 @@ let(:commit2) { double('commit2', safe_message: "Fixes #{issue1.to_reference}") } before do + subject.project.team << [subject.author, :developer] allow(subject).to receive(:commits).and_return([commit0, commit1, commit2]) end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index de1757bf67a5f36f98bd9c51553fa0d170978494..72a4ea702281ae5d8dd3b8fbe2c6e86768e1821b 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -32,6 +32,7 @@ let(:milestone) { create(:milestone) } let(:issue) { create(:issue) } + let(:user) { create(:user) } describe "unique milestone title per project" do it "shouldn't accept the same title in a project twice" do @@ -50,18 +51,17 @@ describe "#percent_complete" do it "should not count open issues" do milestone.issues << issue - expect(milestone.percent_complete).to eq(0) + expect(milestone.percent_complete(user)).to eq(0) end it "should count closed issues" do issue.close milestone.issues << issue - expect(milestone.percent_complete).to eq(100) + expect(milestone.percent_complete(user)).to eq(100) end it "should recover from dividing by zero" do - expect(milestone.issues).to receive(:size).and_return(0) - expect(milestone.percent_complete).to eq(0) + expect(milestone.percent_complete(user)).to eq(0) end end @@ -103,7 +103,7 @@ ) end - it { expect(milestone.percent_complete).to eq(75) } + it { expect(milestone.percent_complete(user)).to eq(75) } end describe :items_count do @@ -113,23 +113,23 @@ milestone.merge_requests << create(:merge_request) end - it { expect(milestone.closed_items_count).to eq(1) } - it { expect(milestone.total_items_count).to eq(3) } - it { expect(milestone.is_empty?).to be_falsey } + it { expect(milestone.closed_items_count(user)).to eq(1) } + it { expect(milestone.total_items_count(user)).to eq(3) } + it { expect(milestone.is_empty?(user)).to be_falsey } end describe :can_be_closed? do it { expect(milestone.can_be_closed?).to be_truthy } end - describe :is_empty? do + describe :total_items_count do before do create :closed_issue, milestone: milestone create :merge_request, milestone: milestone end it 'Should return total count of issues and merge requests assigned to milestone' do - expect(milestone.total_items_count).to eq 2 + expect(milestone.total_items_count(user)).to eq 2 end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 571ea2dae4c75263c4bb3c58b596bbe5d5513937..bb2ab058003cfef93f65e18cd16be5c10abca49f 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -3,7 +3,11 @@ describe API::API, api: true do include ApiHelpers let(:user) { create(:user) } - let!(:project) { create(:project, namespace: user.namespace ) } + let(:non_member) { create(:user) } + let(:author) { create(:author) } + let(:assignee) { create(:assignee) } + let(:admin) { create(:admin) } + let!(:project) { create(:project, :public, namespace: user.namespace ) } let!(:closed_issue) do create :closed_issue, author: user, @@ -12,6 +16,13 @@ state: :closed, milestone: milestone end + let!(:confidential_issue) do + create :issue, + :confidential, + project: project, + author: author, + assignee: assignee + end let!(:issue) do create :issue, author: user, @@ -123,10 +134,43 @@ let(:base_url) { "/projects/#{project.id}" } let(:title) { milestone.title } - it "should return project issues" do + it 'should return project issues without confidential issues for non project members' do + get api("#{base_url}/issues", non_member) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + expect(json_response.first['title']).to eq(issue.title) + end + + it 'should return project confidential issues for author' do + get api("#{base_url}/issues", author) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(3) + expect(json_response.first['title']).to eq(issue.title) + end + + it 'should return project confidential issues for assignee' do + get api("#{base_url}/issues", assignee) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(3) + expect(json_response.first['title']).to eq(issue.title) + end + + it 'should return project issues with confidential issues for project members' do get api("#{base_url}/issues", user) expect(response.status).to eq(200) expect(json_response).to be_an Array + expect(json_response.length).to eq(3) + expect(json_response.first['title']).to eq(issue.title) + end + + it 'should return project confidential issues for admin' do + get api("#{base_url}/issues", admin) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(3) expect(json_response.first['title']).to eq(issue.title) end @@ -206,6 +250,41 @@ get api("/projects/#{project.id}/issues/54321", user) expect(response.status).to eq(404) end + + context 'confidential issues' do + it "should return 404 for non project members" do + get api("/projects/#{project.id}/issues/#{confidential_issue.id}", non_member) + expect(response.status).to eq(404) + end + + it "should return confidential issue for project members" do + get api("/projects/#{project.id}/issues/#{confidential_issue.id}", user) + expect(response.status).to eq(200) + expect(json_response['title']).to eq(confidential_issue.title) + expect(json_response['iid']).to eq(confidential_issue.iid) + end + + it "should return confidential issue for author" do + get api("/projects/#{project.id}/issues/#{confidential_issue.id}", author) + expect(response.status).to eq(200) + expect(json_response['title']).to eq(confidential_issue.title) + expect(json_response['iid']).to eq(confidential_issue.iid) + end + + it "should return confidential issue for assignee" do + get api("/projects/#{project.id}/issues/#{confidential_issue.id}", assignee) + expect(response.status).to eq(200) + expect(json_response['title']).to eq(confidential_issue.title) + expect(json_response['iid']).to eq(confidential_issue.iid) + end + + it "should return confidential issue for admin" do + get api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin) + expect(response.status).to eq(200) + expect(json_response['title']).to eq(confidential_issue.title) + expect(json_response['iid']).to eq(confidential_issue.iid) + end + end end describe "POST /projects/:id/issues" do @@ -294,6 +373,35 @@ expect(response.status).to eq(400) expect(json_response['message']['labels']['?']['title']).to eq(['is invalid']) end + + context 'confidential issues' do + it "should return 403 for non project members" do + put api("/projects/#{project.id}/issues/#{confidential_issue.id}", non_member), + title: 'updated title' + expect(response.status).to eq(403) + end + + it "should update a confidential issue for project members" do + put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user), + title: 'updated title' + expect(response.status).to eq(200) + expect(json_response['title']).to eq('updated title') + end + + it "should update a confidential issue for author" do + put api("/projects/#{project.id}/issues/#{confidential_issue.id}", author), + title: 'updated title' + expect(response.status).to eq(200) + expect(json_response['title']).to eq('updated title') + end + + it "should update a confidential issue for admin" do + put api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin), + title: 'updated title' + expect(response.status).to eq(200) + expect(json_response['title']).to eq('updated title') + end + end end describe 'PUT /projects/:id/issues/:issue_id to update labels' do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index d8ad5e689383f2935fdf237863b0eabb3a7cfe91..5fc85600ff953e309a273faa78ca48f6490e16b7 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -228,12 +228,16 @@ let(:commit) { project.commit } before do + project.team << [commit_author, :developer] + project.team << [user, :developer] + allow(commit).to receive_messages( safe_message: "this commit \n mentions #{issue.to_reference}", references: [issue], author_name: commit_author.name, author_email: commit_author.email ) + allow(project.repository).to receive(:commits_between).and_return([commit]) end diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6108c26a78b862808dacd7b9b1d80e6757df8f5f --- /dev/null +++ b/spec/services/projects/autocomplete_service_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +describe Projects::AutocompleteService, services: true do + describe '#issues' do + describe 'confidential issues' do + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let(:project) { create(:empty_project, :public) } + let!(:issue) { create(:issue, project: project, title: 'Issue 1') } + let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) } + + it 'should not list project confidential issues for guests' do + autocomplete = described_class.new(project, nil) + issues = autocomplete.issues.map(&:iid) + + expect(issues).to include issue.iid + expect(issues).not_to include security_issue_1.iid + expect(issues).not_to include security_issue_2.iid + expect(issues.count).to eq 1 + end + + it 'should not list project confidential issues for non project members' do + autocomplete = described_class.new(project, non_member) + issues = autocomplete.issues.map(&:iid) + + expect(issues).to include issue.iid + expect(issues).not_to include security_issue_1.iid + expect(issues).not_to include security_issue_2.iid + expect(issues.count).to eq 1 + end + + it 'should list project confidential issues for author' do + autocomplete = described_class.new(project, author) + issues = autocomplete.issues.map(&:iid) + + expect(issues).to include issue.iid + expect(issues).to include security_issue_1.iid + expect(issues).not_to include security_issue_2.iid + expect(issues.count).to eq 2 + end + + it 'should list project confidential issues for assignee' do + autocomplete = described_class.new(project, assignee) + issues = autocomplete.issues.map(&:iid) + + expect(issues).to include issue.iid + expect(issues).not_to include security_issue_1.iid + expect(issues).to include security_issue_2.iid + expect(issues.count).to eq 2 + end + + it 'should list project confidential issues for project members' do + project.team << [member, :developer] + + autocomplete = described_class.new(project, member) + issues = autocomplete.issues.map(&:iid) + + expect(issues).to include issue.iid + expect(issues).to include security_issue_1.iid + expect(issues).to include security_issue_2.iid + expect(issues.count).to eq 3 + end + + it 'should list all project issues for admin' do + autocomplete = described_class.new(project, admin) + issues = autocomplete.issues.map(&:iid) + + expect(issues).to include issue.iid + expect(issues).to include security_issue_1.iid + expect(issues).to include security_issue_2.iid + expect(issues.count).to eq 3 + end + end + end +end diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index fce91015fd4b9c2a20d2e053834cffe31473fbad..e876d44c166dc384ebc21c8509638b7a532d88f0 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -52,6 +52,8 @@ end set_mentionable_text.call(ref_string) + + project.team << [author, :developer] end end