Skip to content

Commit 1fbb439

Browse files
ZogStriPZogStriP
authored
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
1 parent b12f153 commit 1fbb439

File tree

3 files changed

+144
-37
lines changed

3 files changed

+144
-37
lines changed

‎app/assets/javascripts/discourse/app/routes/login.js

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export default class extends DiscourseRoute {
2929
const { pathname: url } = window.location;
3030
const { referrer } = document;
3131
const { isOnlyOneExternalLoginMethod, singleExternalLogin } = this.login;
32+
const redirect = auth_immediately || login_required || !from || wantsTo;
3233

3334
// Regular users can't log in but staff can when the site is read-only
3435
if (isReadOnly && !isStaffWritesOnly) {
@@ -56,18 +57,19 @@ export default class extends DiscourseRoute {
5657
}
5758
}
5859

59-
// When Discourse Connect is enabled, redirect to the SSO endpoint
60-
if (auth_immediately && enable_discourse_connect) {
61-
const returnPath = cookie("destination_url")
62-
? getURL("/")
63-
: encodeURIComponent(url);
64-
window.location = getURL(`/session/sso?return_path=${returnPath}`);
65-
return;
66-
}
67-
6860
// Automatically kick off the external login if it's the only one available
69-
if (isOnlyOneExternalLoginMethod) {
70-
if (auth_immediately || login_required || !from || wantsTo) {
61+
if (enable_discourse_connect) {
62+
if (redirect) {
63+
this.#isRedirecting = true;
64+
const returnPath = cookie("destination_url")
65+
? getURL("/")
66+
: encodeURIComponent(url);
67+
window.location = getURL(`/session/sso?return_path=${returnPath}`);
68+
} else {
69+
router.replaceWith("discovery.login-required");
70+
}
71+
} else if (isOnlyOneExternalLoginMethod) {
72+
if (redirect) {
7173
this.#isRedirecting = true;
7274
singleExternalLogin();
7375
} else {
@@ -77,8 +79,6 @@ export default class extends DiscourseRoute {
7779
}
7880

7981
setupController(controller) {
80-
const { enable_discourse_connect } = this.siteSettings;
81-
8282
super.setupController(...arguments);
8383

8484
// We're in the middle of an authentication flow
@@ -87,7 +87,6 @@ export default class extends DiscourseRoute {
8787
}
8888

8989
// Shows the loading spinner while waiting for the redirection to external auth
90-
controller.isRedirectingToExternalAuth =
91-
this.#isRedirecting || enable_discourse_connect;
90+
controller.isRedirectingToExternalAuth = this.#isRedirecting;
9291
}
9392
}

‎app/assets/javascripts/discourse/app/routes/signup.js

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export default class extends DiscourseRoute {
3535
const { referrer } = document;
3636
const { canSignUp } = this.controllerFor("application");
3737
const { isOnlyOneExternalLoginMethod, singleExternalLogin } = this.login;
38+
const redirect = auth_immediately || login_required || !from || wantsTo;
3839

3940
// Can't sign up when the site is read-only
4041
if (isReadOnly) {
@@ -69,18 +70,19 @@ export default class extends DiscourseRoute {
6970
}
7071
}
7172

72-
// When Discourse Connect is enabled, redirect to the SSO endpoint
73-
if (auth_immediately && enable_discourse_connect) {
74-
const returnPath = cookie("destination_url")
75-
? getURL("/")
76-
: encodeURIComponent(url);
77-
window.location = getURL(`/session/sso?return_path=${returnPath}`);
78-
return;
79-
}
80-
8173
// Automatically kick off the external login if it's the only one available
82-
if (isOnlyOneExternalLoginMethod) {
83-
if (auth_immediately || login_required || !from || wantsTo) {
74+
if (enable_discourse_connect) {
75+
if (redirect) {
76+
this.#isRedirecting = true;
77+
const returnPath = cookie("destination_url")
78+
? getURL("/")
79+
: encodeURIComponent(url);
80+
window.location = getURL(`/session/sso?return_path=${returnPath}`);
81+
} else {
82+
router.replaceWith("discovery.login-required");
83+
}
84+
} else if (isOnlyOneExternalLoginMethod) {
85+
if (redirect) {
8486
this.#isRedirecting = true;
8587
singleExternalLogin({ signup: true });
8688
} else {
@@ -90,8 +92,6 @@ export default class extends DiscourseRoute {
9092
}
9193

9294
setupController(controller) {
93-
const { enable_discourse_connect } = this.siteSettings;
94-
9595
super.setupController(...arguments);
9696

9797
// We're in the middle of an authentication flow
@@ -100,7 +100,6 @@ export default class extends DiscourseRoute {
100100
}
101101

102102
// Shows the loading spinner while waiting for the redirection to external auth
103-
controller.isRedirectingToExternalAuth =
104-
this.#isRedirecting || enable_discourse_connect;
103+
controller.isRedirectingToExternalAuth = this.#isRedirecting;
105104
}
106105
}

‎spec/system/discourse_connect_spec.rb

Lines changed: 116 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,24 +13,133 @@
1313
fab!(:private_topic) { Fabricate(:topic, category: private_category) }
1414
fab!(:private_post) { Fabricate(:post, topic: private_topic) }
1515

16+
fab!(:topic)
17+
fab!(:post) { Fabricate(:post, topic:) }
18+
1619
before do
1720
setup_test_sso_server
1821
configure_discourse_connect
1922
end
2023

2124
after { shutdown_test_sso_server }
2225

23-
context "when auth_immediately is enabled" do
24-
before { SiteSetting.auth_immediately = true }
26+
shared_examples "redirects to SSO" do
27+
it "redirects to SSO" do
28+
wait_for { has_css?("#current-user") }
29+
expect(page).to have_css("a[data-topic-id='#{private_topic.id}']")
30+
end
31+
end
2532

26-
it "redirects the user back to the landing URL" do
27-
visit private_topic.url
33+
shared_examples "shows the homepage" do
34+
it "shows the homepage" do
35+
expect(page).to have_css("a[data-topic-id='#{topic.id}']")
36+
end
37+
end
2838

29-
find(".login-button").click
39+
shared_examples "shows the login splash" do
40+
it "shows the login splash" do
41+
expect(page).to have_css(".login-page")
42+
end
43+
end
3044

31-
wait_for { has_css?("#current-user") }
45+
context "when login_required is false" do
46+
before { SiteSetting.login_required = false }
47+
48+
context "when auth_immediately is false" do
49+
before { SiteSetting.auth_immediately = false }
50+
51+
context "when visiting /" do
52+
before { visit "/" }
53+
it_behaves_like "shows the homepage"
54+
end
55+
56+
context "when visiting / and clicking the login button" do
57+
before do
58+
visit "/"
59+
find(".login-button").click
60+
end
61+
62+
it_behaves_like "redirects to SSO"
63+
end
64+
65+
context "when visiting /login" do
66+
before { visit "/login" }
67+
it_behaves_like "redirects to SSO"
68+
end
69+
end
70+
71+
context "when auth_immediately is true" do
72+
before { SiteSetting.auth_immediately = true }
73+
74+
context "when visiting /" do
75+
before { visit "/" }
76+
it_behaves_like "shows the homepage"
77+
end
78+
79+
context "when visiting / and clicking the login button" do
80+
before do
81+
visit "/"
82+
find(".login-button").click
83+
end
84+
85+
it_behaves_like "redirects to SSO"
86+
end
87+
88+
context "when visiting /login" do
89+
before { visit "/login" }
90+
it_behaves_like "redirects to SSO"
91+
end
92+
93+
it "redirects the user back to the landing URL" do
94+
visit private_topic.url
95+
96+
find(".login-button").click
97+
98+
wait_for { has_css?("#current-user") }
99+
100+
expect(page).to have_current_path(private_topic.relative_url)
101+
end
102+
end
103+
end
104+
105+
context "when login_required is true" do
106+
before { SiteSetting.login_required = true }
107+
108+
context "when auth_immediately is false" do
109+
before { SiteSetting.auth_immediately = false }
110+
111+
context "when visiting /" do
112+
before { visit "/" }
113+
it_behaves_like "shows the login splash"
114+
end
115+
116+
context "when visiting / and clicking the login button" do
117+
before do
118+
visit "/"
119+
find(".login-button").click
120+
end
121+
122+
it_behaves_like "redirects to SSO"
123+
end
124+
125+
context "when visiting /login" do
126+
before { visit "/login" }
127+
it_behaves_like "redirects to SSO"
128+
end
129+
end
130+
131+
context "when auth_immediately is true" do
132+
before { SiteSetting.auth_immediately = true }
133+
134+
context "when visiting /" do
135+
before { visit "/" }
136+
it_behaves_like "redirects to SSO"
137+
end
32138

33-
expect(page).to have_current_path(private_topic.relative_url)
139+
context "when visiting /login" do
140+
before { visit "/login" }
141+
it_behaves_like "redirects to SSO"
142+
end
34143
end
35144
end
36145

0 commit comments

Comments
 (0)