summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-01-28 13:33:23 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-01-28 13:33:23 +0000
commitb9bf11ef5f63203c6974c4432553270f7b3d29a1 (patch)
treeddf69920d5929b68cb7097c3c586a2fdf483cd60
parent1e8ef329735f06d3b3cfe1966b79fe55eff21f30 (diff)
downloadgitlab-ce-b9bf11ef5f63203c6974c4432553270f7b3d29a1.tar.gz
Add latest changes from gitlab-org/security/gitlab@12-7-stable-ee
-rw-r--r--app/assets/javascripts/frequent_items/components/app.vue6
-rw-r--r--app/assets/javascripts/frequent_items/components/frequent_items_list.vue6
-rw-r--r--app/assets/javascripts/frequent_items/utils.js7
-rw-r--r--app/controllers/dashboard_controller.rb3
-rw-r--r--app/controllers/groups_controller.rb7
-rw-r--r--app/controllers/projects_controller.rb3
-rw-r--r--app/models/generic_commit_status.rb15
-rw-r--r--app/policies/base_policy.rb8
-rw-r--r--app/policies/global_policy.rb7
-rw-r--r--app/presenters/event_presenter.rb12
-rw-r--r--app/services/compare_service.rb2
-rw-r--r--app/services/projects/group_links/destroy_service.rb6
-rw-r--r--changelogs/unreleased/security-35235-todos-cleanup.yml5
-rw-r--r--changelogs/unreleased/security-commits-api-last-pipeline-status.yml5
-rw-r--r--changelogs/unreleased/security-email-confirmation-bypass-via-api-ee.yml5
-rw-r--r--changelogs/unreleased/security-enforce-permissions-for-event-filter-ee.yml5
-rw-r--r--changelogs/unreleased/security-fix-xss-on-frequent-groups-dropdown.yml5
-rw-r--r--changelogs/unreleased/security-proctect-internal-builds-from-external-overrides.yml5
-rw-r--r--changelogs/unreleased/security-reverse-polarity-of-branch-compare.yml5
-rw-r--r--lib/api/commit_statuses.rb2
-rw-r--r--lib/api/commits.rb2
-rw-r--r--lib/api/entities.rb12
-rw-r--r--lib/gitlab/git/cross_repo_comparer.rb56
-rw-r--r--lib/gitlab/git/repository.rb33
-rw-r--r--spec/controllers/dashboard_controller_spec.rb41
-rw-r--r--spec/controllers/groups_controller_spec.rb37
-rw-r--r--spec/controllers/projects_controller_spec.rb40
-rw-r--r--spec/javascripts/frequent_items/utils_spec.js19
-rw-r--r--spec/lib/gitlab/git/cross_repo_comparer_spec.rb117
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb63
-rw-r--r--spec/models/generic_commit_status_spec.rb74
-rw-r--r--spec/policies/global_policy_spec.rb56
-rw-r--r--spec/requests/api/commit_statuses_spec.rb40
-rw-r--r--spec/requests/api/commits_spec.rb131
-rw-r--r--spec/requests/api/oauth_tokens_spec.rb32
-rw-r--r--spec/services/projects/group_links/destroy_service_spec.rb39
36 files changed, 773 insertions, 138 deletions
diff --git a/app/assets/javascripts/frequent_items/components/app.vue b/app/assets/javascripts/frequent_items/components/app.vue
index 8cf939254c1..2ffecce0a56 100644
--- a/app/assets/javascripts/frequent_items/components/app.vue
+++ b/app/assets/javascripts/frequent_items/components/app.vue
@@ -5,7 +5,7 @@ import AccessorUtilities from '~/lib/utils/accessor';
import eventHub from '../event_hub';
import store from '../store/';
import { FREQUENT_ITEMS, STORAGE_KEY } from '../constants';
-import { isMobile, updateExistingFrequentItem } from '../utils';
+import { isMobile, updateExistingFrequentItem, sanitizeItem } from '../utils';
import FrequentItemsSearchInput from './frequent_items_search_input.vue';
import FrequentItemsList from './frequent_items_list.vue';
import frequentItemsMixin from './frequent_items_mixin';
@@ -64,7 +64,9 @@ export default {
this.fetchFrequentItems();
}
},
- logItemAccess(storageKey, item) {
+ logItemAccess(storageKey, unsanitizedItem) {
+ const item = sanitizeItem(unsanitizedItem);
+
if (!AccessorUtilities.isLocalStorageAccessSafe()) {
return false;
}
diff --git a/app/assets/javascripts/frequent_items/components/frequent_items_list.vue b/app/assets/javascripts/frequent_items/components/frequent_items_list.vue
index 67ffa97a046..0ece64692ae 100644
--- a/app/assets/javascripts/frequent_items/components/frequent_items_list.vue
+++ b/app/assets/javascripts/frequent_items/components/frequent_items_list.vue
@@ -1,6 +1,7 @@
<script>
import FrequentItemsListItem from './frequent_items_list_item.vue';
import frequentItemsMixin from './frequent_items_mixin';
+import { sanitizeItem } from '../utils';
export default {
components: {
@@ -48,6 +49,9 @@ export default {
? this.translations.itemListErrorMessage
: this.translations.itemListEmptyMessage;
},
+ sanitizedItems() {
+ return this.items.map(sanitizeItem);
+ },
},
};
</script>
@@ -59,7 +63,7 @@ export default {
{{ listEmptyMessage }}
</li>
<frequent-items-list-item
- v-for="item in items"
+ v-for="item in sanitizedItems"
v-else
:key="item.id"
:item-id="item.id"
diff --git a/app/assets/javascripts/frequent_items/utils.js b/app/assets/javascripts/frequent_items/utils.js
index cc1668b1a0d..5188d6118ac 100644
--- a/app/assets/javascripts/frequent_items/utils.js
+++ b/app/assets/javascripts/frequent_items/utils.js
@@ -1,5 +1,6 @@
import _ from 'underscore';
import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
+import sanitize from 'sanitize-html';
import { FREQUENT_ITEMS, HOUR_IN_MS } from './constants';
export const isMobile = () => ['md', 'sm', 'xs'].includes(bp.getBreakpointSize());
@@ -43,3 +44,9 @@ export const updateExistingFrequentItem = (frequentItem, item) => {
lastAccessedOn: accessedOverHourAgo ? Date.now() : frequentItem.lastAccessedOn,
};
};
+
+export const sanitizeItem = item => ({
+ ...item,
+ name: sanitize(item.name.toString(), { allowedTags: [] }),
+ namespace: sanitize(item.namespace.toString(), { allowedTags: [] }),
+});
diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index 1a97b39d3ae..1668cf004f8 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -19,7 +19,7 @@ class DashboardController < Dashboard::ApplicationController
format.json do
load_events
- pager_json("events/_events", @events.count)
+ pager_json('events/_events', @events.count { |event| event.visible_to_user?(current_user) })
end
end
end
@@ -37,6 +37,7 @@ class DashboardController < Dashboard::ApplicationController
@events = EventCollection
.new(projects, offset: params[:offset].to_i, filter: event_filter)
.to_a
+ .map(&:present)
Events::RenderService.new(current_user).execute(@events)
end
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 0953ca96317..958dc27984f 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -91,7 +91,7 @@ class GroupsController < Groups::ApplicationController
format.json do
load_events
- pager_json("events/_events", @events.count)
+ pager_json("events/_events", @events.count { |event| event.visible_to_user?(current_user) })
end
end
end
@@ -209,8 +209,9 @@ class GroupsController < Groups::ApplicationController
.includes(:namespace)
@events = EventCollection
- .new(projects, offset: params[:offset].to_i, filter: event_filter, groups: groups)
- .to_a
+ .new(projects, offset: params[:offset].to_i, filter: event_filter, groups: groups)
+ .to_a
+ .map(&:present)
Events::RenderService
.new(current_user)
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index bf05defbc2e..9d0901881d3 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -118,7 +118,7 @@ class ProjectsController < Projects::ApplicationController
format.html
format.json do
load_events
- pager_json('events/_events', @events.count)
+ pager_json('events/_events', @events.count { |event| event.visible_to_user?(current_user) })
end
end
end
@@ -343,6 +343,7 @@ class ProjectsController < Projects::ApplicationController
@events = EventCollection
.new(projects, offset: params[:offset].to_i, filter: event_filter)
.to_a
+ .map(&:present)
Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?)
end
diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb
index 8a768b3a2c0..6c8bfc35334 100644
--- a/app/models/generic_commit_status.rb
+++ b/app/models/generic_commit_status.rb
@@ -1,11 +1,14 @@
# frozen_string_literal: true
class GenericCommitStatus < CommitStatus
+ EXTERNAL_STAGE_IDX = 1_000_000
+
before_validation :set_default_values
validates :target_url, addressable_url: true,
length: { maximum: 255 },
allow_nil: true
+ validate :name_uniqueness_across_types, unless: :importing?
# GitHub compatible API
alias_attribute :context, :name
@@ -13,7 +16,7 @@ class GenericCommitStatus < CommitStatus
def set_default_values
self.context ||= 'default'
self.stage ||= 'external'
- self.stage_idx ||= 1000000
+ self.stage_idx ||= EXTERNAL_STAGE_IDX
end
def tags
@@ -25,4 +28,14 @@ class GenericCommitStatus < CommitStatus
.new(self, current_user)
.fabricate!
end
+
+ private
+
+ def name_uniqueness_across_types
+ return if !pipeline || name.blank?
+
+ if pipeline.statuses.by_name(name).where.not(type: type).exists?
+ errors.add(:name, :taken)
+ end
+ end
end
diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb
index 3a16f7dc239..c93a19bdc3d 100644
--- a/app/policies/base_policy.rb
+++ b/app/policies/base_policy.rb
@@ -21,6 +21,14 @@ class BasePolicy < DeclarativePolicy::Base
with_options scope: :user, score: 0
condition(:deactivated) { @user&.deactivated? }
+ desc "User email is unconfirmed or user account is locked"
+ with_options scope: :user, score: 0
+ condition(:inactive) do
+ Feature.enabled?(:inactive_policy_condition, default_enabled: true) &&
+ @user &&
+ !@user&.active_for_authentication?
+ end
+
with_options scope: :user, score: 0
condition(:external_user) { @user.nil? || @user.external? }
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
index f212bb06bc9..2187c703760 100644
--- a/app/policies/global_policy.rb
+++ b/app/policies/global_policy.rb
@@ -36,6 +36,13 @@ class GlobalPolicy < BasePolicy
enable :use_slash_commands
end
+ rule { inactive }.policy do
+ prevent :log_in
+ prevent :access_api
+ prevent :access_git
+ prevent :use_slash_commands
+ end
+
rule { blocked | internal }.policy do
prevent :log_in
prevent :access_api
diff --git a/app/presenters/event_presenter.rb b/app/presenters/event_presenter.rb
index f31d362d5fa..5657e0b96bc 100644
--- a/app/presenters/event_presenter.rb
+++ b/app/presenters/event_presenter.rb
@@ -3,6 +3,18 @@
class EventPresenter < Gitlab::View::Presenter::Delegated
presents :event
+ def initialize(subject, **attributes)
+ super
+
+ @visible_to_user_cache = ActiveSupport::Cache::MemoryStore.new
+ end
+
+ # Caching `visible_to_user?` method in the presenter beause it might be called multiple times.
+ def visible_to_user?(user = nil)
+ @visible_to_user_cache.fetch(user&.id) { super(user) }
+ end
+
+ # implement cache here
def resource_parent_name
resource_parent&.full_name || ''
end
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
index 3f0aedfbfb2..569b91de73e 100644
--- a/app/services/compare_service.rb
+++ b/app/services/compare_service.rb
@@ -18,7 +18,7 @@ class CompareService
return unless raw_compare && raw_compare.base && raw_compare.head
Compare.new(raw_compare,
- target_project,
+ start_project,
base_sha: base_sha,
straight: straight)
end
diff --git a/app/services/projects/group_links/destroy_service.rb b/app/services/projects/group_links/destroy_service.rb
index c96dcaae8d5..ea7d05551fd 100644
--- a/app/services/projects/group_links/destroy_service.rb
+++ b/app/services/projects/group_links/destroy_service.rb
@@ -6,6 +6,12 @@ module Projects
def execute(group_link)
return false unless group_link
+ if group_link.project.private?
+ TodosDestroyer::ProjectPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id)
+ else
+ TodosDestroyer::ConfidentialIssueWorker.perform_in(Todo::WAIT_FOR_DELETE, nil, project.id)
+ end
+
group_link.destroy
end
end
diff --git a/changelogs/unreleased/security-35235-todos-cleanup.yml b/changelogs/unreleased/security-35235-todos-cleanup.yml
new file mode 100644
index 00000000000..119220fbc73
--- /dev/null
+++ b/changelogs/unreleased/security-35235-todos-cleanup.yml
@@ -0,0 +1,5 @@
+---
+title: Cleanup todos for users from a removed linked group
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-commits-api-last-pipeline-status.yml b/changelogs/unreleased/security-commits-api-last-pipeline-status.yml
new file mode 100644
index 00000000000..a68151f9732
--- /dev/null
+++ b/changelogs/unreleased/security-commits-api-last-pipeline-status.yml
@@ -0,0 +1,5 @@
+---
+title: Disable access to last_pipeline in commits API for users without read permissions
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-email-confirmation-bypass-via-api-ee.yml b/changelogs/unreleased/security-email-confirmation-bypass-via-api-ee.yml
new file mode 100644
index 00000000000..8bd2b7a452f
--- /dev/null
+++ b/changelogs/unreleased/security-email-confirmation-bypass-via-api-ee.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent API access for unconfirmed users
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-enforce-permissions-for-event-filter-ee.yml b/changelogs/unreleased/security-enforce-permissions-for-event-filter-ee.yml
new file mode 100644
index 00000000000..7d74d6108f8
--- /dev/null
+++ b/changelogs/unreleased/security-enforce-permissions-for-event-filter-ee.yml
@@ -0,0 +1,5 @@
+---
+title: Enforce permission check when counting activity events
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-fix-xss-on-frequent-groups-dropdown.yml b/changelogs/unreleased/security-fix-xss-on-frequent-groups-dropdown.yml
new file mode 100644
index 00000000000..9381efff0c8
--- /dev/null
+++ b/changelogs/unreleased/security-fix-xss-on-frequent-groups-dropdown.yml
@@ -0,0 +1,5 @@
+---
+title: Fix xss on frequent groups dropdown
+merge_request: 50
+author:
+type: security
diff --git a/changelogs/unreleased/security-proctect-internal-builds-from-external-overrides.yml b/changelogs/unreleased/security-proctect-internal-builds-from-external-overrides.yml
new file mode 100644
index 00000000000..b540172d95c
--- /dev/null
+++ b/changelogs/unreleased/security-proctect-internal-builds-from-external-overrides.yml
@@ -0,0 +1,5 @@
+---
+title: Protect internal CI builds from external overrides
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-reverse-polarity-of-branch-compare.yml b/changelogs/unreleased/security-reverse-polarity-of-branch-compare.yml
new file mode 100644
index 00000000000..db6a4f064a4
--- /dev/null
+++ b/changelogs/unreleased/security-reverse-polarity-of-branch-compare.yml
@@ -0,0 +1,5 @@
+---
+title: Make cross-repository comparisons happen in the source repository
+merge_request:
+author:
+type: security
diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 6e26ee309f0..569c4e04dc5 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -85,6 +85,8 @@ module API
protected: user_project.protected_for?(ref))
end
+ authorize! :update_pipeline, pipeline
+
status = GenericCommitStatus.running_or_pending.find_or_initialize_by(
project: user_project,
pipeline: pipeline,
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index 63a7fdfa3ab..9dcf9b015aa 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -154,7 +154,7 @@ module API
not_found! 'Commit' unless commit
- present commit, with: Entities::CommitDetail, stats: params[:stats]
+ present commit, with: Entities::CommitDetail, stats: params[:stats], current_user: current_user
end
desc 'Get the diff for a specific commit of a project' do
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index dfd0e676586..8c4d986eb34 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -491,8 +491,18 @@ module API
class CommitDetail < Commit
expose :stats, using: Entities::CommitStats, if: :stats
expose :status
- expose :last_pipeline, using: 'API::Entities::PipelineBasic'
expose :project_id
+
+ expose :last_pipeline do |commit, options|
+ pipeline = commit.last_pipeline if can_read_pipeline?
+ ::API::Entities::PipelineBasic.represent(pipeline, options)
+ end
+
+ private
+
+ def can_read_pipeline?
+ Ability.allowed?(options[:current_user], :read_pipeline, object.last_pipeline)
+ end
end
class CommitSignature < Grape::Entity
diff --git a/lib/gitlab/git/cross_repo_comparer.rb b/lib/gitlab/git/cross_repo_comparer.rb
new file mode 100644
index 00000000000..3958373f7cb
--- /dev/null
+++ b/lib/gitlab/git/cross_repo_comparer.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Git
+ class CrossRepoComparer
+ attr_reader :source_repo, :target_repo
+
+ def initialize(source_repo, target_repo)
+ @source_repo = source_repo
+ @target_repo = target_repo
+ end
+
+ def compare(source_ref, target_ref, straight:)
+ ensuring_ref_in_source(target_ref) do |target_commit_id|
+ Gitlab::Git::Compare.new(
+ source_repo,
+ target_commit_id,
+ source_ref,
+ straight: straight
+ )
+ end
+ end
+
+ private
+
+ def ensuring_ref_in_source(ref, &blk)
+ return yield ref if source_repo == target_repo
+
+ # If the commit doesn't exist in the target, there's nothing we can do
+ commit_id = target_repo.commit(ref)&.sha
+ return unless commit_id
+
+ # The commit pointed to by ref may exist in the source even when they
+ # are different repositories. This is particularly true of close forks,
+ # but may also be the case if a temporary ref for this comparison has
+ # already been created in the past, and the result hasn't been GC'd yet.
+ return yield commit_id if source_repo.commit(commit_id)
+
+ # Worst case: the commit is not in the source repo so we need to fetch
+ # it. Use a temporary ref and clean up afterwards
+ with_commit_in_source_tmp(commit_id, &blk)
+ end
+
+ # Fetch the ref into source_repo from target_repo, using a temporary ref
+ # name that will be deleted once the method completes. This is a no-op if
+ # fetching the source branch fails
+ def with_commit_in_source_tmp(commit_id, &blk)
+ tmp_ref = "refs/tmp/#{SecureRandom.hex}"
+
+ yield commit_id if source_repo.fetch_source_branch!(target_repo, commit_id, tmp_ref)
+ ensure
+ source_repo.delete_refs(tmp_ref) # best-effort
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index ed3e7a1e39c..0120e3be14c 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -746,29 +746,9 @@ module Gitlab
end
def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
- reachable_ref =
- if source_repository == self
- source_branch_name
- else
- # If a tmp ref was created before for a separate repo comparison (forks),
- # we're able to short-circuit the tmp ref re-creation:
- # 1. Take the SHA from the source repo
- # 2. Read that in the current "target" repo
- # 3. If that SHA is still known (readable), it means GC hasn't
- # cleaned it up yet, so we can use it instead re-writing the tmp ref.
- source_commit_id = source_repository.commit(source_branch_name)&.sha
- commit(source_commit_id)&.sha if source_commit_id
- end
-
- return compare(target_branch_name, reachable_ref, straight: straight) if reachable_ref
-
- tmp_ref = "refs/tmp/#{SecureRandom.hex}"
-
- return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref)
-
- compare(target_branch_name, tmp_ref, straight: straight)
- ensure
- delete_refs(tmp_ref) if tmp_ref
+ CrossRepoComparer
+ .new(source_repository, self)
+ .compare(source_branch_name, target_branch_name, straight: straight)
end
def write_ref(ref_path, ref, old_ref: nil)
@@ -1046,13 +1026,6 @@ module Gitlab
private
- def compare(base_ref, head_ref, straight:)
- Gitlab::Git::Compare.new(self,
- base_ref,
- head_ref,
- straight: straight)
- end
-
def empty_diff_stats
Gitlab::Git::DiffStatsCollection.new([])
end
diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb
index a733c3ecaa1..305419efe96 100644
--- a/spec/controllers/dashboard_controller_spec.rb
+++ b/spec/controllers/dashboard_controller_spec.rb
@@ -23,6 +23,47 @@ describe DashboardController do
end
end
+ describe "GET activity as JSON" do
+ render_views
+
+ let(:user) { create(:user) }
+ let(:project) { create(:project, :public, issues_access_level: ProjectFeature::PRIVATE) }
+
+ before do
+ create(:event, :created, project: project, target: create(:issue))
+
+ sign_in(user)
+
+ request.cookies[:event_filter] = 'all'
+ end
+
+ context 'when user has permission to see the event' do
+ before do
+ project.add_developer(user)
+ end
+
+ it 'returns count' do
+ get :activity, params: { format: :json }
+
+ expect(json_response['count']).to eq(1)
+ end
+ end
+
+ context 'when user has no permission to see the event' do
+ it 'filters out invisible event' do
+ get :activity, params: { format: :json }
+
+ expect(json_response['html']).to include(_('No activities found'))
+ end
+
+ it 'filters out invisible event when calculating the count' do
+ get :activity, params: { format: :json }
+
+ expect(json_response['count']).to eq(0)
+ end
+ end
+ end
+
it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first
it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 2ed2b319298..ddfd2b424e7 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -47,7 +47,7 @@ describe GroupsController do
it 'assigns events for all the projects in the group', :sidekiq_might_not_need_inline do
subject
- expect(assigns(:events)).to contain_exactly(event)
+ expect(assigns(:events).map(&:id)).to contain_exactly(event.id)
end
end
end
@@ -119,12 +119,12 @@ describe GroupsController do
describe 'GET #activity' do
render_views
- before do
- sign_in(user)
- project
- end
-
context 'as json' do
+ before do
+ sign_in(user)
+ project
+ end
+
it 'includes events from all projects in group and subgroups', :sidekiq_might_not_need_inline do
2.times do
project = create(:project, group: group)
@@ -141,6 +141,31 @@ describe GroupsController do
expect(assigns(:projects).limit_value).to be_nil
end
end
+
+ context 'when user has no permission to see the event' do
+ let(:user) { create(:user) }
+ let(:group) { create(:group) }
+ let(:project) { create(:project, group: group) }
+
+ let(:project_with_restricted_access) do
+ create(:project, :public, issues_access_level: ProjectFeature::PRIVATE, group: group)
+ end
+
+ before do
+ create(:event, project: project)
+ create(:event, :created, project: project_with_restricted_access, target: create(:issue))
+
+ group.add_guest(user)
+
+ sign_in(user)
+ end
+
+ it 'filters out invisible event' do
+ get :activity, params: { id: group.to_param }, format: :json
+
+ expect(json_response['count']).to eq(1)
+ end
+ end
end
describe 'POST #create' do
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 9ae1277de26..3870ef5d947 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -64,6 +64,46 @@ describe ProjectsController do
end
end
+ describe "GET #activity as JSON" do
+ render_views
+
+ let(:project) { create(:project, :public, issues_access_level: ProjectFeature::PRIVATE) }
+
+ before do
+ create(:event, :created, project: project, target: create(:issue))
+
+ sign_in(user)
+
+ request.cookies[:event_filter] = 'all'
+ end
+
+ context 'when user has permission to see the event' do
+ before do
+ project.add_developer(user)
+ end
+
+ it 'returns count' do
+ get :activity, params: { namespace_id: project.namespace, id: project, format: :json }
+
+ expect(json_response['count']).to eq(1)
+ end
+ end
+
+ context 'when user has no permission to see the event' do
+ it 'filters out invisible event' do
+ get :activity, params: { namespace_id: project.namespace, id: project, format: :json }
+
+ expect(json_response['html']).to eq("\n")
+ end
+
+ it 'filters out invisible event when calculating the count' do
+ get :activity, params: { namespace_id: project.namespace, id: project, format: :json }
+
+ expect(json_response['count']).to eq(0)
+ end
+ end
+ end
+
describe "GET show" do
context "user not project member" do
before do
diff --git a/spec/javascripts/frequent_items/utils_spec.js b/spec/javascripts/frequent_items/utils_spec.js
index 2480af5b31d..fd5bd002428 100644
--- a/spec/javascripts/frequent_items/utils_spec.js
+++ b/spec/javascripts/frequent_items/utils_spec.js
@@ -1,5 +1,10 @@
import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
-import { isMobile, getTopFrequentItems, updateExistingFrequentItem } from '~/frequent_items/utils';
+import {
+ isMobile,
+ getTopFrequentItems,
+ updateExistingFrequentItem,
+ sanitizeItem,
+} from '~/frequent_items/utils';
import { HOUR_IN_MS, FREQUENT_ITEMS } from '~/frequent_items/constants';
import { mockProject, unsortedFrequentItems, sortedFrequentItems } from './mock_data';
@@ -92,4 +97,16 @@ describe('Frequent Items utils spec', () => {
expect(result.frequency).toBe(mockedProject.frequency);
});
});
+
+ describe('sanitizeItem', () => {
+ it('strips HTML tags for name and namespace', () => {
+ const input = {
+ name: '<br><b>test</b>',
+ namespace: '<br>test',
+ id: 1,
+ };
+
+ expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 });
+ });
+ });
});
diff --git a/spec/lib/gitlab/git/cross_repo_comparer_spec.rb b/spec/lib/gitlab/git/cross_repo_comparer_spec.rb
new file mode 100644
index 00000000000..8b37b6d1667
--- /dev/null
+++ b/spec/lib/gitlab/git/cross_repo_comparer_spec.rb
@@ -0,0 +1,117 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Git::CrossRepoComparer do
+ let(:source_project) { create(:project, :repository) }
+ let(:target_project) { create(:project, :repository) }
+
+ let(:source_repo) { source_project.repository.raw_repository }
+ let(:target_repo) { target_project.repository.raw_repository }
+
+ let(:source_branch) { 'feature' }
+ let(:target_branch) { 'master' }
+ let(:straight) { false }
+
+ let(:source_commit) { source_repo.commit(source_branch) }
+ let(:target_commit) { source_repo.commit(target_branch) }
+
+ subject(:result) { described_class.new(source_repo, target_repo).compare(source_branch, target_branch, straight: straight) }
+
+ describe '#compare' do
+ context 'within a single repository' do
+ let(:target_project) { source_project }
+
+ context 'a non-straight comparison' do
+ it 'compares without fetching from another repo' do
+ expect(source_repo).not_to receive(:fetch_source_branch!)
+
+ expect_compare(result, from: source_commit, to: target_commit)
+ expect(result.straight).to eq(false)
+ end
+ end
+
+ context 'a straight comparison' do
+ let(:straight) { true }
+
+ it 'compares without fetching from another repo' do
+ expect(source_repo).not_to receive(:fetch_source_branch!)
+
+ expect_compare(result, from: source_commit, to: target_commit)
+ expect(result.straight).to eq(true)
+ end
+ end
+ end
+
+ context 'across two repositories' do
+ context 'target ref exists in source repo' do
+ it 'compares without fetching from another repo' do
+ expect(source_repo).not_to receive(:fetch_source_branch!)
+ expect(source_repo).not_to receive(:delete_refs)
+
+ expect_compare(result, from: source_commit, to: target_commit)
+ end
+ end
+
+ context 'target ref does not exist in source repo' do
+ it 'compares in the source repo by fetching from the target to a temporary ref' do
+ new_commit_id = create_commit(target_project.owner, target_repo, target_branch)
+ new_commit = target_repo.commit(new_commit_id)
+
+ # This is how the temporary ref is generated
+ expect(SecureRandom).to receive(:hex).at_least(:once).and_return('foo')
+
+ expect(source_repo)
+ .to receive(:fetch_source_branch!)
+ .with(target_repo, new_commit_id, 'refs/tmp/foo')
+ .and_call_original
+
+ expect(source_repo).to receive(:delete_refs).with('refs/tmp/foo').and_call_original
+
+ expect_compare(result, from: source_commit, to: new_commit)
+ end
+ end
+
+ context 'source ref does not exist in source repo' do
+ let(:source_branch) { 'does-not-exist' }
+
+ it 'returns an empty comparison' do
+ expect(source_repo).not_to receive(:fetch_source_branch!)
+ expect(source_repo).not_to receive(:delete_refs)
+
+ expect(result).to be_a(::Gitlab::Git::Compare)
+ expect(result.commits.size).to eq(0)
+ end
+ end
+
+ context 'target ref does not exist in target repo' do
+ let(:target_branch) { 'does-not-exist' }
+
+ it 'returns nil' do
+ expect(source_repo).not_to receive(:fetch_source_branch!)
+ expect(source_repo).not_to receive(:delete_refs)
+
+ is_expected.to be_nil
+ end
+ end
+ end
+ end
+
+ def expect_compare(of, from:, to:)
+ expect(of).to be_a(::Gitlab::Git::Compare)
+ expect(from).to be_a(::Gitlab::Git::Commit)
+ expect(to).to be_a(::Gitlab::Git::Commit)
+
+ expect(of.commits).not_to be_empty
+ expect(of.head).to eq(from)
+ expect(of.base).to eq(to)
+ end
+
+ def create_commit(user, repo, branch)
+ action = { action: :create, file_path: '/FILE', content: 'content' }
+
+ result = repo.multi_action(user, branch_name: branch, message: 'Commit', actions: [action])
+
+ result.newrev
+ end
+end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 6854d514dcc..07fef203691 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1962,66 +1962,15 @@ describe Gitlab::Git::Repository, :seed_helper do
end
describe '#compare_source_branch' do
- let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') }
-
- context 'within same repository' do
- it 'does not create a temp ref' do
- expect(repository).not_to receive(:fetch_source_branch!)
- expect(repository).not_to receive(:delete_refs)
-
- compare = repository.compare_source_branch('master', repository, 'feature', straight: false)
- expect(compare).to be_a(Gitlab::Git::Compare)
- expect(compare.commits.count).to be > 0
- end
-
- it 'returns empty commits when source ref does not exist' do
- compare = repository.compare_source_branch('master', repository, 'non-existent-branch', straight: false)
+ it 'delegates to Gitlab::Git::CrossRepoComparer' do
+ expect_next_instance_of(::Gitlab::Git::CrossRepoComparer) do |instance|
+ expect(instance.source_repo).to eq(:source_repository)
+ expect(instance.target_repo).to eq(repository)
- expect(compare.commits).to be_empty
+ expect(instance).to receive(:compare).with('feature', 'master', straight: :straight)
end
- end
- context 'with different repositories' do
- context 'when ref is known by source repo, but not by target' do
- before do
- mutable_repository.write_ref('another-branch', 'feature')
- end
-
- it 'creates temp ref' do
- expect(repository).not_to receive(:fetch_source_branch!)
- expect(repository).not_to receive(:delete_refs)
-
- compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false)
- expect(compare).to be_a(Gitlab::Git::Compare)
- expect(compare.commits.count).to be > 0
- end
- end
-
- context 'when ref is known by source and target repos' do
- before do
- mutable_repository.write_ref('another-branch', 'feature')
- repository.write_ref('another-branch', 'feature')
- end
-
- it 'does not create a temp ref' do
- expect(repository).not_to receive(:fetch_source_branch!)
- expect(repository).not_to receive(:delete_refs)
-
- compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false)
- expect(compare).to be_a(Gitlab::Git::Compare)
- expect(compare.commits.count).to be > 0
- end
- end
-
- context 'when ref is unknown by source repo' do
- it 'returns nil when source ref does not exist' do
- expect(repository).to receive(:fetch_source_branch!).and_call_original
- expect(repository).to receive(:delete_refs).and_call_original
-
- compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false)
- expect(compare).to be_nil
- end
- end
+ repository.compare_source_branch('master', :source_repository, 'feature', straight: :straight)
end
end
diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb
index c851810ffb3..c8ed898122b 100644
--- a/spec/models/generic_commit_status_spec.rb
+++ b/spec/models/generic_commit_status_spec.rb
@@ -19,6 +19,74 @@ describe GenericCommitStatus do
it { is_expected.not_to allow_value('javascript:alert(1)').for(:target_url) }
end
+ describe '#name_uniqueness_across_types' do
+ let(:attributes) { {} }
+ let(:commit_status) { described_class.new(attributes) }
+ let(:status_name) { 'test-job' }
+
+ subject(:errors) { commit_status.errors[:name] }
+
+ shared_examples 'it does not have uniqueness errors' do
+ it 'does not return errors' do
+ commit_status.valid?
+
+ is_expected.to be_empty
+ end
+ end
+
+ context 'without attributes' do
+ it_behaves_like 'it does not have uniqueness errors'
+ end
+
+ context 'with only a pipeline' do
+ let(:attributes) { { pipeline: pipeline } }
+
+ context 'without name' do
+ it_behaves_like 'it does not have uniqueness errors'
+ end
+ end
+
+ context 'with only a name' do
+ let(:attributes) { { name: status_name } }
+
+ context 'without pipeline' do
+ it_behaves_like 'it does not have uniqueness errors'
+ end
+ end
+
+ context 'with pipeline and name' do
+ let(:attributes) do
+ {
+ pipeline: pipeline,
+ name: status_name
+ }
+ end
+
+ context 'without other statuses' do
+ it_behaves_like 'it does not have uniqueness errors'
+ end
+
+ context 'with generic statuses' do
+ before do
+ create(:generic_commit_status, pipeline: pipeline, name: status_name)
+ end
+
+ it_behaves_like 'it does not have uniqueness errors'
+ end
+
+ context 'with ci_build statuses' do
+ before do
+ create(:ci_build, pipeline: pipeline, name: status_name)
+ end
+
+ it 'returns name error' do
+ expect(commit_status).to be_invalid
+ is_expected.to include('has already been taken')
+ end
+ end
+ end
+ end
+
describe '#context' do
subject { generic_commit_status.context }
@@ -79,6 +147,12 @@ describe GenericCommitStatus do
it { is_expected.not_to be_nil }
end
+
+ describe '#stage_idx' do
+ subject { generic_commit_status.stage_idx }
+
+ it { is_expected.not_to be_nil }
+ end
end
describe '#present' do
diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb
index f715ecae347..d227c018694 100644
--- a/spec/policies/global_policy_spec.rb
+++ b/spec/policies/global_policy_spec.rb
@@ -141,6 +141,34 @@ describe GlobalPolicy do
it { is_expected.to be_allowed(:access_api) }
end
end
+
+ context 'inactive user' do
+ before do
+ current_user.update!(confirmed_at: nil, confirmation_sent_at: 5.days.ago)
+ end
+
+ context 'when within the confirmation grace period' do
+ before do
+ allow(User).to receive(:allow_unconfirmed_access_for).and_return(10.days)
+ end
+
+ it { is_expected.to be_allowed(:access_api) }
+ end
+
+ context 'when confirmation grace period is expired' do
+ before do
+ allow(User).to receive(:allow_unconfirmed_access_for).and_return(2.days)
+ end
+
+ it { is_expected.not_to be_allowed(:access_api) }
+ end
+
+ it 'when `inactive_policy_condition` feature flag is turned off' do
+ stub_feature_flags(inactive_policy_condition: false)
+
+ is_expected.to be_allowed(:access_api)
+ end
+ end
end
describe 'receive notifications' do
@@ -202,6 +230,20 @@ describe GlobalPolicy do
it { is_expected.not_to be_allowed(:access_git) }
end
+ describe 'inactive user' do
+ before do
+ current_user.update!(confirmed_at: nil)
+ end
+
+ it { is_expected.not_to be_allowed(:access_git) }
+
+ it 'when `inactive_policy_condition` feature flag is turned off' do
+ stub_feature_flags(inactive_policy_condition: false)
+
+ is_expected.to be_allowed(:access_git)
+ end
+ end
+
context 'when terms are enforced' do
before do
enforce_terms
@@ -298,6 +340,20 @@ describe GlobalPolicy do
it { is_expected.not_to be_allowed(:use_slash_commands) }
end
+ describe 'inactive user' do
+ before do
+ current_user.update!(confirmed_at: nil)
+ end
+
+ it { is_expected.not_to be_allowed(:use_slash_commands) }
+
+ it 'when `inactive_policy_condition` feature flag is turned off' do
+ stub_feature_flags(inactive_policy_condition: false)
+
+ is_expected.to be_allowed(:use_slash_commands)
+ end
+ end
+
context 'when access locked' do
before do
current_user.lock_access!
diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb
index 639b8e96343..24ed836996e 100644
--- a/spec/requests/api/commit_statuses_spec.rb
+++ b/spec/requests/api/commit_statuses_spec.rb
@@ -164,6 +164,7 @@ describe API::CommitStatuses do
expect(response).to have_gitlab_http_status(201)
expect(job.status).to eq('pending')
+ expect(job.stage_idx).to eq(GenericCommitStatus::EXTERNAL_STAGE_IDX)
end
end
@@ -331,6 +332,29 @@ describe API::CommitStatuses do
end
end
+ context 'when updating a protected ref' do
+ before do
+ create(:protected_branch, project: project, name: 'master')
+ post api(post_url, user), params: { state: 'running', ref: 'master' }
+ end
+
+ context 'with user as developer' do
+ let(:user) { developer }
+
+ it 'does not create commit status' do
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+
+ context 'with user as maintainer' do
+ let(:user) { create_user(:maintainer) }
+
+ it 'creates commit status' do
+ expect(response).to have_gitlab_http_status(201)
+ end
+ end
+ end
+
context 'when commit SHA is invalid' do
let(:sha) { 'invalid_sha' }
@@ -372,6 +396,22 @@ describe API::CommitStatuses do
.to include 'is blocked: Only allowed schemes are http, https'
end
end
+
+ context 'when trying to update a status of a different type' do
+ let!(:pipeline) { create(:ci_pipeline, project: project, sha: sha, ref: 'ref') }
+ let!(:ci_build) { create(:ci_build, pipeline: pipeline, name: 'test-job') }
+ let(:params) { { state: 'pending', name: 'test-job' } }
+
+ before do
+ post api(post_url, developer), params: params
+ end
+
+ it 'responds with bad request status and validation errors' do
+ expect(response).to have_gitlab_http_status(400)
+ expect(json_response['message']['name'])
+ .to include 'has already been taken'
+ end
+ end
end
context 'reporter user' do
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index d8da1c001b0..e390f3945a9 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -8,6 +8,7 @@ describe API::Commits do
let(:user) { create(:user) }
let(:guest) { create(:user).tap { |u| project.add_guest(u) } }
+ let(:developer) { create(:user).tap { |u| project.add_developer(u) } }
let(:project) { create(:project, :repository, creator: user, path: 'my.project') }
let(:branch_with_dot) { project.repository.find_branch('ends-with.json') }
let(:branch_with_slash) { project.repository.find_branch('improve/awesome') }
@@ -964,6 +965,56 @@ describe API::Commits do
end
end
+ shared_examples_for 'ref with pipeline' do
+ let!(:pipeline) do
+ project
+ .ci_pipelines
+ .create!(source: :push, ref: 'master', sha: commit.sha, protected: false)
+ end
+
+ it 'includes status as "created" and a last_pipeline object' do
+ get api(route, current_user)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(response).to match_response_schema('public_api/v4/commit/detail')
+ expect(json_response['status']).to eq('created')
+ expect(json_response['last_pipeline']['id']).to eq(pipeline.id)
+ expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref)
+ expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha)
+ expect(json_response['last_pipeline']['status']).to eq(pipeline.status)
+ end
+
+ context 'when pipeline succeeds' do
+ before do
+ pipeline.update!(status: 'success')
+ end
+
+ it 'includes a "success" status' do
+ get api(route, current_user)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(response).to match_response_schema('public_api/v4/commit/detail')
+ expect(json_response['status']).to eq('success')
+ end
+ end
+ end
+
+ shared_examples_for 'ref with unaccessible pipeline' do
+ let!(:pipeline) do
+ project
+ .ci_pipelines
+ .create!(source: :push, ref: 'master', sha: commit.sha, protected: false)
+ end
+
+ it 'does not include last_pipeline' do
+ get api(route, current_user)
+
+ expect(response).to match_response_schema('public_api/v4/commit/detail')
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['last_pipeline']).to be_nil
+ end
+ end
+
context 'when stat param' do
let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}" }
@@ -993,6 +1044,15 @@ describe API::Commits do
let(:project) { create(:project, :public, :repository) }
it_behaves_like 'ref commit'
+ it_behaves_like 'ref with pipeline'
+
+ context 'with private builds' do
+ before do
+ project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE)
+ end
+
+ it_behaves_like 'ref with unaccessible pipeline'
+ end
end
context 'when unauthenticated', 'and project is private' do
@@ -1006,6 +1066,17 @@ describe API::Commits do
let(:current_user) { user }
it_behaves_like 'ref commit'
+ it_behaves_like 'ref with pipeline'
+
+ context 'when builds are disabled' do
+ before do
+ project
+ .project_feature
+ .update!(builds_access_level: ProjectFeature::DISABLED)
+ end
+
+ it_behaves_like 'ref with unaccessible pipeline'
+ end
context 'when branch contains a dot' do
let(:commit) { project.repository.commit(branch_with_dot.name) }
@@ -1041,35 +1112,53 @@ describe API::Commits do
it_behaves_like 'ref commit'
end
end
+ end
- context 'when the ref has a pipeline' do
- let!(:pipeline) { project.ci_pipelines.create(source: :push, ref: 'master', sha: commit.sha, protected: false) }
+ context 'when authenticated', 'as a developer' do
+ let(:current_user) { developer }
- it 'includes a "created" status' do
- get api(route, current_user)
+ it_behaves_like 'ref commit'
+ it_behaves_like 'ref with pipeline'
- expect(response).to have_gitlab_http_status(200)
- expect(response).to match_response_schema('public_api/v4/commit/detail')
- expect(json_response['status']).to eq('created')
- expect(json_response['last_pipeline']['id']).to eq(pipeline.id)
- expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref)
- expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha)
- expect(json_response['last_pipeline']['status']).to eq(pipeline.status)
+ context 'with private builds' do
+ before do
+ project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE)
end
- context 'when pipeline succeeds' do
- before do
- pipeline.update(status: 'success')
- end
+ it_behaves_like 'ref with pipeline'
+ end
+ end
- it 'includes a "success" status' do
- get api(route, current_user)
+ context 'when authenticated', 'as a guest' do
+ let(:current_user) { guest }
- expect(response).to have_gitlab_http_status(200)
- expect(response).to match_response_schema('public_api/v4/commit/detail')
- expect(json_response['status']).to eq('success')
- end
+ it_behaves_like '403 response' do
+ let(:request) { get api(route, guest) }
+ let(:message) { '403 Forbidden' }
+ end
+ end
+
+ context 'when authenticated', 'as a non member' do
+ let(:current_user) { create(:user) }
+
+ it_behaves_like '403 response' do
+ let(:request) { get api(route, guest) }
+ let(:message) { '403 Forbidden' }
+ end
+ end
+
+ context 'when authenticated', 'as non_member and project is public' do
+ let(:current_user) { create(:user) }
+ let(:project) { create(:project, :public, :repository) }
+
+ it_behaves_like 'ref with pipeline'
+
+ context 'with private builds' do
+ before do
+ project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE)
end
+
+ it_behaves_like 'ref with unaccessible pipeline'
end
end
end
diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb
index 8d7b3fa3c09..ce03756a19a 100644
--- a/spec/requests/api/oauth_tokens_spec.rb
+++ b/spec/requests/api/oauth_tokens_spec.rb
@@ -30,26 +30,40 @@ describe 'OAuth tokens' do
end
end
- context "when user is blocked" do
- it "does not create an access token" do
- user = create(:user)
+ shared_examples 'does not create an access token' do
+ let(:user) { create(:user) }
+
+ it { expect(response).to have_gitlab_http_status(401) }
+ end
+
+ context 'when user is blocked' do
+ before do
user.block
request_oauth_token(user)
-
- expect(response).to have_gitlab_http_status(401)
end
+
+ include_examples 'does not create an access token'
end
- context "when user is ldap_blocked" do
- it "does not create an access token" do
- user = create(:user)
+ context 'when user is ldap_blocked' do
+ before do
user.ldap_block
request_oauth_token(user)
+ end
- expect(response).to have_gitlab_http_status(401)
+ include_examples 'does not create an access token'
+ end
+
+ context 'when user account is not confirmed' do
+ before do
+ user.update!(confirmed_at: nil)
+
+ request_oauth_token(user)
end
+
+ include_examples 'does not create an access token'
end
end
end
diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb
index d78ab78c3d8..0fd1fcfe1a5 100644
--- a/spec/services/projects/group_links/destroy_service_spec.rb
+++ b/spec/services/projects/group_links/destroy_service_spec.rb
@@ -3,8 +3,8 @@
require 'spec_helper'
describe Projects::GroupLinks::DestroyService, '#execute' do
- let(:group_link) { create :project_group_link }
- let(:project) { group_link.project }
+ let(:project) { create(:project, :private) }
+ let!(:group_link) { create(:project_group_link, project: project) }
let(:user) { create :user }
let(:subject) { described_class.new(project, user) }
@@ -15,4 +15,39 @@ describe Projects::GroupLinks::DestroyService, '#execute' do
it 'returns false if group_link is blank' do
expect { subject.execute(nil) }.not_to change { project.project_group_links.count }
end
+
+ describe 'todos cleanup' do
+ context 'when project is private' do
+ it 'triggers todos cleanup' do
+ expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id)
+ expect(project.private?).to be true
+
+ subject.execute(group_link)
+ end
+ end
+
+ context 'when project is public or internal' do
+ shared_examples_for 'removes confidential todos' do
+ it 'does not trigger todos cleanup' do
+ expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id)
+ expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, nil, project.id)
+ expect(project.private?).to be false
+
+ subject.execute(group_link)
+ end
+ end
+
+ context 'when project is public' do
+ let(:project) { create(:project, :public) }
+
+ it_behaves_like 'removes confidential todos'
+ end
+
+ context 'when project is internal' do
+ let(:project) { create(:project, :public) }
+
+ it_behaves_like 'removes confidential todos'
+ end
+ end
+ end
end