Add Organization Level TOTP Enrollment Configuration UI #9494
Conversation
WalkthroughAdds a TOTP authenticator form and exports it; integrates TOTP into authenticator form factories; disables external-authenticators API usage for TOTP; narrows Settings-tab hiding/default-tab skip to Magic Link only; and adds i18n entries and a changeset for TOTP enrollment. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Factory as AuthenticatorFormFactory
participant Form as TOTPAuthenticatorForm
participant Editor as EditMFAComponent
participant API as BackendAPI
User->>Factory: Open authenticator editor (TOTP)
Factory->>Form: Render with metadata & initialValues
Form->>Form: Normalize initialValues (keys, booleans)
User->>Form: Update fields and submit
Form->>Editor: onSubmit(updated config)
Editor->>API: Persist authenticator configuration
API-->>Editor: Acknowledge save
Editor-->>User: Update UI / close editor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (2)
189-189: Use strict equality operator.Replace loose equality (
!=) with strict equality (!==) for more predictable behavior and to align with JavaScript best practices.Apply this diff:
- if (name != undefined) { + if (name !== undefined) { const moderatedName: string = name.replace(/_/g, ".");
238-241: Consider using i18n for accessibility labels.The
ariaLabelattributes use hardcoded English strings. While the visiblelabelfor the checkbox uses the i18n system viaTrans, consider using i18n for aria labels as well to maintain consistency with the codebase pattern (though the current approach may be acceptable if aria labels are intentionally kept static).Also applies to: 260-260
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
features/admin.connections.v1/components/edit/edit-multi-factor-authenticator.tsx(1 hunks)features/admin.connections.v1/components/edit/forms/authenticators/index.ts(1 hunks)features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx(1 hunks)features/admin.connections.v1/components/edit/forms/factories/authenticator-form-factory.tsx(2 hunks)features/admin.connections.v1/constants/connection-ui-constants.ts(1 hunks)features/admin.connections.v1/meta/authenticator-meta.ts(1 hunks)features/admin.identity-providers.v1/components/forms/factories/authenticator-form-factory.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
features/admin.connections.v1/components/edit/forms/factories/authenticator-form-factory.tsx (1)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (1)
TOTPAuthenticatorForm(113-270)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (1)
features/admin.identity-providers.v1/models/identity-provider.ts (7)
CommonAuthenticatorFormMetaInterface(329-329)CommonAuthenticatorFormInitialValuesInterface(335-335)CommonAuthenticatorFormFieldInterface(347-349)CommonAuthenticatorFormPropertyInterface(341-341)CommonAuthenticatorFormFieldMetaInterface(355-355)CommonPluggableComponentMetaPropertyInterface(357-373)CommonPluggableComponentPropertyInterface(319-323)
features/admin.identity-providers.v1/components/forms/factories/authenticator-form-factory.tsx (1)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (1)
TOTPAuthenticatorForm(113-270)
🔇 Additional comments (11)
features/admin.identity-providers.v1/components/forms/factories/authenticator-form-factory.tsx (2)
23-23: LGTM! Import follows established patterns.The import correctly references the TOTP authenticator form from the sibling authenticators module.
293-306: LGTM! TOTP authenticator integration follows existing patterns.The new case correctly mirrors the pattern used by other local authenticators (SMS OTP, Email OTP, Push), passing all required props consistently.
features/admin.connections.v1/components/edit/edit-multi-factor-authenticator.tsx (2)
254-264: LGTM! Settings tab correctly enabled for TOTP.The condition now appropriately excludes only Magic Link authenticators from the Settings tab, allowing TOTP to surface its configuration UI. The updated comment accurately reflects this narrowed scope.
275-284: LGTM! Default active index logic is consistent.The change correctly mirrors the tab exclusion logic, ensuring TOTP follows the standard default tab behavior.
features/admin.connections.v1/constants/connection-ui-constants.ts (1)
170-199: LGTM! TOTP constraints are well-defined and reasonable.The constraint values align with standard TOTP specifications (4-10 digit OTP length, 0-3 window size for time drift tolerance, 1-1440 minute expiry). The structure follows the established pattern used by EMAIL_OTP and SMS_OTP authenticators.
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (2)
135-175: LGTM! Property name transformation logic is sound.The effect correctly handles the dot-to-underscore transformation and boolean string parsing, maintaining consistency with the form's internal state management.
183-210: LGTM! Form submission logic correctly reverses the transformation.The function properly converts underscore-separated names back to dot notation and ensures all values are stringified for submission.
features/admin.connections.v1/components/edit/forms/authenticators/index.ts (1)
28-28: LGTM! Export follows established barrel pattern.The TOTP authenticator form is correctly exposed through the public API surface.
features/admin.connections.v1/meta/authenticator-meta.ts (1)
373-382: LGTM! API flag correctly aligned with new form integration.Setting
useAuthenticatorsAPI: falsefor TOTP is consistent with similar authenticators (Email OTP, SMS OTP, FIDO, Push) and properly enables the local form flow introduced in this PR.features/admin.connections.v1/components/edit/forms/factories/authenticator-form-factory.tsx (2)
42-43: LGTM! Import organized correctly.The TOTPAuthenticatorForm import is properly placed alongside other authenticator form imports.
249-262: LGTM! TOTP case follows established factory pattern.The implementation correctly mirrors the SMS_OTP authenticator case, passing all standard props and maintaining consistency across the factory.
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9494 +/- ##
=======================================
Coverage 55.88% 55.88%
=======================================
Files 42 42
Lines 1020 1020
Branches 231 231
=======================================
Hits 570 570
Misses 450 450
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
features/admin.identity-providers.v1/components/forms/factories/authenticator-form-factory.tsx (1)
19-19: New import looks fine; ensure package dependency boundaries are intended.
Just verify@wso2is/admin.connections.v1is an allowed dependency for@wso2is/admin.identity-providers.v1(to avoid unintended coupling/version skew).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx(1 hunks)features/admin.identity-providers.v1/components/forms/factories/authenticator-form-factory.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (1)
features/admin.identity-providers.v1/models/identity-provider.ts (6)
CommonAuthenticatorFormMetaInterface(329-329)CommonAuthenticatorFormInitialValuesInterface(335-335)CommonAuthenticatorFormFieldInterface(347-349)CommonAuthenticatorFormPropertyInterface(341-341)CommonPluggableComponentMetaPropertyInterface(357-373)CommonPluggableComponentPropertyInterface(319-323)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
features/admin.identity-providers.v1/components/forms/factories/authenticator-form-factory.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (5)
98-103: Fix type mismatch in error validation interface.
TOTP_EnrolUserInAuthenticationFlowis declared as a requiredstringbut line 211 assignsundefinedto it, creating a type mismatch. Since validation errors are conditionally returned, this field should be optional.Apply this diff:
export interface TOTPAuthenticatorFormErrorValidationsInterface { /** * Enable progressive enrollment. */ - TOTP_EnrolUserInAuthenticationFlow: string; + TOTP_EnrolUserInAuthenticationFlow?: string; }
38-73: Make unused props optional to align interface with implementation.
triggerSubmit,enableSubmitButton, andshowCustomPropertiesare declared as required but never destructured or used in the component. The comment on line 66 explicitly statesshowCustomPropertiesis "Not implemented ATM." This creates a contract mismatch where callers must provide values that the component ignores.Apply this diff to make these props optional:
- triggerSubmit: boolean; + triggerSubmit?: boolean; - enableSubmitButton: boolean; + enableSubmitButton?: boolean; - showCustomProperties: boolean; + showCustomProperties?: boolean;
178-200: Add null safety and remove redundant conditional ingetUpdatedConfigurations.Line 189 has a redundant conditional (
isBoolean(value) ? value.toString() : value.toString()) where both branches calltoString(). More critically,toString()will throw ifvalueisnullorundefined, which can occur as form fields evolve.Apply this diff:
for (const [ name, value ] of Object.entries(values)) { - if (name != undefined) { + if (name) { const moderatedName: string = name.replace(/_/g, "."); + if (value === null || value === undefined) { + continue; + } + properties.push({ name: moderatedName, - value: isBoolean(value) ? value.toString() : value.toString() + value: String(value) }); } }
117-124:triggerSubmitprop is not destructured or implemented.While
triggerSubmitis declared in the props interface (line 59), it's never destructured here or used anywhere in the component. If parent components need to trigger form submission externally, this will fail silently.To implement external form submission, add a ref and a
useEffectto watchtriggerSubmit:const { metadata, initialValues: originalInitialValues, onSubmit, readOnly, isSubmitting, + triggerSubmit, + enableSubmitButton, ["data-testid"]: testId } = props; const { t } = useTranslation(); +const formRef = useRef<any>(); +useEffect(() => { + if (triggerSubmit && formRef.current) { + formRef.current.submitForm(); + } +}, [ triggerSubmit ]);Then attach the ref to the Form component:
<Form + ref={ formRef } id={ FORM_ID }Also destructure
enableSubmitButtonto use it on the submit button (line 253).
128-130: Remove unused state and fix type-unsafe initialization.Line 129 declares
setFormFieldsbut the state is never read—only written at line 168, making it dead code. Additionally, both state variables are initialized withundefined, violating their type contracts (TOTPAuthenticatorFormFieldsInterfaceandTOTPAuthenticatorFormInitialValuesInterfacedon't allowundefined).Apply this diff:
- // This can be used when `meta` support is there. - const [ , setFormFields ] = useState<TOTPAuthenticatorFormFieldsInterface>(undefined); - const [ initialValues, setInitialValues ] = useState<TOTPAuthenticatorFormInitialValuesInterface>(undefined); + const [ initialValues, setInitialValues ] = useState<TOTPAuthenticatorFormInitialValuesInterface | undefined>(undefined);And remove the call to
setFormFieldsat line 168:- setFormFields(resolvedFormFields); setInitialValues(resolvedInitialValues);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (1)
features/admin.identity-providers.v1/models/identity-provider.ts (7)
CommonAuthenticatorFormMetaInterface(329-329)CommonAuthenticatorFormInitialValuesInterface(335-335)CommonAuthenticatorFormFieldInterface(347-349)CommonAuthenticatorFormPropertyInterface(341-341)CommonAuthenticatorFormFieldMetaInterface(355-355)CommonPluggableComponentMetaPropertyInterface(357-373)CommonPluggableComponentPropertyInterface(319-323)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/index.ts
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR adds organization-level TOTP (Time-based One-Time Password) enrollment configuration UI to the authentication provider settings. It enables administrators to configure progressive enrollment for TOTP devices, allowing users to enroll their devices during the login flow.
Changes:
- Added a new TOTP authenticator form component with a progressive enrollment checkbox setting
- Integrated the TOTP form into both identity provider and connection authenticator form factories
- Updated TOTP authenticator metadata to use the local form instead of the external authenticators API
- Fixed settings tab visibility to show settings for TOTP authenticator (previously excluded)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
totp-authenticator-form.tsx |
New form component for TOTP authenticator configuration with progressive enrollment option |
authenticator-form-factory.tsx (identity-providers) |
Added TOTP case to render the new TOTP form |
authenticator-form-factory.tsx (connections) |
Added TOTP case to render the new TOTP form |
index.ts |
Exported the new TOTP authenticator form |
authenticator-meta.ts |
Changed useAuthenticatorsAPI flag to false for TOTP to use local form |
edit-multi-factor-authenticator.tsx |
Removed TOTP from settings tab exclusion list to enable settings UI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.changeset/lazy-paws-greet.md:
- Line 6: Update the changeset note to fix grammar by replacing the phrase "a
org level config to govern TOTP" with "an org-level config to govern TOTP" so it
uses the correct article and hyphenation.
♻️ Duplicate comments (3)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (3)
126-151: Avoid spreadingnulland guard optional property names.
{ ...resolvedFormFields }and{ ...resolvedInitialValues }will throw when the first property is processed because both are initialized tonull. Also,value.nameis optional, soreplace()can crash when it’s missing.🐛 Proposed fix
- let resolvedFormFields: TOTPAuthenticatorFormFieldsInterface = null; - let resolvedInitialValues: TOTPAuthenticatorFormInitialValuesInterface = null; + let resolvedFormFields: Partial<TOTPAuthenticatorFormFieldsInterface> = {}; + let resolvedInitialValues: TOTPAuthenticatorFormInitialValuesInterface = { + TOTP_EnrolUserInAuthenticationFlow: false + }; @@ - const moderatedName: string = value.name.replace(/\./g, "_"); + const rawName: string = value.name ?? value.key ?? ""; + if (!rawName) { + return; + } + const moderatedName: string = rawName.replace(/\./g, "_");
168-175: Guard nullish values beforetoString()in submission mapping.If any field value is
null/undefined,value.toString()will throw. Normalize safely.✅ Proposed fix
- for (const [ name, value ] of Object.entries(values)) { - if (name !== undefined) { + for (const [ name, value ] of Object.entries(values ?? {})) { + if (!name || value === null || value === undefined) { + continue; + } const moderatedName: string = name.replace(/_/g, "."); properties.push({ name: moderatedName, - value: value.toString() + value: String(value) }); } }
83-88: Align validation return type with interface.
TOTP_EnrolUserInAuthenticationFlowis declared as a required string, butvalidateForm()returnsundefined. Make it optional or return an empty object.🔧 Suggested fix
export interface TOTPAuthenticatorFormErrorValidationsInterface { /** * Enable progressive enrollment. */ - TOTP_EnrolUserInAuthenticationFlow: string; + TOTP_EnrolUserInAuthenticationFlow?: string; } @@ - const errors: TOTPAuthenticatorFormErrorValidationsInterface = { - TOTP_EnrolUserInAuthenticationFlow: undefined - }; - - return errors; + return {};Also applies to: 192-199
features/admin.identity-providers.v1/components/forms/factories/authenticator-form-factory.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx`:
- Around line 139-151: In getUpdatedConfigurations, avoid calling
value.toString() unguarded; replicate the pattern used in other forms by
checking isBoolean(value) before converting — i.e., when building
CommonPluggableComponentPropertyInterface entries in the loop over
Object.entries(values), set value to isBoolean(value) ? value.toString() : value
so null/undefined or non-boolean values won't cause a runtime error; reference
the getUpdatedConfigurations function and the properties array where the push
occurs and use the existing isBoolean helper.
♻️ Duplicate comments (3)
.changeset/lazy-paws-greet.md (1)
7-7: Fix changeset grammar (“an org-level”).Minor wording polish for the release note.
✏️ Suggested edit
-Introduce a org level config to govern TOTP +Introduce an org-level config to govern TOTPfeatures/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (2)
116-120: Guard optionalvalue.namebeforereplace().
CommonPluggableComponentPropertyInterface.nameis optional; callingreplace()can throw when it’s missing.🛠️ Suggested fix
- const moderatedName: string = value.name.replace(/\./g, "_"); + const rawName: string = value.name ?? value.key; + if (!rawName) { + return; + } + const moderatedName: string = rawName.replace(/\./g, "_");
103-131: Guard against optionalnameproperty before calling.replace().
value.nameis optional inCommonPluggableComponentPropertyInterfacebut line 120 calls.replace()without a null/undefined check, causing a runtime crash. Additionally, the early return at line 111 leavesinitialValuesasundefined, andresolvedInitialValuesstarts asnull, leading to inconsistent state passed to the Form. Initialize with a safe default object and ensure all code paths set valid values.🛠️ Suggested fix
+ const defaultInitialValues: TOTPAuthenticatorFormInitialValuesInterface = { + TOTP_EnrolUserInAuthenticationFlow: false + }; - - const [ initialValues, setInitialValues ] = useState<TOTPAuthenticatorFormInitialValuesInterface>(undefined); + const [ initialValues, setInitialValues ] = + useState<TOTPAuthenticatorFormInitialValuesInterface>(defaultInitialValues); @@ - if (isEmpty(originalInitialValues?.properties)) { - return; - } - - let resolvedInitialValues: TOTPAuthenticatorFormInitialValuesInterface = null; + if (isEmpty(originalInitialValues?.properties)) { + setInitialValues(defaultInitialValues); + return; + } + + let resolvedInitialValues: TOTPAuthenticatorFormInitialValuesInterface = { + ...defaultInitialValues + }; + - const moderatedName: string = value.name.replace(/\./g, "_"); + const moderatedName: string = (value.name ?? "").replace(/\./g, "_");
🧹 Nitpick comments (1)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (1)
172-176: Use i18n label instead of hardcoded text.You already added the label in i18n; wire it here so localization stays consistent.
♻️ Suggested change
- label="Enable TOTP device progressive enrollment" + label={ t("authenticationProvider:forms.authenticatorSettings.totp.enrollUserInAuthenticationFlow.label") }
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx`:
- Around line 110-121: The loop over originalInitialValues.properties uses
value.name without checking for undefined which can throw; update the code in
the forEach that computes moderatedName (inside
TOTPAuthenticatorFormInitialValues resolution) to guard against missing name by
using optional chaining or a default (e.g. const moderatedName = (value.name ??
"") .replace(/\./g, "_") or skip entries with no name), and ensure you handle
the resulting key being empty (skip or use a safe fallback) before assigning
into resolvedInitialValues; keep all other logic (parsing "true"/"false") the
same and reference the variables/methods: originalInitialValues.properties,
value (CommonAuthenticatorFormPropertyInterface), moderatedName, and
resolvedInitialValues.
♻️ Duplicate comments (3)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (3)
165-183: Use i18n for the checkbox label.The
labelprop on line 168 is hardcoded in English while thehintcorrectly usesTransfor i18n. For consistency and localization support, the label should also uset().Proposed fix
<Field.Checkbox ariaLabel="Enable TOTP device progressive enrollment" name="TOTP_EnrolUserInAuthenticationFlow" - label="Enable TOTP device progressive enrollment" + label={ t("authenticationProvider:forms.authenticatorSettings.totp.enrollUserInAuthenticationFlow.label") } hint={Ensure a corresponding i18n key is added to the translation files.
99-99: Fix type mismatch: state initialized withundefinedbut type doesn't allow it.
TOTPAuthenticatorFormInitialValuesInterfacedoesn't includeundefined, yet the state is initialized withundefined. This causes a TypeScript type mismatch.Proposed fix
- const [ initialValues, setInitialValues ] = useState<TOTPAuthenticatorFormInitialValuesInterface>(undefined); + const [ initialValues, setInitialValues ] = useState<TOTPAuthenticatorFormInitialValuesInterface | undefined>(undefined);
137-146: Add null guard beforevalue.toString()to prevent runtime crash.If a form field value is
nullorundefined, callingvalue.toString()will throw. Add a defensive check consistent with other authenticator forms in the codebase.🐛 Proposed fix
for (const [ name, value ] of Object.entries(values)) { if (name !== undefined) { const moderatedName: string = name.replace(/_/g, "."); + if (value === null || value === undefined) { + continue; + } + properties.push({ name: moderatedName, - value: value.toString() + value: String(value) }); } }
🧹 Nitpick comments (1)
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx (1)
70-75: Consider removing the unused error validation interface.
TOTPAuthenticatorFormErrorValidationsInterfaceis exported but there's no validation logic in this component. If validation is not planned, this interface is dead code and could be removed to improve maintainability.
features/admin.connections.v1/components/edit/forms/authenticators/totp-authenticator-form.tsx
Show resolved
Hide resolved
2 participants
Purpose
$subject
Related Issues
Related PRs
Checklist
Security checks
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.Summary by CodeRabbit
New Features
Behavior Change
Chores
✏️ Tip: You can customize this high-level summary in your review settings.