Refactor all three service files and fix enroll-clears-session bug
- Update module docstrings to concise service descriptions - Add _require_json() helper to Handler in k_proxy and k_client_portal, eliminating repetitive try/except JSON-parse blocks in handler methods - Cache SSL context once in ClientState.__init__ instead of per-request - Fix: ClientState.enroll() now calls /session/logout on k_proxy before re-enrolling, so the old server-side session is invalidated rather than left to expire (discovered via live test where re-register after login caused subsequent logout to fail with missing bearer token) - Add targeted comments explaining non-obvious invariants: _gc_locked lock ownership, _with_direct_ctap2 retry-on-reopen, _require_session None convention, will_close connection reuse, HTTP/1.1 body-drain requirement, 90 s interactive timeout margin, and enroll session-clearing rationale Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e7212b49a0
commit
2cf44e97df
|
|
@ -1,9 +1,11 @@
|
|||
#!/usr/bin/env python3
|
||||
"""
|
||||
Minimal browser-facing client portal for Phase 6 bring-up.
|
||||
k_client_portal — browser-facing portal running in k_client.
|
||||
|
||||
This runs in k_client, keeps a local preferred username, and talks to k_proxy
|
||||
over the localhost-forwarded TLS endpoint.
|
||||
Serves the single-page UI and thin API shim that delegates every auth and
|
||||
resource operation to k_proxy over the localhost-forwarded TLS endpoint.
|
||||
Persists one preferred username locally; all session and enrollment state
|
||||
lives in k_proxy.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -561,18 +563,25 @@ class ClientState:
|
|||
self.proxy_base_url = proxy_base_url.rstrip("/")
|
||||
self.proxy_ca_file = proxy_ca_file
|
||||
self.enroll_db = enroll_db
|
||||
# Registration and login both require a physical card touch, which can
|
||||
# take up to ~60 s in practice; 90 s gives a generous margin.
|
||||
self.interactive_timeout_s = interactive_timeout_s
|
||||
self.default_timeout_s = default_timeout_s
|
||||
self.lock = threading.Lock()
|
||||
self.preferred_enrollment: EnrollmentRecord | None = None
|
||||
self.session_token: str | None = None
|
||||
self.session_expires_at: int | None = None
|
||||
# Build the TLS context once; creating it on every request is expensive
|
||||
# and the CA file doesn't change at runtime.
|
||||
self._ssl_ctx: ssl.SSLContext | None = (
|
||||
ssl.create_default_context(cafile=self.proxy_ca_file)
|
||||
if proxy_base_url.startswith("https://")
|
||||
else None
|
||||
)
|
||||
self._load_preferred_enrollment()
|
||||
|
||||
def _ssl_context(self):
|
||||
if self.proxy_base_url.startswith("https://"):
|
||||
return ssl.create_default_context(cafile=self.proxy_ca_file)
|
||||
return None
|
||||
def _ssl_context(self) -> ssl.SSLContext | None:
|
||||
return self._ssl_ctx
|
||||
|
||||
def _proxy_json(
|
||||
self,
|
||||
|
|
@ -626,6 +635,12 @@ class ClientState:
|
|||
username = username.strip()
|
||||
if not username:
|
||||
return {"ok": False, "error": "username required"}
|
||||
# Best-effort: invalidate any active session on k_proxy before re-enrolling.
|
||||
# The new credential will differ from what the old session was issued for.
|
||||
with self.lock:
|
||||
old_token = self.session_token
|
||||
if old_token:
|
||||
self._proxy_json("POST", "/session/logout")
|
||||
status, data = self._proxy_json(
|
||||
"POST",
|
||||
"/enroll/register",
|
||||
|
|
@ -741,6 +756,15 @@ class Handler(BaseHTTPRequestHandler):
|
|||
return {}
|
||||
return json.loads(raw.decode("utf-8"))
|
||||
|
||||
def _require_json(self) -> dict[str, Any] | None:
|
||||
# Returns None and sends 400 when the body is unparseable; the caller
|
||||
# should return immediately without sending a second response.
|
||||
try:
|
||||
return self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
return None
|
||||
|
||||
def do_GET(self) -> None: # noqa: N802
|
||||
path = urlparse(self.path).path
|
||||
if path == "/":
|
||||
|
|
@ -761,28 +785,22 @@ class Handler(BaseHTTPRequestHandler):
|
|||
def do_POST(self) -> None: # noqa: N802
|
||||
path = urlparse(self.path).path
|
||||
if path == "/api/enroll":
|
||||
try:
|
||||
data = self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
data = self._require_json()
|
||||
if data is None:
|
||||
return
|
||||
result = self.state.enroll(str(data.get("username", "")))
|
||||
self._json(200 if result.get("ok") else 400, result)
|
||||
return
|
||||
if path == "/api/login":
|
||||
try:
|
||||
data = self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
data = self._require_json()
|
||||
if data is None:
|
||||
return
|
||||
status, data = self.state.login(str(data.get("username", "")))
|
||||
self._json(status, data)
|
||||
return
|
||||
if path == "/api/enroll/delete":
|
||||
try:
|
||||
data = self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
data = self._require_json()
|
||||
if data is None:
|
||||
return
|
||||
status, data = self.state.delete_enrollment(str(data.get("username", "")))
|
||||
self._json(status, data)
|
||||
|
|
|
|||
|
|
@ -1,16 +1,14 @@
|
|||
#!/usr/bin/env python3
|
||||
"""
|
||||
Minimal k_proxy service for Phase 5 bring-up.
|
||||
k_proxy — session gateway and card authentication bridge.
|
||||
|
||||
Behavior:
|
||||
- Creates short-lived sessions after a card-backed auth gate.
|
||||
- Reuses valid sessions to access k_server protected counter endpoint.
|
||||
- Supports enrollment, session status, and logout.
|
||||
Creates short-lived bearer sessions after a card-backed auth gate, then
|
||||
proxies authenticated requests to k_server. Enrollment metadata and session
|
||||
state are both process-local; sessions do not survive a restart.
|
||||
|
||||
Notes:
|
||||
- Default runtime still uses the legacy card-presence probe gate.
|
||||
- Experimental direct FIDO2 registration/assertion lives behind `--auth-mode fido2-direct`.
|
||||
- This is still a prototype and not a final production auth design.
|
||||
Default auth mode is a lightweight card-presence probe (subprocess call to
|
||||
fido2_probe.py). Pass --auth-mode fido2-direct for real CTAP2
|
||||
makeCredential/getAssertion against the attached ChromeCard.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -546,6 +544,7 @@ class ProxyState:
|
|||
return time.time()
|
||||
|
||||
def _gc_locked(self) -> None:
|
||||
# Caller must hold self.lock.
|
||||
now = self._now()
|
||||
dead = [token for token, sess in self.sessions.items() if sess.expires_at <= now]
|
||||
for token in dead:
|
||||
|
|
@ -674,6 +673,9 @@ class ProxyState:
|
|||
self._drop_direct_device_locked()
|
||||
|
||||
def _with_direct_ctap2(self, action):
|
||||
# First attempt reuses the cached handle; if it fails (e.g. the card was
|
||||
# briefly removed or the CTAPHID channel desynchronised), we reopen once
|
||||
# and retry before propagating the error.
|
||||
with self.direct_device_lock:
|
||||
last_exc: Exception | None = None
|
||||
for reopen in (False, True):
|
||||
|
|
@ -962,6 +964,8 @@ class UpstreamPool:
|
|||
conn.request("POST", full_path, body=body, headers=req_headers)
|
||||
resp = conn.getresponse()
|
||||
raw = resp.read()
|
||||
# will_close is set by the server when it intends to close the connection
|
||||
# after this response; reusing such a connection would hit an EOF.
|
||||
reusable = not resp.will_close
|
||||
try:
|
||||
data = json.loads(raw.decode("utf-8")) if raw else {}
|
||||
|
|
@ -1004,10 +1008,20 @@ class Handler(BaseHTTPRequestHandler):
|
|||
return json.loads(raw.decode("utf-8"))
|
||||
|
||||
def _discard_request_body(self) -> None:
|
||||
# HTTP/1.1 keep-alive: body must be consumed before the response is sent.
|
||||
length = int(self.headers.get("Content-Length", "0"))
|
||||
if length > 0:
|
||||
self.rfile.read(length)
|
||||
|
||||
def _require_json(self) -> dict[str, Any] | None:
|
||||
# Returns None and sends 400 when the body is unparseable; callers must
|
||||
# return immediately without sending a second response.
|
||||
try:
|
||||
return self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
return None
|
||||
|
||||
def _bearer_token(self) -> str | None:
|
||||
value = self.headers.get("Authorization", "")
|
||||
if not value.startswith("Bearer "):
|
||||
|
|
@ -1016,6 +1030,8 @@ class Handler(BaseHTTPRequestHandler):
|
|||
return token or None
|
||||
|
||||
def _require_session(self) -> tuple[str, Session] | None:
|
||||
# Returns None when auth fails; the 401 has already been sent, so callers
|
||||
# must return immediately without writing a second response.
|
||||
token = self._bearer_token()
|
||||
if not token:
|
||||
self._json(401, {"ok": False, "error": "missing bearer token"})
|
||||
|
|
@ -1076,10 +1092,8 @@ class Handler(BaseHTTPRequestHandler):
|
|||
self.send_error(404)
|
||||
|
||||
def _session_login(self) -> None:
|
||||
try:
|
||||
data = self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
data = self._require_json()
|
||||
if data is None:
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
@ -1110,10 +1124,8 @@ class Handler(BaseHTTPRequestHandler):
|
|||
)
|
||||
|
||||
def _enroll_register(self) -> None:
|
||||
try:
|
||||
data = self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
data = self._require_json()
|
||||
if data is None:
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
@ -1134,10 +1146,8 @@ class Handler(BaseHTTPRequestHandler):
|
|||
self._json(200, enrollment_payload(enrollment, created=enrollment.created_at == enrollment.updated_at))
|
||||
|
||||
def _enroll_update(self) -> None:
|
||||
try:
|
||||
data = self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
data = self._require_json()
|
||||
if data is None:
|
||||
return
|
||||
try:
|
||||
enrollment = self.state.update_enrollment(
|
||||
|
|
@ -1153,10 +1163,8 @@ class Handler(BaseHTTPRequestHandler):
|
|||
self._json(200, enrollment_payload(enrollment))
|
||||
|
||||
def _enroll_delete(self) -> None:
|
||||
try:
|
||||
data = self._read_json()
|
||||
except Exception:
|
||||
self._json(400, {"ok": False, "error": "invalid json"})
|
||||
data = self._require_json()
|
||||
if data is None:
|
||||
return
|
||||
try:
|
||||
enrollment = self.state.delete_enrollment(str(data.get("username", "")))
|
||||
|
|
|
|||
|
|
@ -1,11 +1,10 @@
|
|||
#!/usr/bin/env python3
|
||||
"""
|
||||
Minimal k_server service for Phase 5/5.5 bring-up.
|
||||
k_server — protected resource backend.
|
||||
|
||||
Behavior:
|
||||
- Exposes a protected monotonic counter endpoint.
|
||||
- Accepts only requests from k_proxy via a shared proxy token header.
|
||||
- Uses thread-safe counter increments.
|
||||
Exposes a monotonic counter behind a shared proxy token. Only k_proxy
|
||||
is expected to reach this service; k_client should have no direct path.
|
||||
All state is process-local and resets on restart.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -21,6 +20,7 @@ from urllib.parse import urlparse
|
|||
|
||||
|
||||
class ServerState:
|
||||
# All state is process-local; a restart resets the counter to zero.
|
||||
def __init__(self, proxy_token: str):
|
||||
self.proxy_token = proxy_token
|
||||
self.counter = 0
|
||||
|
|
@ -45,6 +45,8 @@ class Handler(BaseHTTPRequestHandler):
|
|||
self.wfile.write(body)
|
||||
|
||||
def _discard_request_body(self) -> None:
|
||||
# HTTP/1.1 keep-alive: the connection is reused, so the body must be fully
|
||||
# consumed before we send the response, even for endpoints that ignore it.
|
||||
length = int(self.headers.get("Content-Length", "0"))
|
||||
if length > 0:
|
||||
self.rfile.read(length)
|
||||
|
|
|
|||
Loading…
Reference in New Issue