Don't check for second factor when switching to anonymous account (#7803)

discourse
,
#1

Don’t check for second factor when switching to anonymous account (#7803)

diff --git a/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 b/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6
index c9f2bc8..f3a7c2e 100644
--- a/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6
+++ b/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6
@@ -43,6 +43,7 @@ export default RestrictedUserRoute.extend({
       if (
         transition.targetName === "preferences.second-factor" ||
         !user ||
+        (settings.allow_anonymous_posting && user.is_anonymous) ||
         user.second_factor_enabled ||
         (settings.enforce_second_factor === "staff" && !user.staff) ||
         settings.enforce_second_factor === "no"
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 1798ba5..2c0c612 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -745,6 +745,7 @@ class ApplicationController < ActionController::Base
     check_totp = current_user &&
       !request.format.json? &&
       !is_api? &&
+      !(SiteSetting.allow_anonymous_posting && current_user.anonymous?) &&
       ((SiteSetting.enforce_second_factor == 'staff' && current_user.staff?) ||
         SiteSetting.enforce_second_factor == 'all') &&
       !current_user.totp_enabled?
diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb
index 4a40d717..6941a15 100644
--- a/spec/requests/application_controller_spec.rb
+++ b/spec/requests/application_controller_spec.rb
@@ -46,6 +46,18 @@ RSpec.describe ApplicationController do
       expect(response).to redirect_to("/u/#{user.username}/preferences/second-factor")
     end
 
+    it "should not redirect anonymous users when enforce_second_factor is 'all'" do
+      SiteSetting.enforce_second_factor = "all"
+      SiteSetting.allow_anonymous_posting = true
+      sign_in(user)
+
+      post "/u/toggle-anon.json"
+      expect(response.status).to eq(200)
+
+      get "/"
+      expect(response.status).to eq(200)
+    end
+
     it "should redirect admins when enforce_second_factor is 'staff'" do
       SiteSetting.enforce_second_factor = "staff"
       sign_in(admin)
diff --git a/test/javascripts/acceptance/enforce-second-factor-test.js.es6 b/test/javascripts/acceptance/enforce-second-factor-test.js.es6
index 3f7ad66..87b6ca3 100644
--- a/test/javascripts/acceptance/enforce-second-factor-test.js.es6
+++ b/test/javascripts/acceptance/enforce-second-factor-test.js.es6
@@ -57,3 +57,28 @@ QUnit.test("as a user", async assert => {
     "it stays at second-factor preferences"
   );
 });
+
+QUnit.test("as an anonymous user", async assert => {
+  updateCurrentUser({ staff: false, admin: false, is_anonymous: true });
+
+  await visit("/u/eviltrout/preferences/second-factor");
+  Discourse.SiteSettings.enforce_second_factor = "all";
+  Discourse.SiteSettings.allow_anonymous_posting = true;
+
+  await visit("/u/eviltrout/summary");
+
+  assert.notEqual(
+    find(".control-label").text(),
+    "Password",
+    "it will transition from second-factor preferences"
+  );
+
+  await click("#toggle-hamburger-menu");
+  await click("a.about-link");
+
+  assert.notEqual(
+    find(".control-label").text(),
+    "Password",
+    "it is possible to navigate to other pages"
+  );
+});

GitHub sha: 4ba35472

1 Like
#4

I am wondering whether there is any point in checking settings.allow_anonymous_posting. I believe that only using user.is_anonymous should suffice.

#5

The same as above.

#6
#7

Functionally yes, I added it mostly because I wasn’t too well known with the complete inner workings of Discourse. If you want I can make a PR to clean it up.

#8