Skip to content

Commit d2987ee

Browse files
kherrera6219claude
andcommitted
fix(test) + docs: repair 5 pre-existing desktop-auth tests; correct Phase C scope
test_desktop_auto_login_security.py monkeypatched the pre-migration 'routes.auth_routes' module path (now backend.routes.auth_routes) — 5 stale failures fixed, now green. Phase C correction: investigation found auth_routes.py is the desktop Windows-identity auth (NOT a web login), and LoginManager/current_user/session_manager/the API-key branch are all the live keep-path — the original plan's removals were based on a stale multi-user model. Corrected Phase C to: gut admin_routes user-mgmt/ownership (pending frontend check), remove stale CSRF-exempt entries. MFA/tenant_rls confirmed vestigial (auth_routes docstring: web-app patterns removed) -> Phase D stands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e97b35f commit d2987ee

2 files changed

Lines changed: 28 additions & 20 deletions

File tree

docs/audits/DataLogicEngine_Auth_Deprecation_Plan.md

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,13 @@ another* (roles, admin, per-user login/sessions, MFA, multi-tenant isolation, JW
3232
| Zero-trust engine | `backend/security/zero_trust.py` | **0 live importers** (2 tests only) |**REMOVED (Phase A, `57b912da`)** |
3333
| JWT/token manager | `backend/security/token_manager.py` | **0 live importers** (1 test only) |**REMOVED (Phase A, `57b912da`)** |
3434
| RBAC / permissions | `backend/security/rbac.py` | 5 refs: `admin_routes`, `mcp_routes`, `privacy_routes`, `extensions.py`, `scripts/scan_backend_routes.py` |**REMOVED (Phase B, `e710aeb3`)** |
35-
| Multi-session mgr | `backend/security/session_manager.py` | 1 ref: `app.py` | De-wire, delete |
36-
| Per-user MFA | `backend/security/mfa.py` | `extensions.py`, `models.py` (`User.verify_totp`) | De-wire, delete; drop `User.mfa_enabled`/`mfa_secret` |
37-
| Multi-tenant RLS | `backend/security/tenant_rls.py` | 1 ref: `app.py` | De-wire, delete |
38-
| Web login flow | `backend/routes/auth_routes.py`, Flask-Login `LoginManager` in `extensions.py` | registered | Remove web login routes + session login |
39-
| **Admin/user-mgmt routes** | `backend/routes/admin_routes.py` (16 decorators) | registered | **Wholesale obsolete** — every route is user CRUD / role update / `transfer-ownership` / role-gated dashboard. No users/roles/owner under single-mode. Remove the user-mgmt + ownership routes; retain only genuinely-operational endpoints (cache clear, health) as owner-only, ungated. |
35+
| Multi-session mgr | `backend/security/session_manager.py` | 1 ref: `app.py` | ⚠️ **KEEP (correction)** — it's session-cookie *security* hardening (rotation/invalidation/secure storage) for the owner's session, not a multi-user login. `MAX_CONCURRENT_SESSIONS=3` is vestigial but harmless. |
36+
| Per-user MFA | `backend/security/mfa.py` | `extensions.py`, `models.py` (`User.verify_totp`) | De-wire, delete; drop `User.mfa_enabled`/`mfa_secret` (Phase D — confirmed vestigial: `auth_routes` docstring says MFA was removed from the flow). |
37+
| Multi-tenant RLS | `backend/security/tenant_rls.py` | 1 ref: `app.py` | De-wire, delete (Phase D) — verify it's not providing per-row security still relied on. |
38+
| ~~Web login flow~~ | `backend/routes/auth_routes.py`, `LoginManager` | registered | ⚠️ **KEEP (correction)**`auth_routes.py` is the **desktop Windows-identity auth** (its docstring: "Web-app patterns … have been removed"), and `LoginManager`/`current_user` back the owner's session across 25 live files. NOT removable. |
39+
| API-key branch | `check_api_auth` `ukg_`/`ExternalAPIKey` path | many consumers (`api_gateway`, `unified_middleware`, `chat`, …) | ⚠️ **KEEP (correction)** — live, not dead. |
40+
| **Admin/user-mgmt routes** | `backend/routes/admin_routes.py` (16 decorators) | registered | **Vestigial** — user CRUD / role update / `transfer-ownership` / role-gated dashboard. No users/roles/owner under single-mode. Remove the user-mgmt + ownership routes; retain operational endpoints (cache clear, health) ungated. (Phase C — **verify frontend has no admin/user pages calling them first.**) |
41+
| Stale CSRF-exempt entries | `app.py` `CSRF_API_EXEMPT_PATH_PREFIXES` | `/auth/login`, `/auth/register`, `/auth/mfa/verify`, `/auth/callback/sso` | Remove — these reference routes that **no longer exist** (auth_routes only has `/check`, `/csrf-token`, `/desktop/*`). |
4042
| Admin/permission decorators | `api_admin_required`, `require_permission` | part of the 147 decorator usages | Collapse to single-owner pass-through |
4143
| User authz fields | `models.User.role`, `is_admin`, `mfa_*`, possibly `password_hash` | columns + indexes | Drop after de-wiring (migration) |
4244

@@ -94,16 +96,22 @@ dashboard, `/api/security/scan/recent`, `/api/v1/retention/policies`). 302 secur
9496
> when `auth_routes` is handled (these test the desktop auto-login KEEP path, so fix
9597
> the path — don't delete them).
9698
97-
**Phase C — Simplify the keep-decorators to desktop-only + remove obsolete route surfaces.**
98-
Strip the Flask-Login session branch + external-API-key branch from `check_api_auth`
99-
and the decorators. Remove `auth_routes.py` web login, the `LoginManager` wiring, and
100-
`session_manager.py`. **Remove `admin_routes.py` user-management + ownership-transfer
101-
routes** (obsolete — no users/roles/owner); keep only operational endpoints (cache
102-
clear, health) as owner-only ungated. Verify Electron still authenticates (signed
103-
loopback unaffected) and that the frontend has no admin/user-management pages still
104-
calling the removed routes. **Also fix the 5 pre-existing
105-
`test_desktop_auto_login_security.py` failures** (stale `routes.auth_routes`
106-
monkeypatch → `backend.routes.auth_routes`) since this phase touches `auth_routes`.
99+
**Phase C — CORRECTED SCOPE (2026-06-13).** Investigation found the original Phase C
100+
was written from a stale multi-user-web-app model. The live reality: the single-mode
101+
**desktop auth is already built** (`auth_routes.py` = Windows-identity + signed
102+
loopback; `LoginManager`/`current_user` back the owner's session). So the planned
103+
removals (`auth_routes`, `LoginManager`, `session_manager`, API-key branch, the
104+
`check_api_auth` session branch) are all the **keep-path** — NOT removable. The
105+
genuinely-valid Phase C work is:
106+
-**Fix the 5 `test_desktop_auto_login_security.py` failures** — done (`routes.auth_routes`
107+
`backend.routes.auth_routes`; pre-existing, not from this deprecation).
108+
-**Gut `admin_routes.py`** user-management + ownership-transfer routes (vestigial —
109+
no users/roles/owner). Keep `cache/clear` + `health` ungated. **Blocked on:** verify
110+
the frontend has no admin/user pages calling `/api/v1/admin/users*` or
111+
`/transfer-ownership`; migrate `tests/integration_routes/test_admin_routes.py`
112+
(user-list/role/delete tests).
113+
-**Remove stale `CSRF_API_EXEMPT_PATH_PREFIXES`** entries in `app.py` for the
114+
non-existent `/auth/login`, `/auth/register`, `/auth/mfa/verify`, `/auth/callback/sso`.
107115

108116
**Phase D — Remove MFA + tenancy.**
109117
De-wire `mfa` from `extensions.py` + `models.User.verify_totp`; delete `mfa.py`.

tests/integration_routes/test_desktop_auto_login_security.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def _desktop_auth_headers(client, install_secret: str):
2727
def test_desktop_auto_login_requires_desktop_header(app, client, monkeypatch):
2828
monkeypatch.setenv("IS_DESKTOP_APP", "true")
2929
monkeypatch.setenv("DESKTOP_INSTALL_SECRET", "test-install-secret")
30-
monkeypatch.setattr("routes.auth_routes.os.name", "nt", raising=False)
30+
monkeypatch.setattr("backend.routes.auth_routes.os.name", "nt", raising=False)
3131

3232
response = client.post("/api/v1/auth/desktop/auto-login")
3333
assert response.status_code == 403
@@ -38,7 +38,7 @@ def test_desktop_auto_login_requires_desktop_header(app, client, monkeypatch):
3838
def test_desktop_auto_login_rejects_fallback_identity(app, client, monkeypatch):
3939
monkeypatch.setenv("IS_DESKTOP_APP", "true")
4040
monkeypatch.setenv("DESKTOP_INSTALL_SECRET", "test-install-secret")
41-
monkeypatch.setattr("routes.auth_routes.os.name", "nt", raising=False)
41+
monkeypatch.setattr("backend.routes.auth_routes.os.name", "nt", raising=False)
4242
monkeypatch.setattr(
4343
"backend.auth.windows_identity.get_windows_user_identity",
4444
lambda: {
@@ -61,7 +61,7 @@ def test_desktop_auto_login_rejects_fallback_identity(app, client, monkeypatch):
6161
def test_desktop_auto_login_rejects_invalid_challenge_signature(app, client, monkeypatch):
6262
monkeypatch.setenv("IS_DESKTOP_APP", "true")
6363
monkeypatch.setenv("DESKTOP_INSTALL_SECRET", "test-install-secret")
64-
monkeypatch.setattr("routes.auth_routes.os.name", "nt", raising=False)
64+
monkeypatch.setattr("backend.routes.auth_routes.os.name", "nt", raising=False)
6565

6666
nonce = _issue_desktop_challenge(client)
6767
response = client.post(
@@ -79,7 +79,7 @@ def test_desktop_auto_login_defaults_first_user_to_standard_role(app, client, mo
7979
monkeypatch.setenv("IS_DESKTOP_APP", "true")
8080
monkeypatch.setenv("DESKTOP_INSTALL_SECRET", "test-install-secret")
8181
monkeypatch.setenv("DESKTOP_AUTOLOGIN_BOOTSTRAP_OWNER", "false")
82-
monkeypatch.setattr("routes.auth_routes.os.name", "nt", raising=False)
82+
monkeypatch.setattr("backend.routes.auth_routes.os.name", "nt", raising=False)
8383
monkeypatch.setattr(
8484
"backend.auth.windows_identity.get_windows_user_identity",
8585
lambda: {
@@ -111,7 +111,7 @@ def test_desktop_auto_login_can_bootstrap_owner_when_explicitly_enabled(app, cli
111111
monkeypatch.setenv("IS_DESKTOP_APP", "true")
112112
monkeypatch.setenv("DESKTOP_INSTALL_SECRET", "test-install-secret")
113113
monkeypatch.setenv("DESKTOP_AUTOLOGIN_BOOTSTRAP_OWNER", "true")
114-
monkeypatch.setattr("routes.auth_routes.os.name", "nt", raising=False)
114+
monkeypatch.setattr("backend.routes.auth_routes.os.name", "nt", raising=False)
115115
monkeypatch.setattr(
116116
"backend.auth.windows_identity.get_windows_user_identity",
117117
lambda: {

0 commit comments

Comments
 (0)