feat(auth): consolidate to database-backed auth and add offline password reset#5180
feat(auth): consolidate to database-backed auth and add offline password reset#5180OliverBryant wants to merge 5 commits into
Conversation
…ord reset Reduce authentication to two modes and add a recovery path for a lost or stolen admin account. Remove the legacy file-based auth system ---------------------------------------- The in-memory `--auth-config` JSON auth system is removed. Authentication is now either the database-backed advanced system (default) or no auth at all (XINFERENCE_AUTH_ADVANCED=false). "No auth" is now expressed by the advanced service being absent, so `is_authenticated()` returns whether the advanced service exists, and every route/model-access check already keys off it. - Delete oauth2/auth_service.py, oauth2/types.py, oauth2/utils.py. - Drop the `--auth-config` CLI option and its plumbing (cmdline/local/ supervisor/restful_api), and the legacy `/token` login endpoint. - Delete the legacy-auth integration tests and the setup_with_auth fixture. Remove the first-run setup token -------------------------------- Creating the first admin no longer requires a one-time setup token; whoever reaches POST /v1/admin/setup first becomes the admin. The single-admin guard and password-length check are unchanged. This drops get_or_create_setup_token /delete_setup_token, the startup token log notice, and the setup-token field from the web UI setup page and its i18n strings. Add xinference-reset-auth-password ---------------------------------- A new offline console command resets an admin's password directly against the auth SQLite database, bypassing login. It works when nobody can log in (lost password) or when someone else won the first-admin race. It only targets users holding the `admin` permission and revokes the user's refresh tokens on reset. Docs updated to describe the two remaining auth modes and the reset command.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the legacy, in-memory file-based authentication system (--auth-config) entirely, making the database-backed SQLite system the sole authenticated mode. It also simplifies the first-run setup by removing the setup token requirement from the /v1/admin/setup endpoint and the frontend UI. To mitigate security risks from removing the token, a new offline command-line tool xinference-reset-auth-password is introduced to allow operators with shell access to reset admin passwords directly in the database. Feedback on the changes includes addressing a potential TypeError in the password reset script if permissions are stored as None in the database, and replacing a deep relative import with an absolute import.
Use `or []` when reading a user's permissions, matching the pattern in routes.py, so an admin check stays safe even if permissions come back as None.
test_metrics failed on Linux CI: the RESTful API subprocess is created with the fork start method there, which inherits the parent's already-imported constants module. XINFERENCE_AUTH_ADVANCED was frozen as a module-level constant at first import (defaulting to advanced=on), so a forked child ignored the XINFERENCE_AUTH_ADVANCED=false the test set at runtime and started advanced auth anyway -- turning unauthenticated launch requests into "Could not validate credentials". The old file-based auth used to mask this by treating a missing config as no-auth. Replace the frozen constant with is_auth_advanced() and resolve the JWT / encryption secrets via get_auth_jwt_secret_key() / get_auth_encryption_key() at RESTfulAPI construction time, so a forked server honors the environment it is actually running with.
…rt time Same fork-inheritance issue as the auth flag: XINFERENCE_DISABLE_METRICS was a module-level constant frozen at first import. On Linux the supervisor and worker run in a forked subprocess that inherited the parent's frozen value, so setting XINFERENCE_DISABLE_METRICS=1 at runtime did not actually disable the /metrics route (test_disable_metrics_exporter_server saw 200 instead of 404). Replace the constant with is_metrics_disabled(), read at call time in RESTfulAPI and WorkerActor.
qinxuye
changed the title
qinxuye
left a comment
qinxuye
left a comment
There was a problem hiding this comment.
One recovery-path issue remains.
|
|
||
| # Atomically update the password and revoke the user's refresh tokens, so | ||
| # a leaked/old refresh token can't outlive the reset. | ||
| db.update_password_and_revoke_tokens(user["id"], get_password_hash(new_password)) |
There was a problem hiding this comment.
This revokes refresh tokens, but access tokens minted before the reset remain valid until their expiration (30 minutes by default). Because the auth dependency trusts the admin scope embedded in such a JWT, the old token can still call admin routes after the password has changed; I reproduced this with a pre-reset token successfully passing users:manage after reset_admin_password(). For this recovery command, please persist a per-user session/token version (or equivalent password-change timestamp), include it in access tokens, and bump it atomically with the password update so older tokens are rejected. Please add a regression test covering the pre-reset access token.
3 participants
What & why
This consolidates authentication down to two modes and adds a recovery path
for a lost or stolen admin account.
Motivation: the first-run setup token existed only to stop someone from
winning the "create the first admin" race on a freshly exposed instance. With
an offline password-reset command, that race is recoverable without the token
— an operator with shell access can always take control back — so the token
(and the older file-based auth system it grew alongside) is no longer needed.
Changes
1. Remove the legacy file-based auth system
The in-memory
--auth-configJSON auth system is removed. Authentication isnow either:
XINFERENCE_AUTH_ADVANCED=false)."No auth" is now expressed by the advanced service being absent, so
is_authenticated()simply returns whether that service exists — every routeand model-access check already keys off it.
oauth2/auth_service.py,oauth2/types.py,oauth2/utils.py.--auth-configCLI option and its plumbing, plus the legacy/tokenlogin endpoint.setup_with_authfixture.2. Remove the first-run setup token
Creating the first admin via
POST /v1/admin/setupno longer requires a setuptoken. The single-admin guard (only creatable while the user table is empty)
and the password-length check are unchanged.
get_or_create_setup_token/delete_setup_token, the startuptoken log notice, and the setup-token field from the web UI setup page and
its i18n strings (en/zh/ja/ko).
3. Add
xinference-reset-auth-passwordA new offline console command resets an admin's password directly against the
auth SQLite database, bypassing login and permission checks. It:
first-admin race;
adminpermission;update_password_and_revoke_tokens);Docs updated to describe the two remaining auth modes and the reset command.
Verification
test_oauth2_reset_admin_password.py,6 cases) — pass.
main()entry point: resetsucceeds, old password rejected, refresh tokens revoked; non-admin / unknown
user / short password / cancel all rejected with exit 1.
POST /v1/admin/setupdriven end-to-end via a FastAPI test client with nosetup token: returns 201 and creates the admin; second call → 403; short
password → 400.
test_admin_setup_route.py(6 cases) pass.black/isort/flake8clean on changed files; changed frontendTS/TSX parses clean.