fix(auth-ui): admin credential fields are always visible in Authentication settings

[kevinheneveld]

Summary

The Admin Account Management subpanel in Settings → Authentication was gated by v-if="authEnabledComputed", which meant the username/password inputs were invisible whenever the Enable login screen toggle was off. Combined with the toggle reflecting the server-side AuthenticationRequired value from config.json, this produced a hard lockout for new installs:

1. Operator edits config/config.json to set AuthenticationRequired: "true" (or uses the UI toggle and clicks Save — which writes the same value).
2. Login screen activates on the next request.
3. No admin user has been provisioned, because the only UI affordance to do so was inside the gated section that doesn't render until auth is already on.
4. Every login attempt fails. There is no UI path to recover; the operator has to either edit config.json back to false and restart, or curl the /api/v1/configuration/settings endpoint with the API key + a JSON body containing AdminUsername / AdminPassword — which bypasses CSRF and creates the user — neither of which a user should be expected to know.

Fix

Drop the v-if gate. The credential inputs now render whenever the Authentication section is on screen, regardless of the login-screen toggle state. Operators can configure (or pre-configure) admin credentials before, during, or after enabling auth — eliminating the lockout window.

Help text and the password placeholder were updated to reflect the actual backend semantics:

• The fields are accepted at any time and are persisted with Save Settings.
• Both fields must be non-empty for ConfigurationService to create/update the admin user (see listenarr.application/Common/ConfigurationService.cs ~line 245).
• Leaving the password blank is the explicit "keep existing" path.

Test plan

• npm run type-check — clean
• npx vitest run AuthenticationSection.spec.ts — 4 / 4 passing
• New regression test added (renders the admin credential inputs even when authEnabled is false) — asserts the form is visible in the previously-lockout-prone state. Would have caught the original regression.
• Manual: set AuthenticationRequired: "false" in config.json, restart, open Settings → Authentication. Admin Username + Password fields are visible. Enter creds, click Save. Tick Enable login screen. Save again. Reload. Login screen accepts the credentials.

Notes

• Purely a frontend change to fe/src/components/settings/AuthenticationSection.vue. No backend changes — the /api/v1/configuration/settings endpoint already supports the create-or-update flow; it just lacked a UI affordance to drive it from a fresh install.
• Found while operating an instance after the 1.0.1 canary release. Documented as kevinheneveld/Listenarr#7.

[T4g1]

lgtm!

[therobbiedavis]

Thanks for fixing the visibility issue here!

I do have a concern though, that is that the broader save flow still has a lockout path outside this component. SettingsView.saveSettings() saves application settings first, then persists authenticationRequired to startup config. On the backend, ConfigurationService.SaveApplicationSettingsAsync() catches admin user create/update failures, logs them, and still returns success:

listenarr.application/Common/ConfigurationService.cs, around lines 243-270.

So if admin credentials are supplied but user creation/update fails, the frontend can still proceed to save AuthenticationRequired=true. That leaves the instance requiring login without the expected working admin account, which is the same kind of lockout your PR is trying to avoid.

I don’t think the component visibility fix is wrong, just perhaps incompelte, so I’d like us to either:

• surface admin credential provisioning failures back to the caller when credentials were supplied, or
• have the auth-enable flow validate that at least one usable admin exists before persisting AuthenticationRequired=true.

Otherwise this PR fixes the most visible symptom, but not the full auth-enable safety issue.

[kevinheneveld]

Thanks — you're right, the catch at ConfigurationService.cs:268-271 swallows the failure and lets AuthenticationRequired=true proceed regardless. Going with option 1 in 50c79de: the catch now re-throws when admin credentials were supplied so SettingsView.saveSettings() aborts before the startup-config write. The settings row itself is still saved before the admin block (non-admin changes like notification triggers and webhooks shouldn't disappear because credential provisioning failed). The no-credentials path stays unchanged — silent skip is correct there, that's the "I'm just updating other settings" case.

I considered option 2 as well (validating that ≥1 admin exists before persisting AuthenticationRequired=true) as a defense-in-depth backstop in case a future code path ever sidesteps option 1. Happy to add it as a separate PR if you think it's worth the extra query, but I leaned toward keeping this one focused on the immediate surface and letting you call it. LMK either way.

Two new tests on ConfigurationServiceTests cover the throw-on-failure path and the no-credentials carveout.

[therobbiedavis]

The backend failure path looks fixed now, thanks. Although I think we should also probably add in option 2 here because, looking over it again, there is one lockout edge case during first-time setup I missed...

If there are zero admin users, the UI still allows enabling the login screen with blank credentials or username-only. The admin fields are optional, blank values are deleted before save, ConfigurationService silently skips admin provisioning unless both username and password are non-empty, and SettingsView.saveSettings() can still persist AuthenticationRequired=true.

Now just to be clear, these lockouts are recoverable by editing config/config.json back to "AuthenticationRequired": "false",, so I don’t think it’s super severe, but it can still be confusing for the user.

[kevinheneveld]

Good catch — you're right, the blank-credentials-with-auth-toggle-on case isn't covered by option 1 since SaveApplicationSettings silently no-ops when credentials weren't supplied. Added option 2 in 7ce13f44.

SaveStartupConfigAsync now queries IUserService.GetAdminUsersAsync when the incoming save transitions AuthenticationRequired from disabled (or unset) to enabled, and throws when the admin list is empty. The scoped-to-transition shape matters — initially I had it firing on any save with auth-enabled, but that broke RegenerateApiKey_AllowsAdminSession_WhenAuthenticationEnabled (the session-cookie tests set up auth-on factories without populating IUserService). Tightening to the transition edge is correct semantically too: once auth is already on, the admin must have already existed (or no save would have flipped it on through this same check), and API-key regens / port changes / log-level tweaks shouldn't have to re-prove the admin row exists. Demotion/deletion of the last admin while auth is enabled is a separate concern that belongs in the user-management path.

Also rebased onto the new canary 1.0.2 base while I was in there — clean rebase, all three commits replayed cleanly. Three new tests cover refuse-on-transition-no-admin, allow-on-transition-with-admin, and the auth-already-on subsequent-save carveout. 681 / 681 backend tests passing.

[therobbiedavis]

I tested this manually and the backend guard does reject the no-admin auth-enable case, but the frontend currently handles that rejection as a startup-config disk persistence failure.

apiService.saveStartupConfig(newCfg) throws the 400 from the backend, then SettingsView catches it and generates a downloadable config.json containing the same rejected AuthenticationRequired=true value. The toast tells the user to save that file manually, which bypasses the new backend guard and can still produce the lockout state.

Could we distinguish validation failures from actual disk-write failures here? For the no-admin response, the UI should surface the backend error and not offer/download a replacement config. The download fallback should only run for genuine “server could not persist config to disk” cases.

Additionally there's a small test-framework nit: tests/README.md says backend test classes should define class-level Trait("Name", ...) and Trait("Category", ...) metadata. Since this PR is adding several tests to ConfigurationServiceTests, could we add:

[Trait("Name", "ConfigurationServiceTests")]
[Trait("Category", "ConfigurationService")]

above the class declaration?

[kevinheneveld]

Both addressed in 42b85d76. Thanks for the manual-test catch — that download fallback genuinely did defeat the whole guard, embarrassing miss on my part.

FE bypass fix. SettingsView.saveSettings() now inspects the thrown error's status:

• 4xx (validation refusal) → hard error toast surfacing the backend's message, no download fallback. Letting the user manually save a server-rejected config was undoing the entire option-2 guard.
• 5xx / network errors / no status → keep the existing download fallback. That path was added for the legitimate "server wants to save but can't write config.json" case (permission denied, read-only mount, etc.) and is still the right escape hatch for those.

The split keeps the operator-recovery affordance for genuine persistence failures while closing the bypass.

Test traits. Added [Trait("Name", "ConfigurationServiceTests")] and [Trait("Category", "ConfigurationService")] above the class. Followed the pattern in LibraryController_DeleteFilesystemTests per the README example.

One thing I didn't do — happy to add if you'd like: a regression test for the FE branch (mock saveStartupConfig to throw a 400 → assert no <a download="config.json"> element appeared; mock a 500 → assert it does). The existing SettingsView.spec.ts doesn't currently exercise the save flow at all, so adding it would mean a fair amount of multi-mock plumbing for one assertion. I weighed brittleness vs coverage and shipped without it, but if you'd rather have the regression test, I'll do it.

10 / 10 ConfigurationService tests passing on the rebased branch; full backend suite was 681 / 681 in the prior commit and this commit only touches FE + class attribute.

[therobbiedavis]

@kevinheneveld Manual testing confirms the no-admin auth-enable path is refused and does not persist AuthenticationRequired=true therefore i am marking this approved however, yes, I think we eventually need a test after the FE test framework is revamped (#611). Lets add a TODO in the settingsview.spec.ts and comeback to it after.

Additional non-blocking finding: the refused-save toast should eventually show a clearer, user-facing reason. Right now this path can surface raw API wording, and even the backend message could be more direct. Something like: “Login cannot be enabled until an admin username and password are saved. Enter both fields and save again.” No need to handle right now unless you want to go ahead and take care of it, but worth adding another TODO for a later UX pass.

[therobbiedavis]

Hey @kevinheneveld, please rebase this, also there are 2 codeql errors, I am not sure if you can see them (https://github.com/Listenarrs/Listenarr/pull/623/checks?check_run_id=78840520878), if not please let me know and I can provide details.

[kevinheneveld]

The failing CodeQL check (cs/user-controlled-bypass-of-sensitive-method, alerts #362/#363 at ConfigurationService.cs:436) is a false positive, and I don't have access to dismiss it — could you?

The flagged if (config.IsAuthenticationEnabled()) guards a self-lockout safety check, not an authorization decision. The only way to "bypass" it is to send an auth-disabled config, which leaves login off — strictly safe, no lockout, no privilege change. The user controlling their own auth toggle is the intended behavior, the endpoint is already admin-gated, and wasAuthEnabled is read from server state (startupConfigService.GetConfig()), not the request. There's no protected resource being bypassed.

I'd rather not refactor clean validation logic just to dodge the scanner (and pushing would dismiss the approval) — so a dismissal as "false positive" seems right. Happy to add a code comment noting the suppression rationale if you'd prefer that on the record.

[kevinheneveld]

Rebased onto canary (was 3 commits behind) and pushed — build's green locally. The only remaining red check is the CodeQL false positive (alerts #362/#363, cs/user-controlled-bypass-of-sensitive-method at ConfigurationService.cs:436); I still can't dismiss it from my end, so that one's yours when you get a sec. Thanks @therobbiedavis!