FIX: Disallow all client-side routing when action is required (#27817)

When a user action is required and enforced, such as filling up newly added required fields or adding a 2FA method, we disable routing on the client-side. However, this could be bypassed by first loading an always allowed page, such as /faq and then client-side routing away from there.

This commit fixes that by 1) moving the logic for checking if routing is restricted and if a given path is allowed into a service and 2) hoisting the willTransition hook into the application router and use the newly created service to check whether to abort transitions or not.
This commit is contained in:
Ted Johansson 2024-07-10 13:33:52 +08:00 committed by GitHub
parent 3fee52eae3
commit e364ed2ad1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 76 additions and 52 deletions

View File

@ -47,6 +47,7 @@ const ApplicationRoute = DiscourseRoute.extend({
router: service(),
site: service(),
siteSettings: service(),
restrictedRouting: service(),
get isOnlyOneExternalLoginMethod() {
return (
@ -68,6 +69,19 @@ const ApplicationRoute = DiscourseRoute.extend({
return false;
},
@action
willTransition(transition) {
if (
this.restrictedRouting.isRestricted &&
!this.restrictedRouting.isAllowedRoute(transition.to.name)
) {
transition.abort();
return false;
}
return true;
},
@action
willResolveModel(transition) {
this.historyStore.willResolveModel(transition);

View File

@ -1,26 +1,7 @@
import { action } from "@ember/object";
import { service } from "@ember/service";
import RestrictedUserRoute from "discourse/routes/restricted-user";
export default class PreferencesProfile extends RestrictedUserRoute {
@service currentUser;
setupController(controller, model) {
controller.set("model", model);
}
@action
willTransition(transition) {
super.willTransition(...arguments);
if (
this.currentUser?.needs_required_fields_check &&
!transition?.to.name.startsWith("admin")
) {
transition.abort();
return false;
}
return true;
}
}

View File

@ -1,10 +1,7 @@
import { action } from "@ember/object";
import { service } from "@ember/service";
import RestrictedUserRoute from "discourse/routes/restricted-user";
export default class PreferencesSecondFactor extends RestrictedUserRoute {
@service currentUser;
@service siteSettings;
@service router;
model() {
@ -33,34 +30,4 @@ export default class PreferencesSecondFactor extends RestrictedUserRoute {
.catch(controller.popupAjaxError)
.finally(() => controller.set("loading", false));
}
@action
willTransition(transition) {
super.willTransition(...arguments);
// NOTE: Matches the should_enforce_2fa? and disqualified_from_2fa_enforcement
// methods in ApplicationController.
const enforcing2fa =
(this.siteSettings.enforce_second_factor === "staff" &&
this.currentUser.staff) ||
this.siteSettings.enforce_second_factor === "all";
const disqualifiedFrom2faEnforcement =
!this.currentUser ||
this.currentUser.is_anonymous ||
this.currentUser.second_factor_enabled ||
(!this.siteSettings.enforce_second_factor_on_external_auth &&
this.currentUser.login_method === "oauth");
if (
transition.targetName === "preferences.second-factor" ||
disqualifiedFrom2faEnforcement ||
!enforcing2fa
) {
return true;
}
transition.abort();
return false;
}
}

View File

@ -0,0 +1,62 @@
import Service, { inject as service } from "@ember/service";
export default class RestrictedRouting extends Service {
@service currentUser;
@service siteSettings;
get isRestricted() {
return this._needsRequiredFields || this._needs2fa;
}
isAllowedRoute(path) {
const alwaysAllowed = ["faq", "about", "tos", "privacy"];
if (alwaysAllowed.includes(path)) {
return true;
}
if (this._needs2fa) {
if (path === "preferences.second-factor") {
return true;
}
return false;
}
if (this._needsRequiredFields) {
if (path.startsWith("admin")) {
return true;
}
if (path === "preferences.profile") {
return true;
}
return false;
}
return true;
}
get _needs2fa() {
// NOTE: Matches the should_enforce_2fa? and disqualified_from_2fa_enforcement
// methods in ApplicationController.
const enforcing2fa =
(this.siteSettings.enforce_second_factor === "staff" &&
this.currentUser.staff) ||
this.siteSettings.enforce_second_factor === "all";
const exemptedFrom2faEnforcement =
!this.currentUser ||
this.currentUser.is_anonymous ||
this.currentUser.second_factor_enabled ||
(!this.siteSettings.enforce_second_factor_on_external_auth &&
this.currentUser.login_method === "oauth");
return enforcing2fa && !exemptedFrom2faEnforcement;
}
get _needsRequiredFields() {
return this.currentUser?.needs_required_fields_check;
}
}