From 7b5ccadd5e8f1e0f6f04b50d5386e580933b78e6 Mon Sep 17 00:00:00 2001 From: Rodrigo Tomonari Date: Fri, 17 Oct 2025 16:25:23 -0300 Subject: [PATCH 1/5] Fix infinite redirect loop in Gitea importer The current implementation applies a rate limit to the status endpoint to limit repository list fetches from Gitea per minute. This change increases the rate limit threshold to prevent regular users from being rate limited during normal usage. This resolves the infinite redirect issue and improves the overall user experience. Changelog: fixed --- app/controllers/import/gitea_controller.rb | 7 ++----- app/controllers/import/github_controller.rb | 11 ++++++---- lib/gitlab/application_rate_limiter.rb | 2 +- locale/gitlab.pot | 3 +++ .../import/gitea_controller_spec.rb | 21 ++++++++++++------- .../import/github_controller_spec.rb | 8 +++---- ...ubish_import_controller_shared_examples.rb | 8 +++---- 7 files changed, 34 insertions(+), 26 deletions(-) diff --git a/app/controllers/import/gitea_controller.rb b/app/controllers/import/gitea_controller.rb index f030d47a6dbd9a..992ab00cb2c284 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 c84a8c27e40e77..b10db5f6745e61 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,13 @@ 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: :found } + format.html { redirect_to new_import_url } + end end def provider_rate_limit(exception) diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index 1159c3d93f923d..6f04cadeb127c2 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 0a796fc2cc234f..6190ba43050bdc 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 a3c3c9e430dd0c..dc935364a4012d 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 df0a8d0ef7485a..499c4879641797 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 c3539fc04a36c8..1ea40f47d0e2a0 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) -- GitLab From 22ef247b31e5612442ce42bc4d62c626eed7ac4e Mon Sep 17 00:00:00 2001 From: Rodrigo Tomonari Date: Fri, 17 Oct 2025 16:55:24 -0300 Subject: [PATCH 2/5] Remove extra empty lines --- app/controllers/import/github_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index b10db5f6745e61..f03f62323babfd 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -235,9 +235,7 @@ def status_import_url def provider_unauthorized session[access_token_key] = nil - flash[:alert] = s_('Import|Invalid credentials') - respond_to do |format| format.json { render json: { error: { redirect: new_import_url } }, status: :found } format.html { redirect_to new_import_url } -- GitLab From a2ebd778ae7885469c9b5cd7527666beea68a2b4 Mon Sep 17 00:00:00 2001 From: Rodrigo Tomonari Date: Mon, 20 Oct 2025 13:19:12 -0300 Subject: [PATCH 3/5] Update flash message in e2e --- ee/spec/features/projects/new_project_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/features/projects/new_project_spec.rb b/ee/spec/features/projects/new_project_spec.rb index 5eafa85d3df86d..69ed050bc556e1 100644 --- a/ee/spec/features/projects/new_project_spec.rb +++ b/ee/spec/features/projects/new_project_spec.rb @@ -159,7 +159,7 @@ fill_in 'personal_access_token', with: 'unauthorized-fake-token' click_button 'Authenticate' - expect(page).to have_text('Wrong credentials') + expect(page).to have_text('Invalid credentials') expect(page).to have_current_path(new_import_github_path(ci_cd_only: true)) end end -- GitLab From b29bbfec9abf6da0c7863445a4355d0014a3adfb Mon Sep 17 00:00:00 2001 From: Rodrigo Tomonari Date: Wed, 22 Oct 2025 00:47:02 -0300 Subject: [PATCH 4/5] Send ci_cd param in async request --- .../import_entities/import_projects/store/actions.js | 6 ++++-- ee/spec/features/projects/new_project_spec.rb | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) 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 314c0e6111c148..59570567428ed6 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/ee/spec/features/projects/new_project_spec.rb b/ee/spec/features/projects/new_project_spec.rb index 69ed050bc556e1..c49a89ac5acc37 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('Invalid credentials') expect(page).to have_current_path(new_import_github_path(ci_cd_only: true)) end end -- GitLab From 1d51d28d75b36a82b1766ef3083a6e4bee80e8b3 Mon Sep 17 00:00:00 2001 From: Rodrigo Tomonari Date: Wed, 22 Oct 2025 12:06:09 -0300 Subject: [PATCH 5/5] Return not found status --- app/controllers/import/github_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index f03f62323babfd..6de101fce26081 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -237,7 +237,7 @@ def provider_unauthorized session[access_token_key] = nil flash[:alert] = s_('Import|Invalid credentials') respond_to do |format| - format.json { render json: { error: { redirect: new_import_url } }, status: :found } + format.json { render json: { error: { redirect: new_import_url } }, status: :not_found } format.html { redirect_to new_import_url } end end -- GitLab