diff --git a/app/assets/javascripts/import_entities/import_projects/store/actions.js b/app/assets/javascripts/import_entities/import_projects/store/actions.js index 314c0e6111c148822e039a6385694f632c58daad..59570567428ed69dc348c8cd5a1fcbabe1b17d86 100644 --- a/app/assets/javascripts/import_entities/import_projects/store/actions.js +++ b/app/assets/javascripts/import_entities/import_projects/store/actions.js @@ -89,13 +89,15 @@ const fetchReposFactory = ({ state, commit }) => { commit(types.REQUEST_REPOS); - const { provider, filter } = state; + const { provider, filter, ciCdOnly } = state; + const ciCdFilter = ciCdOnly ? { ci_cd_only: ciCdOnly } : {}; return axios .get( pathWithParams({ path: reposPath, ...filter, + ...ciCdFilter, ...paginationParams({ state }), }), ) @@ -106,7 +108,7 @@ const fetchReposFactory = }) .catch((e) => { if (hasRedirectInError(e)) { - redirectToUrlInError(e); + window.location.href = e.response.data.error.redirect; } else if (tooManyRequests(e)) { createAlert({ message: sprintf( diff --git a/app/controllers/import/gitea_controller.rb b/app/controllers/import/gitea_controller.rb index f030d47a6dbd9a0e5a0cde269ba0da6b5c590da3..992ab00cb2c284ae387271e88838dd84e1334c24 100644 --- a/app/controllers/import/gitea_controller.rb +++ b/app/controllers/import/gitea_controller.rb @@ -3,7 +3,8 @@ class Import::GiteaController < Import::GithubController extend ::Gitlab::Utils::Override - before_action -> { check_rate_limit!(:gitea_import, scope: current_user, redirect_back: true) }, only: :status + before_action -> { check_rate_limit!(:gitea_import, scope: current_user) }, + only: :status, if: -> { request.format.json? } before_action :verify_blocked_uri, only: :status def new @@ -16,10 +17,6 @@ def personal_access_token end def status - # Request repos to display error page if provider token is invalid - # Improving in https://gitlab.com/gitlab-org/gitlab/-/issues/25859 - client_repos - respond_to do |format| format.json do render json: { imported_projects: serialized_imported_projects, diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index c84a8c27e40e7730d852b81f8d482410d6ecbf22..6de101fce26081b5c825275f55afd09310d0ad04 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -48,8 +48,6 @@ def personal_access_token end def status - client_repos - respond_to do |format| format.json do render json: { imported_projects: serialized_imported_projects, @@ -237,8 +235,11 @@ def status_import_url def provider_unauthorized session[access_token_key] = nil - redirect_to new_import_url, - alert: "Wrong credentials" + flash[:alert] = s_('Import|Invalid credentials') + respond_to do |format| + format.json { render json: { error: { redirect: new_import_url } }, status: :not_found } + format.html { redirect_to new_import_url } + end end def provider_rate_limit(exception) diff --git a/ee/spec/features/projects/new_project_spec.rb b/ee/spec/features/projects/new_project_spec.rb index 5eafa85d3df86d8cc43a755745d6a2874dc256ef..c49a89ac5acc37f147e4bb3576b59802d7427e1d 100644 --- a/ee/spec/features/projects/new_project_spec.rb +++ b/ee/spec/features/projects/new_project_spec.rb @@ -144,7 +144,7 @@ expect(created_project.project_feature).not_to be_issues_enabled end - it 'stays on GitHub import page after access token failure' do + it 'redirects to configuration page after access token failure' do visit new_project_path click_link 'Run CI/CD for external repository' @@ -159,7 +159,6 @@ fill_in 'personal_access_token', with: 'unauthorized-fake-token' click_button 'Authenticate' - expect(page).to have_text('Wrong credentials') expect(page).to have_current_path(new_import_github_path(ci_cd_only: true)) end end diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index 1159c3d93f923da91c15897f05aa113558938856..6f04cadeb127c2872cff939620bb8481f1992e3e 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -39,7 +39,7 @@ def rate_limits # rubocop:disable Metrics/AbcSize fetch_google_ip_list: { threshold: 10, interval: 1.minute }, github_import: { threshold: 6, interval: 1.minute }, fogbugz_import: { threshold: 1, interval: 1.minute }, - gitea_import: { threshold: 1, interval: 1.minute }, + gitea_import: { threshold: 6, interval: 1.minute }, gitlab_shell_operation: { threshold: application_settings.gitlab_shell_operation_limit, interval: 1.minute }, glql: { threshold: 1, interval: 15.minutes }, group_api: { threshold: -> { application_settings.group_api_limit }, interval: 1.minute }, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0a796fc2cc234fe19eda17dda5868fb24b040dbd..6190ba43050bdc0386fc3522b2ed656658d1fe0a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -34484,6 +34484,9 @@ msgstr "" msgid "Import|In progress" msgstr "" +msgid "Import|Invalid credentials" +msgstr "" + msgid "Import|Maximum decompressed file size for archives from imports (MiB)" msgstr "" diff --git a/spec/controllers/import/gitea_controller_spec.rb b/spec/controllers/import/gitea_controller_spec.rb index a3c3c9e430dd0c86e909997b3d19e5eb37d7e22c..dc935364a4012d40445216a9f88e7f565eca1f32 100644 --- a/spec/controllers/import/gitea_controller_spec.rb +++ b/spec/controllers/import/gitea_controller_spec.rb @@ -30,22 +30,29 @@ def assign_host_url it_behaves_like 'a GitHub-ish import controller: POST personal_access_token' end - describe "GET status" do + describe "GET status", :clean_gitlab_redis_rate_limiting do it_behaves_like 'a GitHub-ish import controller: GET status' do let(:extra_assign_expectations) { { gitea_host_url: host_url } } before do - allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false) assign_host_url end it "requests provider repos list" do - expect(stub_client(repos: [], orgs: [])).to receive(:repos) - get :status expect(response).to have_gitlab_http_status(:ok) end + end + end + + describe 'GET status.json', :clean_gitlab_redis_rate_limiting do + it_behaves_like 'a GitHub-ish import controller: GET status' do + let(:extra_assign_expectations) { { gitea_host_url: host_url } } + + before do + assign_host_url + end shared_examples "unacceptable url" do |url, expected_error| let(:host_url) { url } @@ -107,7 +114,7 @@ def assign_host_url end end - it_behaves_like 'rate limited endpoint', rate_limit_key: :gitea_import, with_redirect: true do + it_behaves_like 'rate limited endpoint', rate_limit_key: :gitea_import do let_it_be(:second_user) { create(:user) } let(:token) { 'gitea token' } @@ -120,14 +127,14 @@ def assign_host_url end def request - get :status + get :status, format: :json end def request_with_second_scope sign_in(second_user) session[:gitea_access_token] = token assign_host_url - get :status + get :status, format: :json end end end diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index df0a8d0ef7485a63ae1617155d2ca48de8294e79..499c4879641797a552adeecd17499056536aac00 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -59,7 +59,7 @@ get :callback expect(controller).to redirect_to(new_import_url) - expect(flash[:alert]).to eq('Wrong credentials') + expect(flash[:alert]).to eq('Invalid credentials') end end @@ -77,7 +77,7 @@ get :callback, params: { state: "different-state" } expect(controller).to redirect_to(new_import_url) - expect(flash[:alert]).to eq('Wrong credentials') + expect(flash[:alert]).to eq('Invalid credentials') end it "updates access token if state param is valid" do @@ -179,8 +179,8 @@ get :status, format: :json expect(session[:"#{provider}_access_token"]).to be_nil - expect(controller).to redirect_to(new_import_url) - expect(flash[:alert]).to eq("Wrong credentials") + expect(json_response.dig("error", "redirect")).to eq(new_import_url) + expect(flash[:alert]).to eq("Invalid credentials") end end diff --git a/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb b/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb index c3539fc04a36c8532ec53c8554639d8eaf6f024f..1ea40f47d0e2a03f86dcc8bfb6777750c99e390d 100644 --- a/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb @@ -112,11 +112,11 @@ def assign_session_token(provider) allow(client).to receive(:repos).and_raise(Octokit::Unauthorized) allow(client).to receive(:each_page).and_raise(Octokit::Unauthorized) - get :status + get :status, format: :json expect(session[:"#{provider}_access_token"]).to be_nil - expect(controller).to redirect_to(new_import_url) - expect(flash[:alert]).to eq("Wrong credentials") + expect(json_response.dig("error", "redirect")).to eq(new_import_url) + expect(flash[:alert]).to eq("Invalid credentials") end it "does not produce N+1 database queries" do @@ -143,8 +143,6 @@ def assign_session_token(provider) let!(:group) { create(:group, developers: user) } it 'returns 404' do - expect(stub_client(repos: [], orgs: [])).to receive(:repos) - get :status, params: { namespace_id: group.id }, format: :html expect(response).to have_gitlab_http_status(:not_found)