FIX: DiscourseConnect & SiteSetting.auth_immediately = false (#34424)

DiscourseConnect "required" auth_immediately to also be enabled to work properly but this was incorrect.

This fixes that assumption and add specs to cover all combinations of

- login_required
- auth_immediately
- visiting /
- visiting / and clicking the login button
- visiting /login directly

Internal ref - t/161485
This commit is contained in:
Régis Hanol
2025-08-20 14:36:04 +02:00
committed by GitHub
parent b12f153a9e
commit 1fbb43925d
3 changed files with 146 additions and 39 deletions
@@ -29,6 +29,7 @@ export default class extends DiscourseRoute {
const { pathname: url } = window.location;
const { referrer } = document;
const { isOnlyOneExternalLoginMethod, singleExternalLogin } = this.login;
const redirect = auth_immediately || login_required || !from || wantsTo;
// Regular users can't log in but staff can when the site is read-only
if (isReadOnly && !isStaffWritesOnly) {
@@ -56,18 +57,19 @@ export default class extends DiscourseRoute {
}
}
// When Discourse Connect is enabled, redirect to the SSO endpoint
if (auth_immediately && enable_discourse_connect) {
const returnPath = cookie("destination_url")
? getURL("/")
: encodeURIComponent(url);
window.location = getURL(`/session/sso?return_path=${returnPath}`);
return;
}
// Automatically kick off the external login if it's the only one available
if (isOnlyOneExternalLoginMethod) {
if (auth_immediately || login_required || !from || wantsTo) {
if (enable_discourse_connect) {
if (redirect) {
this.#isRedirecting = true;
const returnPath = cookie("destination_url")
? getURL("/")
: encodeURIComponent(url);
window.location = getURL(`/session/sso?return_path=${returnPath}`);
} else {
router.replaceWith("discovery.login-required");
}
} else if (isOnlyOneExternalLoginMethod) {
if (redirect) {
this.#isRedirecting = true;
singleExternalLogin();
} else {
@@ -77,8 +79,6 @@ export default class extends DiscourseRoute {
}
setupController(controller) {
const { enable_discourse_connect } = this.siteSettings;
super.setupController(...arguments);
// We're in the middle of an authentication flow
@@ -87,7 +87,6 @@ export default class extends DiscourseRoute {
}
// Shows the loading spinner while waiting for the redirection to external auth
controller.isRedirectingToExternalAuth =
this.#isRedirecting || enable_discourse_connect;
controller.isRedirectingToExternalAuth = this.#isRedirecting;
}
}
@@ -35,6 +35,7 @@ export default class extends DiscourseRoute {
const { referrer } = document;
const { canSignUp } = this.controllerFor("application");
const { isOnlyOneExternalLoginMethod, singleExternalLogin } = this.login;
const redirect = auth_immediately || login_required || !from || wantsTo;
// Can't sign up when the site is read-only
if (isReadOnly) {
@@ -69,18 +70,19 @@ export default class extends DiscourseRoute {
}
}
// When Discourse Connect is enabled, redirect to the SSO endpoint
if (auth_immediately && enable_discourse_connect) {
const returnPath = cookie("destination_url")
? getURL("/")
: encodeURIComponent(url);
window.location = getURL(`/session/sso?return_path=${returnPath}`);
return;
}
// Automatically kick off the external login if it's the only one available
if (isOnlyOneExternalLoginMethod) {
if (auth_immediately || login_required || !from || wantsTo) {
if (enable_discourse_connect) {
if (redirect) {
this.#isRedirecting = true;
const returnPath = cookie("destination_url")
? getURL("/")
: encodeURIComponent(url);
window.location = getURL(`/session/sso?return_path=${returnPath}`);
} else {
router.replaceWith("discovery.login-required");
}
} else if (isOnlyOneExternalLoginMethod) {
if (redirect) {
this.#isRedirecting = true;
singleExternalLogin({ signup: true });
} else {
@@ -90,8 +92,6 @@ export default class extends DiscourseRoute {
}
setupController(controller) {
const { enable_discourse_connect } = this.siteSettings;
super.setupController(...arguments);
// We're in the middle of an authentication flow
@@ -100,7 +100,6 @@ export default class extends DiscourseRoute {
}
// Shows the loading spinner while waiting for the redirection to external auth
controller.isRedirectingToExternalAuth =
this.#isRedirecting || enable_discourse_connect;
controller.isRedirectingToExternalAuth = this.#isRedirecting;
}
}
+118 -9
View File
@@ -13,6 +13,9 @@ describe "Discourse Connect", type: :system do
fab!(:private_topic) { Fabricate(:topic, category: private_category) }
fab!(:private_post) { Fabricate(:post, topic: private_topic) }
fab!(:topic)
fab!(:post) { Fabricate(:post, topic:) }
before do
setup_test_sso_server
configure_discourse_connect
@@ -20,17 +23,123 @@ describe "Discourse Connect", type: :system do
after { shutdown_test_sso_server }
context "when auth_immediately is enabled" do
before { SiteSetting.auth_immediately = true }
it "redirects the user back to the landing URL" do
visit private_topic.url
find(".login-button").click
shared_examples "redirects to SSO" do
it "redirects to SSO" do
wait_for { has_css?("#current-user") }
expect(page).to have_css("a[data-topic-id='#{private_topic.id}']")
end
end
expect(page).to have_current_path(private_topic.relative_url)
shared_examples "shows the homepage" do
it "shows the homepage" do
expect(page).to have_css("a[data-topic-id='#{topic.id}']")
end
end
shared_examples "shows the login splash" do
it "shows the login splash" do
expect(page).to have_css(".login-page")
end
end
context "when login_required is false" do
before { SiteSetting.login_required = false }
context "when auth_immediately is false" do
before { SiteSetting.auth_immediately = false }
context "when visiting /" do
before { visit "/" }
it_behaves_like "shows the homepage"
end
context "when visiting / and clicking the login button" do
before do
visit "/"
find(".login-button").click
end
it_behaves_like "redirects to SSO"
end
context "when visiting /login" do
before { visit "/login" }
it_behaves_like "redirects to SSO"
end
end
context "when auth_immediately is true" do
before { SiteSetting.auth_immediately = true }
context "when visiting /" do
before { visit "/" }
it_behaves_like "shows the homepage"
end
context "when visiting / and clicking the login button" do
before do
visit "/"
find(".login-button").click
end
it_behaves_like "redirects to SSO"
end
context "when visiting /login" do
before { visit "/login" }
it_behaves_like "redirects to SSO"
end
it "redirects the user back to the landing URL" do
visit private_topic.url
find(".login-button").click
wait_for { has_css?("#current-user") }
expect(page).to have_current_path(private_topic.relative_url)
end
end
end
context "when login_required is true" do
before { SiteSetting.login_required = true }
context "when auth_immediately is false" do
before { SiteSetting.auth_immediately = false }
context "when visiting /" do
before { visit "/" }
it_behaves_like "shows the login splash"
end
context "when visiting / and clicking the login button" do
before do
visit "/"
find(".login-button").click
end
it_behaves_like "redirects to SSO"
end
context "when visiting /login" do
before { visit "/login" }
it_behaves_like "redirects to SSO"
end
end
context "when auth_immediately is true" do
before { SiteSetting.auth_immediately = true }
context "when visiting /" do
before { visit "/" }
it_behaves_like "redirects to SSO"
end
context "when visiting /login" do
before { visit "/login" }
it_behaves_like "redirects to SSO"
end
end
end