diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index c17b2ebc859..10d57e441a1 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -5,6 +5,8 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController include InitializesCurrentUserMode include Gitlab::Utils::StrongMemoize + prepend_before_action :set_current_organization + before_action :add_gon_variables before_action :verify_confirmed_email!, :verify_admin_allowed! @@ -50,6 +52,8 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController # Cannot be achieved with a before_action hook, due to the execution order. downgrade_scopes! if action_name == 'new' + params[:organization_id] = ::Current.organization_id + super end @@ -117,6 +121,10 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController *::Gitlab::Auth::REGISTRY_SCOPES ) && !doorkeeper_application&.trusted? end + + def set_current_organization + ::Current.organization = Gitlab::Current::Organization.new(user: current_user).organization + end end Oauth::AuthorizationsController.prepend_mod diff --git a/app/services/ci/runners/unregister_runner_manager_service.rb b/app/services/ci/runners/unregister_runner_manager_service.rb index 9b3bd4a53e2..b0d838d9f7d 100644 --- a/app/services/ci/runners/unregister_runner_manager_service.rb +++ b/app/services/ci/runners/unregister_runner_manager_service.rb @@ -28,7 +28,7 @@ module Ci private def system_id_missing_error - ServiceResponse.error(message: '`system_id` needs to be specified for runners created in the UI.') + ServiceResponse.error(message: '`system_id` needs to be specified.') end end end diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index 8ef83e57b9a..d578405fe33 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -98,11 +98,15 @@ module API delete '/managers', urgency: :low, feature_category: :fleet_visibility do authenticate_runner!(ensure_runner_manager: false) - destroy_conditionally!(current_runner) do + runner_manager = current_runner.runner_managers.find_by_system_xid(params[:system_id]) + not_found!('Runner manager not found') unless runner_manager + + destroy_conditionally!(runner_manager) do ::Ci::Runners::UnregisterRunnerManagerService.new( current_runner, params[:token], - system_id: params[:system_id]).execute + system_id: params[:system_id] + ).execute end end diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index a4b019df1c6..d391807eec1 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe Oauth::AuthorizationsController do - let(:user) { create(:user) } +RSpec.describe Oauth::AuthorizationsController, :with_current_organization, feature_category: :system_access do + let(:user) { create(:user, organizations: [current_organization]) } let(:application_scopes) { 'api read_user' } let(:confidential) { true } @@ -128,6 +128,10 @@ RSpec.describe Oauth::AuthorizationsController do expect(response).to render_template('doorkeeper/authorizations/redirect') end + it "creates access grant on the Current.organization" do + expect { subject }.to change { OauthAccessGrant.where(organization: current_organization).count } + end + context 'when showing applications as provided' do let!(:application) do create( @@ -267,6 +271,8 @@ RSpec.describe Oauth::AuthorizationsController do end context 'when the user is admin' do + let_it_be(:user) { create(:user, :admin, organizations: [current_organization]) } + context 'when disable_admin_oauth_scopes is set' do before do stub_application_setting(disable_admin_oauth_scopes: true) @@ -275,8 +281,6 @@ RSpec.describe Oauth::AuthorizationsController do allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes) end - let(:user) { create(:user, :admin) } - it 'returns 200 and renders forbidden view' do subject @@ -293,7 +297,6 @@ RSpec.describe Oauth::AuthorizationsController do end let(:application_scopes) { 'api' } - let(:user) { create(:user, :admin) } it 'returns 200 and renders redirect view' do subject @@ -309,7 +312,6 @@ RSpec.describe Oauth::AuthorizationsController do end let(:application_scopes) { 'api' } - let(:user) { create(:user, :admin) } it 'returns 200 and renders new view' do subject diff --git a/spec/features/ide_spec.rb b/spec/features/ide_spec.rb index c9bd4d8c7c2..3d2df0719e2 100644 --- a/spec/features/ide_spec.rb +++ b/spec/features/ide_spec.rb @@ -2,14 +2,14 @@ require 'spec_helper' -RSpec.describe 'IDE', :js, feature_category: :web_ide do +RSpec.describe 'IDE', :js, :with_current_organization, feature_category: :web_ide do include Features::WebIdeSpecHelpers let_it_be(:ide_iframe_selector) { '#ide iframe' } let_it_be(:normal_project) { create(:project, :repository) } let(:project) { normal_project } - let(:user) { create(:user) } + let(:user) { create(:user, organizations: [current_organization]) } before do # TODO - We need to be able to handle requests to https://*.cdn.web-ide.gitlab-static.net diff --git a/spec/features/oauth_provider_authorize_spec.rb b/spec/features/oauth_provider_authorize_spec.rb index 77cddcfb3df..1434b3a1e09 100644 --- a/spec/features/oauth_provider_authorize_spec.rb +++ b/spec/features/oauth_provider_authorize_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -RSpec.describe 'OAuth Provider', feature_category: :system_access do +RSpec.describe 'OAuth Provider', :with_current_organization, feature_category: :system_access do + let_it_be(:user) { create(:admin, organizations: [current_organization]) } + def visit_oauth_authorization_path visit oauth_authorization_path( client_id: application.uid, @@ -44,9 +46,8 @@ RSpec.describe 'OAuth Provider', feature_category: :system_access do end context 'when the OAuth application has HTML in the name' do - let(:client_name) { '' } - let(:application) { create(:oauth_application, name: client_name, scopes: 'read_user') } - let(:user) { create(:admin) } + let_it_be(:client_name) { '' } + let_it_be(:application) { create(:oauth_application, name: client_name, scopes: 'read_user') } before do visit_oauth_authorization_path @@ -84,7 +85,6 @@ RSpec.describe 'OAuth Provider', feature_category: :system_access do context 'when brand title has HTML' do let(:application) { create(:oauth_application, scopes: 'read_user') } - let(:user) { create(:user) } let!(:appearance) { create(:appearance, title: '') } before do diff --git a/spec/requests/api/ci/runner/runners_delete_spec.rb b/spec/requests/api/ci/runner/runners_delete_spec.rb index 7ece0bed572..b46decc1518 100644 --- a/spec/requests/api/ci/runner/runners_delete_spec.rb +++ b/spec/requests/api/ci/runner/runners_delete_spec.rb @@ -102,75 +102,67 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego context 'with created runner' do let!(:runner) { create(:ci_runner, :with_runner_manager, registration_type: :authenticated_user) } + let(:delete_params) do + { token: runner.token, system_id: runner.runner_managers.first.system_xid } + end - context 'with matching system_id' do - context 'when no token is provided' do - let(:delete_params) { { system_id: runner.runner_managers.first.system_xid } } + it 'deletes runner manager' do + expect do + delete_request - it 'returns 400 error' do - delete_request + expect(response).to have_gitlab_http_status(:no_content) + end.to change { runner.runner_managers.count }.from(1).to(0) + .and not_change { ::Ci::Runner.count }.from(1) + end - expect(response).to have_gitlab_http_status(:bad_request) - end - end + it_behaves_like '412 response' do + let(:request) { api('/runners/managers') } + let(:params) { delete_params } + end - context 'when invalid token is provided' do - let(:delete_params) { { token: 'invalid', system_id: runner.runner_managers.first.system_xid } } + it_behaves_like 'storing arguments in the application context for the API' do + let(:expected_params) { { client_id: "runner/#{runner.id}" } } + end - it 'returns 403 error' do - delete_request + context 'with unknown system_id' do + let(:delete_params) { { token: runner.token, system_id: 'unknown_system_id' } } - expect(response).to have_gitlab_http_status(:forbidden) - end + it 'returns 404 error' do + delete_request + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to include('Runner manager not found') + expect(::Ci::Runner.count).to eq(1) end end - end - context 'when valid token is provided' do - context 'with created runner' do - let!(:runner) { create(:ci_runner, :with_runner_manager, registration_type: :authenticated_user) } + context 'without system_id' do + let(:delete_params) { { token: runner.token } } - context 'with matching system_id' do - let(:delete_params) { { token: runner.token, system_id: runner.runner_managers.first.system_xid } } + it 'does not delete runner manager nor runner' do + delete_request - it 'deletes runner manager' do - expect do - delete_request - - expect(response).to have_gitlab_http_status(:no_content) - end.to change { runner.runner_managers.count }.from(1).to(0) - - expect(::Ci::Runner.count).to eq(1) - end - - it_behaves_like '412 response' do - let(:request) { api('/runners/managers') } - let(:params) { delete_params } - end - - it_behaves_like 'storing arguments in the application context for the API' do - let(:expected_params) { { client_id: "runner/#{runner.id}" } } - end + expect(response).to have_gitlab_http_status(:bad_request) end + end - context 'with unknown system_id' do - let(:delete_params) { { token: runner.token, system_id: 'unknown_system_id' } } + context 'when no token is provided' do + let(:delete_params) { { system_id: runner.runner_managers.first.system_xid } } - it 'returns 404 error' do - delete_request + it 'returns 400 error' do + delete_request - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:bad_request) end + end - context 'without system_id' do - let(:delete_params) { { token: runner.token } } + context 'when invalid token is provided' do + let(:delete_params) { { token: 'invalid', system_id: runner.runner_managers.first.system_xid } } - it 'does not delete runner manager nor runner' do - delete_request + it 'returns 403 error' do + delete_request - expect(response).to have_gitlab_http_status(:bad_request) - end + expect(response).to have_gitlab_http_status(:forbidden) end end end diff --git a/spec/requests/oauth/authorizations_controller_spec.rb b/spec/requests/oauth/authorizations_controller_spec.rb index 6ef8970a142..a4333a17dc8 100644 --- a/spec/requests/oauth/authorizations_controller_spec.rb +++ b/spec/requests/oauth/authorizations_controller_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe Oauth::AuthorizationsController, feature_category: :system_access do - let_it_be(:user) { create(:user) } +RSpec.describe Oauth::AuthorizationsController, :with_current_organization, feature_category: :system_access do + let_it_be(:user) { create(:user, organizations: [current_organization]) } let_it_be(:application) { create(:oauth_application, redirect_uri: 'custom://test') } let(:params) do diff --git a/spec/scripts/semgrep_result_processor_spec.rb b/spec/scripts/semgrep_result_processor_spec.rb index fbbf31fffa9..9daf0029a1b 100644 --- a/spec/scripts/semgrep_result_processor_spec.rb +++ b/spec/scripts/semgrep_result_processor_spec.rb @@ -15,6 +15,7 @@ RSpec.describe SemgrepResultProcessor, feature_category: :tooling do stub_env('CI_API_V4_URL', 'https://gitlab.com/api/v4') stub_env('CI_MERGE_REQUEST_PROJECT_ID', '1234') stub_env('CI_MERGE_REQUEST_IID', '1234') + stub_env('CI_MERGE_REQUEST_LABELS', '') stub_env('CUSTOM_SAST_RULES_BOT_PAT', 'gl-pat-123') stub_env('BOT_USER_ID', '21564538') stub_request(:any, /gitlab.com/).to_return(status: 400) diff --git a/spec/services/ci/runners/unregister_runner_manager_service_spec.rb b/spec/services/ci/runners/unregister_runner_manager_service_spec.rb index 83b8a5473e7..367cff7863a 100644 --- a/spec/services/ci/runners/unregister_runner_manager_service_spec.rb +++ b/spec/services/ci/runners/unregister_runner_manager_service_spec.rb @@ -74,7 +74,7 @@ RSpec.describe ::Ci::Runners::UnregisterRunnerManagerService, '#execute', :freez it 'returns error and leaves runner_manager1', :aggregate_failures do expect do expect(execute).to be_error - expect(execute.message).to eq('`system_id` needs to be specified for runners created in the UI.') + expect(execute.message).to eq('`system_id` needs to be specified.') end.to not_change { Ci::Runner.count } .and not_change { Ci::RunnerManager.count } end diff --git a/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb b/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb index 738d9453f78..dddda208745 100644 --- a/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb +++ b/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb @@ -2,7 +2,7 @@ RSpec.shared_examples 'Secure OAuth Authorizations' do context 'when user is confirmed' do - let(:user) { create(:user) } + let_it_be(:user) { create(:user, organizations: [current_organization]) } it 'asks the user to authorize the application' do expect(page).to have_text "#{application.name} is requesting access to your account on" @@ -10,7 +10,7 @@ RSpec.shared_examples 'Secure OAuth Authorizations' do end context 'when user is unconfirmed' do - let(:user) { create(:user, :unconfirmed) } + let_it_be(:user) { create(:user, :unconfirmed) } it 'displays an error' do expect(page).to have_text I18n.t('doorkeeper.errors.messages.unconfirmed_email')