T10.4: remove the local password path entirely
Real deletion (D15's 'full replacement'), not a toggle. Okta is now the only credential this app accepts anywhere. Backend: - server/models.py: drop User.password_hash. - server/alembic/versions/1d60a608bb51_...: matching migration (op.drop_column, same plain-drop precedent as project_role/locked_until/etc.; downgrade re-adds it with server_default=''). - server/auth.py: remove hash_password/verify_password/password_problem/ MIN_PASSWORD_LEN/_COMMON_PASSWORDS, create_reset_token/decode_reset_token/ RESET_MINUTES, the bcrypt import. Roles/tokens/cookies/get_current_user untouched. - server/app.py: remove login(), the whole self-service reset-password block (forgot-password/reset-available/reset-password), and change_password() (POST /api/auth/password). Rework create_user() to drop the password field (with a docstring note: the username must exactly match the eventual Okta identity claim, or a later sign-in provisions a second account instead of matching this one). Remove admin_reset_password() outright - nothing left to reset. Fixes a bug this task's own predecessor left behind: okta_callback()'s JIT provisioning (T10.3) was still setting password_hash="", which would have raised TypeError the moment the column was actually dropped. Admin bootstrap (D16): server/manage_users.py moves from creating accounts (create/create-admin/reset-password, all password-based) to a single 'promote <username> --role <role>' command that changes the role on a row Okta's JIT provisioning already created - the documented path for naming the first admin. list/disable/enable unchanged. Frontend: html/users.js drops the password field and validation from createUser(), removes resetPw() and its button (nothing left to reset). html/users.html drops the #nu-password input, adds a tooltip on username explaining the exact-match-to-Okta requirement. html/auth-guard.js removes the wpChangePassword dialog; html/wp-sidenav.js removes the 'Password' menu item that opened it. Tests: tests/browser_check.py and tests/launcher_check.py stop hashing a password to seed fixture rows (and the --keep-server hint now prints a ready-to-use cookie-setting snippet instead of a dead username/password). tests/pipeline_check.py and tests/token_check.py drop an unused PW import. tests/console_dialogs_check.py: the admin password-reset dialog it drove no longer exists, so that scenario is removed - the prompt-with-validate() UI pattern it exercised is still covered via creator_dialogs_check.py's wp-creation-app.js call sites, noted in this file's docstring so the coverage move isn't silent. tests/url_state_check.py: the "next= survives a real sign-in via login" scenario is explicitly marked SKIPPED (not deleted, not faked) - that promise is specific to the login FORM this task removed and can't be honestly re-proven until T10.5 rebuilds it as an Okta redirect; a minted-token cookie now stands in as setup only, so scenarios 3-6 in that file still get a signed-in page to run against. server/smoketest.py and server/seed_demo.py: switched from POST /api/auth/login to minting a session the same way okta_callback() does (auth.create_token(), seeded into the cookie jar) rather than waiting on T10.7. This is a real operational change, documented in both files' own AUTHENTICATION sections: they now need to run where AUTH_SECRET_KEY and the database match the target server's (inside the api container, or local dev) - they can no longer sign in to an arbitrary remote URL from an unrelated workstation, because Okta requires a real browser and these are stdlib scripts. The account must already exist; neither script creates or promotes one. server/requirements.txt: bcrypt dropped, nothing imports it anymore. Verified: full Alembic chain (baseline through this migration) upgrades and downgrades cleanly against a throwaway SQLite DB. okta_callback() JIT provisioning re-tested against the post-migration schema (would have thrown before the password_hash="" fix above). create_user() verified via a live HTTP call with no password field. manage_users.py promote verified end to end (seed a JIT-shaped row at project_user, promote to admin, list). smoketest.py and seed_demo.py both run to completion against a live uvicorn instance using the new minted-session path - 25/25 checks, including logout actually invalidating the session (proving the cookie-jar seeding didn't just fake the sign-in, it preserved the real expiry mechanics). wave-10.md T10.4 / D15 / D16
This commit is contained in:
225
server/app.py
225
server/app.py
@@ -13,8 +13,6 @@ import logging
|
||||
import os
|
||||
import re
|
||||
import uuid
|
||||
from datetime import timedelta, timezone
|
||||
from time import monotonic
|
||||
from typing import Any, Optional
|
||||
from urllib.parse import urlparse
|
||||
|
||||
@@ -609,14 +607,8 @@ def health():
|
||||
|
||||
|
||||
# ── Authentication ─────────────────────────────────────────────────────────────
|
||||
class LoginIn(BaseModel):
|
||||
username: str
|
||||
password: str
|
||||
|
||||
|
||||
class NewUserIn(BaseModel):
|
||||
username: str
|
||||
password: str
|
||||
full_name: str = ""
|
||||
email: str = ""
|
||||
role: str = auth.ROLE_PROJECT_USER # permissions role — see auth.ROLES
|
||||
@@ -638,24 +630,6 @@ class PreferencesIn(BaseModel):
|
||||
timezone: Optional[str] = None
|
||||
|
||||
|
||||
class ForgotPasswordIn(BaseModel):
|
||||
username: str = "" # username or email
|
||||
|
||||
|
||||
class ResetPasswordIn(BaseModel):
|
||||
token: str
|
||||
new_password: str
|
||||
|
||||
|
||||
class PasswordChangeIn(BaseModel):
|
||||
current_password: str
|
||||
new_password: str
|
||||
|
||||
|
||||
class AdminPasswordIn(BaseModel):
|
||||
new_password: str
|
||||
|
||||
|
||||
class ActiveIn(BaseModel):
|
||||
is_active: bool
|
||||
|
||||
@@ -678,47 +652,6 @@ class AutoAddIn(BaseModel):
|
||||
role: str = ""
|
||||
|
||||
|
||||
LOGIN_MAX_ATTEMPTS = int(os.getenv("AUTH_MAX_ATTEMPTS", "5"))
|
||||
LOGIN_LOCKOUT_MINUTES = int(os.getenv("AUTH_LOCKOUT_MINUTES", "15"))
|
||||
|
||||
|
||||
@app.post("/api/auth/login")
|
||||
def login(body: LoginIn, request: Request, response: Response, db: Session = Depends(get_db)):
|
||||
"""Verify credentials and, on success, set the HttpOnly session cookie.
|
||||
Throttles online password guessing: after LOGIN_MAX_ATTEMPTS consecutive
|
||||
failures an account is locked for LOGIN_LOCKOUT_MINUTES."""
|
||||
user = auth.find_user(db, body.username)
|
||||
now = models.utcnow()
|
||||
# Always run the hash comparison first — even for missing or locked accounts —
|
||||
# so response timing doesn't leak which usernames exist. verify_password
|
||||
# tolerates an empty hash.
|
||||
valid = auth.verify_password(body.password, user.password_hash if user else "")
|
||||
locked = user.locked_until if user else None
|
||||
if locked is not None and locked.tzinfo is None:
|
||||
locked = locked.replace(tzinfo=timezone.utc) # SQLite returns naive datetimes; normalize to UTC
|
||||
if locked is not None and locked > now:
|
||||
raise HTTPException(status_code=429, detail="Too many failed attempts. Try again later.")
|
||||
if not user or not valid:
|
||||
if user:
|
||||
user.failed_attempts = (user.failed_attempts or 0) + 1
|
||||
if user.failed_attempts >= LOGIN_MAX_ATTEMPTS:
|
||||
user.locked_until = now + timedelta(minutes=LOGIN_LOCKOUT_MINUTES)
|
||||
user.failed_attempts = 0
|
||||
log_event(db, user.username, "login_locked", "user", user.id, summary=user.username,
|
||||
detail={"minutes": LOGIN_LOCKOUT_MINUTES})
|
||||
db.commit()
|
||||
raise HTTPException(status_code=401, detail="Invalid username or password")
|
||||
if not user.is_active:
|
||||
raise HTTPException(status_code=403, detail="Account is disabled")
|
||||
user.failed_attempts = 0
|
||||
user.locked_until = None
|
||||
user.last_login_at = now
|
||||
db.commit()
|
||||
token = auth.create_token(user)
|
||||
auth.set_session_cookie(response, request, token)
|
||||
return {"user": user.to_dict()}
|
||||
|
||||
|
||||
@app.post("/api/auth/logout")
|
||||
def logout(response: Response):
|
||||
auth.clear_session_cookie(response)
|
||||
@@ -762,14 +695,13 @@ async def okta_callback(request: Request, db: Session = Depends(get_db)):
|
||||
# at all — this app still decides what a first-time sign-in may do. A new
|
||||
# account gets the lowest-privilege role and no project membership; an admin
|
||||
# or project super user grants access afterward, same as any account created
|
||||
# by hand today (create_user() above). No password is ever set — Okta is the
|
||||
# only credential (D15's "no stored password").
|
||||
# by hand today (create_user() above). No password field exists at all —
|
||||
# Okta is the only credential (D15/D16, real deletion as of T10.4).
|
||||
user = models.User(
|
||||
id=gen_id("user"),
|
||||
username=identity,
|
||||
email=(claims.get("email") or "").strip(),
|
||||
full_name=(claims.get("name") or "").strip(),
|
||||
password_hash="",
|
||||
role=auth.ROLE_PROJECT_USER,
|
||||
)
|
||||
db.add(user)
|
||||
@@ -794,113 +726,6 @@ async def okta_callback(request: Request, db: Session = Depends(get_db)):
|
||||
return redirect
|
||||
|
||||
|
||||
# ── Self-service password reset (needs email switched on) ──────────────────────
|
||||
RESET_COOLDOWN_SECONDS = int(os.getenv("AUTH_RESET_COOLDOWN_SECONDS", "120"))
|
||||
# In-process throttle: one reset mail per (account, client) per cooldown. Enough to
|
||||
# stop someone using the form to spam a colleague's inbox. Per-worker and lost on
|
||||
# restart — deliberately simple; the token expiry is the real control.
|
||||
_reset_last: dict[str, float] = {}
|
||||
|
||||
|
||||
def _reset_throttled(request: Request, username: str) -> bool:
|
||||
now = monotonic()
|
||||
key = f"{(username or '').strip().lower()}|{request.client.host if request.client else ''}"
|
||||
prev = _reset_last.get(key)
|
||||
if prev is not None and (now - prev) < RESET_COOLDOWN_SECONDS:
|
||||
return True
|
||||
_reset_last[key] = now
|
||||
if len(_reset_last) > 5000: # bound the dict on a long-lived worker
|
||||
cutoff = now - RESET_COOLDOWN_SECONDS
|
||||
for k in [k for k, t in _reset_last.items() if t < cutoff]:
|
||||
_reset_last.pop(k, None)
|
||||
return False
|
||||
|
||||
|
||||
def reset_body(user: "models.User", link: str, minutes: int) -> str:
|
||||
# No account detail beyond the username, and no customer data — same rule as
|
||||
# the assignment mail. The link is the only sensitive thing in here.
|
||||
who = user.full_name or user.username
|
||||
return (
|
||||
f"Hi {who},\n\n"
|
||||
f"A password reset was requested for your Work Package Suite account "
|
||||
f"({user.username}).\n\n"
|
||||
f"Set a new password:\n{link}\n\n"
|
||||
f"The link expires in {minutes} minutes and can only be used once. "
|
||||
f"If you didn't request this, you can ignore this email — your current "
|
||||
f"password still works.\n"
|
||||
)
|
||||
|
||||
|
||||
@app.get("/api/auth/reset-available")
|
||||
def reset_available(db: Session = Depends(get_db)):
|
||||
"""Whether the login page should offer 'Forgot password'. Self-service reset
|
||||
depends entirely on outbound email, so it's off unless email is enabled AND
|
||||
SMTP is configured — otherwise the only route is an admin reset."""
|
||||
s = notify.get_settings(db)
|
||||
return {"enabled": bool(s.get("email_enabled")) and notify.smtp_ready(s)}
|
||||
|
||||
|
||||
@app.post("/api/auth/forgot-password")
|
||||
def forgot_password(body: ForgotPasswordIn, request: Request, db: Session = Depends(get_db)):
|
||||
"""Email a reset link. Always returns the same 200 response whether or not the
|
||||
account exists — this endpoint is unauthenticated, so it must not become a
|
||||
username/email oracle. Failures are recorded in the audit log instead."""
|
||||
s = notify.get_settings(db)
|
||||
if not (s.get("email_enabled") and notify.smtp_ready(s)):
|
||||
raise HTTPException(
|
||||
status_code=503,
|
||||
detail="Password reset by email isn't available. Ask an administrator to reset it for you.",
|
||||
)
|
||||
if _reset_throttled(request, body.username):
|
||||
# Same shape as the success response — no oracle, no mail bomb.
|
||||
return {"ok": True, "message": "If that account exists, a reset link is on its way."}
|
||||
user = auth.find_user(db, body.username)
|
||||
if user and user.is_active and user.email:
|
||||
base = (s.get("app_base_url") or "").rstrip("/")
|
||||
token = auth.create_reset_token(user)
|
||||
link = f"{base}/login.html?reset={token}" if base else f"/login.html?reset={token}"
|
||||
sent = notify.send_now(
|
||||
db, user.email,
|
||||
"Work Package Suite — reset your password",
|
||||
reset_body(user, link, auth.RESET_MINUTES),
|
||||
)
|
||||
log_event(db, user.username, "password_reset_requested", "user", user.id,
|
||||
summary=user.username, detail={"emailed": bool(sent)})
|
||||
db.commit()
|
||||
else:
|
||||
# Log the miss for the admin's benefit; the caller can't tell the difference.
|
||||
log_event(db, "(anonymous)", "password_reset_miss", "user", "",
|
||||
summary=(body.username or "")[:200],
|
||||
detail={"reason": "no account, inactive, or no email on file"})
|
||||
db.commit()
|
||||
return {"ok": True, "message": "If that account exists, a reset link is on its way."}
|
||||
|
||||
|
||||
@app.post("/api/auth/reset-password")
|
||||
def reset_password(body: ResetPasswordIn, db: Session = Depends(get_db)):
|
||||
"""Complete a reset using the emailed token. The token carries the user's
|
||||
token_version, and finishing a reset bumps it — so the link is single-use and
|
||||
every existing session for that account is signed out."""
|
||||
claims = auth.decode_reset_token(body.token or "")
|
||||
if not claims:
|
||||
raise HTTPException(status_code=400, detail="This reset link is invalid or has expired. Request a new one.")
|
||||
user = db.get(models.User, claims.get("sub"))
|
||||
if not user or not user.is_active:
|
||||
raise HTTPException(status_code=400, detail="This reset link is no longer valid.")
|
||||
if (claims.get("ver", 0) or 0) != (user.token_version or 0):
|
||||
raise HTTPException(status_code=400, detail="This reset link has already been used. Request a new one.")
|
||||
problem = auth.password_problem(body.new_password, user.username, user.email)
|
||||
if problem:
|
||||
raise HTTPException(status_code=400, detail=problem)
|
||||
user.password_hash = auth.hash_password(body.new_password)
|
||||
user.token_version = (user.token_version or 0) + 1 # burns the link + all sessions
|
||||
# A completed reset also clears any login lockout — the person has proven
|
||||
# control of the mailbox, so there's nothing left to throttle.
|
||||
user.failed_attempts = 0
|
||||
user.locked_until = None
|
||||
log_event(db, user.username, "password_reset", "user", user.id, summary=user.username)
|
||||
db.commit()
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@app.get("/api/auth/me")
|
||||
@@ -956,22 +781,6 @@ def set_preferences(body: PreferencesIn, user: models.User = Depends(auth.get_cu
|
||||
return {"user": user.to_dict()}
|
||||
|
||||
|
||||
@app.post("/api/auth/password")
|
||||
def change_password(body: PasswordChangeIn, request: Request, response: Response, user: models.User = Depends(auth.get_current_user), db: Session = Depends(get_db)):
|
||||
if not auth.verify_password(body.current_password, user.password_hash):
|
||||
raise HTTPException(status_code=400, detail="Current password is incorrect")
|
||||
problem = auth.password_problem(body.new_password, user.username, user.email)
|
||||
if problem:
|
||||
raise HTTPException(status_code=400, detail=problem)
|
||||
user.password_hash = auth.hash_password(body.new_password)
|
||||
user.token_version = (user.token_version or 0) + 1 # invalidate all OTHER existing sessions
|
||||
db.commit()
|
||||
db.refresh(user)
|
||||
# Keep this session logged in by re-issuing a cookie carrying the new version.
|
||||
auth.set_session_cookie(response, request, auth.create_token(user))
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
# ── User administration ─────────────────────────────────────────────────────────
|
||||
# Two kinds of caller reach these routes: an app admin, who manages every account,
|
||||
# and a Project Super User, who manages the accounts on the projects they administer.
|
||||
@@ -1059,9 +868,14 @@ def user_scope(user: models.User = Depends(auth.get_current_user), db: Session =
|
||||
|
||||
@app.post("/api/auth/users")
|
||||
def create_user(body: NewUserIn, actor: models.User = Depends(require_user_manager), db: Session = Depends(get_db)):
|
||||
problem = auth.password_problem(body.password, body.username, body.email)
|
||||
if problem:
|
||||
raise HTTPException(status_code=400, detail=problem)
|
||||
"""Create an account ahead of its first Okta sign-in — e.g. to put it on
|
||||
projects or hand it a role before anyone has ever signed in as them.
|
||||
|
||||
`body.username` MUST match what Okta's identity claim will send for this
|
||||
person exactly (see OKTA_IDENTITY_CLAIM, server/okta_auth.py) — auth.find_user()
|
||||
is how a later Okta sign-in locates this row (T10.3). A mismatch doesn't
|
||||
fail loudly; it silently produces a second, JIT-provisioned account instead
|
||||
of signing this person into the one just created here."""
|
||||
allowed = grantable_roles(actor)
|
||||
if body.role not in allowed:
|
||||
raise HTTPException(status_code=400, detail=f"role must be one of {', '.join(allowed)}")
|
||||
@@ -1093,7 +907,6 @@ def create_user(body: NewUserIn, actor: models.User = Depends(require_user_manag
|
||||
username=body.username.strip(),
|
||||
email=body.email.strip(),
|
||||
full_name=body.full_name.strip(),
|
||||
password_hash=auth.hash_password(body.password),
|
||||
role=body.role,
|
||||
project_role=body.project_role.strip()[:120],
|
||||
)
|
||||
@@ -1120,24 +933,6 @@ def create_user(body: NewUserIn, actor: models.User = Depends(require_user_manag
|
||||
return directory_entry(db, u, actor)
|
||||
|
||||
|
||||
@app.post("/api/auth/users/{user_id}/password")
|
||||
def admin_reset_password(user_id: str, body: AdminPasswordIn, actor: models.User = Depends(require_user_manager), db: Session = Depends(get_db)):
|
||||
u = load_target_user(db, user_id)
|
||||
require_see_user(db, actor, u)
|
||||
require_manage_user(db, actor, u)
|
||||
problem = auth.password_problem(body.new_password, u.username, u.email)
|
||||
if problem:
|
||||
raise HTTPException(status_code=400, detail=problem)
|
||||
u.password_hash = auth.hash_password(body.new_password)
|
||||
u.token_version = (u.token_version or 0) + 1 # revoke the user's existing sessions
|
||||
# An administrative password reset was the one user-account change that left no
|
||||
# trace; it is the most impersonation-adjacent thing on this page, so it logs.
|
||||
log_event(db, actor, "password_reset", "user", u.id, summary=u.username,
|
||||
detail={"by": "administrator"})
|
||||
db.commit()
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@app.post("/api/auth/users/{user_id}/active")
|
||||
def set_user_active(user_id: str, body: ActiveIn, actor: models.User = Depends(require_user_manager), db: Session = Depends(get_db)):
|
||||
u = load_target_user(db, user_id)
|
||||
|
||||
Reference in New Issue
Block a user