feat: improve incorrect ROOT_URL warning (#7103)
Some checks are pending
/ release (push) Waiting to run
testing / backend-checks (push) Waiting to run
testing / frontend-checks (push) Waiting to run
testing / test-unit (push) Blocked by required conditions
testing / test-e2e (push) Blocked by required conditions
testing / test-remote-cacher (redis) (push) Blocked by required conditions
testing / test-remote-cacher (valkey) (push) Blocked by required conditions
testing / test-remote-cacher (garnet) (push) Blocked by required conditions
testing / test-remote-cacher (redict) (push) Blocked by required conditions
testing / test-mysql (push) Blocked by required conditions
testing / test-pgsql (push) Blocked by required conditions
testing / test-sqlite (push) Blocked by required conditions
testing / security-check (push) Blocked by required conditions

- In the case that the `ROOT_URL` does not match the site a person is visiting Forgejo gives zero guarantees that any of the functionality will still work.
- Make the error i18n, use `local_next`.
- Reflect in the error that the any part of the application can break, don't be specific - it is plain wrong and should not be used.
- Always check for this case on the login page. This was previously only the case if OAuth2 was enabled, but this code was checking for elements that are always present on the login page regardless if the OAuth2 was enabled or not. Technically nothing changed, but reading the code it is now more clear when this check is being run.
- Add E2E testing.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7103
Reviewed-by: Otto <otto@codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Gusted 2025-03-03 18:05:01 +00:00 committed by 0ko
parent 8971321083
commit 4c641b0fb2
6 changed files with 45 additions and 6 deletions

View file

@ -12,5 +12,6 @@
"one": "wants to merge %[1]d commit from <code>%[2]s</code> into <code id=\"%[4]s\">%[3]s</code>",
"other": "wants to merge %[1]d commits from <code>%[2]s</code> into <code id=\"%[4]s\">%[3]s</code>"
},
"search.milestone_kind": "Search milestones..."
"search.milestone_kind": "Search milestones...",
"incorrect_root_url": "This Forgejo instance is configured to be served on \"%s\". You are currently viewing Forgejo through a different URL, which may cause parts of the application to break. The canonical URL is controlled by Forgejo admins via the ROOT_URL setting in the app.ini."
}

View file

@ -42,6 +42,7 @@ If you introduce mistakes in it, Gitea JavaScript code wouldn't run correctly.
modal_confirm: {{ctx.Locale.Tr "modal.confirm"}},
modal_cancel: {{ctx.Locale.Tr "modal.cancel"}},
more_items: {{ctx.Locale.Tr "more_items"}},
incorrect_root_url: {{ctx.Locale.Tr "incorrect_root_url" AppUrl}},
},
};
{{/* in case some pages don't render the pageData, we make sure it is an object to prevent null access */}}

View file

@ -0,0 +1,33 @@
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: GPL-3.0-or-later
// @watch start
// templates/user/auth/**
// web_src/js/features/user-**
// web_src/js/features/common-global.js
// @watch end
import {expect} from '@playwright/test';
import {test, save_visual, test_context} from './utils_e2e.ts';
test('Mismatched ROOT_URL', async ({browser}) => {
const context = await test_context(browser);
const page = await context.newPage();
// Ugly hack to override the appUrl of `window.config`.
await page.addInitScript(() => {
setInterval(() => {
if (window.config) {
window.config.appUrl = 'https://example.com';
}
}, 1);
});
const response = await page.goto('/user/login');
expect(response?.status()).toBe(200);
await save_visual(page);
const globalError = page.locator('.js-global-error');
await expect(globalError).toContainText('This Forgejo instance is configured to be served on ');
await expect(globalError).toContainText('You are currently viewing Forgejo through a different URL, which may cause parts of the application to break. The canonical URL is controlled by Forgejo admins via the ROOT_URL setting in the app.ini.');
});

View file

@ -458,6 +458,6 @@ export function checkAppUrl() {
if (curUrl.startsWith(appUrl) || `${curUrl}/` === appUrl) {
return;
}
showGlobalErrorMessage(`Your ROOT_URL in app.ini is "${appUrl}", it's unlikely matching the site you are visiting.
Mismatched ROOT_URL config causes wrong URL links for web UI/mail content/webhook notification/OAuth2 sign-in.`);
showGlobalErrorMessage(i18n.incorrect_root_url);
}

View file

@ -5,8 +5,6 @@ export function initUserAuthOauth2() {
if (!outer) return;
const inner = document.getElementById('oauth2-login-navigator-inner');
checkAppUrl();
for (const link of outer.querySelectorAll('.oauth-login-link')) {
link.addEventListener('click', () => {
inner.classList.add('tw-invisible');
@ -20,3 +18,8 @@ export function initUserAuthOauth2() {
});
}
}
export function initUserAuth() {
if (!document.querySelector('.user.signin')) return;
checkAppUrl();
}

View file

@ -24,7 +24,7 @@ import {initFindFileInRepo} from './features/repo-findfile.js';
import {initCommentContent, initMarkupContent} from './markup/content.js';
import {initPdfViewer} from './render/pdf.js';
import {initUserAuthOauth2} from './features/user-auth.js';
import {initUserAuthOauth2, initUserAuth} from './features/user-auth.js';
import {
initRepoIssueDue,
initRepoIssueReferenceRepositorySearch,
@ -184,6 +184,7 @@ onDomReady(() => {
initUserAuthOauth2();
initUserAuthWebAuthn();
initUserAuthWebAuthnRegister();
initUserAuth();
initRepoDiffView();
initPdfViewer();
initScopedAccessTokenCategories();