2
0
mirror of https://github.com/inventree/InvenTree.git synced 2026-01-29 17:43:55 +00:00

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
This commit is contained in:
Matthias Mair
2026-01-15 23:33:10 +01:00
committed by GitHub
parent 07e1a72261
commit 9fa40ae572
16 changed files with 250 additions and 64 deletions

View File

@@ -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

View File

@@ -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(

View File

@@ -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

View File

@@ -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'),

View File

@@ -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):

View File

@@ -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 <key>."""
# These keys are invalid, and should return 404

View File

@@ -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',

View File

@@ -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 {

View File

@@ -34,6 +34,8 @@ export interface Setting {
method?: string;
required?: boolean;
read_only?: boolean;
confirm?: boolean;
confirm_text?: string;
}
export interface SettingChoice {

View File

@@ -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 (
<Alert
color='red'

View File

@@ -1,3 +1,4 @@
import { t } from '@lingui/core/macro';
import {
Button,
Group,
@@ -6,6 +7,7 @@ import {
Stack,
Switch,
Text,
Tooltip,
useMantineColorScheme
} from '@mantine/core';
import { IconEdit } from '@tabler/icons-react';
@@ -20,6 +22,24 @@ import { vars } from '../../theme';
import { Boundary } from '../Boundary';
import { RenderInstance } from '../render/Instance';
type ConfirmResult = {
requires_confirmation: boolean;
confirmed: boolean;
proceed?: boolean;
};
function confirmSettingChange(setting: Setting): ConfirmResult {
if (setting.confirm) {
const confirmed = window.confirm(
setting.confirm_text || t`Do you want to proceed to change this setting?`
);
return {
requires_confirmation: true,
confirmed: confirmed || false,
proceed: confirmed
};
}
return { requires_confirmation: false, confirmed: false, proceed: true };
}
/**
* Render a single setting value
*/
@@ -29,8 +49,8 @@ function SettingValue({
onToggle
}: Readonly<{
setting: Setting;
onEdit: (setting: Setting) => 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({
<Text size='xs'>{setting.description}</Text>
</Stack>
<Boundary label={`setting-value-${setting.key}`}>
<SettingValue setting={setting} onEdit={onEdit} onToggle={onToggle} />
<Group gap='xs' justify='right'>
{setting.confirm && (
<Tooltip label={t`This setting requires confirmation`}>
<IconEdit color={vars.colors.yellow[7]} size={16} />
</Tooltip>
)}
<SettingValue
setting={setting}
onEdit={onEdit}
onToggle={onToggle}
/>
</Group>
</Boundary>
</Group>
</Paper>

View File

@@ -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');

View File

@@ -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<any>,
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<any>
navigate: NavigateFunction,
location: Location<any>
) {
const { setAuthContext } = useServerApiState.getState();

View File

@@ -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}
/>
<RemoveWebauthnModal
tokenId={webauthnToken}
opened={removeWebauthnModalOpen}
setOpen={setRemoveWebauthnModalOpen}
onReauthFlow={reauthenticate}
onSuccess={refetch}
onSuccess={refetchAfterRemoval}
/>
<RegisterTOTPModal
opened={registerTOTPModalOpen}

View File

@@ -20,6 +20,7 @@ import {
TextInput
} from '@mantine/core';
import { hideNotification, showNotification } from '@mantine/notifications';
import { ErrorBoundary } from '@sentry/react';
import {
IconAlertCircle,
IconAt,
@@ -29,6 +30,7 @@ import {
import { useQuery } from '@tanstack/react-query';
import { useCallback, useMemo, useState } from 'react';
import { useShallow } from 'zustand/react/shallow';
import { DefaultFallback } from '../../../../components/Boundary';
import { StylishText } from '../../../../components/items/StylishText';
import { ProviderLogin, authApi } from '../../../../functions/auth';
import { useServerApiState } from '../../../../states/ServerApiState';
@@ -43,6 +45,13 @@ export function SecurityContent() {
const user = useUserState();
const onError = useCallback(
(error: unknown, componentStack: string | undefined, eventId: string) => {
console.error(`ERR: Error rendering component: ${error}`);
},
[]
);
return (
<Stack>
<Accordion multiple defaultValue={['email']}>
@@ -85,7 +94,12 @@ export function SecurityContent() {
<StylishText size='lg'>{t`Access Tokens`}</StylishText>
</Accordion.Control>
<Accordion.Panel>
<ApiTokenTable only_myself />
<ErrorBoundary
fallback={<DefaultFallback title={'API Table'} />}
onError={onError}
>
<ApiTokenTable only_myself />
</ErrorBoundary>
</Accordion.Panel>
</Accordion.Item>
{user.isSuperuser() && (

View File

@@ -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()) {