diff options
Diffstat (limited to 'spec')
42 files changed, 1005 insertions, 594 deletions
diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 5f0e8c5eca9..b23f183fec8 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -1,127 +1,162 @@ require 'spec_helper' -describe OmniauthCallbacksController do +describe OmniauthCallbacksController, type: :controller do include LoginHelpers - let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) } - - before do - mock_auth_hash(provider.to_s, extern_uid, user.email) - stub_omniauth_provider(provider, context: request) - end - - context 'when the user is on the last sign in attempt' do - let(:extern_uid) { 'my-uid' } + describe 'omniauth' do + let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) } before do - user.update(failed_attempts: User.maximum_attempts.pred) - subject.response = ActionDispatch::Response.new + mock_auth_hash(provider.to_s, extern_uid, user.email) + stub_omniauth_provider(provider, context: request) end - context 'when using a form based provider' do - let(:provider) { :ldap } - - it 'locks the user when sign in fails' do - allow(subject).to receive(:params).and_return(ActionController::Parameters.new(username: user.username)) - request.env['omniauth.error.strategy'] = OmniAuth::Strategies::LDAP.new(nil) - - subject.send(:failure) + context 'when the user is on the last sign in attempt' do + let(:extern_uid) { 'my-uid' } - expect(user.reload).to be_access_locked + before do + user.update(failed_attempts: User.maximum_attempts.pred) + subject.response = ActionDispatch::Response.new end - end - context 'when using a button based provider' do - let(:provider) { :github } + context 'when using a form based provider' do + let(:provider) { :ldap } - it 'does not lock the user when sign in fails' do - request.env['omniauth.error.strategy'] = OmniAuth::Strategies::GitHub.new(nil) + it 'locks the user when sign in fails' do + allow(subject).to receive(:params).and_return(ActionController::Parameters.new(username: user.username)) + request.env['omniauth.error.strategy'] = OmniAuth::Strategies::LDAP.new(nil) - subject.send(:failure) + subject.send(:failure) - expect(user.reload).not_to be_access_locked + expect(user.reload).to be_access_locked + end end - end - end - context 'strategies' do - context 'github' do - let(:extern_uid) { 'my-uid' } - let(:provider) { :github } + context 'when using a button based provider' do + let(:provider) { :github } - it 'allows sign in' do - post provider + it 'does not lock the user when sign in fails' do + request.env['omniauth.error.strategy'] = OmniAuth::Strategies::GitHub.new(nil) - expect(request.env['warden']).to be_authenticated - end - - shared_context 'sign_up' do - let(:user) { double(email: 'new@example.com') } + subject.send(:failure) - before do - stub_omniauth_setting(block_auto_created_users: false) + expect(user.reload).not_to be_access_locked end end + end - context 'sign up' do - include_context 'sign_up' + context 'strategies' do + context 'github' do + let(:extern_uid) { 'my-uid' } + let(:provider) { :github } - it 'is allowed' do + it 'allows sign in' do post provider expect(request.env['warden']).to be_authenticated end - end - - context 'when OAuth is disabled' do - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - settings = Gitlab::CurrentSettings.current_application_settings - settings.update(disabled_oauth_sign_in_sources: [provider.to_s]) - end - it 'prevents login via POST' do - post provider + shared_context 'sign_up' do + let(:user) { double(email: 'new@example.com') } - expect(request.env['warden']).not_to be_authenticated + before do + stub_omniauth_setting(block_auto_created_users: false) + end end - it 'shows warning when attempting login' do - post provider - - expect(response).to redirect_to new_user_session_path - expect(flash[:alert]).to eq('Signing in using GitHub has been disabled') - end + context 'sign up' do + include_context 'sign_up' - it 'allows linking the disabled provider' do - user.identities.destroy_all - sign_in(user) + it 'is allowed' do + post provider - expect { post provider }.to change { user.reload.identities.count }.by(1) + expect(request.env['warden']).to be_authenticated + end end - context 'sign up' do - include_context 'sign_up' + context 'when OAuth is disabled' do + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + settings = Gitlab::CurrentSettings.current_application_settings + settings.update(disabled_oauth_sign_in_sources: [provider.to_s]) + end - it 'is prevented' do + it 'prevents login via POST' do post provider expect(request.env['warden']).not_to be_authenticated end + + it 'shows warning when attempting login' do + post provider + + expect(response).to redirect_to new_user_session_path + expect(flash[:alert]).to eq('Signing in using GitHub has been disabled') + end + + it 'allows linking the disabled provider' do + user.identities.destroy_all + sign_in(user) + + expect { post provider }.to change { user.reload.identities.count }.by(1) + end + + context 'sign up' do + include_context 'sign_up' + + it 'is prevented' do + post provider + + expect(request.env['warden']).not_to be_authenticated + end + end + end + end + + context 'auth0' do + let(:extern_uid) { '' } + let(:provider) { :auth0 } + + it 'does not allow sign in without extern_uid' do + post 'auth0' + + expect(request.env['warden']).not_to be_authenticated + expect(response.status).to eq(302) + expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.') end end end + end + + describe '#saml' do + let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml') } + let(:mock_saml_response) { File.read('spec/fixtures/authentication/saml_response.xml') } + let(:saml_config) { mock_saml_config_with_upstream_two_factor_authn_contexts } + + 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) + request.env["devise.mapping"] = Devise.mappings[:user] + request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth'] + post :saml, params: { SAMLResponse: mock_saml_response } + end - context 'auth0' do - let(:extern_uid) { '' } - let(:provider) { :auth0 } + context 'when worth two factors' do + let(:mock_saml_response) do + File.read('spec/fixtures/authentication/saml_response.xml') + .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN') + end - it 'does not allow sign in without extern_uid' do - post 'auth0' + it 'expects user to be signed_in' do + expect(request.env['warden']).to be_authenticated + end + end + context 'when not worth two factors' do + it 'expects user to provide second factor' do + expect(response).to render_template('devise/sessions/two_factor') expect(request.env['warden']).not_to be_authenticated - expect(response.status).to eq(302) - expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.') end end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 2b61e0d4a85..cdec26bd421 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -53,21 +53,22 @@ describe SessionsController do include UserActivitiesHelpers let(:user) { create(:user) } + let(:user_params) { { login: user.username, password: user.password } } it 'authenticates user correctly' do - post(:create, user: { login: user.username, password: user.password }) + post(:create, user: user_params) expect(subject.current_user). to eq user end it 'creates an audit log record' do - expect { post(:create, user: { login: user.username, password: user.password }) }.to change { SecurityEvent.count }.by(1) + expect { post(:create, user: user_params) }.to change { SecurityEvent.count }.by(1) expect(SecurityEvent.last.details[:with]).to eq('standard') end include_examples 'user login request with unique ip limit', 302 do def request - post(:create, user: { login: user.username, password: user.password }) + post(:create, user: user_params) expect(subject.current_user).to eq user subject.sign_out user end @@ -75,10 +76,40 @@ describe SessionsController do it 'updates the user activity' do expect do - post(:create, user: { login: user.username, password: user.password }) + post(:create, user: user_params) end.to change { user_activity(user) } end end + + context 'when reCAPTCHA is enabled' do + let(:user) { create(:user) } + let(:user_params) { { login: user.username, password: user.password } } + + before do + stub_application_setting(recaptcha_enabled: true) + request.headers[described_class::CAPTCHA_HEADER] = 1 + end + + it 'displays an error when the reCAPTCHA is not solved' do + # Without this, `verify_recaptcha` arbitraily returns true in test env + Recaptcha.configuration.skip_verify_env.delete('test') + + post(:create, user: user_params) + + expect(response).to render_template(:new) + expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' + expect(subject.current_user).to be_nil + end + + it 'successfully logs in a user when reCAPTCHA is solved' do + # Avoid test ordering issue and ensure `verify_recaptcha` returns true + Recaptcha.configuration.skip_verify_env << 'test' + + post(:create, user: user_params) + + expect(subject.current_user).to eq user + end + end end context 'when using two-factor authentication via OTP' do diff --git a/spec/dependencies/omniauth_saml_spec.rb b/spec/dependencies/omniauth_saml_spec.rb new file mode 100644 index 00000000000..ccc604dc230 --- /dev/null +++ b/spec/dependencies/omniauth_saml_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' +require 'omniauth/strategies/saml' + +describe 'processing of SAMLResponse in dependencies' do + let(:mock_saml_response) { File.read('spec/fixtures/authentication/saml_response.xml') } + let(:saml_strategy) { OmniAuth::Strategies::SAML.new({}) } + let(:session_mock) { {} } + let(:settings) { OpenStruct.new({ soft: false, idp_cert_fingerprint: 'something' }) } + let(:auth_hash) { Gitlab::Auth::Saml::AuthHash.new(saml_strategy) } + + subject { auth_hash.authn_context } + + before do + allow(saml_strategy).to receive(:session).and_return(session_mock) + allow_any_instance_of(OneLogin::RubySaml::Response).to receive(:is_valid?).and_return(true) + saml_strategy.send(:handle_response, mock_saml_response, {}, settings ) { } + end + + it 'can extract AuthnContextClassRef from SAMLResponse param' do + is_expected.to eq 'urn:oasis:names:tc:SAML:2.0:ac:classes:Password' + end +end diff --git a/spec/features/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index d5e603baeae..a4226d7a682 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -31,6 +31,7 @@ feature 'Admin Groups' do path_component = 'gitlab' group_name = 'GitLab group name' group_description = 'Description of group for GitLab' + fill_in 'group_path', with: path_component fill_in 'group_name', with: group_name fill_in 'group_description', with: group_description diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index bc42618306f..8dca81a8627 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -10,6 +10,7 @@ describe 'Filter issues', :js do # When the name is longer, the filtered search input can end up scrolling # horizontally, and PhantomJS can't handle it. let(:user) { create(:user, name: 'Ann') } + let(:user2) { create(:user, name: 'jane') } let!(:bug_label) { create(:label, project: project, title: 'bug') } let!(:caps_sensitive_label) { create(:label, project: project, title: 'CaPs') } @@ -25,8 +26,6 @@ describe 'Filter issues', :js do before do project.add_master(user) - user2 = create(:user) - create(:issue, project: project, author: user2, title: "Bug report 1") create(:issue, project: project, author: user2, title: "Bug report 2") @@ -113,6 +112,24 @@ describe 'Filter issues', :js do expect_issues_list_count(3) expect_filtered_search_input_empty end + + it 'filters issues by invalid assignee' do + skip('to be tested, issue #26546') + end + + it 'filters issues by multiple assignees' do + create(:issue, project: project, author: user, assignees: [user2, user]) + + input_filtered_search("assignee:@#{user.username} assignee:@#{user2.username}") + + expect_tokens([ + assignee_token(user.name), + assignee_token(user2.name) + ]) + + expect_issues_list_count(1) + expect_filtered_search_input_empty + end end end @@ -491,6 +508,21 @@ describe 'Filter issues', :js do it_behaves_like 'updates atom feed link', :group do let(:path) { issues_group_path(group, milestone_title: milestone.title, assignee_id: user.id) } end + + it 'updates atom feed link for group issues' do + visit issues_group_path(group, milestone_title: milestone.title, assignee_id: user.id) + link = find('.nav-controls a[title="Subscribe to RSS feed"]', visible: false) + params = CGI.parse(URI.parse(link[:href]).query) + auto_discovery_link = find('link[type="application/atom+xml"]', visible: false) + auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query) + + expect(params).to include('feed_token' => [user.feed_token]) + expect(params).to include('milestone_title' => [milestone.title]) + expect(params).to include('assignee_id' => [user.id.to_s]) + expect(auto_discovery_params).to include('feed_token' => [user.feed_token]) + expect(auto_discovery_params).to include('milestone_title' => [milestone.title]) + expect(auto_discovery_params).to include('assignee_id' => [user.id.to_s]) + end end context 'URL has a trailing slash' do diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 1f8d31a5c88..24a2c89f50b 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -177,14 +177,35 @@ feature 'Login' do end context 'logging in via OAuth' do - it 'shows 2FA prompt after OAuth login' do - stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config]) - user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml') - gitlab_sign_in_via('saml', user, 'my-uid') + let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')} + let(:mock_saml_response) do + File.read('spec/fixtures/authentication/saml_response.xml') + end - expect(page).to have_content('Two-Factor Authentication') - enter_code(user.current_otp) - expect(current_path).to eq root_path + before do + stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], + providers: [mock_saml_config_with_upstream_two_factor_authn_contexts]) + gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response) + end + + context 'when authn_context is worth two factors' do + let(:mock_saml_response) do + File.read('spec/fixtures/authentication/saml_response.xml') + .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + end + + it 'signs user in without prompting for second factor' do + expect(page).not_to have_content('Two-Factor Authentication') + expect(current_path).to eq root_path + end + end + + context 'when authn_context is not worth two factors' do + it 'shows 2FA prompt after OAuth login' do + expect(page).to have_content('Two-Factor Authentication') + enter_code(user.current_otp) + expect(current_path).to eq root_path + end end end end diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index b51ca5d130b..bfe11ddf673 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -40,6 +40,15 @@ describe 'Signup' do expect(find('.username')).to have_css '.gl-field-error-outline' end + + it 'shows an error message on submit if the username contains special characters' do + fill_in 'new_user_username', with: 'new$user!username' + wait_for_requests + + click_button "Register" + + expect(page).to have_content("Please create a username with only alphanumeric characters.") + end end context 'with no errors' do diff --git a/spec/fixtures/authentication/saml_response.xml b/spec/fixtures/authentication/saml_response.xml new file mode 100644 index 00000000000..ac7b662be22 --- /dev/null +++ b/spec/fixtures/authentication/saml_response.xml @@ -0,0 +1,42 @@ +<?xml version='1.0'?> +<samlp:Response xmlns:samlp='urn:oasis:names:tc:SAML:2.0:protocol' xmlns:saml='urn:oasis:names:tc:SAML:2.0:assertion' ID='pfxb9b71715-2202-9a51-8ae5-689d5b9dd25a' Version='2.0' IssueInstant='2014-07-17T01:01:48Z' Destination='http://sp.example.com/demo1/index.php?acs' InResponseTo='ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685'> + <saml:Issuer>http://idp.example.com/metadata.php</saml:Issuer><ds:Signature xmlns:ds='http://www.w3.org/2000/09/xmldsig#'> + <ds:SignedInfo><ds:CanonicalizationMethod Algorithm='http://www.w3.org/2001/10/xml-exc-c14n#'/> + <ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/> + <ds:Reference URI='#pfxb9b71715-2202-9a51-8ae5-689d5b9dd25a'><ds:Transforms><ds:Transform Algorithm='http://www.w3.org/2000/09/xmldsig#enveloped-signature'/><ds:Transform Algorithm='http://www.w3.org/2001/10/xml-exc-c14n#'/></ds:Transforms><ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#sha1'/><ds:DigestValue>z0Y25hsUHVJJnYhgB5LzPVjqbgM=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>NSdsZopzNX4kJETipLNbU+7dG4GPTj5e40iSBaUeUMc1UUSX4UCe9Qx6R9ADEkEQgNekgYaCFOuY90kLNh9Ky0Czq8gd4w7ykQJEVJ7VF7LakmG8dPedHAKyAMAuZ8y3mNGye31vtR9frYaznCVoxB3eAi9rbVOXkQtdOTRMHec=</ds:SignatureValue> + <ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIICajCCAdOgAwIBAgIBADANBgkqhkiG9w0BAQ0FADBSMQswCQYDVQQGEwJ1czETMBEGA1UECAwKQ2FsaWZvcm5pYTEVMBMGA1UECgwMT25lbG9naW4gSW5jMRcwFQYDVQQDDA5zcC5leGFtcGxlLmNvbTAeFw0xNDA3MTcxNDEyNTZaFw0xNTA3MTcxNDEyNTZaMFIxCzAJBgNVBAYTAnVzMRMwEQYDVQQIDApDYWxpZm9ybmlhMRUwEwYDVQQKDAxPbmVsb2dpbiBJbmMxFzAVBgNVBAMMDnNwLmV4YW1wbGUuY29tMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDZx+ON4IUoIWxgukTb1tOiX3bMYzYQiwWPUNMp+Fq82xoNogso2bykZG0yiJm5o8zv/sd6pGouayMgkx/2FSOdc36T0jGbCHuRSbtia0PEzNIRtmViMrt3AeoWBidRXmZsxCNLwgIV6dn2WpuE5Az0bHgpZnQxTKFek0BMKU/d8wIDAQABo1AwTjAdBgNVHQ4EFgQUGHxYqZYyX7cTxKVODVgZwSTdCnwwHwYDVR0jBBgwFoAUGHxYqZYyX7cTxKVODVgZwSTdCnwwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQ0FAAOBgQByFOl+hMFICbd3DJfnp2Rgd/dqttsZG/tyhILWvErbio/DEe98mXpowhTkC04ENprOyXi7ZbUqiicF89uAGyt1oqgTUCD1VsLahqIcmrzgumNyTwLGWo17WDAa1/usDhetWAMhgzF/Cnf5ek0nK00m0YZGyc4LzgD0CROMASTWNg==</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature> + <samlp:Status> + <samlp:StatusCode Value='urn:oasis:names:tc:SAML:2.0:status:Success'/> + </samlp:Status> + <saml:Assertion xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xmlns:xs='http://www.w3.org/2001/XMLSchema' ID='_d71a3a8e9fcc45c9e9d248ef7049393fc8f04e5f75' Version='2.0' IssueInstant='2014-07-17T01:01:48Z'> + <saml:Issuer>http://idp.example.com/metadata.php</saml:Issuer> + <saml:Subject> + <saml:NameID SPNameQualifier='http://sp.example.com/demo1/metadata.php' Format='urn:oasis:names:tc:SAML:2.0:nameid-format:transient'>_ce3d2948b4cf20146dee0a0b3dd6f69b6cf86f62d7</saml:NameID> + <saml:SubjectConfirmation Method='urn:oasis:names:tc:SAML:2.0:cm:bearer'> + <saml:SubjectConfirmationData NotOnOrAfter='2024-01-18T06:21:48Z' Recipient='http://sp.example.com/demo1/index.php?acs' InResponseTo='ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685'/> + </saml:SubjectConfirmation> + </saml:Subject> + <saml:Conditions NotBefore='2014-07-17T01:01:18Z' NotOnOrAfter='2024-01-18T06:21:48Z'> + <saml:AudienceRestriction> + <saml:Audience>http://sp.example.com/demo1/metadata.php</saml:Audience> + </saml:AudienceRestriction> + </saml:Conditions> + <saml:AuthnStatement AuthnInstant='2014-07-17T01:01:48Z' SessionNotOnOrAfter='2024-07-17T09:01:48Z' SessionIndex='_be9967abd904ddcae3c0eb4189adbe3f71e327cf93'> + <saml:AuthnContext> + <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:Password</saml:AuthnContextClassRef> + </saml:AuthnContext> + </saml:AuthnStatement> + <saml:AttributeStatement> + <saml:Attribute Name='uid' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'> + <saml:AttributeValue xsi:type='xs:string'>test</saml:AttributeValue> + </saml:Attribute> + <saml:Attribute Name='mail' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'> + <saml:AttributeValue xsi:type='xs:string'>test@example.com</saml:AttributeValue> + </saml:Attribute> + <saml:Attribute Name='eduPersonAffiliation' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'> + <saml:AttributeValue xsi:type='xs:string'>users</saml:AttributeValue> + <saml:AttributeValue xsi:type='xs:string'>examplerole1</saml:AttributeValue> + </saml:Attribute> + </saml:AttributeStatement> + </saml:Assertion> +</samlp:Response> diff --git a/spec/javascripts/filtered_search/filtered_search_token_keys_spec.js b/spec/javascripts/filtered_search/filtered_search_token_keys_spec.js index fbc3926d332..68158cf52e4 100644 --- a/spec/javascripts/filtered_search/filtered_search_token_keys_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_token_keys_spec.js @@ -17,6 +17,17 @@ describe('Filtered Search Token Keys', () => { }); }); + describe('getKeys', () => { + it('should return keys', () => { + const getKeys = FilteredSearchTokenKeys.getKeys(); + const keys = FilteredSearchTokenKeys.get().map(i => i.key); + + keys.forEach((key, i) => { + expect(key).toEqual(getKeys[i]); + }); + }); + }); + describe('getConditions', () => { let conditions; diff --git a/spec/javascripts/ide/components/repo_tab_spec.js b/spec/javascripts/ide/components/repo_tab_spec.js index 8cabc6e8935..fc0695a4263 100644 --- a/spec/javascripts/ide/components/repo_tab_spec.js +++ b/spec/javascripts/ide/components/repo_tab_spec.js @@ -38,6 +38,26 @@ describe('RepoTab', () => { expect(name.textContent.trim()).toEqual(vm.tab.name); }); + it('does not call openPendingTab when tab is active', done => { + vm = createComponent({ + tab: { + ...file(), + pending: true, + active: true, + }, + }); + + spyOn(vm, 'openPendingTab'); + + vm.$el.click(); + + vm.$nextTick(() => { + expect(vm.openPendingTab).not.toHaveBeenCalled(); + + done(); + }); + }); + it('fires clickFile when the link is clicked', () => { vm = createComponent({ tab: file(), @@ -112,9 +132,9 @@ describe('RepoTab', () => { }); it('renders a tooltip', () => { - expect( - vm.$el.querySelector('span:nth-child(2)').dataset.originalTitle, - ).toContain('Locked by testuser'); + expect(vm.$el.querySelector('span:nth-child(2)').dataset.originalTitle).toContain( + 'Locked by testuser', + ); }); }); diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 2626b439ca6..0eff98bcc9d 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -3,7 +3,6 @@ import $ from 'jquery'; import 'vendor/jasmine-jquery'; import '~/commons'; - import Vue from 'vue'; import VueResource from 'vue-resource'; import Translate from '~/vue_shared/translate'; @@ -136,7 +135,7 @@ if (process.env.BABEL_ENV === 'coverage') { // exempt these files from the coverage report const troubleMakers = [ './blob_edit/blob_bundle.js', - './boards/components/modal/empty_state.js', + './boards/components/modal/empty_state.vue', './boards/components/modal/footer.js', './boards/components/modal/header.js', './cycle_analytics/cycle_analytics_bundle.js', diff --git a/spec/javascripts/u2f/authenticate_spec.js b/spec/javascripts/u2f/authenticate_spec.js index d84b13b07c4..57e0caa692c 100644 --- a/spec/javascripts/u2f/authenticate_spec.js +++ b/spec/javascripts/u2f/authenticate_spec.js @@ -6,7 +6,7 @@ import MockU2FDevice from './mock_u2f_device'; describe('U2FAuthenticate', function () { preloadFixtures('u2f/authenticate.html.raw'); - beforeEach((done) => { + beforeEach(() => { loadFixtures('u2f/authenticate.html.raw'); this.u2fDevice = new MockU2FDevice(); this.container = $('#js-authenticate-u2f'); @@ -19,46 +19,70 @@ describe('U2FAuthenticate', function () { document.querySelector('#js-login-2fa-device'), document.querySelector('.js-2fa-form'), ); + }); - // bypass automatic form submission within renderAuthenticated - spyOn(this.component, 'renderAuthenticated').and.returnValue(true); + describe('with u2f unavailable', () => { + beforeEach(() => { + spyOn(this.component, 'switchToFallbackUI'); + this.oldu2f = window.u2f; + window.u2f = null; + }); - this.component.start().then(done).catch(done.fail); - }); + afterEach(() => { + window.u2f = this.oldu2f; + }); - it('allows authenticating via a U2F device', () => { - const inProgressMessage = this.container.find('p'); - expect(inProgressMessage.text()).toContain('Trying to communicate with your device'); - this.u2fDevice.respondToAuthenticateRequest({ - deviceData: 'this is data from the device', + it('falls back to normal 2fa', (done) => { + this.component.start().then(() => { + expect(this.component.switchToFallbackUI).toHaveBeenCalled(); + done(); + }).catch(done.fail); }); - expect(this.component.renderAuthenticated).toHaveBeenCalledWith('{"deviceData":"this is data from the device"}'); }); - describe('errors', () => { - it('displays an error message', () => { - const setupButton = this.container.find('#js-login-u2f-device'); - setupButton.trigger('click'); - this.u2fDevice.respondToAuthenticateRequest({ - errorCode: 'error!', - }); - const errorMessage = this.container.find('p'); - return expect(errorMessage.text()).toContain('There was a problem communicating with your device'); + describe('with u2f available', () => { + beforeEach((done) => { + // bypass automatic form submission within renderAuthenticated + spyOn(this.component, 'renderAuthenticated').and.returnValue(true); + this.u2fDevice = new MockU2FDevice(); + + this.component.start().then(done).catch(done.fail); }); - return it('allows retrying authentication after an error', () => { - let setupButton = this.container.find('#js-login-u2f-device'); - setupButton.trigger('click'); - this.u2fDevice.respondToAuthenticateRequest({ - errorCode: 'error!', - }); - const retryButton = this.container.find('#js-u2f-try-again'); - retryButton.trigger('click'); - setupButton = this.container.find('#js-login-u2f-device'); - setupButton.trigger('click'); + + it('allows authenticating via a U2F device', () => { + const inProgressMessage = this.container.find('p'); + expect(inProgressMessage.text()).toContain('Trying to communicate with your device'); this.u2fDevice.respondToAuthenticateRequest({ deviceData: 'this is data from the device', }); expect(this.component.renderAuthenticated).toHaveBeenCalledWith('{"deviceData":"this is data from the device"}'); }); + + describe('errors', () => { + it('displays an error message', () => { + const setupButton = this.container.find('#js-login-u2f-device'); + setupButton.trigger('click'); + this.u2fDevice.respondToAuthenticateRequest({ + errorCode: 'error!', + }); + const errorMessage = this.container.find('p'); + return expect(errorMessage.text()).toContain('There was a problem communicating with your device'); + }); + return it('allows retrying authentication after an error', () => { + let setupButton = this.container.find('#js-login-u2f-device'); + setupButton.trigger('click'); + this.u2fDevice.respondToAuthenticateRequest({ + errorCode: 'error!', + }); + const retryButton = this.container.find('#js-u2f-try-again'); + retryButton.trigger('click'); + setupButton = this.container.find('#js-login-u2f-device'); + setupButton.trigger('click'); + this.u2fDevice.respondToAuthenticateRequest({ + deviceData: 'this is data from the device', + }); + expect(this.component.renderAuthenticated).toHaveBeenCalledWith('{"deviceData":"this is data from the device"}'); + }); + }); }); }); diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb index 64f3d09a25b..3a8667e434d 100644 --- a/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -779,4 +779,12 @@ describe Gitlab::Auth::OAuth::User do end end end + + describe '#bypass_two_factor?' do + subject { oauth_user.bypass_two_factor? } + + it 'returns always false' do + is_expected.to be_falsey + end + end end diff --git a/spec/lib/gitlab/auth/saml/auth_hash_spec.rb b/spec/lib/gitlab/auth/saml/auth_hash_spec.rb index bb950e6bbf8..76f49e778fb 100644 --- a/spec/lib/gitlab/auth/saml/auth_hash_spec.rb +++ b/spec/lib/gitlab/auth/saml/auth_hash_spec.rb @@ -37,4 +37,55 @@ describe Gitlab::Auth::Saml::AuthHash do end end end + + describe '#authn_context' do + let(:auth_hash_data) do + { + provider: 'saml', + uid: 'some_uid', + info: + { + name: 'mockuser', + email: 'mock@email.ch', + image: 'mock_user_thumbnail_url' + }, + credentials: + { + token: 'mock_token', + secret: 'mock_secret' + }, + extra: + { + raw_info: + { + info: + { + name: 'mockuser', + email: 'mock@email.ch', + image: 'mock_user_thumbnail_url' + } + } + } + } + end + + subject(:saml_auth_hash) { described_class.new(OmniAuth::AuthHash.new(auth_hash_data)) } + + context 'with response_object' do + before do + auth_hash_data[:extra][:response_object] = { document: + saml_xml(File.read('spec/fixtures/authentication/saml_response.xml')) } + end + + it 'can extract authn_context' do + expect(saml_auth_hash.authn_context).to eq 'urn:oasis:names:tc:SAML:2.0:ac:classes:Password' + end + end + + context 'without response_object' do + it 'returns an empty string' do + expect(saml_auth_hash.authn_context).to be_nil + end + end + end end diff --git a/spec/lib/gitlab/auth/saml/user_spec.rb b/spec/lib/gitlab/auth/saml/user_spec.rb index 62514ca0688..c523f5e177f 100644 --- a/spec/lib/gitlab/auth/saml/user_spec.rb +++ b/spec/lib/gitlab/auth/saml/user_spec.rb @@ -400,4 +400,45 @@ describe Gitlab::Auth::Saml::User do end end end + + describe '#bypass_two_factor?' do + let(:saml_config) { mock_saml_config_with_upstream_two_factor_authn_contexts } + + subject { saml_user.bypass_two_factor? } + + context 'with authn_contexts_worth_two_factors configured' do + before do + stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [saml_config]) + end + + it 'returns true when authn_context is worth two factors' do + allow(saml_user.auth_hash).to receive(:authn_context).and_return('urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + is_expected.to be_truthy + end + + it 'returns false when authn_context is not worth two factors' do + allow(saml_user.auth_hash).to receive(:authn_context).and_return('urn:oasis:names:tc:SAML:2.0:ac:classes:Password') + is_expected.to be_falsey + end + + it 'returns false when authn_context is blank' do + is_expected.to be_falsey + end + end + + context 'without auth_contexts_worth_two_factors_configured' do + before do + stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config]) + end + + it 'returns false when authn_context is present' do + allow(saml_user.auth_hash).to receive(:authn_context).and_return('urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + is_expected.to be_falsey + end + + it 'returns false when authn_context is blank' do + is_expected.to be_falsey + end + end + end end diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb index e79f0a7f257..adb3ff4321f 100644 --- a/spec/lib/gitlab/ci/variables/collection/item_spec.rb +++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb @@ -1,19 +1,69 @@ require 'spec_helper' describe Gitlab::Ci::Variables::Collection::Item do + let(:variable_key) { 'VAR' } + let(:variable_value) { 'something' } + let(:expected_value) { variable_value } + let(:variable) do - { key: 'VAR', value: 'something', public: true } + { key: variable_key, value: variable_value, public: true } end describe '.new' do - it 'raises error if unknown key i specified' do - expect { described_class.new(key: 'VAR', value: 'abc', files: true) } - .to raise_error ArgumentError, 'unknown keyword: files' + context 'when unknown keyword is specified' do + it 'raises error' do + expect { described_class.new(key: variable_key, value: 'abc', files: true) } + .to raise_error ArgumentError, 'unknown keyword: files' + end + end + + context 'when required keywords are not specified' do + it 'raises error' do + expect { described_class.new(key: variable_key) } + .to raise_error ArgumentError, 'missing keyword: value' + end end - it 'raises error when required keywords are not specified' do - expect { described_class.new(key: 'VAR') } - .to raise_error ArgumentError, 'missing keyword: value' + shared_examples 'creates variable' do + subject { described_class.new(key: variable_key, value: variable_value) } + + it 'saves given value' do + expect(subject[:key]).to eq variable_key + expect(subject[:value]).to eq expected_value + end + end + + shared_examples 'raises error for invalid type' do + it do + expect { described_class.new(key: variable_key, value: variable_value) } + .to raise_error ArgumentError, /`value` must be of type String, while it was:/ + end + end + + it_behaves_like 'creates variable' + + context "when it's nil" do + let(:variable_value) { nil } + let(:expected_value) { nil } + + it_behaves_like 'creates variable' + end + + context "when it's an empty string" do + let(:variable_value) { '' } + let(:expected_value) { '' } + + it_behaves_like 'creates variable' + end + + context 'when provided value is not a string' do + [1, false, [], {}, Object.new].each do |val| + context "when it's #{val}" do + let(:variable_value) { val } + + it_behaves_like 'raises error for invalid type' + end + end end end diff --git a/spec/lib/gitlab/ci/variables/collection_spec.rb b/spec/lib/gitlab/ci/variables/collection_spec.rb index cb2f7718c9c..5c91816a586 100644 --- a/spec/lib/gitlab/ci/variables/collection_spec.rb +++ b/spec/lib/gitlab/ci/variables/collection_spec.rb @@ -29,7 +29,7 @@ describe Gitlab::Ci::Variables::Collection do end it 'appends an internal resource' do - collection = described_class.new([{ key: 'TEST', value: 1 }]) + collection = described_class.new([{ key: 'TEST', value: '1' }]) subject.append(collection.first) @@ -74,15 +74,15 @@ describe Gitlab::Ci::Variables::Collection do describe '#+' do it 'makes it possible to combine with an array' do - collection = described_class.new([{ key: 'TEST', value: 1 }]) + collection = described_class.new([{ key: 'TEST', value: '1' }]) variables = [{ key: 'TEST', value: 'something' }] expect((collection + variables).count).to eq 2 end it 'makes it possible to combine with another collection' do - collection = described_class.new([{ key: 'TEST', value: 1 }]) - other = described_class.new([{ key: 'TEST', value: 2 }]) + collection = described_class.new([{ key: 'TEST', value: '1' }]) + other = described_class.new([{ key: 'TEST', value: '2' }]) expect((collection + other).count).to eq 2 end @@ -90,10 +90,10 @@ describe Gitlab::Ci::Variables::Collection do describe '#to_runner_variables' do it 'creates an array of hashes in a runner-compatible format' do - collection = described_class.new([{ key: 'TEST', value: 1 }]) + collection = described_class.new([{ key: 'TEST', value: '1' }]) expect(collection.to_runner_variables) - .to eq [{ key: 'TEST', value: 1, public: true }] + .to eq [{ key: 'TEST', value: '1', public: true }] end end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 6015086f002..b6061df349d 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::Git::Blob, seed_helper: true do end end - shared_examples 'finding blobs' do + describe '.find' do context 'nil path' do let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, nil) } @@ -125,16 +125,6 @@ describe Gitlab::Git::Blob, seed_helper: true do end end - describe '.find' do - context 'when project_raw_show Gitaly feature is enabled' do - it_behaves_like 'finding blobs' - end - - context 'when project_raw_show Gitaly feature is disabled', :skip_gitaly_mock do - it_behaves_like 'finding blobs' - end - end - shared_examples 'finding blobs by ID' do let(:raw_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::RubyBlob::ID) } let(:bad_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::BigCommit::ID) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 45f0006dc85..b78fe4ba310 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1871,49 +1871,39 @@ describe Gitlab::Git::Repository, seed_helper: true do repository_rugged.config["gitlab.fullpath"] = repository_path end - shared_examples 'writing repo config' do - context 'is given a path' do - it 'writes it to disk' do - repository.write_config(full_path: "not-the/real-path.git") + context 'is given a path' do + it 'writes it to disk' do + repository.write_config(full_path: "not-the/real-path.git") - config = File.read(File.join(repository_path, "config")) + config = File.read(File.join(repository_path, "config")) - expect(config).to include("[gitlab]") - expect(config).to include("fullpath = not-the/real-path.git") - end + expect(config).to include("[gitlab]") + expect(config).to include("fullpath = not-the/real-path.git") end + end - context 'it is given an empty path' do - it 'does not write it to disk' do - repository.write_config(full_path: "") + context 'it is given an empty path' do + it 'does not write it to disk' do + repository.write_config(full_path: "") - config = File.read(File.join(repository_path, "config")) + config = File.read(File.join(repository_path, "config")) - expect(config).to include("[gitlab]") - expect(config).to include("fullpath = #{repository_path}") - end + expect(config).to include("[gitlab]") + expect(config).to include("fullpath = #{repository_path}") end + end - context 'repository does not exist' do - it 'raises NoRepository and does not call Gitaly WriteConfig' do - repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '') + context 'repository does not exist' do + it 'raises NoRepository and does not call Gitaly WriteConfig' do + repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '') - expect(repository.gitaly_repository_client).not_to receive(:write_config) + expect(repository.gitaly_repository_client).not_to receive(:write_config) - expect do - repository.write_config(full_path: 'foo/bar.git') - end.to raise_error(Gitlab::Git::Repository::NoRepository) - end + expect do + repository.write_config(full_path: 'foo/bar.git') + end.to raise_error(Gitlab::Git::Repository::NoRepository) end end - - context "when gitaly_write_config is enabled" do - it_behaves_like "writing repo config" - end - - context "when gitaly_write_config is disabled", :disable_gitaly do - it_behaves_like "writing repo config" - end end describe '#merge' do @@ -2160,43 +2150,33 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#create_from_bundle' do - shared_examples 'creating repo from bundle' do - let(:bundle_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } - let(:project) { create(:project) } - let(:imported_repo) { project.repository.raw } - - before do - expect(repository.bundle_to_disk(bundle_path)).to be true - end - - after do - FileUtils.rm_rf(bundle_path) - end + let(:bundle_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } + let(:project) { create(:project) } + let(:imported_repo) { project.repository.raw } - it 'creates a repo from a bundle file' do - expect(imported_repo).not_to exist + before do + expect(repository.bundle_to_disk(bundle_path)).to be_truthy + end - result = imported_repo.create_from_bundle(bundle_path) + after do + FileUtils.rm_rf(bundle_path) + end - expect(result).to be true - expect(imported_repo).to exist - expect { imported_repo.fsck }.not_to raise_exception - end + it 'creates a repo from a bundle file' do + expect(imported_repo).not_to exist - it 'creates a symlink to the global hooks dir' do - imported_repo.create_from_bundle(bundle_path) - hooks_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { File.join(imported_repo.path, 'hooks') } + result = imported_repo.create_from_bundle(bundle_path) - expect(File.readlink(hooks_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) - end + expect(result).to be_truthy + expect(imported_repo).to exist + expect { imported_repo.fsck }.not_to raise_exception end - context 'when Gitaly create_repo_from_bundle feature is enabled' do - it_behaves_like 'creating repo from bundle' - end + it 'creates a symlink to the global hooks dir' do + imported_repo.create_from_bundle(bundle_path) + hooks_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { File.join(imported_repo.path, 'hooks') } - context 'when Gitaly create_repo_from_bundle feature is disabled', :disable_gitaly do - it_behaves_like 'creating repo from bundle' + expect(File.readlink(hooks_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) end end diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 013b8895f67..7ffa84f906d 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::ImportExport::RepoRestorer do end it 'restores the repo successfully' do - expect(restorer.restore).to be true + expect(restorer.restore).to be_truthy end it 'has the webhooks' do diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 091645ee86f..7972ff253fe 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -45,7 +45,7 @@ describe Gitlab::Metrics::Samplers::RubySampler do it 'adds a metric containing garbage collection time statistics' do expect(GC::Profiler).to receive(:total_time).and_return(0.24) - expect(sampler.metrics[:total_time]).to receive(:set).with({}, 240) + expect(sampler.metrics[:total_time]).to receive(:increment).with({}, 0.24) sampler.sample end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 51b9b518117..6758adc59eb 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1871,7 +1871,11 @@ describe Ci::Build do end context 'when yaml_variables are undefined' do - let(:pipeline) { create(:ci_pipeline, project: project) } + let(:pipeline) do + create(:ci_pipeline, project: project, + sha: project.commit.id, + ref: project.default_branch) + end before do build.yaml_variables = nil diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index b5a6d959ccb..c5d550cba1b 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -125,14 +125,6 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end end - - context 'when data_store is others' do - before do - build_trace_chunk.send(:write_attribute, :data_store, -1) - end - - it { expect { subject }.to raise_error('Unsupported data store') } - end end describe '#truncate' do diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index b4249d72fc8..48c01fc4d4e 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -47,6 +47,45 @@ describe MergeRequestDiff do end describe '#diffs' do + let(:merge_request) { create(:merge_request, :with_diffs) } + let!(:diff) { merge_request.merge_request_diff.reload } + + context 'when it was not cleaned by the system' do + it 'returns persisted diffs' do + expect(diff).to receive(:load_diffs) + + diff.diffs + end + end + + context 'when diff was cleaned by the system' do + before do + diff.clean! + end + + it 'returns diffs from repository if can compare with current diff refs' do + expect(diff).not_to receive(:load_diffs) + + expect(Compare) + .to receive(:new) + .with(instance_of(Gitlab::Git::Compare), merge_request.target_project, + base_sha: diff.base_commit_sha, straight: false) + .and_call_original + + diff.diffs + end + + it 'returns persisted diffs if cannot compare with diff refs' do + expect(diff).to receive(:load_diffs) + + diff.update!(head_commit_sha: 'invalid-sha') + + diff.diffs + end + end + end + + describe '#raw_diffs' do context 'when the :ignore_whitespace_change option is set' do it 'creates a new compare object instead of loading from the DB' do expect(diff_with_commits).not_to receive(:load_diffs) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 7ae70c3afb4..ec72fefd137 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1630,28 +1630,17 @@ describe MergeRequest do end describe "#reload_diff" do - let(:discussion) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject).to_discussion } - let(:commit) { subject.project.commit(sample_commit.id) } - - it "does not change existing merge request diff" do - expect(subject.merge_request_diff).not_to receive(:save_git_content) - subject.reload_diff - end - - it "creates new merge request diff" do - expect { subject.reload_diff }.to change { subject.merge_request_diffs.count }.by(1) - end - - it "executes diff cache service" do - expect_any_instance_of(MergeRequests::MergeRequestDiffCacheService).to receive(:execute).with(subject, an_instance_of(MergeRequestDiff)) + it 'calls MergeRequests::ReloadDiffsService#execute with correct params' do + user = create(:user) + service = instance_double(MergeRequests::ReloadDiffsService, execute: nil) - subject.reload_diff - end + expect(MergeRequests::ReloadDiffsService) + .to receive(:new).with(subject, user) + .and_return(service) - it "calls update_diff_discussion_positions" do - expect(subject).to receive(:update_diff_discussion_positions) + subject.reload_diff(user) - subject.reload_diff + expect(service).to have_received(:execute) end context 'when using the after_update hook to update' do @@ -2145,8 +2134,7 @@ describe MergeRequest do describe 'transition to cannot_be_merged' do let(:notification_service) { double(:notification_service) } let(:todo_service) { double(:todo_service) } - - subject { create(:merge_request, merge_status: :unchecked) } + subject { create(:merge_request, state, merge_status: :unchecked) } before do allow(NotificationService).to receive(:new).and_return(notification_service) @@ -2155,33 +2143,52 @@ describe MergeRequest do allow(subject.project.repository).to receive(:can_be_merged?).and_return(false) end - it 'notifies conflict, but does not notify again if rechecking still results in cannot_be_merged' do - expect(notification_service).to receive(:merge_request_unmergeable).with(subject).once - expect(todo_service).to receive(:merge_request_became_unmergeable).with(subject).once + [:opened, :locked].each do |state| + context state do + let(:state) { state } - subject.mark_as_unmergeable - subject.mark_as_unchecked - subject.mark_as_unmergeable - end + it 'notifies conflict, but does not notify again if rechecking still results in cannot_be_merged' do + expect(notification_service).to receive(:merge_request_unmergeable).with(subject).once + expect(todo_service).to receive(:merge_request_became_unmergeable).with(subject).once - it 'notifies conflict, whenever newly unmergeable' do - expect(notification_service).to receive(:merge_request_unmergeable).with(subject).twice - expect(todo_service).to receive(:merge_request_became_unmergeable).with(subject).twice + subject.mark_as_unmergeable + subject.mark_as_unchecked + subject.mark_as_unmergeable + end + + it 'notifies conflict, whenever newly unmergeable' do + expect(notification_service).to receive(:merge_request_unmergeable).with(subject).twice + expect(todo_service).to receive(:merge_request_became_unmergeable).with(subject).twice + + subject.mark_as_unmergeable + subject.mark_as_unchecked + subject.mark_as_mergeable + subject.mark_as_unchecked + subject.mark_as_unmergeable + end + + it 'does not notify whenever merge request is newly unmergeable due to other reasons' do + allow(subject.project.repository).to receive(:can_be_merged?).and_return(true) - subject.mark_as_unmergeable - subject.mark_as_unchecked - subject.mark_as_mergeable - subject.mark_as_unchecked - subject.mark_as_unmergeable + expect(notification_service).not_to receive(:merge_request_unmergeable) + expect(todo_service).not_to receive(:merge_request_became_unmergeable) + + subject.mark_as_unmergeable + end + end end - it 'does not notify whenever merge request is newly unmergeable due to other reasons' do - allow(subject.project.repository).to receive(:can_be_merged?).and_return(true) + [:closed, :merged].each do |state| + let(:state) { state } - expect(notification_service).not_to receive(:merge_request_unmergeable) - expect(todo_service).not_to receive(:merge_request_became_unmergeable) + context state do + it 'does not notify' do + expect(notification_service).not_to receive(:merge_request_unmergeable) + expect(todo_service).not_to receive(:merge_request_became_unmergeable) - subject.mark_as_unmergeable + subject.mark_as_unmergeable + end + end end end diff --git a/spec/rubocop/cop/migration/update_large_table_spec.rb b/spec/rubocop/cop/migration/update_large_table_spec.rb index ef724fc8bad..5e08eb4f772 100644 --- a/spec/rubocop/cop/migration/update_large_table_spec.rb +++ b/spec/rubocop/cop/migration/update_large_table_spec.rb @@ -32,6 +32,14 @@ describe RuboCop::Cop::Migration::UpdateLargeTable do include_examples 'large tables', 'add_column_with_default' end + context 'for the change_column_type_concurrently method' do + include_examples 'large tables', 'change_column_type_concurrently' + end + + context 'for the rename_column_concurrently method' do + include_examples 'large tables', 'rename_column_concurrently' + end + context 'for the update_column_in_batches method' do include_examples 'large tables', 'update_column_in_batches' end @@ -60,6 +68,18 @@ describe RuboCop::Cop::Migration::UpdateLargeTable do expect(cop.offenses).to be_empty end + it 'registers no offense for change_column_type_concurrently' do + inspect_source("change_column_type_concurrently :#{table}, :column, default: true") + + expect(cop.offenses).to be_empty + end + + it 'registers no offense for update_column_in_batches' do + inspect_source("rename_column_concurrently :#{table}, :column, default: true") + + expect(cop.offenses).to be_empty + end + it 'registers no offense for update_column_in_batches' do inspect_source("add_column_with_default :#{table}, :column, default: true") diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index da8e660c16b..fce73e0ac1f 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -21,6 +21,11 @@ describe Auth::ContainerRegistryAuthenticationService do allow_any_instance_of(JSONWebToken::RSAToken).to receive(:key).and_return(rsa_key) end + shared_examples 'an authenticated' do + it { is_expected.to include(:token) } + it { expect(payload).to include('access') } + end + shared_examples 'a valid token' do it { is_expected.to include(:token) } it { expect(payload).to include('access') } @@ -380,6 +385,14 @@ describe Auth::ContainerRegistryAuthenticationService do current_project.add_developer(current_user) end + context 'allow to use offline_token' do + let(:current_params) do + { offline_token: true } + end + + it_behaves_like 'an authenticated' + end + it_behaves_like 'a valid token' context 'allow to pull and push images' do diff --git a/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb b/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb new file mode 100644 index 00000000000..1c632847940 --- /dev/null +++ b/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe MergeRequests::DeleteNonLatestDiffsService, :clean_gitlab_redis_shared_state do + let(:merge_request) { create(:merge_request) } + + let!(:subject) { described_class.new(merge_request) } + + describe '#execute' do + before do + stub_const("#{described_class.name}::BATCH_SIZE", 2) + + 3.times { merge_request.create_merge_request_diff } + end + + it 'schedules non-latest merge request diffs removal' do + diffs = merge_request.merge_request_diffs + + expect(diffs.count).to eq(4) + + Timecop.freeze do + expect(DeleteDiffFilesWorker) + .to receive(:bulk_perform_in) + .with(5.minutes, [[diffs.first.id], [diffs.second.id]]) + expect(DeleteDiffFilesWorker) + .to receive(:bulk_perform_in) + .with(10.minutes, [[diffs.third.id]]) + + subject.execute + end + end + + it 'schedules no removal if it is already cleaned' do + merge_request.merge_request_diffs.each(&:clean!) + + expect(DeleteDiffFilesWorker).not_to receive(:bulk_perform_in) + + subject.execute + end + + it 'schedules no removal if it is empty' do + merge_request.merge_request_diffs.each { |diff| diff.update!(state: :empty) } + + expect(DeleteDiffFilesWorker).not_to receive(:bulk_perform_in) + + subject.execute + end + + it 'schedules no removal if there is no non-latest diffs' do + merge_request + .merge_request_diffs + .where.not(id: merge_request.latest_merge_request_diff_id) + .destroy_all + + expect(DeleteDiffFilesWorker).not_to receive(:bulk_perform_in) + + subject.execute + end + end +end diff --git a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb deleted file mode 100644 index 57b6165cfb0..00000000000 --- a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'spec_helper' - -describe MergeRequests::MergeRequestDiffCacheService, :use_clean_rails_memory_store_caching do - let(:subject) { described_class.new } - let(:merge_request) { create(:merge_request) } - - describe '#execute' do - before do - allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(true) - allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(true) - end - - it 'retrieves the diff files to cache the highlighted result' do - new_diff = merge_request.merge_request_diff - cache_key = new_diff.diffs.cache_key - - expect(Rails.cache).to receive(:read).with(cache_key).and_call_original - expect(Rails.cache).to receive(:write).with(cache_key, anything, anything).and_call_original - - subject.execute(merge_request, new_diff) - end - - it 'clears the cache for older diffs on the merge request' do - old_diff = merge_request.merge_request_diff - old_cache_key = old_diff.diffs.cache_key - - subject.execute(merge_request, old_diff) - - new_diff = merge_request.create_merge_request_diff - new_cache_key = new_diff.diffs.cache_key - - expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original - expect(Rails.cache).to receive(:read).with(new_cache_key).and_call_original - expect(Rails.cache).to receive(:write).with(new_cache_key, anything, anything).and_call_original - - subject.execute(merge_request, new_diff) - end - end -end diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index 70957431942..790ecce8ded 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -35,5 +35,17 @@ describe MergeRequests::PostMergeService do described_class.new(project, user, {}).execute(merge_request) end + + it 'deletes non-latest diffs' do + diff_removal_service = instance_double(MergeRequests::DeleteNonLatestDiffsService, execute: nil) + + expect(MergeRequests::DeleteNonLatestDiffsService) + .to receive(:new).with(merge_request) + .and_return(diff_removal_service) + + described_class.new(project, user, {}).execute(merge_request) + + expect(diff_removal_service).to have_received(:execute) + end end end diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb new file mode 100644 index 00000000000..a0a27d247fc --- /dev/null +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_caching do + let(:current_user) { create(:user) } + let(:merge_request) { create(:merge_request) } + let(:subject) { described_class.new(merge_request, current_user) } + + describe '#execute' do + it 'creates new merge request diff' do + expect { subject.execute }.to change { merge_request.merge_request_diffs.count }.by(1) + end + + it 'calls update_diff_discussion_positions with correct params' do + old_diff_refs = merge_request.diff_refs + new_diff = merge_request.create_merge_request_diff + new_diff_refs = merge_request.diff_refs + + expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) + expect(merge_request).to receive(:update_diff_discussion_positions) + .with(old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs, + current_user: current_user) + + subject.execute + end + + it 'does not change existing merge request diff' do + expect(merge_request.merge_request_diff).not_to receive(:save_git_content) + + subject.execute + end + + context 'cache clearing' do + before do + allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(true) + allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(true) + end + + it 'retrieves the diff files to cache the highlighted result' do + new_diff = merge_request.create_merge_request_diff + cache_key = new_diff.diffs_collection.cache_key + + expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) + expect(Rails.cache).to receive(:read).with(cache_key).and_call_original + expect(Rails.cache).to receive(:write).with(cache_key, anything, anything).and_call_original + + subject.execute + end + + it 'clears the cache for older diffs on the merge request' do + old_diff = merge_request.merge_request_diff + old_cache_key = old_diff.diffs_collection.cache_key + new_diff = merge_request.create_merge_request_diff + new_cache_key = new_diff.diffs_collection.cache_key + + expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) + expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original + expect(Rails.cache).to receive(:read).with(new_cache_key).and_call_original + expect(Rails.cache).to receive(:write).with(new_cache_key, anything, anything).and_call_original + subject.execute + end + end + end +end diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index 723cb374c37..5c2e79ff9af 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Projects::UpdateRemoteMirrorService do - let(:project) { create(:project, :repository) } + set(:project) { create(:project, :repository) } + let(:owner) { project.owner } let(:remote_project) { create(:forked_project_with_submodules) } let(:repository) { project.repository } let(:raw_repository) { repository.raw } @@ -9,13 +10,11 @@ describe Projects::UpdateRemoteMirrorService do subject { described_class.new(project, project.creator) } - describe "#execute", :skip_gitaly_mock do + describe "#execute" do before do - create_branch(repository, 'existing-branch') - allow(raw_repository).to receive(:remote_tags) do - generate_tags(repository, 'v1.0.0', 'v1.1.0') - end - allow(raw_repository).to receive(:push_remote_branches).and_return(true) + repository.add_branch(owner, 'existing-branch', 'master') + + allow(remote_mirror).to receive(:update_repository).and_return(true) end it "fetches the remote repository" do @@ -34,307 +33,57 @@ describe Projects::UpdateRemoteMirrorService do expect(result[:status]).to eq(:success) end - describe 'Syncing branches' do + context 'when syncing all branches' do it "push all the branches the first time" do allow(repository).to receive(:fetch_remote) - expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, local_branch_names) - - subject.execute(remote_mirror) - end - - it "does not push anything is remote is up to date" do - allow(repository).to receive(:fetch_remote) { sync_remote(repository, remote_mirror.remote_name, local_branch_names) } - - expect(raw_repository).not_to receive(:push_remote_branches) - - subject.execute(remote_mirror) - end - - it "sync new branches" do - # call local_branch_names early so it is not called after the new branch has been created - current_branches = local_branch_names - allow(repository).to receive(:fetch_remote) { sync_remote(repository, remote_mirror.remote_name, current_branches) } - create_branch(repository, 'my-new-branch') - - expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, ['my-new-branch']) - - subject.execute(remote_mirror) - end - - it "sync updated branches" do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - update_branch(repository, 'existing-branch') - end - - expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, ['existing-branch']) + expect(remote_mirror).to receive(:update_repository).with({}) subject.execute(remote_mirror) end - - context 'when push only protected branches option is set' do - let(:unprotected_branch_name) { 'existing-branch' } - let(:protected_branch_name) do - project.repository.branch_names.find { |n| n != unprotected_branch_name } - end - let!(:protected_branch) do - create(:protected_branch, project: project, name: protected_branch_name) - end - - before do - project.reload - remote_mirror.only_protected_branches = true - end - - it "sync updated protected branches" do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - update_branch(repository, protected_branch_name) - end - - expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, [protected_branch_name]) - - subject.execute(remote_mirror) - end - - it 'does not sync unprotected branches' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - update_branch(repository, unprotected_branch_name) - end - - expect(raw_repository).not_to receive(:push_remote_branches).with(remote_mirror.remote_name, [unprotected_branch_name]) - - subject.execute(remote_mirror) - end - end - - context 'when branch exists in local and remote repo' do - context 'when it has diverged' do - it 'syncs branches' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - update_remote_branch(repository, remote_mirror.remote_name, 'markdown') - end - - expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, ['markdown']) - - subject.execute(remote_mirror) - end - end - end - - describe 'for delete' do - context 'when branch exists in local and remote repo' do - it 'deletes the branch from remote repo' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - delete_branch(repository, 'existing-branch') - end - - expect(raw_repository).to receive(:delete_remote_branches).with(remote_mirror.remote_name, ['existing-branch']) - - subject.execute(remote_mirror) - end - end - - context 'when push only protected branches option is set' do - before do - remote_mirror.only_protected_branches = true - end - - context 'when branch exists in local and remote repo' do - let!(:protected_branch_name) { local_branch_names.first } - - before do - create(:protected_branch, project: project, name: protected_branch_name) - project.reload - end - - it 'deletes the protected branch from remote repo' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - delete_branch(repository, protected_branch_name) - end - - expect(raw_repository).not_to receive(:delete_remote_branches).with(remote_mirror.remote_name, [protected_branch_name]) - - subject.execute(remote_mirror) - end - - it 'does not delete the unprotected branch from remote repo' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - delete_branch(repository, 'existing-branch') - end - - expect(raw_repository).not_to receive(:delete_remote_branches).with(remote_mirror.remote_name, ['existing-branch']) - - subject.execute(remote_mirror) - end - end - - context 'when branch only exists on remote repo' do - let!(:protected_branch_name) { 'remote-branch' } - - before do - create(:protected_branch, project: project, name: protected_branch_name) - end - - context 'when it has diverged' do - it 'does not delete the remote branch' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - - rev = repository.find_branch('markdown').dereferenced_target - create_remote_branch(repository, remote_mirror.remote_name, 'remote-branch', rev.id) - end - - expect(raw_repository).not_to receive(:delete_remote_branches) - - subject.execute(remote_mirror) - end - end - - context 'when it has not diverged' do - it 'deletes the remote branch' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - - masterrev = repository.find_branch('master').dereferenced_target - create_remote_branch(repository, remote_mirror.remote_name, protected_branch_name, masterrev.id) - end - - expect(raw_repository).to receive(:delete_remote_branches).with(remote_mirror.remote_name, [protected_branch_name]) - - subject.execute(remote_mirror) - end - end - end - end - - context 'when branch only exists on remote repo' do - context 'when it has diverged' do - it 'does not delete the remote branch' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - - rev = repository.find_branch('markdown').dereferenced_target - create_remote_branch(repository, remote_mirror.remote_name, 'remote-branch', rev.id) - end - - expect(raw_repository).not_to receive(:delete_remote_branches) - - subject.execute(remote_mirror) - end - end - - context 'when it has not diverged' do - it 'deletes the remote branch' do - allow(repository).to receive(:fetch_remote) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - - masterrev = repository.find_branch('master').dereferenced_target - create_remote_branch(repository, remote_mirror.remote_name, 'remote-branch', masterrev.id) - end - - expect(raw_repository).to receive(:delete_remote_branches).with(remote_mirror.remote_name, ['remote-branch']) - - subject.execute(remote_mirror) - end - end - end - end end - describe 'Syncing tags' do - before do - allow(repository).to receive(:fetch_remote) { sync_remote(repository, remote_mirror.remote_name, local_branch_names) } + context 'when only syncing protected branches' do + let(:unprotected_branch_name) { 'existing-branch' } + let(:protected_branch_name) do + project.repository.branch_names.find { |n| n != unprotected_branch_name } end - - context 'when there are not tags to push' do - it 'does not try to push tags' do - allow(repository).to receive(:remote_tags) { {} } - allow(repository).to receive(:tags) { [] } - - expect(repository).not_to receive(:push_tags) - - subject.execute(remote_mirror) - end + let!(:protected_branch) do + create(:protected_branch, project: project, name: protected_branch_name) end - context 'when there are some tags to push' do - it 'pushes tags to remote' do - allow(raw_repository).to receive(:remote_tags) { {} } - - expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, ['v1.0.0', 'v1.1.0']) - - subject.execute(remote_mirror) - end + before do + project.reload + remote_mirror.only_protected_branches = true end - context 'when there are some tags to delete' do - it 'deletes tags from remote' do - remote_tags = generate_tags(repository, 'v1.0.0', 'v1.1.0') - allow(raw_repository).to receive(:remote_tags) { remote_tags } - - repository.rm_tag(create(:user), 'v1.0.0') - - expect(raw_repository).to receive(:delete_remote_branches).with(remote_mirror.remote_name, ['v1.0.0']) + it "sync updated protected branches" do + allow(repository).to receive(:fetch_remote) + expect(remote_mirror).to receive(:update_repository).with(only_branches_matching: [protected_branch_name]) - subject.execute(remote_mirror) - end + subject.execute(remote_mirror) end end end - def create_branch(repository, branch_name) - rugged = repository.rugged - masterrev = repository.find_branch('master').dereferenced_target - parentrev = repository.commit(masterrev).parent_id - - rugged.references.create("refs/heads/#{branch_name}", parentrev) - - repository.expire_branches_cache - end - - def create_remote_branch(repository, remote_name, branch_name, source_id) - rugged = repository.rugged - - rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_id) - end - def sync_remote(repository, remote_name, local_branch_names) - rugged = repository.rugged - local_branch_names.each do |branch| - target = repository.find_branch(branch).try(:dereferenced_target) - rugged.references.create("refs/remotes/#{remote_name}/#{branch}", target.id) if target + commit = repository.commit(branch) + repository.write_ref("refs/remotes/#{remote_name}/#{branch}", commit.id) if commit end end def update_remote_branch(repository, remote_name, branch) - rugged = repository.rugged - masterrev = repository.find_branch('master').dereferenced_target.id + masterrev = repository.commit('master').id - rugged.references.create("refs/remotes/#{remote_name}/#{branch}", masterrev, force: true) + repository.write_ref("refs/remotes/#{remote_name}/#{branch}", masterrev, force: true) repository.expire_branches_cache end def update_branch(repository, branch) - rugged = repository.rugged - masterrev = repository.find_branch('master').dereferenced_target.id - - # Updated existing branch - rugged.references.create("refs/heads/#{branch}", masterrev, force: true) - repository.expire_branches_cache - end - - def delete_branch(repository, branch) - rugged = repository.rugged + masterrev = repository.commit('master').id - rugged.references.delete("refs/heads/#{branch}") + repository.write_ref("refs/heads/#{branch}", masterrev, force: true) repository.expire_branches_cache end diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 76f1e625fda..f82d4b483e7 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -19,7 +19,9 @@ describe Users::DestroyService do end it 'will delete the project' do - expect_any_instance_of(Projects::DestroyService).to receive(:execute).once + expect_next_instance_of(Projects::DestroyService) do |destroy_service| + expect(destroy_service).to receive(:execute).once + end service.execute(user) end @@ -32,7 +34,9 @@ describe Users::DestroyService do end it 'destroys a project in pending_delete' do - expect_any_instance_of(Projects::DestroyService).to receive(:execute).once + expect_next_instance_of(Projects::DestroyService) do |destroy_service| + expect(destroy_service).to receive(:execute).once + end service.execute(user) diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 7995f2c9ae7..622e56e1da5 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -60,6 +60,36 @@ describe WebHookService do ).once end + context 'when auth credentials are present' do + let(:url) {'https://example.org'} + let(:project_hook) { create(:project_hook, url: 'https://demo:demo@example.org/') } + + it 'uses the credentials' do + WebMock.stub_request(:post, url) + + service_instance.execute + + expect(WebMock).to have_requested(:post, url).with( + headers: headers.merge('Authorization' => 'Basic ZGVtbzpkZW1v') + ).once + end + end + + context 'when auth credentials are partial present' do + let(:url) {'https://example.org'} + let(:project_hook) { create(:project_hook, url: 'https://demo@example.org/') } + + it 'uses the credentials anyways' do + WebMock.stub_request(:post, url) + + service_instance.execute + + expect(WebMock).to have_requested(:post, url).with( + headers: headers.merge('Authorization' => 'Basic ZGVtbzo=') + ).once + end + end + it 'catches exceptions' do WebMock.stub_request(:post, project_hook.url).to_raise(StandardError.new('Some error')) diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index 329f18cd288..87cfb6c04dc 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -46,8 +46,8 @@ module LoginHelpers @current_user = user end - def gitlab_sign_in_via(provider, user, uid) - mock_auth_hash(provider, uid, user.email) + def gitlab_sign_in_via(provider, user, uid, saml_response = nil) + mock_auth_hash(provider, uid, user.email, saml_response) visit new_user_session_path click_link provider end @@ -87,7 +87,7 @@ module LoginHelpers click_link "oauth-login-#{provider}" end - def mock_auth_hash(provider, uid, email) + def mock_auth_hash(provider, uid, email, saml_response = 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({ @@ -109,12 +109,21 @@ module LoginHelpers email: email, image: 'mock_user_thumbnail_url' } + }, + response_object: { + document: saml_xml(saml_response) } } }) Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym] end + def saml_xml(raw_saml_response) + return '' if raw_saml_response.blank? + + XMLSecurity::SignedDocument.new(raw_saml_response, []) + end + def mock_saml_config OpenStruct.new(name: 'saml', label: 'saml', args: { assertion_consumer_service_url: 'https://localhost:3443/users/auth/saml/callback', @@ -125,6 +134,14 @@ module LoginHelpers }) end + def mock_saml_config_with_upstream_two_factor_authn_contexts + config = mock_saml_config + config.args[:upstream_two_factor_authn_contexts] = %w(urn:oasis:names:tc:SAML:2.0:ac:classes:CertificateProtectedTransport + urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS + urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN) + config + end + def stub_omniauth_provider(provider, context: Rails.application) env = env_from_context(context) @@ -140,13 +157,16 @@ module LoginHelpers env['omniauth.error.strategy'] = strategy end - def stub_omniauth_saml_config(messages) - set_devise_mapping(context: Rails.application) - Rails.application.routes.disable_clear_and_finalize = true - Rails.application.routes.draw do + def stub_omniauth_saml_config(messages, context: Rails.application) + set_devise_mapping(context: context) + routes = Rails.application.routes + routes.disable_clear_and_finalize = true + routes.formatter.clear + routes.draw do post '/users/auth/saml' => 'omniauth_callbacks#saml' end - allow(Gitlab::Auth::OAuth::Provider).to receive_messages(providers: [:saml], config_for: mock_saml_config) + saml_config = messages.key?(:providers) ? messages[:providers].first : mock_saml_config + allow(Gitlab::Auth::OAuth::Provider).to receive_messages(providers: [:saml], config_for: saml_config) stub_omniauth_setting(messages) stub_saml_authorize_path_helpers end diff --git a/spec/support/matchers/match_ids.rb b/spec/support/matchers/match_ids.rb index d8424405b96..1cb6b74acac 100644 --- a/spec/support/matchers/match_ids.rb +++ b/spec/support/matchers/match_ids.rb @@ -10,6 +10,13 @@ RSpec::Matchers.define :match_ids do |*expected| 'matches elements by ids' end + failure_message do + actual_ids = map_ids(actual) + expected_ids = map_ids(expected) + + "expected IDs #{actual_ids} in:\n\n #{actual.inspect}\n\nto match IDs #{expected_ids} in:\n\n #{expected.inspect}" + end + def map_ids(elements) elements = elements.flatten if elements.respond_to?(:flatten) diff --git a/spec/support/redis/redis_shared_examples.rb b/spec/support/redis/redis_shared_examples.rb index 8676f895a83..e650a176041 100644 --- a/spec/support/redis/redis_shared_examples.rb +++ b/spec/support/redis/redis_shared_examples.rb @@ -65,6 +65,14 @@ RSpec.shared_examples "redis_shared_examples" do end describe '.url' do + it 'withstands mutation' do + url1 = described_class.url + url2 = described_class.url + url1 << 'foobar' unless url1.frozen? + + expect(url2).not_to end_with('foobar') + end + context 'when yml file with env variable' do let(:config_file_name) { config_with_environment_variable_inside } @@ -101,7 +109,6 @@ RSpec.shared_examples "redis_shared_examples" do before do clear_pool end - after do clear_pool end diff --git a/spec/support/shared_examples/features/project_features_apply_to_issuables_shared_examples.rb b/spec/support/shared_examples/features/project_features_apply_to_issuables_shared_examples.rb index 639b0924197..64c3b80136d 100644 --- a/spec/support/shared_examples/features/project_features_apply_to_issuables_shared_examples.rb +++ b/spec/support/shared_examples/features/project_features_apply_to_issuables_shared_examples.rb @@ -18,7 +18,7 @@ shared_examples 'project features apply to issuables' do |klass| before do _ = issuable - gitlab_sign_in(user) if user + sign_in(user) if user visit path end diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index c7f5694ff43..7e673681c31 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -191,6 +191,18 @@ describe ObjectStorage do it "calls a cache path" do expect { |b| uploader.use_file(&b) }.to yield_with_args(%r[tmp/cache]) end + + it "cleans up the cached file" do + cached_path = '' + + uploader.use_file do |path| + cached_path = path + + expect(File.exist?(cached_path)).to be_truthy + end + + expect(File.exist?(cached_path)).to be_falsey + end end end diff --git a/spec/views/devise/shared/_signin_box.html.haml_spec.rb b/spec/views/devise/shared/_signin_box.html.haml_spec.rb index 0870b8f09f9..66c064e3fba 100644 --- a/spec/views/devise/shared/_signin_box.html.haml_spec.rb +++ b/spec/views/devise/shared/_signin_box.html.haml_spec.rb @@ -6,6 +6,7 @@ describe 'devise/shared/_signin_box' do stub_devise assign(:ldap_servers, []) allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) + allow(view).to receive(:captcha_enabled?).and_return(false) end it 'is shown when Crowd is enabled' do diff --git a/spec/workers/delete_diff_files_worker_spec.rb b/spec/workers/delete_diff_files_worker_spec.rb new file mode 100644 index 00000000000..e0edd313922 --- /dev/null +++ b/spec/workers/delete_diff_files_worker_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe DeleteDiffFilesWorker do + describe '#perform' do + let(:merge_request) { create(:merge_request) } + let(:merge_request_diff) { merge_request.merge_request_diff } + + it 'deletes all merge request diff files' do + expect { described_class.new.perform(merge_request_diff.id) } + .to change { merge_request_diff.merge_request_diff_files.count } + .from(20).to(0) + end + + it 'updates state to without_files' do + expect { described_class.new.perform(merge_request_diff.id) } + .to change { merge_request_diff.reload.state } + .from('collected').to('without_files') + end + + it 'does nothing if diff was already marked as "without_files"' do + merge_request_diff.clean! + + expect_any_instance_of(MergeRequestDiff).not_to receive(:clean!) + + described_class.new.perform(merge_request_diff.id) + end + + it 'rollsback if something goes wrong' do + expect(MergeRequestDiffFile).to receive_message_chain(:where, :delete_all) + .and_raise + + expect { described_class.new.perform(merge_request_diff.id) } + .to raise_error + + merge_request_diff.reload + + expect(merge_request_diff.state).to eq('collected') + expect(merge_request_diff.merge_request_diff_files.count).to eq(20) + end + end +end diff --git a/spec/workers/delete_user_worker_spec.rb b/spec/workers/delete_user_worker_spec.rb index 36594515005..06d9e125105 100644 --- a/spec/workers/delete_user_worker_spec.rb +++ b/spec/workers/delete_user_worker_spec.rb @@ -5,15 +5,17 @@ describe DeleteUserWorker do let!(:current_user) { create(:user) } it "calls the DeleteUserWorker with the params it was given" do - expect_any_instance_of(Users::DestroyService).to receive(:execute) - .with(user, {}) + expect_next_instance_of(Users::DestroyService) do |service| + expect(service).to receive(:execute).with(user, {}) + end described_class.new.perform(current_user.id, user.id) end it "uses symbolized keys" do - expect_any_instance_of(Users::DestroyService).to receive(:execute) - .with(user, test: "test") + expect_next_instance_of(Users::DestroyService) do |service| + expect(service).to receive(:execute).with(user, test: "test") + end described_class.new.perform(current_user.id, user.id, "test" => "test") end |