From 9fa40ae5724d7e4ba29fb478263537961eb7da78 Mon Sep 17 00:00:00 2001 From: Matthias Mair Date: Thu, 15 Jan 2026 23:33:10 +0100 Subject: [PATCH] fix: MFA enforce flows / interactions (#10796) * Add a explicit confirm when MFA Enforcing is turned on https://github.com/inventree/InvenTree/issues/10754 * add error boundary for the case of a login enforcement * ensure registration setup is redirected to * fix auth url * adjust error boundary * update test * be more specific in enforcement flow * ensure we log the admin also out immidiatly after removing all mfa * small cleanup * sml chg * fix execution order issues * clean up args * cleanup * add test for mfa change logout * fix IP in test * add option to require an explicit confirm * adapt ui to ask before patching * bump API version --- .../InvenTree/InvenTree/api_version.py | 5 +- src/backend/InvenTree/common/models.py | 16 +++ src/backend/InvenTree/common/serializers.py | 45 +++++++-- .../InvenTree/common/setting/system.py | 19 ++++ src/backend/InvenTree/common/setting/type.py | 4 + src/backend/InvenTree/common/tests.py | 14 +++ src/frontend/lib/enums/ApiEndpoints.tsx | 1 + src/frontend/lib/types/Auth.tsx | 3 +- src/frontend/lib/types/Settings.tsx | 2 + src/frontend/src/components/Boundary.tsx | 4 +- .../src/components/settings/SettingItem.tsx | 49 ++++++++-- .../src/components/settings/SettingList.tsx | 14 ++- src/frontend/src/functions/auth.tsx | 97 +++++++++++++------ .../Settings/AccountSettings/MFASettings.tsx | 20 +++- .../AccountSettings/SecurityContent.tsx | 16 ++- src/frontend/src/states/states.tsx | 5 +- 16 files changed, 250 insertions(+), 64 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/api_version.py b/src/backend/InvenTree/InvenTree/api_version.py index 85f6e6b86b..474730c9da 100644 --- a/src/backend/InvenTree/InvenTree/api_version.py +++ b/src/backend/InvenTree/InvenTree/api_version.py @@ -1,11 +1,14 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 439 +INVENTREE_API_VERSION = 440 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v440 -> 2026-01-15 : https://github.com/inventree/InvenTree/pull/10796 + - Adds confirm and confirm_text to all settings + v439 -> 2026-01-09 : https://github.com/inventree/InvenTree/pull/11092 - Add missing nullable annotations diff --git a/src/backend/InvenTree/common/models.py b/src/backend/InvenTree/common/models.py index 6e1dbd2e5f..e0f8ef34f5 100644 --- a/src/backend/InvenTree/common/models.py +++ b/src/backend/InvenTree/common/models.py @@ -970,6 +970,22 @@ class BaseInvenTreeSetting(models.Model): return setting.get('model', None) + def confirm(self) -> bool: + """Return if this setting requires confirmation on change.""" + setting = self.get_setting_definition( + self.key, **self.get_filters_for_instance() + ) + + return setting.get('confirm', False) + + def confirm_text(self) -> str: + """Return the confirmation text for this setting, if provided.""" + setting = self.get_setting_definition( + self.key, **self.get_filters_for_instance() + ) + + return setting.get('confirm_text', '') + def model_filters(self) -> Optional[dict]: """Return the model filters associated with this setting.""" setting = self.get_setting_definition( diff --git a/src/backend/InvenTree/common/serializers.py b/src/backend/InvenTree/common/serializers.py index c5fa490497..ae4278a3d1 100644 --- a/src/backend/InvenTree/common/serializers.py +++ b/src/backend/InvenTree/common/serializers.py @@ -88,6 +88,18 @@ class SettingsSerializer(InvenTreeModelSerializer): choices = serializers.SerializerMethodField() + def get_choices(self, obj) -> list: + """Returns the choices available for a given item.""" + results = [] + + choices = obj.choices() + + if choices: + for choice in choices: + results.append({'value': choice[0], 'display_name': choice[1]}) + + return results + model_name = serializers.CharField(read_only=True, allow_null=True) model_filters = serializers.DictField(read_only=True) @@ -108,17 +120,26 @@ class SettingsSerializer(InvenTreeModelSerializer): typ = serializers.CharField(read_only=True) - def get_choices(self, obj) -> list: - """Returns the choices available for a given item.""" - results = [] + confirm = serializers.BooleanField( + read_only=True, + help_text=_('Indicates if changing this setting requires confirmation'), + ) - choices = obj.choices() + confirm_text = serializers.CharField(read_only=True) - if choices: - for choice in choices: - results.append({'value': choice[0], 'display_name': choice[1]}) - - return results + def is_valid(self, *, raise_exception=False): + """Validate the setting, including confirmation if required.""" + ret = super().is_valid(raise_exception=raise_exception) + # Check if confirmation was provided if required + if self.instance.confirm(): + req_data = self.context['request'].data + if not 'manual_confirm' in req_data or not req_data['manual_confirm']: + raise serializers.ValidationError({ + 'manual_confirm': _( + 'This setting requires confirmation before changing. Please confirm the change.' + ) + }) + return ret class GlobalSettingsSerializer(SettingsSerializer): @@ -141,6 +162,8 @@ class GlobalSettingsSerializer(SettingsSerializer): 'api_url', 'typ', 'read_only', + 'confirm', + 'confirm_text', ] read_only = serializers.SerializerMethodField( @@ -184,6 +207,8 @@ class UserSettingsSerializer(SettingsSerializer): 'model_name', 'api_url', 'typ', + 'confirm', + 'confirm_text', ] user = serializers.PrimaryKeyRelatedField(read_only=True) @@ -232,6 +257,8 @@ class GenericReferencedSettingSerializer(SettingsSerializer): 'typ', 'units', 'required', + 'confirm', + 'confirm_text', ] # set Meta class diff --git a/src/backend/InvenTree/common/setting/system.py b/src/backend/InvenTree/common/setting/system.py index 087e494400..a216b5d1f5 100644 --- a/src/backend/InvenTree/common/setting/system.py +++ b/src/backend/InvenTree/common/setting/system.py @@ -106,6 +106,20 @@ def reload_plugin_registry(setting): registry.reload_plugins(full_reload=True, force_reload=True, collect=True) +def enforce_mfa(setting): + """Enforce multifactor authentication for all users.""" + from allauth.usersessions.models import UserSession + + from common.models import logger + + logger.info( + 'Enforcing multifactor authentication for all users by signing out all sessions.' + ) + for session in UserSession.objects.all(): + session.end() + logger.info('All user sessions have been ended.') + + def barcode_plugins() -> list: """Return a list of plugin choices which can be used for barcode generation.""" try: @@ -1007,6 +1021,11 @@ SYSTEM_SETTINGS: dict[str, InvenTreeSettingsKeyType] = { 'description': _('Users must use multifactor security.'), 'default': False, 'validator': bool, + 'confirm': True, + 'confirm_text': _( + 'Enabling this setting will require all users to set up multifactor authentication. All sessions will be disconnected immediately.' + ), + 'after_save': enforce_mfa, }, 'PLUGIN_ON_STARTUP': { 'name': _('Check plugins on startup'), diff --git a/src/backend/InvenTree/common/setting/type.py b/src/backend/InvenTree/common/setting/type.py index 5ec029f998..1971533663 100644 --- a/src/backend/InvenTree/common/setting/type.py +++ b/src/backend/InvenTree/common/setting/type.py @@ -32,6 +32,8 @@ class SettingsKeyType(TypedDict, total=False): protected: Protected values are not returned to the client, instead "***" is returned (optional, default: False) required: Is this setting required to work, can be used in combination with .check_all_settings(...) (optional, default: False) model: Auto create a dropdown menu to select an associated model instance (e.g. 'company.company', 'auth.user' and 'auth.group' are possible too, optional) + confirm: Require an explicit confirmation before changing the setting (optional, default: False) + confirm_text: Text to display in the confirmation dialog (optional) """ name: str @@ -46,6 +48,8 @@ class SettingsKeyType(TypedDict, total=False): protected: bool required: bool model: str + confirm: bool + confirm_text: str class InvenTreeSettingsKeyType(SettingsKeyType): diff --git a/src/backend/InvenTree/common/tests.py b/src/backend/InvenTree/common/tests.py index 5c03ad6db4..7df489209a 100644 --- a/src/backend/InvenTree/common/tests.py +++ b/src/backend/InvenTree/common/tests.py @@ -408,6 +408,8 @@ class SettingsTest(InvenTreeTestCase): 'requires_restart', 'after_save', 'before_save', + 'confirm', + 'confirm_text', ] for k in setting: @@ -641,6 +643,18 @@ class GlobalSettingsApiTest(InvenTreeAPITestCase): setting.refresh_from_db() self.assertEqual(setting.value, val) + def test_mfa_change(self): + """Test that changes in LOGIN_ENFORCE_MFA are handled correctly.""" + # Setup admin users + self.user.usersession_set.create(ip='192.168.1.1') + self.assertEqual(self.user.usersession_set.count(), 1) + + # Enable enforced MFA + set_global_setting('LOGIN_ENFORCE_MFA', True) + + # There should be no user sessions now + self.assertEqual(self.user.usersession_set.count(), 0) + def test_api_detail(self): """Test that we can access the detail view for a setting based on the .""" # These keys are invalid, and should return 404 diff --git a/src/frontend/lib/enums/ApiEndpoints.tsx b/src/frontend/lib/enums/ApiEndpoints.tsx index c46d50af71..5c61f4e6ec 100644 --- a/src/frontend/lib/enums/ApiEndpoints.tsx +++ b/src/frontend/lib/enums/ApiEndpoints.tsx @@ -20,6 +20,7 @@ export enum ApiEndpoints { user_simple_login = 'email/generate/', // User auth endpoints + auth_base = '/auth/', user_reset = 'auth/v1/auth/password/request', user_reset_set = 'auth/v1/auth/password/reset', auth_pwd_change = 'auth/v1/account/password/change', diff --git a/src/frontend/lib/types/Auth.tsx b/src/frontend/lib/types/Auth.tsx index 959b64e5fe..50990f7ddc 100644 --- a/src/frontend/lib/types/Auth.tsx +++ b/src/frontend/lib/types/Auth.tsx @@ -25,7 +25,8 @@ export enum FlowEnum { MfaAuthenticate = 'mfa_authenticate', Reauthenticate = 'reauthenticate', MfaReauthenticate = 'mfa_reauthenticate', - MfaTrust = 'mfa_trust' + MfaTrust = 'mfa_trust', + MfaRegister = 'mfa_register' } export interface Flow { diff --git a/src/frontend/lib/types/Settings.tsx b/src/frontend/lib/types/Settings.tsx index 08b89a9f5f..46e1b10abe 100644 --- a/src/frontend/lib/types/Settings.tsx +++ b/src/frontend/lib/types/Settings.tsx @@ -34,6 +34,8 @@ export interface Setting { method?: string; required?: boolean; read_only?: boolean; + confirm?: boolean; + confirm_text?: string; } export interface SettingChoice { diff --git a/src/frontend/src/components/Boundary.tsx b/src/frontend/src/components/Boundary.tsx index 8509b1b932..b96ec01108 100644 --- a/src/frontend/src/components/Boundary.tsx +++ b/src/frontend/src/components/Boundary.tsx @@ -4,7 +4,9 @@ import { ErrorBoundary, type FallbackRender } from '@sentry/react'; import { IconExclamationCircle } from '@tabler/icons-react'; import { type ReactNode, useCallback } from 'react'; -function DefaultFallback({ title }: Readonly<{ title: string }>): ReactNode { +export function DefaultFallback({ + title +}: Readonly<{ title: string }>): ReactNode { return ( void; - onToggle: (setting: Setting, value: boolean) => void; + onEdit: (setting: Setting, confirmed: boolean) => void; + onToggle: (setting: Setting, value: boolean, confirmed: boolean) => void; }>) { // Determine the text to display for the setting value const valueText: string = useMemo(() => { @@ -54,7 +74,9 @@ function SettingValue({ // Launch the edit dialog for this setting const editSetting = useCallback(() => { if (!setting.read_only) { - onEdit(setting); + const confirm = confirmSettingChange(setting); + if (!confirm.proceed) return; + onEdit(setting, confirm.confirmed); } }, [setting, onEdit]); @@ -62,7 +84,9 @@ function SettingValue({ const toggleSetting = useCallback( (event: any) => { if (!setting.read_only) { - onToggle(setting, event.currentTarget.checked); + const confirm = confirmSettingChange(setting); + if (!confirm.proceed) return; + onToggle(setting, event.currentTarget.checked, confirm.confirmed); } }, [setting, onToggle] @@ -170,8 +194,8 @@ export function SettingItem({ }: Readonly<{ setting: Setting; shaded: boolean; - onEdit: (setting: Setting) => void; - onToggle: (setting: Setting, value: boolean) => void; + onEdit: (setting: Setting, confirmed: boolean) => void; + onToggle: (setting: Setting, value: boolean, confirmed: boolean) => void; }>) { const { colorScheme } = useMantineColorScheme(); @@ -192,7 +216,18 @@ export function SettingItem({ {setting.description} - + + {setting.confirm && ( + + + + )} + + diff --git a/src/frontend/src/components/settings/SettingList.tsx b/src/frontend/src/components/settings/SettingList.tsx index ca1e76627f..db5a3492bb 100644 --- a/src/frontend/src/components/settings/SettingList.tsx +++ b/src/frontend/src/components/settings/SettingList.tsx @@ -91,7 +91,7 @@ export function SettingList({ // Callback for editing a single setting instance const onValueEdit = useCallback( - (setting: Setting) => { + (setting: Setting, confirmed: boolean) => { setSetting(setting); editSettingModal.open(); }, @@ -100,13 +100,17 @@ export function SettingList({ // Callback for toggling a single boolean setting instance const onValueToggle = useCallback( - (setting: Setting, value: boolean) => { + (setting: Setting, value: boolean, confirmed: boolean) => { + let data: any = { + value: value + }; + if (confirmed) { + data = { ...data, manual_confirm: true }; + } api .patch( apiUrl(settingsState.endpoint, setting.key, settingsState.pathParams), - { - value: value - } + data ) .then(() => { notifications.hide('setting'); diff --git a/src/frontend/src/functions/auth.tsx b/src/frontend/src/functions/auth.tsx index b8969aed3b..ae3c481e57 100644 --- a/src/frontend/src/functions/auth.tsx +++ b/src/frontend/src/functions/auth.tsx @@ -90,7 +90,7 @@ export async function doBasicLogin( const host: string = getHost(); - // Attempt login with + // Attempt login with basic info await api .post( apiUrl(ApiEndpoints.auth_login), @@ -147,10 +147,16 @@ export async function doBasicLogin( } }); + // see if mfa registration is required + if (loginDone) { + // stop further processing if mfa setup is required + if (!(await MfaSetupOk(navigate))) loginDone = false; + } + + // we are successfully logged in - gather required states for app if (loginDone) { await fetchUserState(); - // see if mfa registration is required - await fetchGlobalStates(navigate); + await fetchGlobalStates(); observeProfile(); } else if (!success) { clearUserState(); @@ -238,6 +244,26 @@ export const doSimpleLogin = async (email: string) => { return mail; }; +function MfaSetupOk(navigate: NavigateFunction) { + return api + .get(apiUrl(ApiEndpoints.auth_base)) + .then(() => { + return true; + }) + .catch((err) => { + if (err?.response?.status == 401) { + const mfa_register = err.response.data.id == FlowEnum.MfaRegister; + if (mfa_register && navigate != undefined) { + navigate('/mfa-setup'); + return false; + } + } else { + console.error(err); + } + return true; + }); +} + function observeProfile() { // overwrite language and theme info in session with profile info @@ -326,19 +352,14 @@ export async function handleMfaLogin( ) { const { setAuthContext } = useServerApiState.getState(); - const result = await authApi( - apiUrl(ApiEndpoints.auth_login_2fa), - undefined, - 'post', - { - code: values.code - } - ) + return await authApi(apiUrl(ApiEndpoints.auth_login_2fa), undefined, 'post', { + code: values.code + }) .then((response) => { handleSuccessFullAuth(response, navigate, location, setError); return true; }) - .catch((err) => { + .catch(async (err) => { // Already logged in, but with a different session if (err?.response?.status == 409) { notifications.show({ @@ -354,11 +375,12 @@ export async function handleMfaLogin( ); if (mfa_trust?.is_pending) { setAuthContext(err.response.data.data); - authApi(apiUrl(ApiEndpoints.auth_trust), undefined, 'post', { + await authApi(apiUrl(ApiEndpoints.auth_trust), undefined, 'post', { trust: values.remember ?? false }).then((response) => { handleSuccessFullAuth(response, navigate, location, setError); }); + return true; } } else { const errors = err.response?.data?.errors; @@ -371,7 +393,6 @@ export async function handleMfaLogin( } return false; }); - return result; } /** @@ -382,7 +403,7 @@ export async function handleMfaLogin( * - An existing CSRF cookie is stored in the browser */ export const checkLoginState = async ( - navigate: any, + navigate: NavigateFunction, redirect?: any, no_redirect?: boolean ) => { @@ -396,22 +417,25 @@ export const checkLoginState = async ( const { isLoggedIn, fetchUserState } = useUserState.getState(); // Callback function when login is successful - const loginSuccess = () => { + const loginSuccess = async () => { setLoginChecked(true); showLoginNotification({ title: t`Logged In`, message: t`Successfully logged in` }); + MfaSetupOk(navigate).then(async (isOk) => { + if (isOk) { + observeProfile(); + await fetchGlobalStates(); - observeProfile(); - - fetchGlobalStates(navigate); - followRedirect(navigate, redirect); + followRedirect(navigate, redirect); + } + }); }; if (isLoggedIn()) { // Already logged in - loginSuccess(); + await loginSuccess(); return; } @@ -420,7 +444,7 @@ export const checkLoginState = async ( await fetchUserState(); if (isLoggedIn()) { - loginSuccess(); + await loginSuccess(); } else if (!no_redirect) { setLoginChecked(true); navigate('/login', { state: redirect }); @@ -429,8 +453,8 @@ export const checkLoginState = async ( }; function handleSuccessFullAuth( - response?: any, - navigate?: NavigateFunction, + response: any, + navigate: NavigateFunction, location?: Location, setError?: (message: string | undefined) => void ) { @@ -447,12 +471,16 @@ function handleSuccessFullAuth( } setAuthenticated(); - fetchUserState().finally(() => { - observeProfile(); - fetchGlobalStates(navigate); + // see if mfa registration is required + MfaSetupOk(navigate).then(async (isOk) => { + if (isOk) { + await fetchUserState(); + observeProfile(); + await fetchGlobalStates(); - if (navigate && location) { - followRedirect(navigate, location?.state); + if (location !== undefined) { + followRedirect(navigate, location?.state); + } } }); } @@ -545,7 +573,12 @@ export function handleVerifyTotp( authApi(apiUrl(ApiEndpoints.auth_totp), undefined, 'post', { code: value }).then(() => { - followRedirect(navigate, location?.state); + showNotification({ + title: t`MFA Setup successful`, + message: t`MFA via TOTP has been set up successfully; you will need to login again.`, + color: 'green' + }); + doLogout(navigate); }); }; } @@ -689,8 +722,8 @@ export function handleChangePassword( } export async function handleWebauthnLogin( - navigate?: NavigateFunction, - location?: Location + navigate: NavigateFunction, + location: Location ) { const { setAuthContext } = useServerApiState.getState(); diff --git a/src/frontend/src/pages/Index/Settings/AccountSettings/MFASettings.tsx b/src/frontend/src/pages/Index/Settings/AccountSettings/MFASettings.tsx index 52c811cf8a..c59121d306 100644 --- a/src/frontend/src/pages/Index/Settings/AccountSettings/MFASettings.tsx +++ b/src/frontend/src/pages/Index/Settings/AccountSettings/MFASettings.tsx @@ -28,12 +28,14 @@ import { } from '@tabler/icons-react'; import { useQuery } from '@tanstack/react-query'; import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useNavigate } from 'react-router-dom'; import { useShallow } from 'zustand/react/shallow'; import { api, queryClient } from '../../../../App'; import { CopyButton } from '../../../../components/buttons/CopyButton'; import { StylishText } from '../../../../components/items/StylishText'; -import { authApi } from '../../../../functions/auth'; +import { authApi, doLogout } from '../../../../functions/auth'; import { useServerApiState } from '../../../../states/ServerApiState'; +import { useGlobalSettingsState } from '../../../../states/SettingsStates'; import { QrRegistrationForm } from './QrRegistrationForm'; import { parseDate } from './SecurityContent'; @@ -697,6 +699,7 @@ export default function MFASettings() { const [auth_config] = useServerApiState( useShallow((state) => [state.auth_config]) ); + const navigate = useNavigate(); // Fetch list of MFA methods currently configured for the user const { isLoading, data, refetch } = useQuery({ @@ -708,6 +711,17 @@ export default function MFASettings() { .catch(() => []) }); + const refetchAfterRemoval = () => { + refetch(); + if ( + data == undefined && + useGlobalSettingsState.getState().isSet('LOGIN_ENFORCE_MFA') + ) { + console.log('MFA enforced but no MFA methods remain - logging out now'); + doLogout(navigate); + } + }; + // Memoize the list of currently used MFA factors const usedFactors: string[] = useMemo(() => { if (isLoading || !data) return []; @@ -921,14 +935,14 @@ export default function MFASettings() { opened={removeTOTPModalOpen} setOpen={setRemoveTOTPModalOpen} onReauthFlow={reauthenticate} - onSuccess={refetch} + onSuccess={refetchAfterRemoval} /> { + console.error(`ERR: Error rendering component: ${error}`); + }, + [] + ); + return ( @@ -85,7 +94,12 @@ export function SecurityContent() { {t`Access Tokens`} - + } + onError={onError} + > + + {user.isSuperuser() && ( diff --git a/src/frontend/src/states/states.tsx b/src/frontend/src/states/states.tsx index 82e015e757..8369e66631 100644 --- a/src/frontend/src/states/states.tsx +++ b/src/frontend/src/states/states.tsx @@ -1,5 +1,4 @@ import type { PluginProps } from '@lib/types/Plugins'; -import type { NavigateFunction } from 'react-router-dom'; import { setApiDefaults } from '../App'; import { useGlobalStatusState } from './GlobalStatusState'; import { useIconState } from './IconState'; @@ -45,9 +44,7 @@ export interface ServerAPIProps { * Refetch all global state information. * Necessary on login, or if locale is changed. */ -export async function fetchGlobalStates( - navigate?: NavigateFunction | undefined -) { +export async function fetchGlobalStates() { const { isLoggedIn } = useUserState.getState(); if (!isLoggedIn()) {