...@@ -7247,13 +7247,9 @@ ...@@ -7247,13 +7247,9 @@
   
], ],
"type": { "type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "String", "name": "String",
"ofType": null "ofType": null
}
}, },
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
... ...
......
...@@ -221,7 +221,7 @@ Autogenerated return type of DestroySnippet ...@@ -221,7 +221,7 @@ Autogenerated return type of DestroySnippet
| Name | Type | Description | | Name | Type | Description |
| --- | ---- | ---------- | | --- | ---- | ---------- |
| `headSha` | String! | SHA of the HEAD at the time the comment was made | | `headSha` | String! | SHA of the HEAD at the time the comment was made |
| `baseSha` | String! | Merge base of the branch the comment was made on | | `baseSha` | String | Merge base of the branch the comment was made on |
| `startSha` | String! | SHA of the branch being compared against | | `startSha` | String! | SHA of the branch being compared against |
## Discussion ## Discussion
... ...
......
...@@ -4,6 +4,8 @@ module API ...@@ -4,6 +4,8 @@ module API
class Triggers < Grape::API class Triggers < Grape::API
include PaginationParams include PaginationParams
HTTP_GITLAB_EVENT_HEADER = "HTTP_#{WebHookService::GITLAB_EVENT_HEADER}".underscore.upcase
params do params do
requires :id, type: String, desc: 'The ID of a project' requires :id, type: String, desc: 'The ID of a project'
end end
...@@ -19,6 +21,8 @@ module API ...@@ -19,6 +21,8 @@ module API
post ":id/(ref/:ref/)trigger/pipeline", requirements: { ref: /.+/ } do post ":id/(ref/:ref/)trigger/pipeline", requirements: { ref: /.+/ } do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42283') Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42283')
forbidden! if gitlab_pipeline_hook_request?
# validate variables # validate variables
params[:variables] = params[:variables].to_h params[:variables] = params[:variables].to_h
unless params[:variables].all? { |key, value| key.is_a?(String) && value.is_a?(String) } unless params[:variables].all? { |key, value| key.is_a?(String) && value.is_a?(String) }
...@@ -128,5 +132,11 @@ module API ...@@ -128,5 +132,11 @@ module API
destroy_conditionally!(trigger) destroy_conditionally!(trigger)
end end
end end
helpers do
def gitlab_pipeline_hook_request?
request.get_header(HTTP_GITLAB_EVENT_HEADER) == WebHookService.hook_to_event(:pipeline_hooks)
end
end
end end
end end
...@@ -7,6 +7,8 @@ module Gitlab ...@@ -7,6 +7,8 @@ module Gitlab
GIT_INVALID_URL_REGEX = /^git\+#{URL_REGEX}/.freeze GIT_INVALID_URL_REGEX = /^git\+#{URL_REGEX}/.freeze
REPO_REGEX = %r{[^/'" ]+/[^/'" ]+}.freeze REPO_REGEX = %r{[^/'" ]+/[^/'" ]+}.freeze
include ActionView::Helpers::SanitizeHelper
class_attribute :file_type class_attribute :file_type
def self.support?(blob_name) def self.support?(blob_name)
...@@ -62,7 +64,10 @@ module Gitlab ...@@ -62,7 +64,10 @@ module Gitlab
end end
def link_tag(name, url) def link_tag(name, url)
%{<a href="#{ERB::Util.html_escape_once(url)}" rel="nofollow noreferrer noopener" target="_blank">#{ERB::Util.html_escape_once(name)}</a>}.html_safe sanitize(
%{<a href="#{ERB::Util.html_escape_once(url)}" rel="nofollow noreferrer noopener" target="_blank">#{ERB::Util.html_escape_once(name)}</a>},
attributes: %w[href rel target]
)
end end
# Links package names based on regex. # Links package names based on regex.
... ...
......
...@@ -62,6 +62,7 @@ module Gitlab ...@@ -62,6 +62,7 @@ module Gitlab
cte = Gitlab::SQL::RecursiveCTE.new(:namespaces_cte) cte = Gitlab::SQL::RecursiveCTE.new(:namespaces_cte)
members = Member.arel_table members = Member.arel_table
namespaces = Namespace.arel_table namespaces = Namespace.arel_table
group_group_links = GroupGroupLink.arel_table
# Namespaces the user is a member of. # Namespaces the user is a member of.
cte << user.groups cte << user.groups
...@@ -69,7 +70,10 @@ module Gitlab ...@@ -69,7 +70,10 @@ module Gitlab
.except(:order) .except(:order)
# Namespaces shared with any of the group # Namespaces shared with any of the group
cte << Group.select([namespaces[:id], 'group_group_links.group_access AS access_level']) cte << Group.select([namespaces[:id],
least(members[:access_level],
group_group_links[:group_access],
'access_level')])
.joins(join_group_group_links) .joins(join_group_group_links)
.joins(join_members_on_group_group_links) .joins(join_members_on_group_group_links)
... ...
......
...@@ -67,7 +67,13 @@ module Gitlab ...@@ -67,7 +67,13 @@ module Gitlab
return false unless can_access_git? return false unless can_access_git?
return false unless project return false unless project
# Checking for an internal project to prevent an infinite loop:
# https://gitlab.com/gitlab-org/gitlab/issues/36805
if project.internal?
return false unless user.can?(:push_code, project)
else
return false if !user.can?(:push_code, project) && !project.branch_allows_collaboration?(user, ref) return false if !user.can?(:push_code, project) && !project.branch_allows_collaboration?(user, ref)
end
if protected?(ProtectedBranch, project, ref) if protected?(ProtectedBranch, project, ref)
protected_branch_accessible_to?(ref, action: :push) protected_branch_accessible_to?(ref, action: :push)
... ...
......
...@@ -6,9 +6,13 @@ describe Groups::GroupLinksController do ...@@ -6,9 +6,13 @@ describe Groups::GroupLinksController do
let(:shared_with_group) { create(:group, :private) } let(:shared_with_group) { create(:group, :private) }
let(:shared_group) { create(:group, :private) } let(:shared_group) { create(:group, :private) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:group_member) { create(:user) }
let!(:project) { create(:project, group: shared_group) }
before do before do
sign_in(user) sign_in(user)
shared_with_group.add_developer(group_member)
end end
describe '#create' do describe '#create' do
...@@ -22,13 +26,9 @@ describe Groups::GroupLinksController do ...@@ -22,13 +26,9 @@ describe Groups::GroupLinksController do
end end
context 'when user has correct access to both groups' do context 'when user has correct access to both groups' do
let(:group_member) { create(:user) }
before do before do
shared_with_group.add_developer(user) shared_with_group.add_developer(user)
shared_group.add_owner(user) shared_group.add_owner(user)
shared_with_group.add_developer(group_member)
end end
it 'links group with selected group' do it 'links group with selected group' do
...@@ -45,6 +45,10 @@ describe Groups::GroupLinksController do ...@@ -45,6 +45,10 @@ describe Groups::GroupLinksController do
expect { subject }.to change { group_member.can?(:read_group, shared_group) }.from(false).to(true) expect { subject }.to change { group_member.can?(:read_group, shared_group) }.from(false).to(true)
end end
it 'updates project permissions' do
expect { subject }.to change { group_member.can?(:read_project, project) }.from(false).to(true)
end
context 'when shared with group id is not present' do context 'when shared with group id is not present' do
let(:shared_with_group_id) { nil } let(:shared_with_group_id) { nil }
...@@ -130,6 +134,7 @@ describe Groups::GroupLinksController do ...@@ -130,6 +134,7 @@ describe Groups::GroupLinksController do
context 'when user has admin access to the shared group' do context 'when user has admin access to the shared group' do
before do before do
shared_group.add_owner(user) shared_group.add_owner(user)
shared_with_group.refresh_members_authorized_projects
end end
it 'updates existing link' do it 'updates existing link' do
...@@ -143,6 +148,10 @@ describe Groups::GroupLinksController do ...@@ -143,6 +148,10 @@ describe Groups::GroupLinksController do
expect(link.group_access).to eq(Gitlab::Access::GUEST) expect(link.group_access).to eq(Gitlab::Access::GUEST)
expect(link.expires_at).to eq(expiry_date) expect(link.expires_at).to eq(expiry_date)
end end
it 'updates project permissions' do
expect { subject }.to change { group_member.can?(:create_release, project) }.from(true).to(false)
end
end end
context 'when user does not have admin access to the shared group' do context 'when user does not have admin access to the shared group' do
...@@ -180,11 +189,16 @@ describe Groups::GroupLinksController do ...@@ -180,11 +189,16 @@ describe Groups::GroupLinksController do
context 'when user has admin access to the shared group' do context 'when user has admin access to the shared group' do
before do before do
shared_group.add_owner(user) shared_group.add_owner(user)
shared_with_group.refresh_members_authorized_projects
end end
it 'deletes existing link' do it 'deletes existing link' do
expect { subject }.to change(GroupGroupLink, :count).by(-1) expect { subject }.to change(GroupGroupLink, :count).by(-1)
end end
it 'updates project permissions' do
expect { subject }.to change { group_member.can?(:create_release, project) }.from(true).to(false)
end
end end
context 'when user does not have admin access to the shared group' do context 'when user does not have admin access to the shared group' do
... ...
......
import { createLocalVue, shallowMount } from '@vue/test-utils'; import { createLocalVue, shallowMount } from '@vue/test-utils';
import Vuex from 'vuex'; import Vuex from 'vuex';
import { GlLoadingIcon, GlLink, GlBadge, GlFormInput } from '@gitlab/ui'; import { GlLoadingIcon, GlLink, GlBadge, GlFormInput, GlSprintf } from '@gitlab/ui';
import LoadingButton from '~/vue_shared/components/loading_button.vue'; import LoadingButton from '~/vue_shared/components/loading_button.vue';
import Stacktrace from '~/error_tracking/components/stacktrace.vue'; import Stacktrace from '~/error_tracking/components/stacktrace.vue';
import ErrorDetails from '~/error_tracking/components/error_details.vue'; import ErrorDetails from '~/error_tracking/components/error_details.vue';
...@@ -22,7 +22,7 @@ describe('ErrorDetails', () => { ...@@ -22,7 +22,7 @@ describe('ErrorDetails', () => {
function mountComponent() { function mountComponent() {
wrapper = shallowMount(ErrorDetails, { wrapper = shallowMount(ErrorDetails, {
stubs: { LoadingButton }, stubs: { LoadingButton, GlSprintf },
localVue, localVue,
store, store,
mocks, mocks,
...@@ -128,6 +128,30 @@ describe('ErrorDetails', () => { ...@@ -128,6 +128,30 @@ describe('ErrorDetails', () => {
expect(wrapper.findAll('button').length).toBe(3); expect(wrapper.findAll('button').length).toBe(3);
}); });
describe('unsafe chars for culprit field', () => {
const findReportedText = () => wrapper.find('[data-qa-selector="reported_text"]');
const culprit = '<script>console.log("surprise!")</script>';
beforeEach(() => {
store.state.details.loadingStacktrace = false;
store.state.details.loading = false;
mountComponent();
wrapper.setData({
GQLerror: {
culprit,
},
});
});
it('should not convert interpolated text to html entities', () => {
expect(findReportedText().findAll('script').length).toEqual(0);
expect(findReportedText().findAll('strong').length).toEqual(1);
});
it('should render text instead of converting to html entities', () => {
expect(findReportedText().text()).toContain(culprit);
});
});
describe('Badges', () => { describe('Badges', () => {
it('should show language and error level badges', () => { it('should show language and error level badges', () => {
store.state.details.error.tags = { level: 'error', logger: 'ruby' }; store.state.details.error.tags = { level: 'error', logger: 'ruby' };
... ...
......
...@@ -5,5 +5,9 @@ require 'spec_helper' ...@@ -5,5 +5,9 @@ require 'spec_helper'
describe GitlabSchema.types['DiffRefs'] do describe GitlabSchema.types['DiffRefs'] do
it { expect(described_class.graphql_name).to eq('DiffRefs') } it { expect(described_class.graphql_name).to eq('DiffRefs') }
it { expect(described_class).to have_graphql_fields(:base_sha, :head_sha, :start_sha) } it { is_expected.to have_graphql_fields(:head_sha, :base_sha, :start_sha).only }
it { expect(described_class.fields['headSha'].type).to be_non_null }
it { expect(described_class.fields['baseSha'].type).not_to be_non_null }
it { expect(described_class.fields['startSha'].type).to be_non_null }
end end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::DependencyLinker::BaseLinker do
let(:linker_class) do
Class.new(described_class) do
def link_dependencies
link_regex(%r{^(?<name>https?://[^ ]+)}, &:itself)
end
end
end
let(:plain_content) do
<<~CONTENT
http://\\njavascript:alert(1)
https://gitlab.com/gitlab-org/gitlab
CONTENT
end
let(:highlighted_content) do
<<~CONTENT
<span><span>http://</span><span>\\n</span><span>javascript:alert(1)</span></span>
<span><span>https://gitlab.com/gitlab-org/gitlab</span></span>
CONTENT
end
let(:linker) { linker_class.new(plain_content, highlighted_content) }
describe '#link' do
subject { linker.link }
it 'only converts valid links' do
expect(subject).to eq(
<<~CONTENT
<span><span>#{link('http://')}</span><span>#{link('\n', url: '%5Cn')}</span><span>#{link('javascript:alert(1)', url: nil)}</span></span>
<span><span>#{link('https://gitlab.com/gitlab-org/gitlab')}</span></span>
CONTENT
)
end
end
def link(text, url: text)
attrs = [
'rel="nofollow noreferrer noopener"',
'target="_blank"'
]
attrs.unshift(%{href="#{url}"}) if url
%{<a #{attrs.join(' ')}>#{text}</a>}
end
end
...@@ -109,6 +109,20 @@ describe Gitlab::ProjectAuthorizations do ...@@ -109,6 +109,20 @@ describe Gitlab::ProjectAuthorizations do
end end
end end
context 'with lower group access level than max access level for share' do
let(:user) { create(:user) }
it 'creates proper authorizations' do
group.add_reporter(user)
mapping = map_access_levels(authorizations)
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to eq(Gitlab::Access::REPORTER)
expect(mapping[project_child.id]).to eq(Gitlab::Access::REPORTER)
end
end
context 'parent group user' do context 'parent group user' do
let(:user) { parent_group_user } let(:user) { parent_group_user }
... ...
......
...@@ -30,6 +30,17 @@ describe Gitlab::UserAccess do ...@@ -30,6 +30,17 @@ describe Gitlab::UserAccess do
end end
end end
describe 'push to branch in an internal project' do
it 'will not infinitely loop when a project is internal' do
project.visibility_level = Gitlab::VisibilityLevel::INTERNAL
project.save!
expect(project).not_to receive(:branch_allows_collaboration?)
access.can_push_to_branch?('master')
end
end
describe 'push to empty project' do describe 'push to empty project' do
let(:empty_project) { create(:project_empty_repo) } let(:empty_project) { create(:project_empty_repo) }
let(:project_access) { described_class.new(user, project: empty_project) } let(:project_access) { described_class.new(user, project: empty_project) }
... ...
......
...@@ -563,6 +563,18 @@ describe Group do ...@@ -563,6 +563,18 @@ describe Group do
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER) expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER) expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER)
end end
context 'with lower group access level than max access level for share' do
let(:user) { create(:user) }
it 'returns correct access level' do
group.add_reporter(user)
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER)
end
end
end end
context 'with user in the parent group' do context 'with user in the parent group' do
...@@ -584,6 +596,33 @@ describe Group do ...@@ -584,6 +596,33 @@ describe Group do
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end end
end end
context 'unrelated project owner' do
let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 }
let!(:group) { create(:group, id: common_id) }
let!(:unrelated_project) { create(:project, id: common_id) }
let(:user) { unrelated_project.owner }
it 'returns correct access level' do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end
end
context 'user without accepted access request' do
let!(:user) { create(:user) }
before do
create(:group_member, :developer, :access_request, user: user, group: group)
end
it 'returns correct access level' do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end
end
end end
context 'when feature flag share_group_with_group is disabled' do context 'when feature flag share_group_with_group is disabled' do
... ...
......
...@@ -4795,6 +4795,38 @@ describe Project do ...@@ -4795,6 +4795,38 @@ describe Project do
end end
end end
context 'with cross internal project merge requests' do
let(:project) { create(:project, :repository, :internal) }
let(:forked_project) { fork_project(project, nil, repository: true) }
let(:user) { double(:user) }
it "does not endlessly loop for internal projects with MRs to each other", :sidekiq_inline do
allow(user).to receive(:can?).and_return(true, false, true)
allow(user).to receive(:id).and_return(1)
create(
:merge_request,
target_project: project,
target_branch: 'merge-test',
source_project: forked_project,
source_branch: 'merge-test',
allow_collaboration: true
)
create(
:merge_request,
target_project: forked_project,
target_branch: 'merge-test',
source_project: project,
source_branch: 'merge-test',
allow_collaboration: true
)
expect(user).to receive(:can?).at_most(5).times
project.branch_allows_collaboration?(user, "merge-test")
end
end
context 'with cross project merge requests' do context 'with cross project merge requests' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:target_project) { create(:project, :repository) } let(:target_project) { create(:project, :repository) }
... ...
......
...@@ -6,6 +6,7 @@ describe Ci::PipelinePresenter do ...@@ -6,6 +6,7 @@ describe Ci::PipelinePresenter do
include Gitlab::Routing include Gitlab::Routing
let(:user) { create(:user) } let(:user) { create(:user) }
let(:current_user) { user }
let(:project) { create(:project) } let(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: project) } let(:pipeline) { create(:ci_pipeline, project: project) }
...@@ -15,7 +16,7 @@ describe Ci::PipelinePresenter do ...@@ -15,7 +16,7 @@ describe Ci::PipelinePresenter do
before do before do
project.add_developer(user) project.add_developer(user)
allow(presenter).to receive(:current_user) { user } allow(presenter).to receive(:current_user) { current_user }
end end
it 'inherits from Gitlab::View::Presenter::Delegated' do it 'inherits from Gitlab::View::Presenter::Delegated' do
...@@ -215,10 +216,90 @@ describe Ci::PipelinePresenter do ...@@ -215,10 +216,90 @@ describe Ci::PipelinePresenter do
describe '#all_related_merge_requests' do describe '#all_related_merge_requests' do
it 'memoizes the returned relation' do it 'memoizes the returned relation' do
query_count = ActiveRecord::QueryRecorder.new do query_count = ActiveRecord::QueryRecorder.new do
2.times { presenter.send(:all_related_merge_requests).count } 3.times { presenter.send(:all_related_merge_requests).count }
end.count end.count
expect(query_count).to eq(1) expect(query_count).to eq(2)
end
context 'permissions' do
let!(:merge_request) do
create(:merge_request, project: project, source_project: project)
end
subject(:all_related_merge_requests) do
presenter.send(:all_related_merge_requests)
end
shared_examples 'private merge requests' do
context 'when not logged in' do
let(:current_user) {}
it { is_expected.to be_empty }
end
context 'when logged in as a non_member' do
let(:current_user) { create(:user) }
it { is_expected.to be_empty }
end
context 'when logged in as a guest' do
let(:current_user) { create(:user) }
before do
project.add_guest(current_user)
end
it { is_expected.to be_empty }
end
context 'when logged in as a developer' do
it { is_expected.to contain_exactly(merge_request) }
end
context 'when logged in as a maintainer' do
let(:current_user) { create(:user) }
before do
project.add_maintainer(current_user)
end
it { is_expected.to contain_exactly(merge_request) }
end
end
context 'with a private project' do
it_behaves_like 'private merge requests'
end
context 'with a public project with private merge requests' do
before do
project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
project
.project_feature
.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
end
it_behaves_like 'private merge requests'
end
context 'with a public project with public merge requests' do
before do
project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
project
.project_feature
.update!(merge_requests_access_level: ProjectFeature::ENABLED)
end
context 'when not logged in' do
let(:current_user) {}
it { is_expected.to contain_exactly(merge_request) }
end
end
end end
end end
... ...
......
...@@ -116,6 +116,18 @@ describe API::Triggers do ...@@ -116,6 +116,18 @@ describe API::Triggers do
end end
end end
end end
context 'when is triggered by a pipeline hook' do
it 'does not create a new pipeline' do
expect do
post api("/projects/#{project.id}/ref/master/trigger/pipeline?token=#{trigger_token}"),
params: { ref: 'refs/heads/other-branch' },
headers: { WebHookService::GITLAB_EVENT_HEADER => 'Pipeline Hook' }
end.not_to change(Ci::Pipeline, :count)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end end
describe 'GET /projects/:id/triggers' do describe 'GET /projects/:id/triggers' do
... ...
......
...@@ -40,24 +40,11 @@ describe Groups::GroupLinks::DestroyService, '#execute' do ...@@ -40,24 +40,11 @@ describe Groups::GroupLinks::DestroyService, '#execute' do
end end
it 'updates project authorization once per group' do it 'updates project authorization once per group' do
expect(GroupGroupLink).to receive(:delete) expect(GroupGroupLink).to receive(:delete).and_call_original
expect(group).to receive(:refresh_members_authorized_projects).once expect(group).to receive(:refresh_members_authorized_projects).once
expect(another_group).to receive(:refresh_members_authorized_projects).once expect(another_group).to receive(:refresh_members_authorized_projects).once
subject.execute(links) subject.execute(links)
end end
it 'rolls back changes when error happens' do
group.add_developer(user)
expect(group).to receive(:refresh_members_authorized_projects).once.and_call_original
expect(another_group).to(
receive(:refresh_members_authorized_projects).and_raise('boom'))
expect { subject.execute(links) }.to raise_error('boom')
expect(GroupGroupLink.count).to eq(links.length)
expect(Ability.allowed?(user, :read_project, project)).to be_truthy
end
end end
end end
# frozen_string_literal: true
require 'spec_helper'
describe Groups::GroupLinks::UpdateService, '#execute' do
let(:user) { create(:user) }
let_it_be(:group) { create(:group, :private) }
let_it_be(:shared_group) { create(:group, :private) }
let_it_be(:project) { create(:project, group: shared_group) }
let(:group_member) { create(:user) }
let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
let(:expiry_date) { 1.month.from_now.to_date }
let(:group_link_params) do
{ group_access: Gitlab::Access::GUEST,
expires_at: expiry_date }
end
subject { described_class.new(link).execute(group_link_params) }
before do
group.add_developer(group_member)
end
it 'updates existing link' do
expect(link.group_access).to eq(Gitlab::Access::DEVELOPER)
expect(link.expires_at).to be_nil
subject
link.reload
expect(link.group_access).to eq(Gitlab::Access::GUEST)
expect(link.expires_at).to eq(expiry_date)
end
it 'updates project permissions' do
expect { subject }.to change { group_member.can?(:create_release, project) }.from(true).to(false)
end
it 'executes UserProjectAccessChangedService' do
expect_next_instance_of(UserProjectAccessChangedService) do |service|
expect(service).to receive(:execute)
end
subject
end
context 'with only param not requiring authorization refresh' do
let(:group_link_params) { { expires_at: Date.tomorrow } }
it 'does not execute UserProjectAccessChangedService' do
expect(UserProjectAccessChangedService).not_to receive(:new)
subject
end
end
end
...@@ -133,6 +133,21 @@ describe Projects::LfsPointers::LfsDownloadService do ...@@ -133,6 +133,21 @@ describe Projects::LfsPointers::LfsDownloadService do
end end
end end
context 'when an lfs object with the same oid already exists' do
let!(:existing_lfs_object) { create(:lfs_object, oid: oid) }
before do
stub_full_request(download_link).to_return(body: lfs_content)
end
it_behaves_like 'no lfs object is created'
it 'does not update the file attached to the existing LfsObject' do
expect { subject.execute }
.not_to change { existing_lfs_object.reload.file.file.file }
end
end
context 'when credentials present' do context 'when credentials present' do
let(:download_link_with_credentials) { "http://user:password@gitlab.com/#{oid}" } let(:download_link_with_credentials) { "http://user:password@gitlab.com/#{oid}" }
let(:lfs_object) { LfsDownloadObject.new(oid: oid, size: size, link: download_link_with_credentials) } let(:lfs_object) { LfsDownloadObject.new(oid: oid, size: size, link: download_link_with_credentials) }
...@@ -210,17 +225,5 @@ describe Projects::LfsPointers::LfsDownloadService do ...@@ -210,17 +225,5 @@ describe Projects::LfsPointers::LfsDownloadService do
subject.execute subject.execute
end end
end end
context 'when an lfs object with the same oid already exists' do
before do
create(:lfs_object, oid: oid)
end
it 'does not download the file' do
expect(subject).not_to receive(:download_lfs_file!)
subject.execute
end
end
end end
end end
...@@ -24,7 +24,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do ...@@ -24,7 +24,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do
describe '#execute' do describe '#execute' do
context 'when no lfs pointer is linked' do context 'when no lfs pointer is linked' do
before do before do
allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return([])
allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links) allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links)
expect(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:new).with(project, remote_uri: URI.parse(default_endpoint)).and_call_original expect(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:new).with(project, remote_uri: URI.parse(default_endpoint)).and_call_original
end end
...@@ -35,12 +34,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do ...@@ -35,12 +34,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do
subject.execute subject.execute
end end
it 'links existent lfs objects to the project' do
expect_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute)
subject.execute
end
it 'retrieves the download links of non existent objects' do it 'retrieves the download links of non existent objects' do
expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(all_oids) expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(all_oids)
...@@ -48,32 +41,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do ...@@ -48,32 +41,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do
end end
end end
context 'when some lfs objects are linked' do
before do
allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return(existing_lfs_objects.keys)
allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links)
end
it 'retrieves the download links of non existent objects' do
expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(oids)
subject.execute
end
end
context 'when all lfs objects are linked' do
before do
allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return(all_oids.keys)
allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute)
end
it 'retrieves no download links' do
expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with({}).and_call_original
expect(subject.execute).to be_empty
end
end
context 'when lfsconfig file exists' do context 'when lfsconfig file exists' do
before do before do
allow(project.repository).to receive(:lfsconfig_for).and_return("[lfs]\n\turl = #{lfs_endpoint}\n") allow(project.repository).to receive(:lfsconfig_for).and_return("[lfs]\n\turl = #{lfs_endpoint}\n")
... ...
......