diff --git a/app/assets/javascripts/discourse/app/controllers/exception.js b/app/assets/javascripts/discourse/app/controllers/exception.js index 65af0845a14..83a28674410 100644 --- a/app/assets/javascripts/discourse/app/controllers/exception.js +++ b/app/assets/javascripts/discourse/app/controllers/exception.js @@ -7,6 +7,22 @@ import DiscourseURL from "discourse/lib/url"; import discourseComputed, { on } from "discourse-common/utils/decorators"; import I18n from "discourse-i18n"; +/** + * You can throw an instance of this error during a route's beforeModel/model/afterModel hooks. + * It will be caught by the top-level ApplicationController, and cause this Exception controller/template + * to be rendered without changing the URL. + */ +export class RouteException { + status; + reason; + + constructor({ status, reason, desc }) { + this.status = status; + this.reason = reason; + this.desc = desc; + } +} + // The controller for the nice error page export default Controller.extend({ thrown: null, @@ -48,7 +64,9 @@ export default Controller.extend({ @discourseComputed("isNetwork", "thrown.status", "thrown") reason(isNetwork, thrownStatus, thrown) { - if (isNetwork) { + if (thrown.reason) { + return thrown.reason; + } else if (isNetwork) { return I18n.t("errors.reasons.network"); } else if (thrownStatus >= 500) { return I18n.t("errors.reasons.server"); @@ -74,7 +92,9 @@ export default Controller.extend({ "thrown" ) desc(networkFixed, isNetwork, thrownStatus, thrownStatusText, thrown) { - if (networkFixed) { + if (thrown.desc) { + return thrown.desc; + } else if (networkFixed) { return I18n.t("errors.desc.network_fixed"); } else if (isNetwork) { return I18n.t("errors.desc.network"); @@ -138,8 +158,7 @@ export default Controller.extend({ back() { // Strip off subfolder const currentURL = DiscourseURL.router.location.getURL(); - if (this.lastTransition && currentURL !== "/exception") { - this.lastTransition.abort(); + if (this.lastTransition?.method === "replace") { this.setProperties({ lastTransition: null, thrown: null }); // Can't use routeTo because it handles navigation to the same page DiscourseURL.handleURL(currentURL); diff --git a/app/assets/javascripts/discourse/app/lib/url.js b/app/assets/javascripts/discourse/app/lib/url.js index a47edc3710b..f1d12162f10 100644 --- a/app/assets/javascripts/discourse/app/lib/url.js +++ b/app/assets/javascripts/discourse/app/lib/url.js @@ -177,6 +177,11 @@ const DiscourseURL = EmberObject.extend({ } }, + pushState(path) { + path = withoutPrefix(path); + this.router._routerMicrolib.updateURL(path); + }, + routeToTag(a) { // skip when we are provided nowhere to route to if (!a || !a.href) { diff --git a/app/assets/javascripts/discourse/app/mixins/card-contents-base.js b/app/assets/javascripts/discourse/app/mixins/card-contents-base.js index 5f6ac864f58..93d70a93753 100644 --- a/app/assets/javascripts/discourse/app/mixins/card-contents-base.js +++ b/app/assets/javascripts/discourse/app/mixins/card-contents-base.js @@ -45,7 +45,7 @@ export default Mixin.create({ _show(username, target, event) { // No user card for anon if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) { - return false; + return true; } username = escapeExpression(username.toString()); @@ -157,9 +157,15 @@ export default Mixin.create({ return true; } - event.preventDefault(); - event.stopPropagation(); - return this._show(transformText(matchingEl), matchingEl, event); + const shouldBubble = this._show( + transformText(matchingEl), + matchingEl, + event + ); + if (!shouldBubble) { + event.preventDefault(); + event.stopPropagation(); + } } return false; diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js index d659d753be0..076c5979560 100644 --- a/app/assets/javascripts/discourse/app/routes/application.js +++ b/app/assets/javascripts/discourse/app/routes/application.js @@ -4,6 +4,7 @@ import CreateAccount from "discourse/components/modal/create-account"; import ForgotPassword from "discourse/components/modal/forgot-password"; import KeyboardShortcutsHelp from "discourse/components/modal/keyboard-shortcuts-help"; import LoginModal from "discourse/components/modal/login"; +import { RouteException } from "discourse/controllers/exception"; import { setting } from "discourse/lib/computed"; import cookie from "discourse/lib/cookie"; import logout from "discourse/lib/logout"; @@ -125,23 +126,27 @@ const ApplicationRoute = DiscourseRoute.extend({ error(err, transition) { const xhrOrErr = err.jqXHR ? err.jqXHR : err; const exceptionController = this.controllerFor("exception"); + let shouldBubble = false; const themeOrPluginSource = identifySource(err); - // eslint-disable-next-line no-console - console.error( - ...[consolePrefix(err, themeOrPluginSource), xhrOrErr].filter(Boolean) - ); - - if (xhrOrErr && xhrOrErr.status === 404) { - return this.router.transitionTo("exception-unknown"); - } - - if (themeOrPluginSource) { - this.clientErrorHandler.displayErrorNotice( - "Error loading route", - themeOrPluginSource + if (!(xhrOrErr instanceof RouteException)) { + shouldBubble = true; + // eslint-disable-next-line no-console + console.error( + ...[consolePrefix(err, themeOrPluginSource), xhrOrErr].filter(Boolean) ); + + if (xhrOrErr && xhrOrErr.status === 404) { + return this.router.transitionTo("exception-unknown"); + } + + if (themeOrPluginSource) { + this.clientErrorHandler.displayErrorNotice( + "Error loading route", + themeOrPluginSource + ); + } } exceptionController.setProperties({ @@ -149,8 +154,16 @@ const ApplicationRoute = DiscourseRoute.extend({ thrown: xhrOrErr, }); + if (transition.intent.url) { + if (transition.method === "replace") { + DiscourseURL.replaceState(transition.intent.url); + } else { + DiscourseURL.pushState(transition.intent.url); + } + } + this.intermediateTransitionTo("exception"); - return true; + return shouldBubble; }, showLogin: unlessStrictlyReadOnly( diff --git a/app/assets/javascripts/discourse/app/routes/user.js b/app/assets/javascripts/discourse/app/routes/user.js index 48b42f721c1..88372c9a0d1 100644 --- a/app/assets/javascripts/discourse/app/routes/user.js +++ b/app/assets/javascripts/discourse/app/routes/user.js @@ -1,8 +1,10 @@ import { action } from "@ember/object"; import { service } from "@ember/service"; +import { RouteException } from "discourse/controllers/exception"; import User from "discourse/models/user"; import DiscourseRoute from "discourse/routes/discourse"; import { bind } from "discourse-common/utils/decorators"; +import I18n from "discourse-i18n"; export default DiscourseRoute.extend({ router: service(), @@ -12,7 +14,10 @@ export default DiscourseRoute.extend({ beforeModel() { if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) { - this.router.replaceWith("discovery"); + throw new RouteException({ + status: 403, + desc: I18n.t("user.login_to_view_profile"), + }); } }, diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 159657b88eb..92a04329ff6 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1164,6 +1164,7 @@ en: warning: "Are you sure you want to clear your featured topic?" use_current_timezone: "Use Current Timezone" profile_hidden: "This user's public profile is hidden." + login_to_view_profile: "You must log in to view user profiles" inactive_user: "This user is no longer active." expand_profile: "Expand" sr_expand_profile: "Expand profile details" diff --git a/spec/system/user_page/hide_from_public_spec.rb b/spec/system/user_page/hide_from_public_spec.rb new file mode 100644 index 00000000000..c3fb78ef7cb --- /dev/null +++ b/spec/system/user_page/hide_from_public_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +describe "hide_user_profiles_from_public", type: :system do + let(:user) { Fabricate(:user) } + before { SiteSetting.hide_user_profiles_from_public = true } + + it "displays an error when navigating straight to a profile" do + visit("/u/#{user.username}") + expect(page).to have_css(".error-page .reason", text: I18n.t("js.errors.reasons.forbidden")) + expect(page).to have_css(".error-page .desc", text: I18n.t("js.user.login_to_view_profile")) + end + + it "displays an error when navigating from an internal link" do + Fabricate(:post, user: user) + visit("/latest") + find("[data-user-card='#{user.username}']").click + + expect(page).to have_css(".error-page .reason", text: I18n.t("js.errors.reasons.forbidden")) + expect(page).to have_css(".error-page .desc", text: I18n.t("js.user.login_to_view_profile")) + expect(page).to have_current_path("/u/#{user.username}") + + find(".error-page .buttons .btn-primary", text: "Back").click + expect(page).to have_current_path("/latest") + end +end