From dfe44bcaeffe2fb908e149193fd06d4ae8946798 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 11 Feb 2026 15:34:09 +0100 Subject: [PATCH] Add ability to require 2FA for specific roles (including Everybody) (#37701) --- app/controllers/admin/roles_controller.rb | 2 +- app/controllers/application_controller.rb | 12 ++++- .../concerns/challengable_concern.rb | 2 +- .../oauth/authorizations_controller.rb | 4 ++ .../base_controller.rb | 9 ++++ .../confirmations_controller.rb | 7 +++ .../otp_authentication_controller.rb | 2 +- ...actor_authentication_methods_controller.rb | 2 +- app/models/user.rb | 6 ++- app/models/user_role.rb | 14 ++++-- app/views/admin/roles/_form.html.haml | 5 ++ app/views/admin/roles/_role.html.haml | 4 ++ .../confirmations/new.html.haml | 2 + .../otp_authentication/show.html.haml | 5 +- .../recovery_codes/index.html.haml | 6 +++ .../index.html.haml | 5 +- config/locales/en.yml | 3 ++ config/locales/simple_form.en.yml | 2 + ...211132603_add_require2_fa_to_user_roles.rb | 7 +++ db/schema.rb | 3 +- spec/models/user_role_spec.rb | 11 +++++ spec/system/log_in_spec.rb | 46 ++++++++++++++++++- spec/system/oauth_spec.rb | 40 ++++++++++++++++ 23 files changed, 184 insertions(+), 15 deletions(-) create mode 100644 app/controllers/settings/two_factor_authentication/base_controller.rb create mode 100644 db/migrate/20260211132603_add_require2_fa_to_user_roles.rb diff --git a/app/controllers/admin/roles_controller.rb b/app/controllers/admin/roles_controller.rb index 2f9af8a6fc7..238d75bf798 100644 --- a/app/controllers/admin/roles_controller.rb +++ b/app/controllers/admin/roles_controller.rb @@ -62,7 +62,7 @@ module Admin def resource_params params - .expect(user_role: [:name, :color, :highlighted, :position, permissions_as_keys: []]) + .expect(user_role: [:name, :color, :highlighted, :position, :require_2fa, permissions_as_keys: []]) end end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f6d3ce35ab4..a19fcc7a0ae 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -61,19 +61,25 @@ class ApplicationController < ActionController::Base return if request.referer.blank? redirect_uri = URI(request.referer) - return if redirect_uri.path.start_with?('/auth') + return if redirect_uri.path.start_with?('/auth', '/settings/two_factor_authentication', '/settings/otp_authentication') stored_url = redirect_uri.to_s if redirect_uri.host == request.host && redirect_uri.port == request.port store_location_for(:user, stored_url) end + def mfa_setup_path(path_params = {}) + settings_two_factor_authentication_methods_path(path_params) + end + def require_functional! return if current_user.functional? respond_to do |format| format.any do - if current_user.confirmed? + if current_user.missing_2fa? + redirect_to mfa_setup_path + elsif current_user.confirmed? redirect_to edit_user_registration_path else redirect_to auth_setup_path @@ -85,6 +91,8 @@ class ApplicationController < ActionController::Base render json: { error: 'Your login is missing a confirmed e-mail address' }, status: 403 elsif !current_user.approved? render json: { error: 'Your login is currently pending approval' }, status: 403 + elsif current_user.missing_2fa? + render json: { error: 'Your account requires two-factor authentication' }, status: 403 elsif !current_user.functional? render json: { error: 'Your login is currently disabled' }, status: 403 end diff --git a/app/controllers/concerns/challengable_concern.rb b/app/controllers/concerns/challengable_concern.rb index 7fbc469bdf1..bd97037da60 100644 --- a/app/controllers/concerns/challengable_concern.rb +++ b/app/controllers/concerns/challengable_concern.rb @@ -42,7 +42,7 @@ module ChallengableConcern end def render_challenge - render 'auth/challenges/new', layout: 'auth' + render 'auth/challenges/new', layout: params[:oauth] ? 'modal' : 'auth' end def challenge_passed? diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index 75f0b42e832..8b3b41e72fe 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -24,4 +24,8 @@ class OAuth::AuthorizationsController < Doorkeeper::AuthorizationsController def truthy_param?(key) ActiveModel::Type::Boolean.new.cast(params[key]) end + + def mfa_setup_path + super({ oauth: true }) + end end diff --git a/app/controllers/settings/two_factor_authentication/base_controller.rb b/app/controllers/settings/two_factor_authentication/base_controller.rb new file mode 100644 index 00000000000..8770f927e76 --- /dev/null +++ b/app/controllers/settings/two_factor_authentication/base_controller.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Settings + module TwoFactorAuthentication + class BaseController < ::Settings::BaseController + layout -> { truthy_param?(:oauth) ? 'modal' : 'admin' } + end + end +end diff --git a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb index eae990e79b2..61e2aef5a81 100644 --- a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb +++ b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb @@ -4,12 +4,15 @@ module Settings module TwoFactorAuthentication class ConfirmationsController < BaseController include ChallengableConcern + include Devise::Controllers::StoreLocation skip_before_action :require_functional! before_action :require_challenge! before_action :ensure_otp_secret + helper_method :return_to_app_url + def new prepare_two_factor_form end @@ -37,6 +40,10 @@ module Settings private + def return_to_app_url + stored_location_for(:user) + end + def confirmation_params params.expect(form_two_factor_confirmation: [:otp_attempt]) end diff --git a/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb b/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb index ca8d46afe48..5460448d995 100644 --- a/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb +++ b/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb @@ -17,7 +17,7 @@ module Settings def create session[:new_otp_secret] = User.generate_otp_secret - redirect_to new_settings_two_factor_authentication_confirmation_path + redirect_to new_settings_two_factor_authentication_confirmation_path(params.permit(:oauth)) end private diff --git a/app/controllers/settings/two_factor_authentication_methods_controller.rb b/app/controllers/settings/two_factor_authentication_methods_controller.rb index a6d5c1fe2dd..49579b36779 100644 --- a/app/controllers/settings/two_factor_authentication_methods_controller.rb +++ b/app/controllers/settings/two_factor_authentication_methods_controller.rb @@ -22,7 +22,7 @@ module Settings private def require_otp_enabled - redirect_to settings_otp_authentication_path unless current_user.otp_enabled? + redirect_to settings_otp_authentication_path(params.permit(:oauth)) unless current_user.otp_enabled? end end end diff --git a/app/models/user.rb b/app/models/user.rb index be5dc8e517c..88599ab75c4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -226,7 +226,11 @@ class User < ApplicationRecord end def functional_or_moved? - confirmed? && approved? && !disabled? && !account.unavailable? && !account.memorial? + confirmed? && approved? && !disabled? && !account.unavailable? && !account.memorial? && !missing_2fa? + end + + def missing_2fa? + !two_factor_enabled? && role.require_2fa? end def unconfirmed_or_pending? diff --git a/app/models/user_role.rb b/app/models/user_role.rb index 31c8ff20a65..afeed324cc9 100644 --- a/app/models/user_role.rb +++ b/app/models/user_role.rb @@ -5,11 +5,12 @@ # Table name: user_roles # # id :bigint(8) not null, primary key -# name :string default(""), not null # color :string default(""), not null -# position :integer default(0), not null -# permissions :bigint(8) default(0), not null # highlighted :boolean default(FALSE), not null +# name :string default(""), not null +# permissions :bigint(8) default(0), not null +# position :integer default(0), not null +# require_2fa :boolean default(FALSE), not null # created_at :datetime not null # updated_at :datetime not null # @@ -160,7 +161,7 @@ class UserRole < ApplicationRecord @computed_permissions ||= begin permissions = self.class.everyone.permissions | self.permissions - if permissions & FLAGS[:administrator] == FLAGS[:administrator] + if administrator? Flags::ALL else permissions @@ -172,6 +173,10 @@ class UserRole < ApplicationRecord name end + def administrator? + permissions & FLAGS[:administrator] == FLAGS[:administrator] + end + private def in_permissions?(privilege) @@ -189,6 +194,7 @@ class UserRole < ApplicationRecord errors.add(:permissions_as_keys, :own_role) if permissions_changed? errors.add(:position, :own_role) if position_changed? + errors.add(:require_2fa, :own_role) if require_2fa_changed? && !administrator? end def validate_permissions_elevation diff --git a/app/views/admin/roles/_form.html.haml b/app/views/admin/roles/_form.html.haml index 0b1c3101624..f76a594534b 100644 --- a/app/views/admin/roles/_form.html.haml +++ b/app/views/admin/roles/_form.html.haml @@ -25,6 +25,11 @@ = form.input :highlighted, wrapper: :with_label + - if current_user.role.administrator? || current_user.role != form.object + .fields-group + = form.input :require_2fa, + wrapper: :with_label + %hr.spacer/ - unless current_user.role == form.object diff --git a/app/views/admin/roles/_role.html.haml b/app/views/admin/roles/_role.html.haml index df813bbaf43..0b1d541035e 100644 --- a/app/views/admin/roles/_role.html.haml +++ b/app/views/admin/roles/_role.html.haml @@ -21,9 +21,13 @@ .announcements-list__item__meta - if role.everyone? = t('admin.roles.everyone_full_description_html') + = t('admin.roles.requires_2fa') if role.require_2fa? - else = link_to t('admin.roles.assigned_users', count: role.users.count), admin_accounts_path(role_ids: role.id) · %abbr{ title: role.permissions_as_keys.map { |privilege| I18n.t("admin.roles.privileges.#{privilege}") }.join(', ') }= t('admin.roles.permissions_count', count: role.permissions_as_keys.size) + - if role.require_2fa? + · + = t('admin.roles.requires_2fa') .announcements-list__item__actions = table_link_to 'edit', t('admin.accounts.edit'), edit_admin_role_path(role) if can?(:update, role) diff --git a/app/views/settings/two_factor_authentication/confirmations/new.html.haml b/app/views/settings/two_factor_authentication/confirmations/new.html.haml index a35479b84e6..2ce422e997a 100644 --- a/app/views/settings/two_factor_authentication/confirmations/new.html.haml +++ b/app/views/settings/two_factor_authentication/confirmations/new.html.haml @@ -12,6 +12,8 @@ %samp.qr-alternative__code= @new_otp_secret.scan(/.{4}/).join(' ') .fields-group + = hidden_field_tag :oauth, params[:oauth] + = f.input :otp_attempt, hint: t('otp_authentication.code_hint'), input_html: { autocomplete: 'off' }, diff --git a/app/views/settings/two_factor_authentication/otp_authentication/show.html.haml b/app/views/settings/two_factor_authentication/otp_authentication/show.html.haml index 0ba4ad68934..ef87f998db2 100644 --- a/app/views/settings/two_factor_authentication/otp_authentication/show.html.haml +++ b/app/views/settings/two_factor_authentication/otp_authentication/show.html.haml @@ -1,9 +1,12 @@ - content_for :page_title do = t('settings.two_factor_authentication') +- if current_user.role.require_2fa? + .flash-message= t('two_factor_authentication.role_requirement', domain: site_hostname) + .simple_form %p.hint= t('otp_authentication.description_html') %hr.spacer/ - = link_to t('otp_authentication.setup'), settings_otp_authentication_path, data: { method: :post }, class: 'button button--block' + = link_to t('otp_authentication.setup'), settings_otp_authentication_path(params.permit(:oauth)), data: { method: :post }, class: 'button button--block' diff --git a/app/views/settings/two_factor_authentication/recovery_codes/index.html.haml b/app/views/settings/two_factor_authentication/recovery_codes/index.html.haml index d47ee840e1e..c56c71c0e83 100644 --- a/app/views/settings/two_factor_authentication/recovery_codes/index.html.haml +++ b/app/views/settings/two_factor_authentication/recovery_codes/index.html.haml @@ -7,3 +7,9 @@ - @recovery_codes.each do |code| %li< %samp= code + +- if params[:oauth] + %hr.spacer/ + + .simple_form + = link_to t('two_factor_authentication.resume_app_authorization'), return_to_app_url, class: 'button button--block' diff --git a/app/views/settings/two_factor_authentication_methods/index.html.haml b/app/views/settings/two_factor_authentication_methods/index.html.haml index 8088b4423a4..4863a279c06 100644 --- a/app/views/settings/two_factor_authentication_methods/index.html.haml +++ b/app/views/settings/two_factor_authentication_methods/index.html.haml @@ -2,7 +2,10 @@ = t('settings.two_factor_authentication') - content_for :heading_actions do - = link_to t('two_factor_authentication.disable'), disable_settings_two_factor_authentication_methods_path, class: 'button button--destructive', method: :post + = link_to t('two_factor_authentication.disable'), disable_settings_two_factor_authentication_methods_path, class: 'button button--destructive', method: :post unless current_user.role.require_2fa? + +- if current_user.role.require_2fa? + .flash-message= t('two_factor_authentication.role_requirement', domain: site_hostname) %p.hint %span.positive-hint diff --git a/config/locales/en.yml b/config/locales/en.yml index 8d8657d1d21..2c777e72d32 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -802,6 +802,7 @@ en: view_devops_description: Allows users to access Sidekiq and pgHero dashboards view_feeds: View live and topic feeds view_feeds_description: Allows users to access the live and topic feeds regardless of server settings + requires_2fa: Requires two-factor authentication title: Roles rules: add_new: Add rule @@ -2047,6 +2048,8 @@ en: recovery_codes: Backup recovery codes recovery_codes_regenerated: Recovery codes successfully regenerated recovery_instructions_html: If you ever lose access to your phone, you can use one of the recovery codes below to regain access to your account. Keep the recovery codes safe. For example, you may print them and store them with other important documents. + resume_app_authorization: Resume application authorization + role_requirement: "%{domain} requires you to set up Two-Factor Authentication before you can use Mastodon." webauthn: Security keys user_mailer: announcement_published: diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 3a68f3816f2..7fff363ff17 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -164,6 +164,7 @@ en: name: Public name of the role, if role is set to be displayed as a badge permissions_as_keys: Users with this role will have access to... position: Higher role decides conflict resolution in certain situations. Certain actions can only be performed on roles with a lower priority + require_2fa: Users with this role will be required to set up two-factor authentication to use Mastodon username_block: allow_with_approval: Instead of preventing sign-up outright, matching sign-ups will require your approval comparison: Please be mindful of the Scunthorpe Problem when blocking partial matches @@ -387,6 +388,7 @@ en: name: Name permissions_as_keys: Permissions position: Priority + require_2fa: Require two-factor authentication username_block: allow_with_approval: Allow registrations with approval comparison: Method of comparison diff --git a/db/migrate/20260211132603_add_require2_fa_to_user_roles.rb b/db/migrate/20260211132603_add_require2_fa_to_user_roles.rb new file mode 100644 index 00000000000..316b1f36d29 --- /dev/null +++ b/db/migrate/20260211132603_add_require2_fa_to_user_roles.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddRequire2FaToUserRoles < ActiveRecord::Migration[8.0] + def change + add_column :user_roles, :require_2fa, :boolean, null: false, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 44b71d97034..a3ee1f23b01 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_02_09_143308) do +ActiveRecord::Schema[8.0].define(version: 2026_02_11_132603) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -1285,6 +1285,7 @@ ActiveRecord::Schema[8.0].define(version: 2026_02_09_143308) do t.boolean "highlighted", default: false, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "require_2fa", default: false, null: false end create_table "username_blocks", force: :cascade do |t| diff --git a/spec/models/user_role_spec.rb b/spec/models/user_role_spec.rb index 48768719a17..547fc718cc3 100644 --- a/spec/models/user_role_spec.rb +++ b/spec/models/user_role_spec.rb @@ -47,6 +47,17 @@ RSpec.describe UserRole do it { is_expected.to_not allow_value(100).for(:permissions).against(:permissions_as_keys).with_message(:own_role) } it { is_expected.to_not allow_value(100).for(:position).with_message(:own_role) } + it { is_expected.to_not allow_value(true).for(:require_2fa).with_message(:own_role) } + end + + context 'when current_account is changing their own role and is an admin' do + subject { Fabricate(:user_role, permissions: UserRole::FLAGS[:administrator]) } + + let(:account) { Fabricate :account, user: Fabricate(:user, role: subject) } + + it { is_expected.to_not allow_value(100).for(:permissions).against(:permissions_as_keys).with_message(:own_role) } + it { is_expected.to_not allow_value(100).for(:position).with_message(:own_role) } + it { is_expected.to allow_value(true).for(:require_2fa) } end end end diff --git a/spec/system/log_in_spec.rb b/spec/system/log_in_spec.rb index af3a99164f7..aec9a5627f8 100644 --- a/spec/system/log_in_spec.rb +++ b/spec/system/log_in_spec.rb @@ -13,16 +13,19 @@ RSpec.describe 'Log in' do before do as_a_registered_user - visit new_user_session_path end it 'A valid email and password user is able to log in' do + visit new_user_session_path + fill_in_auth_details(email, password) expect(subject).to have_css('div.app-holder') end it 'A invalid email and password user is not able to log in' do + visit new_user_session_path + fill_in_auth_details('invalid_email', 'invalid_password') expect(subject).to have_css('.flash-message', text: /#{failure_message_invalid}/i) @@ -32,12 +35,53 @@ RSpec.describe 'Log in' do let(:confirmed_at) { nil } it 'A unconfirmed user is able to log in' do + visit new_user_session_path + fill_in_auth_details(email, password) expect(subject).to have_css('.title', text: I18n.t('auth.setup.title')) end end + context 'when the user role requires 2FA' do + before do + bob.role.update!(require_2fa: true) + end + + context 'when the user has not configured 2FA' do + it 'they are redirected to 2FA setup' do + visit new_user_session_path + + fill_in_auth_details(email, password) + + expect(subject).to have_no_css('div.app-holder') + expect(subject).to have_title(I18n.t('settings.two_factor_authentication')) + end + end + + context 'when the user has configured 2FA' do + before do + bob.update!(otp_required_for_login: true, otp_secret: User.generate_otp_secret) + end + + it 'they are able to log in' do + visit new_user_session_path + + fill_in_auth_details(email, password) + fill_in_otp_details(bob.current_otp) + + expect(subject).to have_css('div.app-holder') + end + end + end + + private + + def fill_in_otp_details(value) + fill_in 'user_otp_attempt', with: value + click_on I18n.t('auth.login') + end + def failure_message_invalid keys = User.authentication_keys.map { |key| User.human_attribute_name(key) } I18n.t('devise.failure.invalid', authentication_keys: keys.join('support.array.words_connector')) diff --git a/spec/system/oauth_spec.rb b/spec/system/oauth_spec.rb index caed5ea9af1..19235eab425 100644 --- a/spec/system/oauth_spec.rb +++ b/spec/system/oauth_spec.rb @@ -121,6 +121,46 @@ RSpec.describe 'Using OAuth from an external app' do end end end + + context 'when the user has yet to enable TOTP' do + let(:new_otp_secret) { ROTP::Base32.random(User.otp_secret_length) } + + before do + allow(User).to receive(:generate_otp_secret).and_return(new_otp_secret) + user.role.update!(require_2fa: true) + end + + it 'when accepting the authorization request' do + subject + + # It presents the user with the 2FA setup page + expect(page).to have_content(I18n.t('two_factor_authentication.role_requirement', domain: local_domain_uri.host)) + click_on I18n.t('otp_authentication.setup') + + # Fill in challenge form + fill_in 'form_challenge_current_password', with: user.password + click_on I18n.t('challenge.confirm') + + # It presents the user with the TOTP confirmation screen + expect(page).to have_title(I18n.t('settings.two_factor_authentication')) + + fill_in 'form_two_factor_confirmation_otp_attempt', with: ROTP::TOTP.new(new_otp_secret).at(Time.now.utc) + click_on I18n.t('otp_authentication.enable') + + # It presents the user with recovery codes + click_on I18n.t('two_factor_authentication.resume_app_authorization') + + # It presents the user with an authorization page + expect(page).to have_content(oauth_authorize_text) + + # It grants the app access to the account + expect { click_on oauth_authorize_text } + .to change { user_has_grant_with_client_app? }.to(true) + + # Upon authorizing, it redirects to the apps' callback URL + expect(page).to redirect_to_callback_url + end + end end end