Skip to content

Mfa flow editor - #3551

Open
j-chmielewski wants to merge 25 commits into
devfrom
mfa-flow
Open

Mfa flow editor#3551
j-chmielewski wants to merge 25 commits into
devfrom
mfa-flow

Conversation

@j-chmielewski

Copy link
Copy Markdown
Contributor

Related issue: #3044

Implements the multi-step MFA flow editor.

@j-chmielewski
j-chmielewski marked this pull request as ready for review August 17, 2026 10:11
},
});
const form = useAppForm({
defaultValues: flow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memo this

import { MfaFormPage } from '../../../../../pages/MfaPage/MfaFormPage';
import api from '../../../../../shared/api/api';

export const Route = createFileRoute('/_authorized/_default/mfa-flow/$id/edit')({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usage of loader without any loader component, we switched to skeletons a while ago.

return (await api.mfaFlow.get(id)).data;
} catch (error) {
if (axios.isAxiosError(error) && error.response?.status === 404) throw notFound();
throw error;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would crash the app better to throw redirect and show snackbar

export const Route = createFileRoute('/_authorized/_default/mfa-flow/$id/edit')({
loader: async ({ params }) => {
const id = Number(params.id);
if (!Number.isSafeInteger(id) || id <= 0) throw notFound();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if i remember correctly we have no 404 implemented yet for core.

return undefined;
}
};
/** Builds one selectable MFA method menu item. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/** Builds one selectable MFA method menu item. */

),
}));

const deleteStep = (id: MfaConfigurationStepData['id']) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you pass methods to components it is required to wrap them in the useCallback

[MfaFlowMethod.Biometric]: m.mfa_flow_method_biometric(),
};
/** Maps a backend availability reason to menu guidance. */
const getDisabledHelper = (reason: MfaMethodAvailabilityReasonValue) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't need to be in component scope

export const MfaConfiguration = ({ onChange, steps, error }: MfaConfigurationProps) => {
const { data: methodAvailability } = useQuery(getMfaMethodAvailabilityQueryOptions);
const availableMethods = methodAvailability?.map(({ method }) => method) ?? [];
const methodLabels: Record<MfaFlowMethodValue, string> = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't need to be in component scope


export const MfaConfiguration = ({ onChange, steps, error }: MfaConfigurationProps) => {
const { data: methodAvailability } = useQuery(getMfaMethodAvailabilityQueryOptions);
const availableMethods = methodAvailability?.map(({ method }) => method) ?? [];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use select in query for this.

}

/** MFA methods grouped for display in an add-method menu. */
export type MfaMethodGroup = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if those are not from API then name should relate to the component used, if this is from API then it should be in api types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants