FEATURE: optional 2FA enforcement (#27506)

A new admin setting called `enforce_second_factor_on_external_auth`. It allows users to authenticate using external providers even when 2FA is forced with `enforce_second_factor` site setting.
This commit is contained in:
Krzysztof Kotlarek 2024-06-19 09:32:30 +10:00 committed by GitHub
parent 9568a7e542
commit cc4c199680
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 56 additions and 37 deletions

View File

@ -20,6 +20,7 @@ import I18n from "discourse-i18n";
export default Controller.extend(CanCheckEmails, {
dialog: service(),
modal: service(),
siteSettings: service(),
loading: false,
dirty: false,
errorMessage: null,
@ -34,13 +35,34 @@ export default Controller.extend(CanCheckEmails, {
},
@discourseComputed
displayOAuthWarning() {
hasOAuth() {
return findAll().length > 0;
},
@discourseComputed
displayOAuthWarning() {
return (
this.hasOAuth && this.siteSettings.enforce_second_factor_on_external_auth
);
},
@discourseComputed("currentUser")
showEnforcedWithOAuthNotice(user) {
return (
user &&
user.enforcedSecondFactor &&
this.hasOAuth &&
!this.siteSettings.enforce_second_factor_on_external_auth
);
},
@discourseComputed("currentUser")
showEnforcedNotice(user) {
return user && user.enforcedSecondFactor;
return (
user &&
user.enforcedSecondFactor &&
this.siteSettings.enforce_second_factor_on_external_auth
);
},
@discourseComputed("totps", "security_keys")

View File

@ -4,9 +4,15 @@
<ConditionalLoadingSpinner @condition={{this.loading}}>
<form class="form-vertical">
{{#if this.showEnforcedNotice}}
<div class="alert alert-error">{{i18n
"user.second_factor.enforced_notice"
}}</div>
<div class="alert alert-error">
{{i18n "user.second_factor.enforced_notice"}}
</div>
{{/if}}
{{#if this.showEnforcedWithOAuthNotice}}
<div class="alert alert-error">
{{i18n "user.second_factor.enforced_with_oauth_notice"}}
</div>
{{/if}}
{{#if this.displayOAuthWarning}}

View File

@ -143,7 +143,8 @@ class Users::OmniauthCallbacksController < ApplicationController
end
def user_found(user)
if user.has_any_second_factor_methods_enabled?
if user.has_any_second_factor_methods_enabled? &&
SiteSetting.enforce_second_factor_on_external_auth
@auth_result.omniauth_disallow_totp = true
@auth_result.email = user.email
return

View File

@ -1539,6 +1539,7 @@ en:
Two-factor authentication adds extra security to your account by requiring a one-time token in addition to your password. Tokens can be generated on <a href="https://www.google.com/search?q=authenticator+apps+for+android" target='_blank'>Android</a> and <a href="https://www.google.com/search?q=authenticator+apps+for+ios">iOS</a> devices.
oauth_enabled_warning: "Please note that social logins will be disabled once two-factor authentication has been enabled on your account."
use: "Use Authenticator app"
enforced_with_oauth_notice: "You are required to enable two-factor authentication. You will only be prompted to use this when logging in with a password, not with external authentication or social login methods."
enforced_notice: "You are required to enable two-factor authentication before accessing this site."
disable: "Disable"
disable_confirm: "Are you sure you want to disable two-factor authentication?"

View File

@ -348,7 +348,6 @@ en:
secure_uploads_requirements: "S3 uploads and S3 ACLs must be enabled before enabling secure uploads."
s3_use_acls_requirements: "You must have S3 ACLs enabled when secure uploads are enabled."
share_quote_facebook_requirements: "You must set a Facebook app id to enable quote sharing for Facebook."
second_factor_cannot_enforce_with_socials: "You cannot enforce 2FA with social logins enabled. You must first disable login via: %{auth_provider_names}"
second_factor_cannot_be_enforced_with_disabled_local_login: "You cannot enforce 2FA if local logins are disabled."
second_factor_cannot_be_enforced_with_discourse_connect_enabled: "You cannot enforce 2FA if DiscourseConnect is enabled."
local_login_cannot_be_disabled_if_second_factor_enforced: "You cannot disable local login if 2FA is enforced. Disable enforced 2FA before disabling local logins."
@ -1762,7 +1761,8 @@ en:
email_custom_headers: "A pipe-delimited list of custom email headers"
email_subject: "Customizable subject format for standard emails. See <a href='https://meta.discourse.org/t/customize-subject-format-for-standard-emails/20801' target='_blank'>https://meta.discourse.org/t/customize-subject-format-for-standard-emails/20801</a>"
detailed_404: "Provides more details to users about why they cant access a particular topic. Note: This is less secure because users will know if a URL links to a valid topic."
enforce_second_factor: "Require users to enable two-factor authentication before they can access the Discourse UI. Select 'all' to enforce it to all users. Select 'staff' to enforce it to staff users only. This setting does not affect API or 'DiscourseConnect provider' authentication."
enforce_second_factor: "Require users to enable two-factor authentication before they can access the Discourse UI. This setting does not affect API or 'DiscourseConnect provider' authentication. If enforce_second_factor_on_external_auth is enabled, users will not be able to log in with external authentication providers after they set up two-factor authentication."
enforce_second_factor_on_external_auth: "Require users to use two-factor authentication at all times. When enabled this will prevent users logging in with external authentication methods like social logins if they have two-factor authentication enabled. When disabled users will only need to confirm their two-factor authentication when logging in with a username and password. Also see the `enforce_second_factor` setting."
force_https: "Force your site to use HTTPS only. WARNING: do NOT enable this until you verify HTTPS is fully set up and working absolutely everywhere! Did you check your CDN, all social logins, and any external logos / dependencies to make sure they are all HTTPS compatible, too?"
summary_score_threshold: "The minimum score required for a post to be included in 'Summarize This Topic'"

View File

@ -1913,6 +1913,9 @@ trust:
security:
detailed_404: false
enforce_second_factor_on_external_auth:
client: true
default: true
enforce_second_factor:
client: true
type: enum

View File

@ -219,15 +219,6 @@ module SiteSettings::Validations
if new_val != "no" && SiteSetting.enable_discourse_connect?
return validate_error :second_factor_cannot_be_enforced_with_discourse_connect_enabled
end
if new_val == "all" && Discourse.enabled_auth_providers.count > 0
auth_provider_names = Discourse.enabled_auth_providers.map(&:name).join(", ")
return(
validate_error(
:second_factor_cannot_enforce_with_socials,
auth_provider_names: auth_provider_names,
)
)
end
return if SiteSetting.enable_local_logins
return if new_val == "no"
validate_error :second_factor_cannot_be_enforced_with_disabled_local_login

View File

@ -159,26 +159,6 @@ RSpec.describe SiteSettings::Validations do
end
end
context "when social logins are enabled" do
let(:error_message) do
I18n.t(
"errors.site_settings.second_factor_cannot_enforce_with_socials",
auth_provider_names: "facebook, github",
)
end
before do
SiteSetting.enable_facebook_logins = true
SiteSetting.enable_github_logins = true
end
it "raises and error, and specifies the auth providers" do
expect { validations.validate_enforce_second_factor("all") }.to raise_error(
Discourse::InvalidParameters,
error_message,
)
end
end
context "when SSO is enabled" do
let(:error_message) do
I18n.t(

View File

@ -631,6 +631,21 @@ RSpec.describe Users::OmniauthCallbacksController do
end
end
context "when user has TOTP enabled but enforce_second_factor_on_external_auth is false" do
before { user.create_totp(enabled: true) }
it "should return the right response" do
SiteSetting.enforce_second_factor_on_external_auth = false
get "/auth/google_oauth2/callback.json"
expect(response.status).to eq(302)
data = JSON.parse(cookies[:authentication_data])
expect(data["authenticated"]).to eq(true)
end
end
context "when user has security key enabled" do
before { Fabricate(:user_security_key_with_random_credential, user: user) }