Include this:
') + end + end + context 'with path to non-existing file' do let(:include_path) { 'not-exists.adoc' } diff --git a/spec/lib/gitlab/git/cross_repo_comparer_spec.rb b/spec/lib/gitlab/git/cross_repo_comparer_spec.rb new file mode 100644 index 0000000000..8b37b6d166 --- /dev/null +++ b/spec/lib/gitlab/git/cross_repo_comparer_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Git::CrossRepoComparer do + let(:source_project) { create(:project, :repository) } + let(:target_project) { create(:project, :repository) } + + let(:source_repo) { source_project.repository.raw_repository } + let(:target_repo) { target_project.repository.raw_repository } + + let(:source_branch) { 'feature' } + let(:target_branch) { 'master' } + let(:straight) { false } + + let(:source_commit) { source_repo.commit(source_branch) } + let(:target_commit) { source_repo.commit(target_branch) } + + subject(:result) { described_class.new(source_repo, target_repo).compare(source_branch, target_branch, straight: straight) } + + describe '#compare' do + context 'within a single repository' do + let(:target_project) { source_project } + + context 'a non-straight comparison' do + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + + expect_compare(result, from: source_commit, to: target_commit) + expect(result.straight).to eq(false) + end + end + + context 'a straight comparison' do + let(:straight) { true } + + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + + expect_compare(result, from: source_commit, to: target_commit) + expect(result.straight).to eq(true) + end + end + end + + context 'across two repositories' do + context 'target ref exists in source repo' do + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + expect_compare(result, from: source_commit, to: target_commit) + end + end + + context 'target ref does not exist in source repo' do + it 'compares in the source repo by fetching from the target to a temporary ref' do + new_commit_id = create_commit(target_project.owner, target_repo, target_branch) + new_commit = target_repo.commit(new_commit_id) + + # This is how the temporary ref is generated + expect(SecureRandom).to receive(:hex).at_least(:once).and_return('foo') + + expect(source_repo) + .to receive(:fetch_source_branch!) + .with(target_repo, new_commit_id, 'refs/tmp/foo') + .and_call_original + + expect(source_repo).to receive(:delete_refs).with('refs/tmp/foo').and_call_original + + expect_compare(result, from: source_commit, to: new_commit) + end + end + + context 'source ref does not exist in source repo' do + let(:source_branch) { 'does-not-exist' } + + it 'returns an empty comparison' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + expect(result).to be_a(::Gitlab::Git::Compare) + expect(result.commits.size).to eq(0) + end + end + + context 'target ref does not exist in target repo' do + let(:target_branch) { 'does-not-exist' } + + it 'returns nil' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + is_expected.to be_nil + end + end + end + end + + def expect_compare(of, from:, to:) + expect(of).to be_a(::Gitlab::Git::Compare) + expect(from).to be_a(::Gitlab::Git::Commit) + expect(to).to be_a(::Gitlab::Git::Commit) + + expect(of.commits).not_to be_empty + expect(of.head).to eq(from) + expect(of.base).to eq(to) + end + + def create_commit(user, repo, branch) + action = { action: :create, file_path: '/FILE', content: 'content' } + + result = repo.multi_action(user, branch_name: branch, message: 'Commit', actions: [action]) + + result.newrev + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 6854d514dc..07fef20369 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1962,66 +1962,15 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#compare_source_branch' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') } + it 'delegates to Gitlab::Git::CrossRepoComparer' do + expect_next_instance_of(::Gitlab::Git::CrossRepoComparer) do |instance| + expect(instance.source_repo).to eq(:source_repository) + expect(instance.target_repo).to eq(repository) - context 'within same repository' do - it 'does not create a temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', repository, 'feature', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 + expect(instance).to receive(:compare).with('feature', 'master', straight: :straight) end - it 'returns empty commits when source ref does not exist' do - compare = repository.compare_source_branch('master', repository, 'non-existent-branch', straight: false) - - expect(compare.commits).to be_empty - end - end - - context 'with different repositories' do - context 'when ref is known by source repo, but not by target' do - before do - mutable_repository.write_ref('another-branch', 'feature') - end - - it 'creates temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 - end - end - - context 'when ref is known by source and target repos' do - before do - mutable_repository.write_ref('another-branch', 'feature') - repository.write_ref('another-branch', 'feature') - end - - it 'does not create a temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 - end - end - - context 'when ref is unknown by source repo' do - it 'returns nil when source ref does not exist' do - expect(repository).to receive(:fetch_source_branch!).and_call_original - expect(repository).to receive(:delete_refs).and_call_original - - compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false) - expect(compare).to be_nil - end - end + repository.compare_source_branch('master', :source_repository, 'feature', straight: :straight) end end diff --git a/spec/lib/gitlab/no_cache_headers_spec.rb b/spec/lib/gitlab/no_cache_headers_spec.rb new file mode 100644 index 0000000000..f011b55006 --- /dev/null +++ b/spec/lib/gitlab/no_cache_headers_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::NoCacheHeaders do + class NoCacheTester + include Gitlab::NoCacheHeaders + end + + describe "#no_cache_headers" do + subject { NoCacheTester.new } + + it "raises a RuntimeError" do + expect { subject.no_cache_headers }.to raise_error(RuntimeError) + end + end +end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 6bc9b6365d..0faaaa5062 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ReferenceExtractor do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } before do project.add_developer(project.creator) @@ -293,4 +293,34 @@ describe Gitlab::ReferenceExtractor do end end end + + describe '#references' do + let_it_be(:user) { create(:user) } + let_it_be(:issue) { create(:issue, project: project) } + let(:text) { "Ref. #{issue.to_reference}" } + + subject { described_class.new(project, user) } + + before do + subject.analyze(text) + end + + context 'when references are visible' do + before do + project.add_developer(user) + end + + it 'returns visible references of given type' do + expect(subject.references(:issue)).to eq([issue]) + end + + it 'does not increase stateful_not_visible_counter' do + expect { subject.references(:issue) }.not_to change { subject.stateful_not_visible_counter } + end + end + + it 'increases stateful_not_visible_counter' do + expect { subject.references(:issue) }.to change { subject.stateful_not_visible_counter }.by(1) + end + end end diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index c851810ffb..c8ed898122 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -19,6 +19,74 @@ describe GenericCommitStatus do it { is_expected.not_to allow_value('javascript:alert(1)').for(:target_url) } end + describe '#name_uniqueness_across_types' do + let(:attributes) { {} } + let(:commit_status) { described_class.new(attributes) } + let(:status_name) { 'test-job' } + + subject(:errors) { commit_status.errors[:name] } + + shared_examples 'it does not have uniqueness errors' do + it 'does not return errors' do + commit_status.valid? + + is_expected.to be_empty + end + end + + context 'without attributes' do + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with only a pipeline' do + let(:attributes) { { pipeline: pipeline } } + + context 'without name' do + it_behaves_like 'it does not have uniqueness errors' + end + end + + context 'with only a name' do + let(:attributes) { { name: status_name } } + + context 'without pipeline' do + it_behaves_like 'it does not have uniqueness errors' + end + end + + context 'with pipeline and name' do + let(:attributes) do + { + pipeline: pipeline, + name: status_name + } + end + + context 'without other statuses' do + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with generic statuses' do + before do + create(:generic_commit_status, pipeline: pipeline, name: status_name) + end + + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with ci_build statuses' do + before do + create(:ci_build, pipeline: pipeline, name: status_name) + end + + it 'returns name error' do + expect(commit_status).to be_invalid + is_expected.to include('has already been taken') + end + end + end + end + describe '#context' do subject { generic_commit_status.context } @@ -79,6 +147,12 @@ describe GenericCommitStatus do it { is_expected.not_to be_nil } end + + describe '#stage_idx' do + subject { generic_commit_status.stage_idx } + + it { is_expected.not_to be_nil } + end end describe '#present' do diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb index 615865e17b..662e8b1dd6 100644 --- a/spec/models/grafana_integration_spec.rb +++ b/spec/models/grafana_integration_spec.rb @@ -9,7 +9,7 @@ describe GrafanaIntegration do describe 'validations' do it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:token) } + it { is_expected.to validate_presence_of(:encrypted_token) } it 'disallows invalid urls for grafana_url' do unsafe_url = %{https://replaceme.com/'>} @@ -66,4 +66,24 @@ describe GrafanaIntegration do end end end + + describe 'attribute encryption' do + subject(:grafana_integration) { create(:grafana_integration, token: 'super-secret') } + + context 'token' do + it 'encrypts original value into encrypted_token attribute' do + expect(grafana_integration.encrypted_token).not_to be_nil + end + + it 'locks access to raw value in private method', :aggregate_failures do + expect { grafana_integration.token }.to raise_error(NoMethodError, /private method .token. called/) + expect(grafana_integration.send(:token)).to eql('super-secret') + end + + it 'prevents overriding token value with its encrypted or masked version', :aggregate_failures do + expect { grafana_integration.update(token: grafana_integration.encrypted_token) }.not_to change { grafana_integration.reload.send(:token) } + expect { grafana_integration.update(token: grafana_integration.masked_token) }.not_to change { grafana_integration.reload.send(:token) } + end + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index a6d9ecaa7c..2fa3f426da 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -350,12 +350,12 @@ describe Note do end describe "cross_reference_not_visible_for?" do - let(:private_user) { create(:user) } - let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } } - let(:private_issue) { create(:issue, project: private_project) } + let_it_be(:private_user) { create(:user) } + let_it_be(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } } + let_it_be(:private_issue) { create(:issue, project: private_project) } - let(:ext_proj) { create(:project, :public) } - let(:ext_issue) { create(:issue, project: ext_proj) } + let_it_be(:ext_proj) { create(:project, :public) } + let_it_be(:ext_issue) { create(:issue, project: ext_proj) } shared_examples "checks references" do it "returns true" do @@ -393,10 +393,24 @@ describe Note do it_behaves_like "checks references" end - context "when there are two references in note" do + context "when there is a reference to a label" do + let_it_be(:private_label) { create(:label, project: private_project) } let(:note) do create :note, noteable: ext_issue, project: ext_proj, + note: "added label #{private_label.to_reference(ext_proj)}", + system: true + end + let!(:system_note_metadata) { create(:system_note_metadata, note: note, action: :label) } + + it_behaves_like "checks references" + end + + context "when there are two references in note" do + let_it_be(:ext_issue2) { create(:issue, project: ext_proj) } + let(:note) do + create :note, + noteable: ext_issue2, project: ext_proj, note: "mentioned in issue #{private_issue.to_reference(ext_proj)} and " \ "public issue #{ext_issue.to_reference(ext_proj)}", system: true diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index f715ecae34..d227c01869 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -141,6 +141,34 @@ describe GlobalPolicy do it { is_expected.to be_allowed(:access_api) } end end + + context 'inactive user' do + before do + current_user.update!(confirmed_at: nil, confirmation_sent_at: 5.days.ago) + end + + context 'when within the confirmation grace period' do + before do + allow(User).to receive(:allow_unconfirmed_access_for).and_return(10.days) + end + + it { is_expected.to be_allowed(:access_api) } + end + + context 'when confirmation grace period is expired' do + before do + allow(User).to receive(:allow_unconfirmed_access_for).and_return(2.days) + end + + it { is_expected.not_to be_allowed(:access_api) } + end + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:access_api) + end + end end describe 'receive notifications' do @@ -202,6 +230,20 @@ describe GlobalPolicy do it { is_expected.not_to be_allowed(:access_git) } end + describe 'inactive user' do + before do + current_user.update!(confirmed_at: nil) + end + + it { is_expected.not_to be_allowed(:access_git) } + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:access_git) + end + end + context 'when terms are enforced' do before do enforce_terms @@ -298,6 +340,20 @@ describe GlobalPolicy do it { is_expected.not_to be_allowed(:use_slash_commands) } end + describe 'inactive user' do + before do + current_user.update!(confirmed_at: nil) + end + + it { is_expected.not_to be_allowed(:use_slash_commands) } + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:use_slash_commands) + end + end + context 'when access locked' do before do current_user.lock_access! diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 639b8e9634..24ed836996 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -164,6 +164,7 @@ describe API::CommitStatuses do expect(response).to have_gitlab_http_status(201) expect(job.status).to eq('pending') + expect(job.stage_idx).to eq(GenericCommitStatus::EXTERNAL_STAGE_IDX) end end @@ -331,6 +332,29 @@ describe API::CommitStatuses do end end + context 'when updating a protected ref' do + before do + create(:protected_branch, project: project, name: 'master') + post api(post_url, user), params: { state: 'running', ref: 'master' } + end + + context 'with user as developer' do + let(:user) { developer } + + it 'does not create commit status' do + expect(response).to have_gitlab_http_status(403) + end + end + + context 'with user as maintainer' do + let(:user) { create_user(:maintainer) } + + it 'creates commit status' do + expect(response).to have_gitlab_http_status(201) + end + end + end + context 'when commit SHA is invalid' do let(:sha) { 'invalid_sha' } @@ -372,6 +396,22 @@ describe API::CommitStatuses do .to include 'is blocked: Only allowed schemes are http, https' end end + + context 'when trying to update a status of a different type' do + let!(:pipeline) { create(:ci_pipeline, project: project, sha: sha, ref: 'ref') } + let!(:ci_build) { create(:ci_build, pipeline: pipeline, name: 'test-job') } + let(:params) { { state: 'pending', name: 'test-job' } } + + before do + post api(post_url, developer), params: params + end + + it 'responds with bad request status and validation errors' do + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['name']) + .to include 'has already been taken' + end + end end context 'reporter user' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index d8da1c001b..e390f3945a 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -8,6 +8,7 @@ describe API::Commits do let(:user) { create(:user) } let(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let(:developer) { create(:user).tap { |u| project.add_developer(u) } } let(:project) { create(:project, :repository, creator: user, path: 'my.project') } let(:branch_with_dot) { project.repository.find_branch('ends-with.json') } let(:branch_with_slash) { project.repository.find_branch('improve/awesome') } @@ -964,6 +965,56 @@ describe API::Commits do end end + shared_examples_for 'ref with pipeline' do + let!(:pipeline) do + project + .ci_pipelines + .create!(source: :push, ref: 'master', sha: commit.sha, protected: false) + end + + it 'includes status as "created" and a last_pipeline object' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(json_response['status']).to eq('created') + expect(json_response['last_pipeline']['id']).to eq(pipeline.id) + expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref) + expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha) + expect(json_response['last_pipeline']['status']).to eq(pipeline.status) + end + + context 'when pipeline succeeds' do + before do + pipeline.update!(status: 'success') + end + + it 'includes a "success" status' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(json_response['status']).to eq('success') + end + end + end + + shared_examples_for 'ref with unaccessible pipeline' do + let!(:pipeline) do + project + .ci_pipelines + .create!(source: :push, ref: 'master', sha: commit.sha, protected: false) + end + + it 'does not include last_pipeline' do + get api(route, current_user) + + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(response).to have_gitlab_http_status(200) + expect(json_response['last_pipeline']).to be_nil + end + end + context 'when stat param' do let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}" } @@ -993,6 +1044,15 @@ describe API::Commits do let(:project) { create(:project, :public, :repository) } it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' + + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + end + + it_behaves_like 'ref with unaccessible pipeline' + end end context 'when unauthenticated', 'and project is private' do @@ -1006,6 +1066,17 @@ describe API::Commits do let(:current_user) { user } it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' + + context 'when builds are disabled' do + before do + project + .project_feature + .update!(builds_access_level: ProjectFeature::DISABLED) + end + + it_behaves_like 'ref with unaccessible pipeline' + end context 'when branch contains a dot' do let(:commit) { project.repository.commit(branch_with_dot.name) } @@ -1041,35 +1112,53 @@ describe API::Commits do it_behaves_like 'ref commit' end end + end - context 'when the ref has a pipeline' do - let!(:pipeline) { project.ci_pipelines.create(source: :push, ref: 'master', sha: commit.sha, protected: false) } + context 'when authenticated', 'as a developer' do + let(:current_user) { developer } - it 'includes a "created" status' do - get api(route, current_user) + it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' - expect(response).to have_gitlab_http_status(200) - expect(response).to match_response_schema('public_api/v4/commit/detail') - expect(json_response['status']).to eq('created') - expect(json_response['last_pipeline']['id']).to eq(pipeline.id) - expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref) - expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha) - expect(json_response['last_pipeline']['status']).to eq(pipeline.status) + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) end - context 'when pipeline succeeds' do - before do - pipeline.update(status: 'success') - end + it_behaves_like 'ref with pipeline' + end + end - it 'includes a "success" status' do - get api(route, current_user) + context 'when authenticated', 'as a guest' do + let(:current_user) { guest } - expect(response).to have_gitlab_http_status(200) - expect(response).to match_response_schema('public_api/v4/commit/detail') - expect(json_response['status']).to eq('success') - end + it_behaves_like '403 response' do + let(:request) { get api(route, guest) } + let(:message) { '403 Forbidden' } + end + end + + context 'when authenticated', 'as a non member' do + let(:current_user) { create(:user) } + + it_behaves_like '403 response' do + let(:request) { get api(route, guest) } + let(:message) { '403 Forbidden' } + end + end + + context 'when authenticated', 'as non_member and project is public' do + let(:current_user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'ref with pipeline' + + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) end + + it_behaves_like 'ref with unaccessible pipeline' end end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index ab915af8ab..efad443de3 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -447,6 +447,18 @@ describe API::Files do expect(response).to have_gitlab_http_status(200) end + it 'sets no-cache headers' do + url = route('.gitignore') + "/raw" + expect(Gitlab::Workhorse).to receive(:send_git_blob) + + get api(url, current_user), params: params + + expect(response.headers["Cache-Control"]).to include("no-store") + expect(response.headers["Cache-Control"]).to include("no-cache") + expect(response.headers["Pragma"]).to eq("no-cache") + expect(response.headers["Expires"]).to eq("Fri, 01 Jan 1990 00:00:00 GMT") + end + context 'when mandatory params are not given' do it_behaves_like '400 response' do let(:request) { get api(route("any%2Ffile"), current_user) } diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index 8d7b3fa3c0..ce03756a19 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -30,26 +30,40 @@ describe 'OAuth tokens' do end end - context "when user is blocked" do - it "does not create an access token" do - user = create(:user) + shared_examples 'does not create an access token' do + let(:user) { create(:user) } + + it { expect(response).to have_gitlab_http_status(401) } + end + + context 'when user is blocked' do + before do user.block request_oauth_token(user) - - expect(response).to have_gitlab_http_status(401) end + + include_examples 'does not create an access token' end - context "when user is ldap_blocked" do - it "does not create an access token" do - user = create(:user) + context 'when user is ldap_blocked' do + before do user.ldap_block request_oauth_token(user) - - expect(response).to have_gitlab_http_status(401) end + + include_examples 'does not create an access token' + end + + context 'when user account is not confirmed' do + before do + user.update!(confirmed_at: nil) + + request_oauth_token(user) + end + + include_examples 'does not create an access token' end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index cc6cadb190..8f82bd405d 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1509,7 +1509,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do authorize_artifacts - expect(response).to have_gitlab_http_status(500) + expect(response).to have_gitlab_http_status(:forbidden) end context 'authorization token is invalid' do @@ -1639,6 +1639,18 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + context 'Is missing GitLab Workhorse token headers' do + let(:jwt_token) { JWT.encode({ 'iss' => 'invalid-header' }, Gitlab::Workhorse.secret, 'HS256') } + + it 'fails to post artifacts without GitLab-Workhorse' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once + + upload_artifacts(file_upload, headers_with_token) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + context 'when setting an expire date' do let(:default_artifacts_expire_in) {} let(:post_data) do diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb index d78ab78c3d..0fd1fcfe1a 100644 --- a/spec/services/projects/group_links/destroy_service_spec.rb +++ b/spec/services/projects/group_links/destroy_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Projects::GroupLinks::DestroyService, '#execute' do - let(:group_link) { create :project_group_link } - let(:project) { group_link.project } + let(:project) { create(:project, :private) } + let!(:group_link) { create(:project_group_link, project: project) } let(:user) { create :user } let(:subject) { described_class.new(project, user) } @@ -15,4 +15,39 @@ describe Projects::GroupLinks::DestroyService, '#execute' do it 'returns false if group_link is blank' do expect { subject.execute(nil) }.not_to change { project.project_group_links.count } end + + describe 'todos cleanup' do + context 'when project is private' do + it 'triggers todos cleanup' do + expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id) + expect(project.private?).to be true + + subject.execute(group_link) + end + end + + context 'when project is public or internal' do + shared_examples_for 'removes confidential todos' do + it 'does not trigger todos cleanup' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id) + expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, nil, project.id) + expect(project.private?).to be false + + subject.execute(group_link) + end + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + it_behaves_like 'removes confidential todos' + end + + context 'when project is internal' do + let(:project) { create(:project, :public) } + + it_behaves_like 'removes confidential todos' + end + end + end end diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb index a557e61da7..0069b73d31 100644 --- a/spec/services/projects/import_export/export_service_spec.rb +++ b/spec/services/projects/import_export/export_service_spec.rb @@ -10,6 +10,10 @@ describe Projects::ImportExport::ExportService do let(:service) { described_class.new(project, user) } let!(:after_export_strategy) { Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy.new } + before do + project.add_maintainer(user) + end + it 'saves the version' do expect(Gitlab::ImportExport::VersionSaver).to receive(:new).and_call_original @@ -133,5 +137,18 @@ describe Projects::ImportExport::ExportService do expect(service).not_to receive(:execute_after_export_action) end end + + context 'when user does not have admin_project permission' do + let!(:another_user) { create(:user) } + + subject(:service) { described_class.new(project, another_user) } + + it 'fails' do + expected_message = + "User with ID: %s does not have permission to Project %s with ID: %s." % + [another_user.id, project.name, project.id] + expect { service.execute }.to raise_error(Gitlab::ImportExport::Error).with_message(expected_message) + end + end end end diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 81d59a98b9..e258c379bf 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -210,7 +210,7 @@ describe Projects::Operations::UpdateService do integration = project.reload.grafana_integration expect(integration.grafana_url).to eq(expected_attrs[:grafana_url]) - expect(integration.token).to eq(expected_attrs[:token]) + expect(integration.send(:token)).to eq(expected_attrs[:token]) end end @@ -226,7 +226,7 @@ describe Projects::Operations::UpdateService do integration = project.reload.grafana_integration expect(integration.grafana_url).to eq(expected_attrs[:grafana_url]) - expect(integration.token).to eq(expected_attrs[:token]) + expect(integration.send(:token)).to eq(expected_attrs[:token]) end context 'with all grafana attributes blank in params' do diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index fe92b53cd9..9597059034 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -5,7 +5,7 @@ require "spec_helper" describe Projects::UpdatePagesService do set(:project) { create(:project, :repository) } set(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) } - set(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } + let(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } let(:invalid_file) { fixture_file_upload('spec/fixtures/dk.png') } let(:file) { fixture_file_upload("spec/fixtures/pages.zip") } @@ -242,6 +242,32 @@ describe Projects::UpdatePagesService do end end + context 'when file size is spoofed' do + let(:metadata) { spy('metadata') } + + include_context 'pages zip with spoofed size' + + before do + file = fixture_file_upload(fake_zip_path, 'pages.zip') + metafile = fixture_file_upload('spec/fixtures/pages.zip.meta') + + create(:ci_job_artifact, :archive, file: file, job: build) + create(:ci_job_artifact, :metadata, file: metafile, job: build) + + allow(build).to receive(:artifacts_metadata_entry) + .and_return(metadata) + allow(metadata).to receive(:total_size).and_return(100) + end + + it 'raises an error' do + expect do + subject.execute + end.to raise_error(Projects::UpdatePagesService::FailedToExtractError, + 'Entry public/index.html should be 1B but is larger when inflated') + expect(deploy_status).to be_script_failure + end + end + def deploy_status GenericCommitStatus.find_by(name: 'pages:deploy') end diff --git a/spec/support/helpers/reference_parser_helpers.rb b/spec/support/helpers/reference_parser_helpers.rb index f96a01d15b..9084265b58 100644 --- a/spec/support/helpers/reference_parser_helpers.rb +++ b/spec/support/helpers/reference_parser_helpers.rb @@ -5,6 +5,11 @@ module ReferenceParserHelpers Nokogiri::HTML.fragment('').children[0] end + def expect_gathered_references(result, visible, not_visible_count) + expect(result[:visible]).to eq(visible) + expect(result[:not_visible].count).to eq(not_visible_count) + end + shared_examples 'no project N+1 queries' do it 'avoids N+1 queries in #nodes_visible_to_user', :request_store do context = Banzai::RenderContext.new(project, user) diff --git a/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb b/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb new file mode 100644 index 0000000000..4cec5ab3b7 --- /dev/null +++ b/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +# the idea of creating zip archive with spoofed size is borrowed from +# https://github.com/rubyzip/rubyzip/pull/403/files#diff-118213fb4baa6404a40f89e1147661ebR88 +RSpec.shared_context 'pages zip with spoofed size' do + let(:real_zip_path) { Tempfile.new(['real', '.zip']).path } + let(:fake_zip_path) { Tempfile.new(['fake', '.zip']).path } + + before do + full_file_name = 'public/index.html' + true_size = 500_000 + fake_size = 1 + + ::Zip::File.open(real_zip_path, ::Zip::File::CREATE) do |zf| + zf.get_output_stream(full_file_name) do |os| + os.write 'a' * true_size + end + end + + compressed_size = nil + ::Zip::File.open(real_zip_path) do |zf| + a_entry = zf.find_entry(full_file_name) + compressed_size = a_entry.compressed_size + end + + true_size_bytes = [compressed_size, true_size, full_file_name.size].pack('LLS') + fake_size_bytes = [compressed_size, fake_size, full_file_name.size].pack('LLS') + + data = File.binread(real_zip_path) + data.gsub! true_size_bytes, fake_size_bytes + + File.open(fake_zip_path, 'wb') do |file| + file.write data + end + end + + after do + File.delete(real_zip_path) if File.exist?(real_zip_path) + File.delete(fake_zip_path) if File.exist?(fake_zip_path) + end +end