adding a password reset system for admins

[vlad-a-c]

since we don't have a mandatory email address entered, the only way of changing a password is for an admin to create a temp pass. i'm also thinking of making the user able to change his own password from the options menu.

[zarvox32]

Reviewed and tested locally on the merge ref. All 49 tests in the three modified test files pass, plus 43 tests in test_server_authorization.py, test_server_message_flow.py, and test_server_core_misc.py — no regressions in the auth/menu surface this PR touches.

The auth changes are the most valuable part: reset_password now invalidates in-memory sessions AND revokes refresh tokens at the DB layer, which closes a real gap where a reset password wouldn't actually log the target out everywhere. The new revoke_user_refresh_tokens correctly uses lower(username) = lower(?) matching the rest of the table.

A few non-blocking observations (take or leave):

nit: getattr(self, "_password_min_length", 8) and the matching _password_max_length default in _handle_reset_password_editbox — Server.__init__ always sets these, so the fallback is dead code. Registration in core/server.py uses self._password_min_length directly. Minor consistency thing.

nit: reset-user-password-prompt doesn't include the "or press Escape to cancel" hint that decline-reason-prompt has. An admin who selects the wrong user and wants to back out has no clear way to discover that.

future consideration: The editbox protocol has no masked/password input mode, so the temporary password is likely echoed and spoken by the screen reader as the admin types it. Pre-existing limitation, not introduced here, but worth filing as an issue if we ever add a self-service password change flow — it'd be the same problem at a worse blast radius.

future consideration: After a successful reset the admin lands back on the user list, which matches _ban_user's pattern so it's consistent — but for password reset specifically, returning to the admin menu might be more useful since the typical flow is "reset one user, done." Not worth changing in isolation.

The double trust-level check (decorator + explicit target_record.trust_level.value >= TrustLevel.ADMIN.value guard inside _reset_user_password) is good defense in depth.

LGTM.

[zarvox32]

Re-tested the merge ref now that 05ad0f1 Fix mac app sound resource paths has landed on top of the password-reset commits.

Scope concern. The PR title and description only cover admin password reset, but 05ad0f1 bundles in 84 LOC of unrelated macOS PyInstaller work (PlayPalace.spec, pyinstaller_runtime.py, sound_manager.py, a sound-manager test). That commit landed after my earlier review of the password-reset feature, so it hasn't actually been reviewed here. Splitting it into its own PR would keep history readable and let the password-reset feature merge on its own merits.

Test results.

Server side (the password-reset feature) is clean — test_administration.py, test_database.py, test_integration.py: 49/49 pass on the merge ref. Nothing in 79061d9 Address password reset review feedback regressed.

Desktop side has a real bug introduced by 05ad0f1:

clients/desktop/tests/test_sound_manager.py::test_default_sounds_folder_is_absolute FAILED

E       AssertionError: assert False
E        +  where False = 'C:\Users\me\Documents\PlayPalace11\clients\desktop\sounds'.startswith('/')

Line 48 checks absoluteness with manager.sounds_folder.startswith(\"/\"), which is POSIX-only — Windows absolute paths start with a drive letter. The fix is one line: replace it with os.path.isabs(manager.sounds_folder), which is also a more accurate assertion of what's actually being claimed.

CI. The claude-review failure is infrastructure — OIDC token exchange returned 401 on all three retries (Invalid OIDC token). Not a code issue.

Other notes on the mac commit (non-blocking).

• pyinstaller_runtime.py does os.chdir(os.path.dirname(sys.executable)) at runtime-hook load when frozen. That mutates cwd for the whole process — fine for a one-shot launch, but anything else in the client using relative paths inherits it. Worth being aware of even if nothing breaks today.
• _default_sounds_folder() itself is correct in both source and frozen modes; the production logic is fine, only the test is broken.
• The spec file is mac-only (.app / Contents/MacOS/sounds). The project's build story now has a Windows/Linux gap that someone will need to fill before this can ship to non-mac users.

Recommendation. Two clean paths:

1. Split the mac fix into a separate PR. Cleanest history, lets the password-reset feature land today.
2. Keep it bundled, but update the PR title/description to reflect both changes and fix the Windows-incompatible test before merge.

Either way, the startswith(\"/\") assertion needs to go. The password-reset feature itself I'm still happy with — the auth-layer gap-closing (session invalidation + refresh-token revocation on reset) is the most valuable part of this PR.

[zarvox32]

Re-tested on the fresh merge ref (commit 0217e40 reverts the mac sound paths change as requested).

Revert is clean. Net diff vs main is back to exactly the password-reset feature: 8 files, +250/-2, all under server/. No mac/PyInstaller/sound_manager artifacts in the diff.

Earlier review nits addressed in 79061d9. The getattr(..., 8/128) dead-code defaults are gone, and reset-user-password-prompt now includes the "(or press Escape to cancel)" hint.

One test failure on the merge ref, but it's not caused by this PR — it's a collision with #226 (dcf3140 Type safety and cleanup refactor), which landed on main after this PR opened:

tests/test_database.py::test_revoke_user_refresh_tokens_revokes_only_target FAILED
TypeError: 'RefreshTokenRecord' object is not subscriptable

dcf3140 changed get_refresh_token's return type from dict to the RefreshTokenRecord dataclass, so the new test's db.get_refresh_token("tok-a1")["revoked_at"] no longer works. Fix is one line per assertion (.revoked_at instead of ["revoked_at"]). The production code in revoke_user_refresh_tokens is untouched by that refactor and remains correct.

@zarvox32 will land that one-liner as a follow-up — it's small enough that gating this PR on it isn't worth the round trip.

Other tests on the merge ref:

• test_administration.py, test_integration.py: 31/31 pass
• test_database.py: 17/18 pass (the one above)
• test_server_authorization.py, test_server_message_flow.py: 29/29 pass — no regressions in the auth/menu surface this PR touches.

Ready for merge from my side.