feat: add TOTP/MFA support for local controller auth#10
feat: add TOTP/MFA support for local controller auth#10mi-skam wants to merge 2 commits intovedanta:mainfrom
Conversation
UDM controllers with SSO accounts that have MFA enabled return HTTP 499 requiring a TOTP token. This adds UNIFI_CONTROLLER_TOTP env var support to pass the token during login, and proper error messaging when MFA is required but no token is provided. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The status command called login() directly, bypassing the session cache. This forced a fresh auth on every invocation, which breaks MFA accounts since a new TOTP code would be needed each time. Changed to ensure_authenticated() which reuses the cached session when valid. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for adding this, will test and merge soon |
There was a problem hiding this comment.
Pull request overview
Adds support for MFA-enabled local controller authentication by optionally including a TOTP token in the UDM login flow.
Changes:
- Introduces
UNIFI_CONTROLLER_TOTP(controller_totp) configuration and wires it into UDM login payloads astoken. - Improves UDM login failure handling by surfacing a clearer error when HTTP 499 indicates MFA is required.
- Updates local controller status check to use cached authentication via
ensure_authenticated().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/ui_cli/local_client.py |
Adds optional token to UDM login payload and handles HTTP 499 MFA-required responses. |
src/ui_cli/config.py |
Adds a new controller_totp setting for UNIFI_CONTROLLER_TOTP. |
src/ui_cli/commands/status.py |
Switches status check from login() to ensure_authenticated() (uses cached session). |
Comments suppressed due to low confidence (1)
src/ui_cli/commands/status.py:162
check_local_controllernow callsensure_authenticated(), which may return successfully without any network request when a cached session exists. That meansconnection_time_mscan become just a disk read, and worse, the code setsauthentication="Valid"before any request—if the controller is offline and the first API call later raisesLocalConnectionError, the handler only flipsconnectionto FAILED and leavesauthenticationas Valid. Consider measuring time around an actual request (e.g., a lightweight GET using the session) and only setting connection/authentication fields after that request succeeds (or ensuring the LocalConnectionError path also resets authentication).
client = UniFiLocalClient(timeout=STATUS_CHECK_TIMEOUT)
start = time.perf_counter()
await client.ensure_authenticated()
elapsed_ms = (time.perf_counter() - start) * 1000
result["connection"] = "OK"
result["connection_time_ms"] = round(elapsed_ms, 1)
result["authentication"] = "Valid"
result["controller_type"] = "UDM" if client._is_udm else "Cloud Key/Self-hosted"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self._is_udm: | ||
| login_data = { | ||
| "username": self.username, | ||
| "password": self.password, | ||
| "remember": True, | ||
| } | ||
| if self.totp: | ||
| login_data["token"] = self.totp | ||
| response = await client.post( | ||
| f"{self.controller_url}/api/auth/login", | ||
| json={ | ||
| "username": self.username, | ||
| "password": self.password, | ||
| "remember": True, | ||
| }, | ||
| json=login_data, | ||
| ) | ||
|
|
||
| if response.status_code == 200: | ||
| self._cookies = dict(response.cookies) | ||
| self._csrf_token = response.headers.get("X-CSRF-Token") | ||
| self._save_session() | ||
| return True | ||
| elif response.status_code == 499: | ||
| raise LocalAuthenticationError( | ||
| "MFA token required. Set UNIFI_CONTROLLER_TOTP to your current TOTP code." | ||
| ) |
There was a problem hiding this comment.
The MFA/TOTP flow adds new behavior in login() (including conditional inclusion of token and a dedicated 499 error path), but there are no unit tests covering UDM login payload construction or the 499/MFA-required error handling. Please add tests that mock the httpx client to assert token is omitted when unset, included when set, and that a 499 response raises LocalAuthenticationError with the expected message.
| controller_totp: str = Field( | ||
| default="", | ||
| description="TOTP code for MFA-enabled accounts", | ||
| ) |
There was a problem hiding this comment.
The new controller_totp setting is user-facing, but .env.example (and any related setup docs) doesn’t include UNIFI_CONTROLLER_TOTP, so it’s hard to discover/configure. Please add the new variable to the example env file and any relevant documentation sections so users know how to supply the MFA token.
Summary
UNIFI_CONTROLLER_TOTPenv var to pass a TOTP code during local controller loginContext
UDM controllers where the admin account has MFA enabled (via UI.com SSO) reject
/api/auth/loginwith status 499 andMFA_AUTH_REQUIRED. The fix includes thetokenfield in the login payload whenUNIFI_CONTROLLER_TOTPis set.Once authenticated, the session cookie is cached (existing behavior), so the TOTP is only needed for the initial login.
Test plan
ui local health,ui local devices list,ui local clients list,ui local networks listall work after auth🤖 Generated with Claude Code