Add docs for MFA totp#704
Conversation
📝 WalkthroughWalkthroughThis PR adds complete documentation for the new Local MFA (TOTP-based) authentication feature in self-hosted NetBird. It introduces a new documentation page explaining how to enable and configure local MFA, adds configuration field documentation, and updates the navigation structure to link to the new guidance. ChangesLocal MFA Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/components/NavigationDocs.jsxOops! Something went wrong! :( ESLint: 9.39.4 TypeError: Converting circular structure to JSON src/pages/selfhosted/configuration-files.mdxOops! Something went wrong! :( ESLint: 9.39.4 TypeError: Converting circular structure to JSON src/pages/selfhosted/identity-providers/enable-local-mfa.mdxOops! Something went wrong! :( ESLint: 9.39.4 TypeError: Converting circular structure to JSON 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/selfhosted/identity-providers/enable-local-mfa.mdx`:
- Line 9: Update the opening behavior sentence to match the "MFA session"
section: change the claim that TOTP is required "on every login" to state that
TOTP is required for each authentication unless an active MFA session is present
(referencing the "MFA session" section describing session-based skipping);
ensure the text makes clear the first login or any authentication after session
expiry always requires TOTP while active sessions allow skipping MFA until they
expire.
- Around line 114-115: The warning sentence "Disable MFA removes the TOTP
requirement but **does not** remove the stored TOTP authenticator information.
If you enable MFA again previously enrolled users will be prompted for the same
authenticator they registered previously." is awkward and should be replaced
with a clearer, grammatically correct phrasing; update the text in
src/pages/selfhosted/identity-providers/enable-local-mfa.mdx to something like:
"Disabling MFA removes the TOTP requirement but does not delete stored TOTP
authenticator data; if you re-enable MFA, previously enrolled users will be
prompted to use the same authenticator they registered earlier." Ensure the new
sentence preserves the original security warning and punctuation for clarity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a23a340-e378-4435-b2f8-6b9541b4c4a5
⛔ Files ignored due to path filters (2)
public/docs-static/img/selfhosted/identity-providers/local-mfa/totp-setup.pngis excluded by!**/*.pngpublic/docs-static/img/selfhosted/identity-providers/local-mfa/totp-verify.pngis excluded by!**/*.png
📒 Files selected for processing (3)
src/components/NavigationDocs.jsxsrc/pages/selfhosted/configuration-files.mdxsrc/pages/selfhosted/identity-providers/enable-local-mfa.mdx
|
|
||
| NetBird supports multi-factor authentication (MFA) for local users managed by the embedded identity provider. When enabled, all local users are required to set up and verify a time-based one-time password (TOTP) using an authenticator app before they can log in. | ||
|
|
||
| This is useful when you want to add an extra layer of security to local users, as it will require **all** local users to authenticate using the TOTP code on every login. |
There was a problem hiding this comment.
Behavior statement conflicts with MFA session semantics.
Line 9 says users must enter TOTP on every login, but the MFA session section (Lines 58-62) says users can skip MFA while the session is active. Please align this wording to avoid misleading operators.
Suggested wording
-This is useful when you want to add an extra layer of security to local users, as it will require **all** local users to authenticate using the TOTP code on every login.
+This is useful when you want to add an extra layer of security to local users, as it requires **all** local users to complete TOTP during sign-in whenever their MFA session is not active.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This is useful when you want to add an extra layer of security to local users, as it will require **all** local users to authenticate using the TOTP code on every login. | |
| This is useful when you want to add an extra layer of security to local users, as it requires **all** local users to complete TOTP during sign-in whenever their MFA session is not active. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/selfhosted/identity-providers/enable-local-mfa.mdx` at line 9,
Update the opening behavior sentence to match the "MFA session" section: change
the claim that TOTP is required "on every login" to state that TOTP is required
for each authentication unless an active MFA session is present (referencing the
"MFA session" section describing session-based skipping); ensure the text makes
clear the first login or any authentication after session expiry always requires
TOTP while active sessions allow skipping MFA until they expire.
| Disable MFA removes the TOTP requirement but **does not** remove the stored TOTP authenticator information. If you enable MFA again | ||
| previously enrolled users will be prompted for the same authenticator they registered previously. |
There was a problem hiding this comment.
Clarify the warning sentence grammar.
The warning text is grammatically off and harder to parse in a security-sensitive section.
Suggested wording
-Disable MFA removes the TOTP requirement but **does not** remove the stored TOTP authenticator information. If you enable MFA again
-previously enrolled users will be prompted for the same authenticator they registered previously.
+Disabling MFA removes the TOTP requirement but **does not** remove stored authenticator enrollment data. If you enable MFA again, previously enrolled users will be prompted to use the same authenticator.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Disable MFA removes the TOTP requirement but **does not** remove the stored TOTP authenticator information. If you enable MFA again | |
| previously enrolled users will be prompted for the same authenticator they registered previously. | |
| Disabling MFA removes the TOTP requirement but **does not** remove stored authenticator enrollment data. If you enable MFA again, previously enrolled users will be prompted to use the same authenticator. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/selfhosted/identity-providers/enable-local-mfa.mdx` around lines
114 - 115, The warning sentence "Disable MFA removes the TOTP requirement but
**does not** remove the stored TOTP authenticator information. If you enable MFA
again previously enrolled users will be prompted for the same authenticator they
registered previously." is awkward and should be replaced with a clearer,
grammatically correct phrasing; update the text in
src/pages/selfhosted/identity-providers/enable-local-mfa.mdx to something like:
"Disabling MFA removes the TOTP requirement but does not delete stored TOTP
authenticator data; if you re-enable MFA, previously enrolled users will be
prompted to use the same authenticator they registered earlier." Ensure the new
sentence preserves the original security warning and punctuation for clarity.
configuration files updates:
Enable MFA for local users
Summary by CodeRabbit
mfaSessionMaxLifetimeandmfaSessionIdleTimeout).