diff options
| author | Yorick Peterse <yorickpeterse@gmail.com> | 2019-03-04 19:44:46 +0100 |
|---|---|---|
| committer | Yorick Peterse <yorickpeterse@gmail.com> | 2019-03-04 19:44:46 +0100 |
| commit | 59db98a0cabea4421434655d7f7873110363d21a (patch) | |
| tree | f3f72bca43724e230090aeae7aa0c15e56b707bf /spec | |
| parent | 5c80bbb33c12490bc5fa711642a40fc16bdb79a4 (diff) | |
| parent | 025015048f7eaad29ee7816c6040fb3e0c06eb8d (diff) | |
| download | gitlab-ce-59db98a0cabea4421434655d7f7873110363d21a.tar.gz | |
Merge dev master into GitLab.com master
Diffstat (limited to 'spec')
57 files changed, 1096 insertions, 301 deletions
diff --git a/spec/controllers/dashboard/milestones_controller_spec.rb b/spec/controllers/dashboard/milestones_controller_spec.rb index 4b164d0aa6b..ab40b4eb178 100644 --- a/spec/controllers/dashboard/milestones_controller_spec.rb +++ b/spec/controllers/dashboard/milestones_controller_spec.rb @@ -13,7 +13,7 @@ describe Dashboard::MilestonesController do ) end let(:issue) { create(:issue, project: project, milestone: project_milestone) } - let(:group_issue) { create(:issue, milestone: group_milestone) } + let(:group_issue) { create(:issue, milestone: group_milestone, project: create(:project, group: group)) } let!(:label) { create(:label, project: project, title: 'Issue Label', issues: [issue]) } let!(:group_label) { create(:group_label, group: group, title: 'Group Issue Label', issues: [group_issue]) } diff --git a/spec/controllers/google_api/authorizations_controller_spec.rb b/spec/controllers/google_api/authorizations_controller_spec.rb index 1e8e82da4f3..d9ba85cf56a 100644 --- a/spec/controllers/google_api/authorizations_controller_spec.rb +++ b/spec/controllers/google_api/authorizations_controller_spec.rb @@ -6,7 +6,7 @@ describe GoogleApi::AuthorizationsController do let(:token) { 'token' } let(:expires_at) { 1.hour.since.strftime('%s') } - subject { get :callback, params: { code: 'xxx', state: @state } } + subject { get :callback, params: { code: 'xxx', state: state } } before do sign_in(user) @@ -15,35 +15,57 @@ describe GoogleApi::AuthorizationsController do .to receive(:get_token).and_return([token, expires_at]) end - it 'sets token and expires_at in session' do - subject + shared_examples_for 'access denied' do + it 'returns a 404' do + subject - expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token]) - .to eq(token) - expect(session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at]) - .to eq(expires_at) + expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token]).to be_nil + expect(response).to have_http_status(:not_found) + end end - context 'when redirect uri key is stored in state' do - set(:project) { create(:project) } - let(:redirect_uri) { project_clusters_url(project).to_s } + context 'session key is present' do + let(:session_key) { 'session-key' } + let(:redirect_uri) { 'example.com' } before do - @state = GoogleApi::CloudPlatform::Client - .new_session_key_for_redirect_uri do |key| - session[key] = redirect_uri + session[GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(session_key)] = redirect_uri + end + + context 'session key matches state param' do + let(:state) { session_key } + + it 'sets token and expires_at in session' do + subject + + expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token]) + .to eq(token) + expect(session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at]) + .to eq(expires_at) + end + + it 'redirects to the URL stored in state param' do + expect(subject).to redirect_to(redirect_uri) end end - it 'redirects to the URL stored in state param' do - expect(subject).to redirect_to(redirect_uri) + context 'session key does not match state param' do + let(:state) { 'bad-key' } + + it_behaves_like 'access denied' end - end - context 'when redirection url is not stored in state' do - it 'redirects to root_path' do - expect(subject).to redirect_to(root_path) + context 'state param is blank' do + let(:state) { '' } + + it_behaves_like 'access denied' end end + + context 'state param is present, but session key is blank' do + let(:state) { 'session-key' } + + it_behaves_like 'access denied' + end end end diff --git a/spec/controllers/groups/shared_projects_controller_spec.rb b/spec/controllers/groups/shared_projects_controller_spec.rb index dab7700cf64..b0c20fb5a90 100644 --- a/spec/controllers/groups/shared_projects_controller_spec.rb +++ b/spec/controllers/groups/shared_projects_controller_spec.rb @@ -6,6 +6,8 @@ describe Groups::SharedProjectsController do end def share_project(project) + group.add_developer(user) + Projects::GroupLinks::CreateService.new( project, user, diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 232a5e2793b..e0da23ca0b8 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -193,7 +193,7 @@ describe OmniauthCallbacksController, type: :controller do before do stub_omniauth_saml_config({ enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [saml_config] }) - mock_auth_hash('saml', 'my-uid', user.email, mock_saml_response) + mock_auth_hash_with_saml_xml('saml', 'my-uid', user.email, mock_saml_response) request.env["devise.mapping"] = Devise.mappings[:user] request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth'] post :saml, params: { SAMLResponse: mock_saml_response } diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb index 4bc72042710..a9a058e7e17 100644 --- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb +++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb @@ -35,4 +35,35 @@ describe Projects::AutocompleteSourcesController do avatar_url: user.avatar_url) end end + + describe 'GET milestones' do + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, namespace: group) } + let!(:project_milestone) { create(:milestone, project: project) } + let!(:group_milestone) { create(:milestone, group: group) } + + before do + sign_in(user) + end + + it 'lists milestones' do + group.add_owner(user) + + get :milestones, format: :json, params: { namespace_id: group.path, project_id: project.path } + + milestone_titles = json_response.map { |milestone| milestone["title"] } + expect(milestone_titles).to match_array([project_milestone.title, group_milestone.title]) + end + + context 'when user cannot read project issues and merge requests' do + it 'renders 404' do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + + get :milestones, format: :json, params: { namespace_id: group.path, project_id: project.path } + + expect(response).to have_gitlab_http_status(404) + end + end + end end diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb index 675eeff8d12..ce021b2f085 100644 --- a/spec/controllers/projects/group_links_controller_spec.rb +++ b/spec/controllers/projects/group_links_controller_spec.rb @@ -65,8 +65,24 @@ describe Projects::GroupLinksController do end end + context 'when user does not have access to the public group' do + let(:group) { create(:group, :public) } + + include_context 'link project to group' + + it 'renders 404' do + expect(response.status).to eq 404 + end + + it 'does not share project with that group' do + expect(group.shared_projects).not_to include project + end + end + context 'when project group id equal link group id' do before do + group2.add_developer(user) + post(:create, params: { namespace_id: project.namespace, project_id: project, @@ -102,5 +118,26 @@ describe Projects::GroupLinksController do expect(flash[:alert]).to eq('Please select a group.') end end + + context 'when link is not persisted in the database' do + before do + allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute) + .and_return({ status: :error, http_status: 409, message: 'error' }) + + post(:create, params: { + namespace_id: project.namespace, + project_id: project, + link_group_id: group.id, + link_group_access: ProjectGroupLink.default_access + }) + end + + it 'redirects to project group links page' do + expect(response).to redirect_to( + project_project_members_path(project) + ) + expect(flash[:alert]).to eq('error') + end + end end end diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 5c6858dc7b2..77a94f26d8c 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -205,6 +205,8 @@ describe SnippetsController do end context 'when the snippet description contains a file' do + include FileMoverHelpers + let(:picture_file) { '/-/system/temp/secret56/picture.jpg' } let(:text_file) { '/-/system/temp/secret78/text.txt' } let(:description) do @@ -215,6 +217,8 @@ describe SnippetsController do before do allow(FileUtils).to receive(:mkdir_p) allow(FileUtils).to receive(:move) + stub_file_mover(text_file) + stub_file_mover(picture_file) end subject { create_snippet({ description: description }, { files: [picture_file, text_file] }) } diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 406e80e91aa..9bc340ed4bb 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -233,8 +233,8 @@ describe 'Issues' do created_at: Time.now - (index * 60)) end end - let(:newer_due_milestone) { create(:milestone, due_date: '2013-12-11') } - let(:later_due_milestone) { create(:milestone, due_date: '2013-12-12') } + let(:newer_due_milestone) { create(:milestone, project: project, due_date: '2013-12-11') } + let(:later_due_milestone) { create(:milestone, project: project, due_date: '2013-12-12') } it 'sorts by newest' do visit project_issues_path(project, sort: sort_value_created_date) diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index aa91ade46ca..5c45e363997 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -1,7 +1,11 @@ require 'rails_helper' describe 'Merge request > User sees versions', :js do - let(:merge_request) { create(:merge_request, importing: true) } + let(:merge_request) do + create(:merge_request).tap do |mr| + mr.merge_request_diff.destroy + end + end let(:project) { merge_request.source_project } let(:user) { project.creator } let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index ef7ae490b0f..c691011b9ca 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -13,7 +13,7 @@ describe 'Merge requests > User lists merge requests' do source_project: project, source_branch: 'fix', assignee: user, - milestone: create(:milestone, due_date: '2013-12-11'), + milestone: create(:milestone, project: project, due_date: '2013-12-11'), created_at: 1.minute.ago, updated_at: 1.minute.ago) create(:merge_request, @@ -21,7 +21,7 @@ describe 'Merge requests > User lists merge requests' do source_project: project, source_branch: 'markdown', assignee: user, - milestone: create(:milestone, due_date: '2013-12-12'), + milestone: create(:milestone, project: project, due_date: '2013-12-12'), created_at: 2.minutes.ago, updated_at: 2.minutes.ago) create(:merge_request, diff --git a/spec/features/profiles/active_sessions_spec.rb b/spec/features/profiles/active_sessions_spec.rb index d3050760c06..2aa0177af5d 100644 --- a/spec/features/profiles/active_sessions_spec.rb +++ b/spec/features/profiles/active_sessions_spec.rb @@ -7,6 +7,8 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do end end + let(:admin) { create(:admin) } + around do |example| Timecop.freeze(Time.zone.parse('2018-03-12 09:06')) do example.run @@ -16,6 +18,7 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do it 'User sees their active sessions' do Capybara::Session.new(:session1) Capybara::Session.new(:session2) + Capybara::Session.new(:session3) # note: headers can only be set on the non-js (aka. rack-test) driver using_session :session1 do @@ -37,9 +40,27 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do gitlab_sign_in(user) end + # set an admin session impersonating the user + using_session :session3 do + Capybara.page.driver.header( + 'User-Agent', + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36' + ) + + gitlab_sign_in(admin) + + visit admin_user_path(user) + + click_link 'Impersonate' + end + using_session :session1 do visit profile_active_sessions_path + expect(page).to( + have_selector('ul.list-group li.list-group-item', { text: 'Signed in on', + count: 2 })) + expect(page).to have_content( '127.0.0.1 ' \ 'This is your current session ' \ @@ -57,33 +78,8 @@ describe 'Profile > Active Sessions', :clean_gitlab_redis_shared_state do ) expect(page).to have_selector '[title="Smartphone"]', count: 1 - end - end - - it 'User can revoke a session', :js, :redis_session_store do - Capybara::Session.new(:session1) - Capybara::Session.new(:session2) - - # set an additional session in another browser - using_session :session2 do - gitlab_sign_in(user) - end - - using_session :session1 do - gitlab_sign_in(user) - visit profile_active_sessions_path - - expect(page).to have_link('Revoke', count: 1) - - accept_confirm { click_on 'Revoke' } - - expect(page).not_to have_link('Revoke') - end - - using_session :session2 do - visit profile_active_sessions_path - expect(page).to have_content('You need to sign in or sign up before continuing.') + expect(page).not_to have_content('Chrome on Windows') end end end diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 3edcc7ac2cd..a7aa63018fd 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -548,10 +548,7 @@ describe 'File blob', :js do it 'displays an auxiliary viewer' do aggregate_failures do # shows names of dependency manager and package - expect(page).to have_content('This project manages its dependencies using RubyGems and defines a gem named activerecord.') - - # shows a link to the gem - expect(page).to have_link('activerecord', href: 'https://rubygems.org/gems/activerecord') + expect(page).to have_content('This project manages its dependencies using RubyGems.') # shows a learn more link expect(page).to have_link('Learn more', href: 'https://rubygems.org/') diff --git a/spec/features/projects/members/invite_group_spec.rb b/spec/features/projects/members/invite_group_spec.rb index fceead0b45e..b2d2dba55f1 100644 --- a/spec/features/projects/members/invite_group_spec.rb +++ b/spec/features/projects/members/invite_group_spec.rb @@ -27,6 +27,7 @@ describe 'Project > Members > Invite group', :js do before do project.add_maintainer(maintainer) + group_to_share_with.add_guest(maintainer) sign_in(maintainer) end @@ -112,6 +113,7 @@ describe 'Project > Members > Invite group', :js do before do project.add_maintainer(maintainer) + group.add_guest(maintainer) sign_in(maintainer) visit project_settings_members_path(project) diff --git a/spec/features/projects/settings/user_manages_group_links_spec.rb b/spec/features/projects/settings/user_manages_group_links_spec.rb index 676659b90c3..e5a58c44e41 100644 --- a/spec/features/projects/settings/user_manages_group_links_spec.rb +++ b/spec/features/projects/settings/user_manages_group_links_spec.rb @@ -10,6 +10,7 @@ describe 'Projects > Settings > User manages group links' do before do project.add_maintainer(user) + group_market.add_guest(user) sign_in(user) share_link = project.project_group_links.new(group_access: Gitlab::Access::MAINTAINER) diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb index 4705cd12d23..3238e07fe15 100644 --- a/spec/features/security/group/private_access_spec.rb +++ b/spec/features/security/group/private_access_spec.rb @@ -27,7 +27,7 @@ describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_allowed_for(project_guest) } + it { is_expected.to be_denied_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -42,7 +42,7 @@ describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_allowed_for(project_guest) } + it { is_expected.to be_denied_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -58,7 +58,7 @@ describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_allowed_for(project_guest) } + it { is_expected.to be_denied_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -73,7 +73,7 @@ describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_allowed_for(project_guest) } + it { is_expected.to be_denied_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -93,4 +93,28 @@ describe 'Private Group access' do it { is_expected.to be_denied_for(:visitor) } it { is_expected.to be_denied_for(:external) } end + + describe 'GET /groups/:path for shared projects' do + let(:project) { create(:project, :public) } + before do + Projects::GroupLinks::CreateService.new( + project, + create(:user), + link_group_access: ProjectGroupLink::DEVELOPER + ).execute(group) + end + + subject { group_path(group) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(group) } + it { is_expected.to be_allowed_for(:maintainer).of(group) } + it { is_expected.to be_allowed_for(:developer).of(group) } + it { is_expected.to be_allowed_for(:reporter).of(group) } + it { is_expected.to be_allowed_for(:guest).of(group) } + it { is_expected.to be_denied_for(project_guest) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:external) } + it { is_expected.to be_denied_for(:visitor) } + end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 107da08a0a9..79f854cdb96 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -31,7 +31,7 @@ describe MergeRequestsFinder do p end end - let(:project4) { create_project_without_n_plus_1(group: subgroup) } + let(:project4) { create_project_without_n_plus_1(:repository, group: subgroup) } let(:project5) { create_project_without_n_plus_1(group: subgroup) } let(:project6) { create_project_without_n_plus_1(group: subgroup) } @@ -68,6 +68,15 @@ describe MergeRequestsFinder do expect(merge_requests.size).to eq(2) end + it 'filters by commit sha' do + merge_requests = described_class.new( + user, + commit_sha: merge_request5.merge_request_diff.last_commit_sha + ).execute + + expect(merge_requests).to contain_exactly(merge_request5) + end + context 'filtering by group' do it 'includes all merge requests when user has access' do params = { group_id: group.id } @@ -269,6 +278,21 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request) end end + + context 'when project restricts merge requests' do + let(:non_member) { create(:user) } + let(:project) { create(:project, :repository, :public, :merge_requests_private) } + let!(:merge_request) { create(:merge_request, source_project: project) } + + it "returns nothing to to non members" do + merge_requests = described_class.new( + non_member, + project_id: project.id + ).execute + + expect(merge_requests).to be_empty + end + end end describe '#row_count', :request_store do diff --git a/spec/lib/constraints/project_url_constrainer_spec.rb b/spec/lib/constraints/project_url_constrainer_spec.rb index c96e7ab8495..3496b01ebcc 100644 --- a/spec/lib/constraints/project_url_constrainer_spec.rb +++ b/spec/lib/constraints/project_url_constrainer_spec.rb @@ -16,6 +16,10 @@ describe Constraints::ProjectUrlConstrainer do let(:request) { build_request('foo', 'bar') } it { expect(subject.matches?(request)).to be_falsey } + + context 'existence_check is false' do + it { expect(subject.matches?(request, existence_check: false)).to be_truthy } + end end context "project id ending with .git" do diff --git a/spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb b/spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb index 4d222564fd0..0ebd8994636 100644 --- a/spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb +++ b/spec/lib/gitlab/dependency_linker/composer_json_linker_spec.rb @@ -50,8 +50,8 @@ describe Gitlab::DependencyLinker::ComposerJsonLinker do %{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>} end - it 'links the module name' do - expect(subject).to include(link('laravel/laravel', 'https://packagist.org/packages/laravel/laravel')) + it 'does not link the module name' do + expect(subject).not_to include(link('laravel/laravel', 'https://packagist.org/packages/laravel/laravel')) end it 'links the homepage' do diff --git a/spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb b/spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb index a97803b119e..f00f6b47b94 100644 --- a/spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb +++ b/spec/lib/gitlab/dependency_linker/gemfile_linker_spec.rb @@ -41,13 +41,16 @@ describe Gitlab::DependencyLinker::GemfileLinker do end it 'links dependencies' do - expect(subject).to include(link('rails', 'https://rubygems.org/gems/rails')) expect(subject).to include(link('rails-deprecated_sanitizer', 'https://rubygems.org/gems/rails-deprecated_sanitizer')) - expect(subject).to include(link('responders', 'https://rubygems.org/gems/responders')) - expect(subject).to include(link('sprockets', 'https://rubygems.org/gems/sprockets')) expect(subject).to include(link('default_value_for', 'https://rubygems.org/gems/default_value_for')) end + it 'links to external dependencies' do + expect(subject).to include(link('rails', 'https://github.com/rails/rails')) + expect(subject).to include(link('responders', 'https://github.com/rails/responders')) + expect(subject).to include(link('sprockets', 'https://gitlab.example.com/gems/sprockets')) + end + it 'links GitHub repos' do expect(subject).to include(link('rails/rails', 'https://github.com/rails/rails')) expect(subject).to include(link('rails/responders', 'https://github.com/rails/responders')) diff --git a/spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb b/spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb index 24ad7d12f4c..6c6a5d70576 100644 --- a/spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb +++ b/spec/lib/gitlab/dependency_linker/gemspec_linker_spec.rb @@ -43,8 +43,8 @@ describe Gitlab::DependencyLinker::GemspecLinker do %{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>} end - it 'links the gem name' do - expect(subject).to include(link('gitlab_git', 'https://rubygems.org/gems/gitlab_git')) + it 'does not link the gem name' do + expect(subject).not_to include(link('gitlab_git', 'https://rubygems.org/gems/gitlab_git')) end it 'links the license' do diff --git a/spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb b/spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb index 1e8b72afb7b..9050127af7f 100644 --- a/spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb +++ b/spec/lib/gitlab/dependency_linker/package_json_linker_spec.rb @@ -33,7 +33,8 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do "express": "4.2.x", "bigpipe": "bigpipe/pagelet", "plates": "https://github.com/flatiron/plates/tarball/master", - "karma": "^1.4.1" + "karma": "^1.4.1", + "random": "git+https://EdOverflow@github.com/example/example.git" }, "devDependencies": { "vows": "^0.7.0", @@ -51,8 +52,8 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do %{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>} end - it 'links the module name' do - expect(subject).to include(link('module-name', 'https://npmjs.com/package/module-name')) + it 'does not link the module name' do + expect(subject).not_to include(link('module-name', 'https://npmjs.com/package/module-name')) end it 'links the homepage' do @@ -71,14 +72,21 @@ describe Gitlab::DependencyLinker::PackageJsonLinker do expect(subject).to include(link('primus', 'https://npmjs.com/package/primus')) expect(subject).to include(link('async', 'https://npmjs.com/package/async')) expect(subject).to include(link('express', 'https://npmjs.com/package/express')) - expect(subject).to include(link('bigpipe', 'https://npmjs.com/package/bigpipe')) - expect(subject).to include(link('plates', 'https://npmjs.com/package/plates')) expect(subject).to include(link('karma', 'https://npmjs.com/package/karma')) expect(subject).to include(link('vows', 'https://npmjs.com/package/vows')) expect(subject).to include(link('assume', 'https://npmjs.com/package/assume')) expect(subject).to include(link('pre-commit', 'https://npmjs.com/package/pre-commit')) end + it 'links dependencies to URL detected on value' do + expect(subject).to include(link('bigpipe', 'https://github.com/bigpipe/pagelet')) + expect(subject).to include(link('plates', 'https://github.com/flatiron/plates/tarball/master')) + end + + it 'does not link to NPM when invalid git URL' do + expect(subject).not_to include(link('random', 'https://npmjs.com/package/random')) + end + it 'links GitHub repos' do expect(subject).to include(link('bigpipe/pagelet', 'https://github.com/bigpipe/pagelet')) end diff --git a/spec/lib/gitlab/dependency_linker/parser/gemfile_spec.rb b/spec/lib/gitlab/dependency_linker/parser/gemfile_spec.rb new file mode 100644 index 00000000000..f81dbcf62da --- /dev/null +++ b/spec/lib/gitlab/dependency_linker/parser/gemfile_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +describe Gitlab::DependencyLinker::Parser::Gemfile do + describe '#parse' do + let(:file_content) do + <<-CONTENT.strip_heredoc + source 'https://rubygems.org' + + gem "rails", '4.2.6', github: "rails/rails" + gem 'rails-deprecated_sanitizer', '~> 1.0.3' + gem 'responders', '~> 2.0', :github => 'rails/responders' + gem 'sprockets', '~> 3.6.0', git: 'https://gitlab.example.com/gems/sprockets' + gem 'default_value_for', '~> 3.0.0' + CONTENT + end + + subject { described_class.new(file_content).parse(keyword: 'gem') } + + def fetch_package(name) + subject.find { |package| package.name == name } + end + + it 'returns parsed packages' do + expect(subject.size).to eq(5) + expect(subject).to all(be_a(Gitlab::DependencyLinker::Package)) + end + + it 'packages respond to name and external_ref accordingly' do + expect(fetch_package('rails')).to have_attributes(name: 'rails', + github_ref: 'rails/rails', + git_ref: nil) + + expect(fetch_package('sprockets')).to have_attributes(name: 'sprockets', + github_ref: nil, + git_ref: 'https://gitlab.example.com/gems/sprockets') + + expect(fetch_package('default_value_for')).to have_attributes(name: 'default_value_for', + github_ref: nil, + git_ref: nil) + end + end +end diff --git a/spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb b/spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb index cdfd7ad9826..8f1b523653e 100644 --- a/spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb +++ b/spec/lib/gitlab/dependency_linker/podfile_linker_spec.rb @@ -43,7 +43,10 @@ describe Gitlab::DependencyLinker::PodfileLinker do it 'links packages' do expect(subject).to include(link('AFNetworking', 'https://cocoapods.org/pods/AFNetworking')) - expect(subject).to include(link('Interstellar/Core', 'https://cocoapods.org/pods/Interstellar')) + end + + it 'links external packages' do + expect(subject).to include(link('Interstellar/Core', 'https://github.com/ashfurrow/Interstellar.git')) end it 'links Git repos' do diff --git a/spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb b/spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb index ed60ab45955..bacec830103 100644 --- a/spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb +++ b/spec/lib/gitlab/dependency_linker/podspec_linker_spec.rb @@ -42,8 +42,8 @@ describe Gitlab::DependencyLinker::PodspecLinker do %{<a href="#{url}" rel="nofollow noreferrer noopener" target="_blank">#{name}</a>} end - it 'links the gem name' do - expect(subject).to include(link('Reachability', 'https://cocoapods.org/pods/Reachability')) + it 'does not link the pod name' do + expect(subject).not_to include(link('Reachability', 'https://cocoapods.org/pods/Reachability')) end it 'links the license' do diff --git a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb index 68eaa70e6b6..4b234411a44 100644 --- a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb +++ b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb @@ -41,4 +41,20 @@ describe Gitlab::ImportExport::MergeRequestParser do expect(parsed_merge_request).to eq(merge_request) end + + context 'when the merge request has diffs' do + let(:merge_request) do + build(:merge_request, source_project: forked_project, target_project: project) + end + + context 'when the diff is invalid' do + let(:merge_request_diff) { build(:merge_request_diff, merge_request: merge_request, base_commit_sha: 'foobar') } + + it 'sets the diff to nil' do + expect(merge_request_diff).to be_invalid + expect(merge_request_diff.merge_request).to eq merge_request + expect(parsed_merge_request.merge_request_diff).to be_nil + end + end + end end diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb index 02364e92149..978e64c4407 100644 --- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb +++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb @@ -50,6 +50,36 @@ describe Gitlab::Kubernetes::KubeClient do end end + describe '#initialize' do + shared_examples 'local address' do + it 'blocks local addresses' do + expect { client }.to raise_error(Gitlab::UrlBlocker::BlockedUrlError) + end + + context 'when local requests are allowed' do + before do + stub_application_setting(allow_local_requests_from_hooks_and_services: true) + end + + it 'allows local addresses' do + expect { client }.not_to raise_error + end + end + end + + context 'localhost address' do + let(:api_url) { 'http://localhost:22' } + + it_behaves_like 'local address' + end + + context 'private network address' do + let(:api_url) { 'http://192.168.1.2:3003' } + + it_behaves_like 'local address' + end + end + describe '#core_client' do subject { client.core_client } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 15b04c9d066..67af2c36669 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -208,6 +208,7 @@ describe Notify do let(:new_issue) { create(:issue) } subject { described_class.issue_moved_email(recipient, issue, new_issue, current_user) } +<<<<<<< HEAD it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end @@ -215,18 +216,55 @@ describe Notify do it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' it_behaves_like 'appearance header and footer not enabled' +======= + context 'when a user has permissions to access the new issue' do + before do + new_issue.project.add_developer(recipient) + end + + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } + end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' + + it 'contains description about action taken' do + is_expected.to have_body_text 'Issue was moved to another project' + end + + it 'has the correct subject and body' do + new_issue_url = project_issue_path(new_issue.project, new_issue) +>>>>>>> dev/master + + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text(new_issue_url) + is_expected.to have_body_text(project_issue_path(project, issue)) + end + end - it 'contains description about action taken' do - is_expected.to have_body_text 'Issue was moved to another project' + it 'contains the issue title' do + is_expected.to have_body_text new_issue.title + end end - it 'has the correct subject and body' do - new_issue_url = project_issue_path(new_issue.project, new_issue) + context 'when a user does not permissions to access the new issue' do + it 'has the correct subject and body' do + new_issue_url = project_issue_path(new_issue.project, new_issue) - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_body_text(new_issue_url) - is_expected.to have_body_text(project_issue_path(project, issue)) + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.not_to have_body_text(new_issue_url) + is_expected.to have_body_text(project_issue_path(project, issue)) + end + end + + it 'does not contain the issue title' do + is_expected.not_to have_body_text new_issue.title + end + + it 'contains information about missing permissions' do + is_expected.to have_body_text "You don't have access to the project." end end end diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 129b2f92683..e128fe8a4b7 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -7,7 +7,10 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do end end - let(:session) { double(:session, id: '6919a6f1bb119dd7396fadc38fd18d0d') } + let(:session) do + double(:session, { id: '6919a6f1bb119dd7396fadc38fd18d0d', + '[]': {} }) + end let(:request) do double(:request, { diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 4068d98d8f7..3b32ca8df05 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -98,6 +98,22 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it { expect(kubernetes.save).to be_truthy } end + + context 'when api_url is localhost' do + let(:api_url) { 'http://localhost:22' } + + it { expect(kubernetes.save).to be_falsey } + + context 'Application settings allows local requests' do + before do + allow(ApplicationSetting) + .to receive(:current) + .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true)) + end + + it { expect(kubernetes.save).to be_truthy } + end + end end context 'when validates token' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 41159348e04..72c6161424b 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -32,17 +32,56 @@ describe Issuable do end describe "Validation" do - subject { build(:issue) } + context 'general validations' do + subject { build(:issue) } - before do - allow(InternalId).to receive(:generate_next).and_return(nil) + before do + allow(InternalId).to receive(:generate_next).and_return(nil) + end + + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:iid) } + it { is_expected.to validate_presence_of(:author) } + it { is_expected.to validate_presence_of(:title) } + it { is_expected.to validate_length_of(:title).is_at_most(255) } end - it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:iid) } - it { is_expected.to validate_presence_of(:author) } - it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_length_of(:title).is_at_most(255) } + describe 'milestone' do + let(:project) { create(:project) } + let(:milestone_id) { create(:milestone, project: project).id } + let(:params) do + { + title: 'something', + project: project, + author: build(:user), + milestone_id: milestone_id + } + end + + subject { issuable_class.new(params) } + + context 'with correct params' do + it { is_expected.to be_valid } + end + + context 'with empty string milestone' do + let(:milestone_id) { '' } + + it { is_expected.to be_valid } + end + + context 'with nil milestone id' do + let(:milestone_id) { nil } + + it { is_expected.to be_valid } + end + + context 'with a milestone id from another project' do + let(:milestone_id) { create(:milestone).id } + + it { is_expected.to be_invalid } + end + end end describe "Scope" do @@ -66,6 +105,48 @@ describe Issuable do end end + describe '#milestone_available?' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + let(:issue) { create(:issue, project: project) } + + def build_issuable(milestone_id) + issuable_class.new(project: project, milestone_id: milestone_id) + end + + it 'returns true with a milestone from the issue project' do + milestone = create(:milestone, project: project) + + expect(build_issuable(milestone.id).milestone_available?).to be_truthy + end + + it 'returns true with a milestone from the issue project group' do + milestone = create(:milestone, group: group) + + expect(build_issuable(milestone.id).milestone_available?).to be_truthy + end + + it 'returns true with a milestone from the the parent of the issue project group', :nested_groups do + parent = create(:group) + group.update(parent: parent) + milestone = create(:milestone, group: parent) + + expect(build_issuable(milestone.id).milestone_available?).to be_truthy + end + + it 'returns false with a milestone from another project' do + milestone = create(:milestone) + + expect(build_issuable(milestone.id).milestone_available?).to be_falsey + end + + it 'returns false with a milestone from another group' do + milestone = create(:milestone, group: create(:group)) + + expect(build_issuable(milestone.id).milestone_available?).to be_falsey + end + end + describe ".search" do let!(:searchable_issue) { create(:issue, title: "Searchable awesome issue") } let!(:searchable_issue2) { create(:issue, title: 'Aw') } diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 87bf731340f..4647eecbdef 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -48,7 +48,7 @@ describe Milestone, 'Milestoneish' do merge_request_2 = create(:labeled_merge_request, labels: [label_1], source_project: project, source_branch: 'branch_2', milestone: milestone) merge_request_3 = create(:labeled_merge_request, labels: [label_3], source_project: project, source_branch: 'branch_3', milestone: milestone) - merge_requests = milestone.sorted_merge_requests + merge_requests = milestone.sorted_merge_requests(member) expect(merge_requests.first).to eq(merge_request_2) expect(merge_requests.second).to eq(merge_request_1) @@ -56,6 +56,51 @@ describe Milestone, 'Milestoneish' do end end + describe '#merge_requests_visible_to_user' do + let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) } + + context 'when project is private' do + before do + project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'does not return any merge request for a non member' do + merge_requests = milestone.merge_requests_visible_to_user(non_member) + expect(merge_requests).to be_empty + end + + it 'returns milestone merge requests for a member' do + merge_requests = milestone.merge_requests_visible_to_user(member) + expect(merge_requests).to contain_exactly(merge_request) + end + end + + context 'when project is public' do + context 'when merge requests are available to anyone' do + it 'returns milestone merge requests for a non member' do + merge_requests = milestone.merge_requests_visible_to_user(non_member) + expect(merge_requests).to contain_exactly(merge_request) + end + end + + context 'when merge requests are available to project members' do + before do + project.project_feature.update(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'does not return any merge request for a non member' do + merge_requests = milestone.merge_requests_visible_to_user(non_member) + expect(merge_requests).to be_empty + end + + it 'returns milestone merge requests for a member' do + merge_requests = milestone.merge_requests_visible_to_user(member) + expect(merge_requests).to contain_exactly(merge_request) + end + end + end + end + describe '#closed_items_count' do it 'does not count confidential issues for non project members' do expect(milestone.closed_items_count(non_member)).to eq 2 diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb index 1bf0ecb98ad..b7291eebe64 100644 --- a/spec/models/issue/metrics_spec.rb +++ b/spec/models/issue/metrics_spec.rb @@ -9,7 +9,7 @@ describe Issue::Metrics do context "milestones" do it "records the first time an issue is associated with a milestone" do time = Time.now - Timecop.freeze(time) { subject.update(milestone: create(:milestone)) } + Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) } metrics = subject.metrics expect(metrics).to be_present @@ -18,9 +18,9 @@ describe Issue::Metrics do it "does not record the second time an issue is associated with a milestone" do time = Time.now - Timecop.freeze(time) { subject.update(milestone: create(:milestone)) } + Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) } Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) } - Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone)) } + Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone, project: project)) } metrics = subject.metrics expect(metrics).to be_present diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 1849d3bac12..e530e0302f5 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -3,6 +3,18 @@ require 'spec_helper' describe MergeRequestDiff do let(:diff_with_commits) { create(:merge_request).merge_request_diff } + describe 'validations' do + subject { diff_with_commits } + + it 'checks sha format of base_commit_sha, head_commit_sha and start_commit_sha' do + subject.base_commit_sha = subject.head_commit_sha = subject.start_commit_sha = 'foobar' + + expect(subject.valid?).to be false + expect(subject.errors.count).to eq 3 + expect(subject.errors).to all(include('is not a valid SHA')) + end + end + describe 'create new record' do subject { diff_with_commits } @@ -78,7 +90,7 @@ describe MergeRequestDiff do it 'returns persisted diffs if cannot compare with diff refs' do expect(diff).to receive(:load_diffs).and_call_original - diff.update!(head_commit_sha: 'invalid-sha') + diff.update!(head_commit_sha: Digest::SHA1.hexdigest(SecureRandom.hex)) diff.diffs.diff_files end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index af7e3d3a6c9..77b7042424c 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -182,7 +182,7 @@ describe Milestone do describe '#total_items_count' do before do create :closed_issue, milestone: milestone, project: project - create :merge_request, milestone: milestone + create :merge_request, milestone: milestone, source_project: project end it 'returns total count of issues and merge requests assigned to milestone' do @@ -192,10 +192,10 @@ describe Milestone do describe '#can_be_closed?' do before do - milestone = create :milestone - create :closed_issue, milestone: milestone + milestone = create :milestone, project: project + create :closed_issue, milestone: milestone, project: project - create :issue + create :issue, project: project end it 'returns true if milestone active and all nested issues closed' do diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index b6cf4c72450..e9c7c94ad70 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -33,18 +33,38 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do describe 'Validations' do context 'when manual_configuration is enabled' do before do - subject.manual_configuration = true + service.manual_configuration = true end - it { is_expected.to validate_presence_of(:api_url) } + it 'validates presence of api_url' do + expect(service).to validate_presence_of(:api_url) + end end context 'when manual configuration is disabled' do before do - subject.manual_configuration = false + service.manual_configuration = false end - it { is_expected.not_to validate_presence_of(:api_url) } + it 'does not validate presence of api_url' do + expect(service).not_to validate_presence_of(:api_url) + end + end + + context 'when the api_url domain points to localhost or local network' do + let(:domain) { Addressable::URI.parse(service.api_url).hostname } + + it 'cannot query' do + expect(service.can_query?).to be true + + aggregate_failures do + ['127.0.0.1', '192.168.2.3'].each do |url| + allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([Addrinfo.tcp(url, 80)]) + + expect(service.can_query?).to be false + end + end + end end end @@ -74,30 +94,35 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do end describe '#prometheus_client' do + let(:api_url) { 'http://some_url' } + + before do + service.active = true + service.api_url = api_url + service.manual_configuration = manual_configuration + end + context 'manual configuration is enabled' do - let(:api_url) { 'http://some_url' } + let(:manual_configuration) { true } - before do - subject.active = true - subject.manual_configuration = true - subject.api_url = api_url + it 'returns rest client from api_url' do + expect(service.prometheus_client.url).to eq(api_url) end - it 'returns rest client from api_url' do - expect(subject.prometheus_client.url).to eq(api_url) + it 'calls valid?' do + allow(service).to receive(:valid?).and_call_original + + expect(service.prometheus_client).not_to be_nil + + expect(service).to have_received(:valid?) end end context 'manual configuration is disabled' do - let(:api_url) { 'http://some_url' } - - before do - subject.manual_configuration = false - subject.api_url = api_url - end + let(:manual_configuration) { false } it 'no client provided' do - expect(subject.prometheus_client).to be_nil + expect(service.prometheus_client).to be_nil end end end diff --git a/spec/policies/commit_policy_spec.rb b/spec/policies/commit_policy_spec.rb new file mode 100644 index 00000000000..2259693cf01 --- /dev/null +++ b/spec/policies/commit_policy_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe CommitPolicy do + describe '#rules' do + let(:user) { create(:user) } + let(:commit) { project.repository.head_commit } + let(:policy) { described_class.new(user, commit) } + + context 'when project is public' do + let(:project) { create(:project, :public, :repository) } + + it 'can read commit and create a note' do + expect(policy).to be_allowed(:read_commit) + end + + context 'when repository access level is private' do + let(:project) { create(:project, :public, :repository, :repository_private) } + + it 'can not read commit and create a note' do + expect(policy).to be_disallowed(:read_commit) + end + + context 'when the user is a project member' do + before do + project.add_developer(user) + end + + it 'can read commit and create a note' do + expect(policy).to be_allowed(:read_commit) + end + end + end + end + + context 'when project is private' do + let(:project) { create(:project, :private, :repository) } + + it 'can not read commit and create a note' do + expect(policy).to be_disallowed(:read_commit) + end + + context 'when the user is a project member' do + before do + project.add_developer(user) + end + + it 'can read commit and create a note' do + expect(policy).to be_allowed(:read_commit) + end + end + end + end +end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index af6d6f084a9..5cb0183df3a 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -74,6 +74,38 @@ describe GroupPolicy do end end + context 'with no user and public project' do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:current_user) { nil } + + before do + Projects::GroupLinks::CreateService.new( + project, + user, + link_group_access: ProjectGroupLink::DEVELOPER + ).execute(group) + end + + it { expect_disallowed(:read_group) } + end + + context 'with foreign user and public project' do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:current_user) { create(:user) } + + before do + Projects::GroupLinks::CreateService.new( + project, + user, + link_group_access: ProjectGroupLink::DEVELOPER + ).execute(group) + end + + it { expect_disallowed(:read_group) } + end + context 'has projects' do let(:current_user) { create(:user) } let(:project) { create(:project, namespace: group) } @@ -82,17 +114,25 @@ describe GroupPolicy do project.add_developer(current_user) end +<<<<<<< HEAD it do expect_allowed(:read_group, :read_list, :read_label) end +======= + it { expect_allowed(:read_label) } +>>>>>>> dev/master context 'in subgroups', :nested_groups do let(:subgroup) { create(:group, :private, parent: group) } let(:project) { create(:project, namespace: subgroup) } +<<<<<<< HEAD it do expect_allowed(:read_group, :read_list, :read_label) end +======= + it { expect_allowed(:read_label) } +>>>>>>> dev/master end end diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb index 0e848c74659..4be7a0266d1 100644 --- a/spec/policies/note_policy_spec.rb +++ b/spec/policies/note_policy_spec.rb @@ -1,28 +1,15 @@ require 'spec_helper' -describe NotePolicy, mdoels: true do +describe NotePolicy do describe '#rules' do let(:user) { create(:user) } let(:project) { create(:project, :public) } let(:issue) { create(:issue, project: project) } - - def policies(noteable = nil) - return @policies if @policies - - noteable ||= issue - note = if noteable.is_a?(Commit) - create(:note_on_commit, commit_id: noteable.id, author: user, project: project) - else - create(:note, noteable: noteable, author: user, project: project) - end - - @policies = described_class.new(user, note) - end + let(:noteable) { issue } + let(:policy) { described_class.new(user, note) } + let(:note) { create(:note, noteable: noteable, author: user, project: project) } shared_examples_for 'a discussion with a private noteable' do - let(:noteable) { issue } - let(:policy) { policies(noteable) } - context 'when the note author can no longer see the noteable' do it 'can not edit nor read the note' do expect(policy).to be_disallowed(:admin_note) @@ -46,12 +33,21 @@ describe NotePolicy, mdoels: true do end end - context 'when the project is private' do - let(:project) { create(:project, :private, :repository) } + context 'when the noteable is a commit' do + let(:commit) { project.repository.head_commit } + let(:note) { create(:note_on_commit, commit_id: commit.id, author: user, project: project) } + + context 'when the project is private' do + let(:project) { create(:project, :private, :repository) } + + it_behaves_like 'a discussion with a private noteable' + end - context 'when the noteable is a commit' do - it_behaves_like 'a discussion with a private noteable' do - let(:noteable) { project.repository.head_commit } + context 'when the project is public' do + context 'when repository access level is private' do + let(:project) { create(:project, :public, :repository, :repository_private) } + + it_behaves_like 'a discussion with a private noteable' end end end @@ -59,44 +55,44 @@ describe NotePolicy, mdoels: true do context 'when the project is public' do context 'when the note author is not a project member' do it 'can edit a note' do - expect(policies).to be_allowed(:admin_note) - expect(policies).to be_allowed(:resolve_note) - expect(policies).to be_allowed(:read_note) + expect(policy).to be_allowed(:admin_note) + expect(policy).to be_allowed(:resolve_note) + expect(policy).to be_allowed(:read_note) end end context 'when the noteable is a project snippet' do - it 'can edit note' do - policies = policies(create(:project_snippet, :public, project: project)) + let(:noteable) { create(:project_snippet, :public, project: project) } - expect(policies).to be_allowed(:admin_note) - expect(policies).to be_allowed(:resolve_note) - expect(policies).to be_allowed(:read_note) + it 'can edit note' do + expect(policy).to be_allowed(:admin_note) + expect(policy).to be_allowed(:resolve_note) + expect(policy).to be_allowed(:read_note) end context 'when it is private' do - it_behaves_like 'a discussion with a private noteable' do - let(:noteable) { create(:project_snippet, :private, project: project) } - end + let(:noteable) { create(:project_snippet, :private, project: project) } + + it_behaves_like 'a discussion with a private noteable' end end context 'when the noteable is a personal snippet' do - it 'can edit note' do - policies = policies(create(:personal_snippet, :public)) + let(:noteable) { create(:personal_snippet, :public) } - expect(policies).to be_allowed(:admin_note) - expect(policies).to be_allowed(:resolve_note) - expect(policies).to be_allowed(:read_note) + it 'can edit note' do + expect(policy).to be_allowed(:admin_note) + expect(policy).to be_allowed(:resolve_note) + expect(policy).to be_allowed(:read_note) end context 'when it is private' do - it 'can not edit nor read the note' do - policies = policies(create(:personal_snippet, :private)) + let(:noteable) { create(:personal_snippet, :private) } - expect(policies).to be_disallowed(:admin_note) - expect(policies).to be_disallowed(:resolve_note) - expect(policies).to be_disallowed(:read_note) + it 'can not edit nor read the note' do + expect(policy).to be_disallowed(:admin_note) + expect(policy).to be_disallowed(:resolve_note) + expect(policy).to be_disallowed(:read_note) end end end @@ -120,20 +116,20 @@ describe NotePolicy, mdoels: true do end it 'can edit a note' do - expect(policies).to be_allowed(:admin_note) - expect(policies).to be_allowed(:resolve_note) - expect(policies).to be_allowed(:read_note) + expect(policy).to be_allowed(:admin_note) + expect(policy).to be_allowed(:resolve_note) + expect(policy).to be_allowed(:read_note) end end context 'when the note author is not a project member' do it 'can not edit a note' do - expect(policies).to be_disallowed(:admin_note) - expect(policies).to be_disallowed(:resolve_note) + expect(policy).to be_disallowed(:admin_note) + expect(policy).to be_disallowed(:resolve_note) end it 'can read a note' do - expect(policies).to be_allowed(:read_note) + expect(policy).to be_allowed(:read_note) end end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 997bdc82af6..47491f708e9 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -131,22 +131,26 @@ describe ProjectPolicy do subject { described_class.new(owner, project) } context 'when the feature is disabled' do - it 'does not include the issues permissions' do + before do project.issues_enabled = false project.save! + end + it 'does not include the issues permissions' do expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue end - end - context 'when the feature is disabled and external tracker configured' do - it 'does not include the issues permissions' do - create(:jira_service, project: project) + it 'disables boards and lists permissions' do + expect_disallowed :read_board, :create_board, :update_board, :admin_board + expect_disallowed :read_list, :create_list, :update_list, :admin_list + end - project.issues_enabled = false - project.save! + context 'when external tracker configured' do + it 'does not include the issues permissions' do + create(:jira_service, project: project) - expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue + expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue + end end end end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 066f1d6862a..a132b85b878 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -1430,8 +1430,8 @@ describe API::Commits do end describe 'GET /projects/:id/repository/commits/:sha/merge_requests' do - let!(:project) { create(:project, :repository, :private) } - let!(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') } + let(:project) { create(:project, :repository, :private) } + let(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') } let(:commit) { merged_mr.merge_request_diff.commits.last } it 'returns the correct merge request' do @@ -1456,6 +1456,17 @@ describe API::Commits do expect(response).to have_gitlab_http_status(404) end + + context 'public project' do + let(:project) { create(:project, :repository, :public, :merge_requests_private) } + let(:non_member) { create(:user) } + + it 'responds 403 when only members are allowed to read merge requests' do + get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", non_member) + + expect(response).to have_gitlab_http_status(403) + end + end end describe 'GET /projects/:id/repository/commits/:sha/signature' do diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 01bab2a1361..f35dabf5d0f 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -49,7 +49,7 @@ describe API::Issues do create(:label, title: 'label', color: '#FFAABB', project: project) end let!(:label_link) { create(:label_link, label: label, target: issue) } - set(:milestone) { create(:milestone, title: '1.0.0', project: project) } + let(:milestone) { create(:milestone, title: '1.0.0', project: project) } set(:empty_milestone) do create(:milestone, title: '2.0.0', project: project) end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 856fe1bbe89..60d9d7fed13 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -136,6 +136,7 @@ describe API::Projects do end let!(:public_project) { create(:project, :public, name: 'public_project') } + before do project project2 @@ -968,8 +969,16 @@ describe API::Projects do describe 'GET /projects/:id' do context 'when unauthenticated' do - it 'returns the public projects' do - public_project = create(:project, :public) + it 'does not return private projects' do + private_project = create(:project, :private) + + get api("/projects/#{private_project.id}") + + expect(response).to have_gitlab_http_status(404) + end + + it 'returns public projects' do + public_project = create(:project, :repository, :public) get api("/projects/#{public_project.id}") @@ -977,8 +986,34 @@ describe API::Projects do expect(json_response['id']).to eq(public_project.id) expect(json_response['description']).to eq(public_project.description) expect(json_response['default_branch']).to eq(public_project.default_branch) + expect(json_response['ci_config_path']).to eq(public_project.ci_config_path) expect(json_response.keys).not_to include('permissions') end + + context 'and the project has a private repository' do + let(:project) { create(:project, :repository, :public, :repository_private) } + let(:protected_attributes) { %w(default_branch ci_config_path) } + + it 'hides protected attributes of private repositories if user is not a member' do + get api("/projects/#{project.id}", user) + + expect(response).to have_gitlab_http_status(200) + protected_attributes.each do |attribute| + expect(json_response.keys).not_to include(attribute) + end + end + + it 'exposes protected attributes of private repositories if user is a member' do + project.add_developer(user) + + get api("/projects/#{project.id}", user) + + expect(response).to have_gitlab_http_status(200) + protected_attributes.each do |attribute| + expect(json_response.keys).to include(attribute) + end + end + end end context 'when authenticated' do @@ -1130,6 +1165,26 @@ describe API::Projects do expect(json_response).to include 'statistics' end + context "and the project has a private repository" do + let(:project) { create(:project, :public, :repository, :repository_private) } + + it "does not include statistics if user is not a member" do + get api("/projects/#{project.id}", user), params: { statistics: true } + + expect(response).to have_gitlab_http_status(200) + expect(json_response).not_to include 'statistics' + end + + it "includes statistics if user is a member" do + project.add_developer(user) + + get api("/projects/#{project.id}", user), params: { statistics: true } + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to include 'statistics' + end + end + it "includes import_error if user can admin project" do get api("/projects/#{project.id}", user) @@ -1510,6 +1565,9 @@ describe API::Projects do describe "POST /projects/:id/share" do let(:group) { create(:group) } + before do + group.add_developer(user) + end it "shares project with group" do expires_at = 10.days.from_now.to_date @@ -1560,6 +1618,15 @@ describe API::Projects do expect(response).to have_gitlab_http_status(400) expect(json_response['error']).to eq 'group_access does not have a valid value' end + + it "returns a 409 error when link is not saved" do + allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute) + .and_return({ status: :error, http_status: 409, message: 'error' }) + + post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: Gitlab::Access::DEVELOPER } + + expect(response).to have_gitlab_http_status(409) + end end describe 'DELETE /projects/:id/share/:group_id' do diff --git a/spec/requests/api/release/links_spec.rb b/spec/requests/api/release/links_spec.rb index ba948e37e2f..3a59052bb29 100644 --- a/spec/requests/api/release/links_spec.rb +++ b/spec/requests/api/release/links_spec.rb @@ -73,6 +73,22 @@ describe API::Release::Links do expect(response).to have_gitlab_http_status(:ok) end end + + context 'when project is public and the repository is private' do + let(:project) { create(:project, :repository, :public, :repository_private) } + + it_behaves_like '403 response' do + let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) } + end + + context 'when the release does not exists' do + let!(:release) { } + + it_behaves_like '403 response' do + let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) } + end + end + end end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 5b625fd47be..bfa178f5cae 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -104,6 +104,70 @@ describe 'Git HTTP requests' do end end + shared_examples_for 'project path without .git suffix' do + context "GET info/refs" do + let(:path) { "/#{project_path}/info/refs" } + + context "when no params are added" do + before do + get path + end + + it "redirects to the .git suffix version" do + expect(response).to redirect_to("/#{project_path}.git/info/refs") + end + end + + context "when the upload-pack service is requested" do + let(:params) { { service: 'git-upload-pack' } } + + before do + get path, params: params + end + + it "redirects to the .git suffix version" do + expect(response).to redirect_to("/#{project_path}.git/info/refs?service=#{params[:service]}") + end + end + + context "when the receive-pack service is requested" do + let(:params) { { service: 'git-receive-pack' } } + + before do + get path, params: params + end + + it "redirects to the .git suffix version" do + expect(response).to redirect_to("/#{project_path}.git/info/refs?service=#{params[:service]}") + end + end + + context "when the params are anything else" do + let(:params) { { service: 'git-implode-pack' } } + + before do + get path, params: params + end + + it "redirects to the sign-in page" do + expect(response).to redirect_to(new_user_session_path) + end + end + end + + context "POST git-upload-pack" do + it "fails to find a route" do + expect { clone_post(project_path) }.to raise_error(ActionController::RoutingError) + end + end + + context "POST git-receive-pack" do + it "fails to find a route" do + expect { push_post(project_path) }.to raise_error(ActionController::RoutingError) + end + end + end + describe "User with no identities" do let(:user) { create(:user) } @@ -143,6 +207,10 @@ describe 'Git HTTP requests' do expect(response).to have_gitlab_http_status(:unprocessable_entity) end end + + it_behaves_like 'project path without .git suffix' do + let(:project_path) { "#{user.namespace.path}/project.git-project" } + end end end @@ -706,70 +774,8 @@ describe 'Git HTTP requests' do end end - context "when the project path doesn't end in .git" do - let(:project) { create(:project, :repository, :public, path: 'project.git-project') } - - context "GET info/refs" do - let(:path) { "/#{project.full_path}/info/refs" } - - context "when no params are added" do - before do - get path - end - - it "redirects to the .git suffix version" do - expect(response).to redirect_to("/#{project.full_path}.git/info/refs") - end - end - - context "when the upload-pack service is requested" do - let(:params) { { service: 'git-upload-pack' } } - - before do - get path, params: params - end - - it "redirects to the .git suffix version" do - expect(response).to redirect_to("/#{project.full_path}.git/info/refs?service=#{params[:service]}") - end - end - - context "when the receive-pack service is requested" do - let(:params) { { service: 'git-receive-pack' } } - - before do - get path, params: params - end - - it "redirects to the .git suffix version" do - expect(response).to redirect_to("/#{project.full_path}.git/info/refs?service=#{params[:service]}") - end - end - - context "when the params are anything else" do - let(:params) { { service: 'git-implode-pack' } } - - before do - get path, params: params - end - - it "redirects to the sign-in page" do - expect(response).to redirect_to(new_user_session_path) - end - end - end - - context "POST git-upload-pack" do - it "fails to find a route" do - expect { clone_post(project.full_path) }.to raise_error(ActionController::RoutingError) - end - end - - context "POST git-receive-pack" do - it "fails to find a route" do - expect { push_post(project.full_path) }.to raise_error(ActionController::RoutingError) - end - end + it_behaves_like 'project path without .git suffix' do + let(:project_path) { create(:project, :repository, :public, path: 'project.git-project').full_path } end context "retrieving an info/refs file" do diff --git a/spec/services/issuable/common_system_notes_service_spec.rb b/spec/services/issuable/common_system_notes_service_spec.rb index fa5d5ebac5c..0edc9016c96 100644 --- a/spec/services/issuable/common_system_notes_service_spec.rb +++ b/spec/services/issuable/common_system_notes_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Issuable::CommonSystemNotesService do let(:user) { create(:user) } let(:project) { create(:project) } - let(:issuable) { create(:issue) } + let(:issuable) { create(:issue, project: project) } context 'on issuable update' do it_behaves_like 'system note creation', { title: 'New title' }, 'changed title' @@ -70,7 +70,7 @@ describe Issuable::CommonSystemNotesService do end context 'on issuable create' do - let(:issuable) { build(:issue) } + let(:issuable) { build(:issue, project: project) } subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) } diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 248e7d5a389..86e58fe06b9 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -8,29 +8,29 @@ describe Issues::BuildService do project.add_developer(user) end + def build_issue(issue_params = {}) + described_class.new(project, user, issue_params).execute + end + context 'for a single discussion' do describe '#execute' do let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) } let(:discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "Almost done").to_discussion } - let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) } - it 'references the noteable title in the issue title' do - issue = service.execute + subject { build_issue(merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) } - expect(issue.title).to include('Hello world') + it 'references the noteable title in the issue title' do + expect(subject.title).to include('Hello world') end it 'adds the note content to the description' do - issue = service.execute - - expect(issue.description).to include('Almost done') + expect(subject.description).to include('Almost done') end end end context 'for discussions in a merge request' do let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) } - let(:issue) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute } describe '#items_for_discussions' do it 'has an item for each discussion' do @@ -66,28 +66,30 @@ describe Issues::BuildService do end describe '#execute' do - it 'has the merge request reference in the title' do - expect(issue.title).to include(merge_request.title) - end + let(:base_params) { { merge_request_to_resolve_discussions_of: merge_request.iid } } - it 'has the reference of the merge request in the description' do - expect(issue.description).to include(merge_request.to_reference) + context 'without additional params' do + subject { build_issue(base_params) } + + it 'has the merge request reference in the title' do + expect(subject.title).to include(merge_request.title) + end + + it 'has the reference of the merge request in the description' do + expect(subject.description).to include(merge_request.to_reference) + end end - it 'does not assign title when a title was given' do - issue = described_class.new(project, user, - merge_request_to_resolve_discussions_of: merge_request, - title: 'What an issue').execute + it 'uses provided title if title param given' do + issue = build_issue(base_params.merge(title: 'What an issue')) expect(issue.title).to eq('What an issue') end - it 'does not assign description when a description was given' do - issue = described_class.new(project, user, - merge_request_to_resolve_discussions_of: merge_request, - description: 'Fix at your earliest conveignance').execute + it 'uses provided description if description param given' do + issue = build_issue(base_params.merge(description: 'Fix at your earliest convenience')) - expect(issue.description).to eq('Fix at your earliest conveignance') + expect(issue.description).to eq('Fix at your earliest convenience') end describe 'with multiple discussions' do @@ -96,20 +98,20 @@ describe Issues::BuildService do it 'mentions all the authors in the description' do authors = merge_request.resolvable_discussions.map(&:author) - expect(issue.description).to include(*authors.map(&:to_reference)) + expect(build_issue(base_params).description).to include(*authors.map(&:to_reference)) end it 'has a link for each unresolved discussion in the description' do notes = merge_request.resolvable_discussions.map(&:first_note) links = notes.map { |note| Gitlab::UrlBuilder.build(note) } - expect(issue.description).to include(*links) + expect(build_issue(base_params).description).to include(*links) end it 'mentions additional notes' do create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, in_reply_to: diff_note) - expect(issue.description).to include('(+2 comments)') + expect(build_issue(base_params).description).to include('(+2 comments)') end end end @@ -120,7 +122,7 @@ describe Issues::BuildService do describe '#execute' do it 'mentions the merge request in the description' do - issue = described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute + issue = build_issue(merge_request_to_resolve_discussions_of: merge_request.iid) expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}") end @@ -128,20 +130,18 @@ describe Issues::BuildService do end describe '#execute' do - let(:milestone) { create(:milestone, project: project) } - it 'builds a new issues with given params' do - issue = described_class.new( - project, - user, - title: 'Issue #1', - description: 'Issue description', - milestone_id: milestone.id - ).execute - - expect(issue.title).to eq('Issue #1') - expect(issue.description).to eq('Issue description') + milestone = create(:milestone, project: project) + issue = build_issue(milestone_id: milestone.id) + expect(issue.milestone).to eq(milestone) end + + it 'sets milestone to nil if it is not available for the project' do + milestone = create(:milestone, project: create(:project)) + issue = build_issue(milestone_id: milestone.id) + + expect(issue.milestone).to be_nil + end end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 931e47d3a77..f1684209729 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -356,7 +356,7 @@ describe Issues::UpdateService, :mailer do it_behaves_like 'system notes for milestones' it 'sends notifications for subscribers of changed milestone' do - issue.milestone = create(:milestone) + issue.milestone = create(:milestone, project: project) issue.save @@ -380,7 +380,7 @@ describe Issues::UpdateService, :mailer do end it 'marks todos as done' do - update_issue(milestone: create(:milestone)) + update_issue(milestone: create(:milestone, project: project)) expect(todo.reload.done?).to eq true end @@ -389,7 +389,7 @@ describe Issues::UpdateService, :mailer do it 'sends notifications for subscribers of changed milestone' do perform_enqueued_jobs do - update_issue(milestone: create(:milestone)) + update_issue(milestone: create(:milestone, project: project)) end should_email(subscriber) diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 536d0d345a4..057e8137a4e 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -229,6 +229,15 @@ describe MergeRequests::BuildService do end end end + + context 'when a milestone is from another project' do + let(:milestone) { create(:milestone, project: create(:project)) } + let(:milestone_id) { milestone.id } + + it 'sets milestone to nil' do + expect(merge_request.milestone).to be_nil + end + end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 20580bf14b9..8e367db031c 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -328,7 +328,7 @@ describe MergeRequests::UpdateService, :mailer do it_behaves_like 'system notes for milestones' it 'sends notifications for subscribers of changed milestone' do - merge_request.milestone = create(:milestone) + merge_request.milestone = create(:milestone, project: project) merge_request.save @@ -352,7 +352,7 @@ describe MergeRequests::UpdateService, :mailer do end it 'marks pending todos as done' do - update_merge_request({ milestone: create(:milestone) }) + update_merge_request({ milestone: create(:milestone, project: project) }) expect(pending_todo.reload).to be_done end @@ -361,7 +361,7 @@ describe MergeRequests::UpdateService, :mailer do it 'sends notifications for subscribers of changed milestone' do perform_enqueued_jobs do - update_merge_request(milestone: create(:milestone)) + update_merge_request(milestone: create(:milestone, project: project)) end should_email(subscriber) diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb index ffb270d277e..68fd82b4cbe 100644 --- a/spec/services/projects/group_links/create_service_spec.rb +++ b/spec/services/projects/group_links/create_service_spec.rb @@ -12,6 +12,10 @@ describe Projects::GroupLinks::CreateService, '#execute' do end let(:subject) { described_class.new(project, user, opts) } + before do + group.add_developer(user) + end + it 'adds group to project' do expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1) end @@ -19,4 +23,8 @@ describe Projects::GroupLinks::CreateService, '#execute' do it 'returns false if group is blank' do expect { subject.execute(nil) }.not_to change { project.project_group_links.count } end + + it 'returns error if user is not allowed to share with a group' do + expect { subject.execute(create :group) }.not_to change { project.project_group_links.count } + end end diff --git a/spec/support/helpers/file_mover_helpers.rb b/spec/support/helpers/file_mover_helpers.rb new file mode 100644 index 00000000000..1ba7cc03354 --- /dev/null +++ b/spec/support/helpers/file_mover_helpers.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module FileMoverHelpers + def stub_file_mover(file_path, stub_real_path: nil) + file_name = File.basename(file_path) + allow(Pathname).to receive(:new).and_call_original + + expect_next_instance_of(Pathname, a_string_including(file_name)) do |pathname| + allow(pathname).to receive(:realpath) { stub_real_path || pathname.cleanpath } + end + end +end diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index 3fee6872498..4a0cf62a661 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -47,7 +47,7 @@ module LoginHelpers end def gitlab_sign_in_via(provider, user, uid, saml_response = nil) - mock_auth_hash(provider, uid, user.email, saml_response) + mock_auth_hash_with_saml_xml(provider, uid, user.email, saml_response) visit new_user_session_path click_link provider end @@ -87,7 +87,12 @@ module LoginHelpers click_link "oauth-login-#{provider}" end - def mock_auth_hash(provider, uid, email, saml_response = nil) + def mock_auth_hash_with_saml_xml(provider, uid, email, saml_response) + response_object = { document: saml_xml(saml_response) } + mock_auth_hash(provider, uid, email, response_object: response_object) + end + + def mock_auth_hash(provider, uid, email, response_object: nil) # The mock_auth configuration allows you to set per-provider (or default) # authentication hashes to return during integration testing. OmniAuth.config.mock_auth[provider.to_sym] = OmniAuth::AuthHash.new({ @@ -110,9 +115,7 @@ module LoginHelpers image: 'mock_user_thumbnail_url' } }, - response_object: { - document: saml_xml(saml_response) - } + response_object: response_object } }) Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym] diff --git a/spec/support/shared_examples/issuable_shared_examples.rb b/spec/support/shared_examples/issuable_shared_examples.rb index c3d40c5b231..d97b21f71cd 100644 --- a/spec/support/shared_examples/issuable_shared_examples.rb +++ b/spec/support/shared_examples/issuable_shared_examples.rb @@ -31,7 +31,7 @@ shared_examples 'system notes for milestones' do context 'project milestones' do it 'creates a system note' do expect do - update_issuable(milestone: create(:milestone)) + update_issuable(milestone: create(:milestone, project: project)) end.to change { Note.system.count }.by(1) end end diff --git a/spec/support/shared_examples/requests/api/discussions.rb b/spec/support/shared_examples/requests/api/discussions.rb index e44da4faa5a..eff8e401bad 100644 --- a/spec/support/shared_examples/requests/api/discussions.rb +++ b/spec/support/shared_examples/requests/api/discussions.rb @@ -86,6 +86,37 @@ shared_examples 'discussions API' do |parent_type, noteable_type, id_name| expect(response).to have_gitlab_http_status(404) end end + + context 'when a project is public with private repo access' do + let!(:parent) { create(:project, :public, :repository, :repository_private, :snippets_private) } + let!(:user_without_access) { create(:user) } + + context 'when user is not a team member of private repo' do + before do + project.team.truncate + end + + context "creating a new note" do + before do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user_without_access), params: { body: 'hi!' } + end + + it 'raises 404 error' do + expect(response).to have_gitlab_http_status(404) + end + end + + context "fetching a discussion" do + before do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{note.discussion_id}", user_without_access) + end + + it 'raises 404 error' do + expect(response).to have_gitlab_http_status(404) + end + end + end + end end describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index de29d0c943f..e474a714b10 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -1,8 +1,9 @@ require 'spec_helper' describe FileMover do + include FileMoverHelpers + let(:filename) { 'banana_sample.gif' } - let(:file) { fixture_file_upload(File.join('spec', 'fixtures', filename)) } let(:temp_file_path) { File.join('uploads/-/system/temp', 'secret55', filename) } let(:temp_description) do @@ -12,7 +13,7 @@ describe FileMover do let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, 'secret55', filename) } let(:snippet) { create(:personal_snippet, description: temp_description) } - subject { described_class.new(file_path, snippet).execute } + subject { described_class.new(temp_file_path, snippet).execute } describe '#execute' do before do @@ -20,6 +21,8 @@ describe FileMover do expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path)) allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true) allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10) + + stub_file_mover(temp_file_path) end context 'when move and field update successful' do @@ -66,4 +69,30 @@ describe FileMover do end end end + + context 'security' do + context 'when relative path is involved' do + let(:temp_file_path) { File.join('uploads/-/system/temp', '..', 'another_subdir_of_temp') } + + it 'does not trigger move if path is outside designated directory' do + stub_file_mover('uploads/-/system/another_subdir_of_temp') + expect(FileUtils).not_to receive(:move) + + subject + + expect(snippet.reload.description).to eq(temp_description) + end + end + + context 'when symlink is involved' do + it 'does not trigger move if path is outside designated directory' do + stub_file_mover(temp_file_path, stub_real_path: Pathname('/etc')) + expect(FileUtils).not_to receive(:move) + + subject + + expect(snippet.reload.description).to eq(temp_description) + end + end + end end diff --git a/spec/validators/sha_validator_spec.rb b/spec/validators/sha_validator_spec.rb new file mode 100644 index 00000000000..b9242ef931e --- /dev/null +++ b/spec/validators/sha_validator_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe ShaValidator do + let(:validator) { described_class.new(attributes: [:base_commit_sha]) } + let(:merge_diff) { build(:merge_request_diff) } + + subject { validator.validate_each(merge_diff, :base_commit_sha, value) } + + context 'with empty value' do + let(:value) { nil } + + it 'does not add any error if value is empty' do + subject + + expect(merge_diff.errors).to be_empty + end + end + + context 'with valid sha' do + let(:value) { Digest::SHA1.hexdigest(SecureRandom.hex) } + + it 'does not add any error if value is empty' do + subject + + expect(merge_diff.errors).to be_empty + end + end + + context 'with invalid sha' do + let(:value) { 'foo' } + + it 'adds error to the record' do + expect(merge_diff.errors).to be_empty + + subject + + expect(merge_diff.errors).not_to be_empty + end + end +end diff --git a/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb b/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb index a13a3046f55..d20d926f5a0 100644 --- a/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb +++ b/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb @@ -18,7 +18,7 @@ describe UpdateHeadPipelineForMergeRequestWorker do context 'when merge request sha does not equal pipeline sha' do before do - merge_request.merge_request_diff.update(head_commit_sha: 'different_sha') + merge_request.merge_request_diff.update(head_commit_sha: Digest::SHA1.hexdigest(SecureRandom.hex)) end it 'does not update head pipeline' do |