diff --git a/app/assets/javascripts/discourse/app/controllers/discovery/login-required.js b/app/assets/javascripts/discourse/app/controllers/discovery/login-required.js new file mode 100644 index 00000000000..022c1613c14 --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/discovery/login-required.js @@ -0,0 +1,5 @@ +import Controller, { inject as controller } from "@ember/controller"; + +export default class LoginRequiredController extends Controller { + @controller application; +} diff --git a/app/assets/javascripts/discourse/app/lib/homepage-router-overrides.js b/app/assets/javascripts/discourse/app/lib/homepage-router-overrides.js index d8a0def3d53..1076d8528f8 100644 --- a/app/assets/javascripts/discourse/app/lib/homepage-router-overrides.js +++ b/app/assets/javascripts/discourse/app/lib/homepage-router-overrides.js @@ -35,6 +35,7 @@ function rewriteIfNeeded(url, transition) { const intentUrl = transition?.intent?.url; if ( intentUrl?.startsWith(homepageDestination()) || + intentUrl?.startsWith("/login-required") || (transition?.intent.name === `discovery.${defaultHomepage()}` && transition?.intent.queryParams[homepageRewriteParam]) ) { diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index d83c3dcf779..3321c871b8f 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -68,6 +68,7 @@ export default function () { this.route("category", { path: "/c/*category_slug_path_with_id" }); this.route("custom"); + this.route("login-required"); }); this.route("groups", { resetNamespace: true, path: "/g" }, function () { diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js index 81f4bcb7cd8..7454793a2ed 100644 --- a/app/assets/javascripts/discourse/app/routes/application.js +++ b/app/assets/javascripts/discourse/app/routes/application.js @@ -271,9 +271,6 @@ export default class ApplicationRoute extends DiscourseRoute { } else { this.router.transitionTo("login").then((login) => { login.controller.set("canSignUp", this.controller.canSignUp); - if (this.siteSettings.login_required) { - login.controller.set("showLogin", true); - } }); } } diff --git a/app/assets/javascripts/discourse/app/routes/discourse.js b/app/assets/javascripts/discourse/app/routes/discourse.js index 8cbeb029ab1..4ffba9a2c76 100644 --- a/app/assets/javascripts/discourse/app/routes/discourse.js +++ b/app/assets/javascripts/discourse/app/routes/discourse.js @@ -36,13 +36,6 @@ export default class DiscourseRoute extends Route { once(this, this._refreshTitleOnce); } - redirectIfLoginRequired() { - const app = this.controllerFor("application"); - if (app.get("loginRequired")) { - this.router.replaceWith("login"); - } - } - isCurrentUser(user) { if (!this.currentUser) { return false; // the current user is anonymous diff --git a/app/assets/javascripts/discourse/app/routes/discovery-login-required.js b/app/assets/javascripts/discourse/app/routes/discovery-login-required.js new file mode 100644 index 00000000000..927432856dc --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/discovery-login-required.js @@ -0,0 +1,8 @@ +import StaticPage from "discourse/models/static-page"; +import DiscourseRoute from "discourse/routes/discourse"; + +export default class LoginRequiredRoute extends DiscourseRoute { + model() { + return StaticPage.find("login"); + } +} diff --git a/app/assets/javascripts/discourse/app/routes/discovery.js b/app/assets/javascripts/discourse/app/routes/discovery.js index 87d745e4e3c..ca310fd0e95 100644 --- a/app/assets/javascripts/discourse/app/routes/discovery.js +++ b/app/assets/javascripts/discourse/app/routes/discovery.js @@ -11,13 +11,17 @@ export default class DiscoveryRoute extends DiscourseRoute { @service router; @service session; @service site; + @service siteSettings; queryParams = { filter: { refreshModel: true }, }; redirect() { - return this.redirectIfLoginRequired(); + if (this.siteSettings.login_required && !this.currentUser) { + this.router.transitionTo("/login-required?_discourse_homepage_rewrite=1"); + return; + } } beforeModel(transition) { diff --git a/app/assets/javascripts/discourse/app/routes/login.js b/app/assets/javascripts/discourse/app/routes/login.js index ac6d35cc3ff..9077202282d 100644 --- a/app/assets/javascripts/discourse/app/routes/login.js +++ b/app/assets/javascripts/discourse/app/routes/login.js @@ -58,10 +58,6 @@ export default class LoginRoute extends DiscourseRoute { ); } - if (this.siteSettings.login_required) { - controller.set("showLogin", false); - } - if (this.login.isOnlyOneExternalLoginMethod) { if (this.siteSettings.auth_immediately) { controller.set("isRedirectingToExternalAuth", true); diff --git a/app/assets/javascripts/discourse/app/routes/topic.js b/app/assets/javascripts/discourse/app/routes/topic.js index 4ac337ccbdc..e072fda8856 100644 --- a/app/assets/javascripts/discourse/app/routes/topic.js +++ b/app/assets/javascripts/discourse/app/routes/topic.js @@ -48,10 +48,6 @@ export default class TopicRoute extends DiscourseRoute { }; } - redirect() { - return this.redirectIfLoginRequired(); - } - titleToken() { const model = this.modelFor("topic"); if (model) { diff --git a/app/assets/javascripts/discourse/app/templates/discovery/login-required.gjs b/app/assets/javascripts/discourse/app/templates/discovery/login-required.gjs new file mode 100644 index 00000000000..201796744b7 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/discovery/login-required.gjs @@ -0,0 +1,62 @@ +import { hash } from "@ember/helper"; +import { htmlSafe } from "@ember/template"; +import RouteTemplate from "ember-route-template"; +import DButton from "discourse/components/d-button"; +import PluginOutlet from "discourse/components/plugin-outlet"; +import bodyClass from "discourse/helpers/body-class"; +import hideApplicationHeaderButtons from "discourse/helpers/hide-application-header-buttons"; +import hideApplicationSidebar from "discourse/helpers/hide-application-sidebar"; +import routeAction from "discourse/helpers/route-action"; + +export default RouteTemplate( + +); diff --git a/app/assets/javascripts/discourse/app/templates/login.gjs b/app/assets/javascripts/discourse/app/templates/login.gjs index c180854188d..89b0e18dd3e 100644 --- a/app/assets/javascripts/discourse/app/templates/login.gjs +++ b/app/assets/javascripts/discourse/app/templates/login.gjs @@ -1,8 +1,7 @@ import { hash } from "@ember/helper"; import { htmlSafe } from "@ember/template"; import RouteTemplate from "ember-route-template"; -import { and, not, or } from "truth-helpers"; -import DButton from "discourse/components/d-button"; +import { and } from "truth-helpers"; import FlashMessage from "discourse/components/flash-message"; import LocalLoginForm from "discourse/components/local-login-form"; import LoginButtons from "discourse/components/login-buttons"; @@ -14,7 +13,6 @@ import concatClass from "discourse/helpers/concat-class"; import hideApplicationHeaderButtons from "discourse/helpers/hide-application-header-buttons"; import hideApplicationSidebar from "discourse/helpers/hide-application-sidebar"; import loadingSpinner from "discourse/helpers/loading-spinner"; -import routeAction from "discourse/helpers/route-action"; import { i18n } from "discourse-i18n"; export default RouteTemplate( @@ -28,201 +26,143 @@ export default RouteTemplate( {{! authentication method and is being automatically redirected to it }} {{loadingSpinner}} {{else}} - {{#if - (or @controller.showLogin (not @controller.siteSettings.login_required)) - }} - {{! Show the full page login form }} -
- + + +
+ -
- - - {{#if @controller.hasNoLoginOptions}} -
- + {{#if @controller.hasNoLoginOptions}} +
+ - {{else}} - {{#if @controller.site.mobileView}} - - + {{else}} + {{#if @controller.site.mobileView}} + + + + {{#if @controller.showLoginButtons}} + + {{/if}} + {{/if}} + + {{#if @controller.canLoginLocal}} +
+ {{#if @controller.site.desktopView}} + + + + {{/if}} + + {{#if @controller.site.desktopView}} + - - {{#if @controller.showLoginButtons}} + {{/if}} +
+ {{/if}} + + {{#if + (and @controller.showLoginButtons @controller.site.desktopView) + }} + {{#unless @controller.canLoginLocal}} + + {{/unless}} + {{#if @controller.hasAtLeastOneLoginButton}} + + {{#if @controller.site.mobileView}} + {{#unless @controller.hasNoLoginOptions}} + + {{/unless}} + {{/if}}
- - {{else}} - {{! Show the login-required splash screen }} - {{bodyClass "static-login"}} -
-
- -
-
- {{/if}} +
{{/if}} ); diff --git a/app/assets/javascripts/discourse/tests/acceptance/login-required-test.js b/app/assets/javascripts/discourse/tests/acceptance/login-required-test.js index 85c600e0262..f78b0f7ead6 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/login-required-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/login-required-test.js @@ -9,8 +9,8 @@ acceptance("Login Required - Full page login", function (needs) { await visit("/"); assert.strictEqual( currentRouteName(), - "login", - "it redirects them to login" + "discovery.login-required", + "it shows the login required splash" ); await click(".login-button"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/static-test.js b/app/assets/javascripts/discourse/tests/acceptance/static-test.js index c89102f576a..ab4d1852ac6 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/static-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/static-test.js @@ -43,11 +43,19 @@ acceptance("Static pages", function () { test("Login-required page", async function (assert) { this.siteSettings.login_required = true; - await visit("/login"); + await visit("/"); - assert.strictEqual(currentRouteName(), "login"); + assert.strictEqual(currentRouteName(), "discovery.login-required"); assert.dom(".body-page").exists("The content is present"); assert.dom(".sign-up-button").exists(); assert.dom(".login-button").exists(); }); + + test("Login-required - Login Route", async function (assert) { + this.siteSettings.login_required = true; + await visit("/login"); + + assert.strictEqual(currentRouteName(), "login"); + assert.dom(".login-fullpage").exists("The login full page form is shown"); + }); }); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 84a6ec562bb..9952dae0167 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -753,6 +753,7 @@ class ApplicationController < ActionController::Base cookies[:destination_url] = destination_url redirect_to path("/auth/#{Discourse.enabled_authenticators.first.name}") else + return if request.path == path("/") && !cookies[:authentication_data] # save original URL in a cookie (javascript redirects after login in this case) cookies[:destination_url] = destination_url redirect_to path("/login") diff --git a/config/routes.rb b/config/routes.rb index 101b2fc63bc..7a4d784686f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -548,6 +548,7 @@ Discourse::Application.routes.draw do post "login" => "static#enter" get "login" => "static#show", :id => "login" + get "login-required" => "static#show", :id => "login" get "login-preferences" => "static#show", :id => "login" get "signup" => "static#show", :id => "signup" get "password-reset" => "static#show", :id => "password_reset" diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 230af6201fa..4a5c7048b43 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -14,9 +14,10 @@ RSpec.describe ApplicationController do expect(response.headers["Cache-Control"]).to eq("no-cache, no-store") end - it "should redirect to login normally" do + it "should not redirect to login" do get "/" - expect(response).to redirect_to("/login") + expect(response).not_to redirect_to("/login") + expect(response.status).to eq(200) end it "should redirect to SSO if enabled" do @@ -27,10 +28,11 @@ RSpec.describe ApplicationController do end it "should redirect to authenticator if only one, and local logins disabled" do - # Local logins and google enabled, direct to login UI + # Local logins and google enabled, show login UI SiteSetting.enable_google_oauth2_logins = true get "/" - expect(response).to redirect_to("/login") + expect(response).not_to redirect_to("/login") + expect(response.status).to eq(200) # Only google enabled, login immediately SiteSetting.enable_local_logins = false @@ -40,7 +42,8 @@ RSpec.describe ApplicationController do # Google and GitHub enabled, direct to login UI SiteSetting.enable_github_logins = true get "/" - expect(response).to redirect_to("/login") + expect(response).not_to redirect_to("/login") + expect(response.status).to eq(200) end it "should not redirect to SSO when auth_immediately is disabled" do @@ -49,7 +52,8 @@ RSpec.describe ApplicationController do SiteSetting.enable_discourse_connect = true get "/" - expect(response).to redirect_to("/login") + expect(response).not_to redirect_to("/login") + expect(response.status).to eq(200) end it "should not redirect to authenticator when auth_immediately is disabled" do @@ -58,7 +62,8 @@ RSpec.describe ApplicationController do SiteSetting.enable_local_logins = false get "/" - expect(response).to redirect_to("/login") + expect(response).not_to redirect_to("/login") + expect(response.status).to eq(200) end context "with omniauth in test mode" do diff --git a/spec/system/login_spec.rb b/spec/system/login_spec.rb index 7ecbdf73629..15021fa758b 100644 --- a/spec/system/login_spec.rb +++ b/spec/system/login_spec.rb @@ -134,7 +134,6 @@ shared_examples "login scenarios" do |login_page_object| Fabricate(:group_private_message_topic, user: user, recipient_group: group) visit "/t/#{pm.id}" - find(".login-welcome .login-button").click login_form.fill(username: "john", password: "supersecurepassword").click_login expect(page).to have_css(".header-dropdown-toggle.current-user") diff --git a/spec/system/social_authentication_spec.rb b/spec/system/social_authentication_spec.rb index 2625b3b5f14..80d1cc15e28 100644 --- a/spec/system/social_authentication_spec.rb +++ b/spec/system/social_authentication_spec.rb @@ -272,8 +272,7 @@ shared_examples "social authentication scenarios" do |signup_page_object, login_ mock_google_auth visit("/login") - expect(page).to have_css(".login-welcome") - expect(page).to have_css(".site-logo") + expect(page).to have_css(".btn-social") visit("/") expect(page).to have_css(".login-welcome")