diff --git a/app/assets/javascripts/discourse/app/routes/preferences-second-factor.js b/app/assets/javascripts/discourse/app/routes/preferences-second-factor.js index c9845d4628d..d6e78fafb5d 100644 --- a/app/assets/javascripts/discourse/app/routes/preferences-second-factor.js +++ b/app/assets/javascripts/discourse/app/routes/preferences-second-factor.js @@ -38,14 +38,24 @@ export default class PreferencesSecondFactor extends RestrictedUserRoute { willTransition(transition) { super.willTransition(...arguments); - if ( - transition.targetName === "preferences.second-factor" || + // 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 === "staff" && - !this.currentUser.staff) || - this.siteSettings.enforce_second_factor === "no" + (!this.siteSettings.enforce_second_factor_on_external_auth && + this.currentUser.login_method === "oauth"); + + if ( + transition.targetName === "preferences.second-factor" || + disqualifiedFrom2faEnforcement || + !enforcing2fa ) { return true; } diff --git a/app/assets/javascripts/discourse/tests/acceptance/enforce-second-factor-test.js b/app/assets/javascripts/discourse/tests/acceptance/enforce-second-factor-test.js index 4ef47d8524d..4cdfca2be29 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/enforce-second-factor-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/enforce-second-factor-test.js @@ -80,3 +80,53 @@ acceptance("Enforce Second Factor for unconfirmed session", function (needs) { ); }); }); + +acceptance("Enforce second factor for OAuth logins", function (needs) { + needs.user(); + needs.pretender((server, helper) => { + server.post("/u/second_factors.json", () => { + return helper.response({ + success: "OK", + unconfirmed_session: "true", + }); + }); + }); + + test("as a user using local login (username + password) when enforce_second_factor_on_external_auth is false", async function (assert) { + updateCurrentUser({ + moderator: false, + admin: false, + login_method: "local", + }); + this.siteSettings.enforce_second_factor = "all"; + this.siteSettings.enforce_second_factor_on_external_auth = false; + + await visit("/u/eviltrout/preferences/second-factor"); + await click(".home-logo-wrapper-outlet a"); + + assert.strictEqual( + currentRouteName(), + "preferences.second-factor", + "it does not let the user leave the second factor preferences" + ); + }); + + test("as a user using oauth login when enforce_second_factor_on_external_auth is false", async function (assert) { + updateCurrentUser({ + moderator: false, + admin: false, + login_method: "oauth", + }); + this.siteSettings.enforce_second_factor = "all"; + this.siteSettings.enforce_second_factor_on_external_auth = false; + + await visit("/u/eviltrout/preferences/second-factor"); + await click(".home-logo-wrapper-outlet a"); + + assert.strictEqual( + currentRouteName(), + "discovery.latest", + "it does let the user leave the second factor preferences" + ); + }); +}); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 482e672f0e3..372b8923c7d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -607,6 +607,11 @@ class ApplicationController < ActionController::Base RateLimiter.new(nil, "second-factor-min-#{user.username}", 6, 1.minute).performed! if user end + def login_method + return if current_user.anonymous? + secure_session["oauth"] == "true" ? Auth::LOGIN_METHOD_OAUTH : Auth::LOGIN_METHOD_LOCAL + end + private def preload_anonymous_data @@ -628,7 +633,7 @@ class ApplicationController < ActionController::Base current_user, scope: guardian, root: false, - navigation_menu_param: params[:navigation_menu], + login_method: login_method, ), ), ) @@ -905,7 +910,10 @@ class ApplicationController < ActionController::Base def disqualified_from_2fa_enforcement request.format.json? || is_api? || current_user.anonymous? || - (!SiteSetting.enforce_second_factor_on_external_auth && secure_session["oauth"] == "true") + ( + !SiteSetting.enforce_second_factor_on_external_auth && + login_method == Auth::LOGIN_METHOD_OAUTH + ) end def redirect_to_profile_if_required diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 4f661afc427..15928ec2bad 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -652,7 +652,7 @@ class SessionController < ApplicationController def current if current_user.present? - render_serialized(current_user, CurrentUserSerializer) + render_serialized(current_user, CurrentUserSerializer, { login_method: login_method }) else render body: nil, status: 404 end diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index 3d1a6e9ae43..4962bc0b002 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -76,7 +76,8 @@ class CurrentUserSerializer < BasicUserSerializer :use_experimental_topic_bulk_actions?, :use_admin_sidebar, :can_view_raw_email, - :use_glimmer_topic_list? + :use_glimmer_topic_list?, + :login_method delegate :user_stat, to: :object, private: true delegate :any_posts, :draft_count, :pending_posts_count, :read_faq?, to: :user_stat @@ -92,6 +93,10 @@ class CurrentUserSerializer < BasicUserSerializer options[:include_status] = true end + def login_method + @options[:login_method] + end + def groups owned_group_ids = GroupUser.where(user_id: id, owner: true).pluck(:group_id).to_set diff --git a/lib/auth.rb b/lib/auth.rb index fec087fd031..d96babe0b8e 100644 --- a/lib/auth.rb +++ b/lib/auth.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module Auth + LOGIN_METHOD_OAUTH = "oauth" + LOGIN_METHOD_LOCAL = "local" end require "auth/auth_provider"