T10.3 D13 - drop users.password_hash and every path that touched it
The irreversible one. The suite no longer stores a credential of any kind.
Removed from app.py: /api/auth/forgot-password, /api/auth/reset-password,
/api/auth/reset-available, /api/auth/password, /api/auth/users/{id}/password,
the four password-bearing input models, the reset throttle and mail body, and
the password arguments to create_user. Removed from auth.py: hash_password,
verify_password, password_problem, MIN_PASSWORD_LEN, _COMMON_PASSWORDS,
create_reset_token, decode_reset_token, RESET_MINUTES, and the bcrypt import.
Removed from notify.py: the password_reset_enabled feature flag. Removed from
manage_users.py: the password prompt and the reset-password command.
token_version STAYS. Password changes no longer exist, but a role change or a
deactivation still has to invalidate sessions that are already issued.
/api/auth/users/{id}/role stays, which is D13 criterion 4 - granting admin to
an existing account must keep working, and it does.
Migration b7e4f1a20c93 uses batch_alter_table because SQLite has no DROP COLUMN
before 3.35 and local dev runs on SQLite while production runs on Postgres.
downgrade() recreates the column NULLABLE rather than NOT NULL as the baseline
declared it: there are no hashes to put back, and a NOT NULL column with no
server default refuses to add itself to a table with rows. The docstring says
plainly that the downgrade does not restore the old login - it exists so the
revision is well-formed, not because stepping back is a recovery path.
Verified:
upgrade head from empty -> password_hash absent from users
downgrade -1 -> column back, nullable (notnull=0)
upgrade head again -> absent again
remaining /api/auth routes -> no password or reset route left
create-admin -> works with no password prompt
grep for the removed symbols -> nothing outside the migration and one
docstring that names the dropped column
NOT verified, and it is a done-when box left open rather than ticked: the
migration has only been round-tripped on SQLite. No Postgres is available here.
batch_alter_table takes the direct ALTER path on Postgres, which is the simpler
of the two, but "simpler" is not "tested".
notify.send_now is now orphaned - its only caller was forgot_password. Logged as
BL-026 rather than deleted in passing, because an immediate unqueued send is a
reasonable primitive to keep and that decision does not belong in an auth task.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
169
server/app.py
169
server/app.py
@@ -616,8 +616,10 @@ class LoginIn(BaseModel):
|
||||
|
||||
|
||||
class NewUserIn(BaseModel):
|
||||
# No password: D13 authenticates against the domain, so an administrator
|
||||
# pre-creating an account only supplies identity and authorization. The person
|
||||
# signs in with their Windows password, or is provisioned on first sign-in.
|
||||
username: str
|
||||
password: str
|
||||
full_name: str = ""
|
||||
email: str = ""
|
||||
role: str = auth.ROLE_PROJECT_USER # permissions role — see auth.ROLES
|
||||
@@ -639,24 +641,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
|
||||
|
||||
@@ -894,115 +878,6 @@ def logout(response: Response):
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
# ── 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")
|
||||
def whoami(user: models.User = Depends(auth.get_current_user)):
|
||||
"""Who is logged in. The frontend guard calls this on every page load.
|
||||
@@ -1056,22 +931,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.
|
||||
@@ -1159,9 +1018,6 @@ 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)
|
||||
allowed = grantable_roles(actor)
|
||||
if body.role not in allowed:
|
||||
raise HTTPException(status_code=400, detail=f"role must be one of {', '.join(allowed)}")
|
||||
@@ -1193,7 +1049,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],
|
||||
)
|
||||
@@ -1220,24 +1075,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