T10.5: login becomes an Okta redirect, not a form
server/app.py: - okta_login(): validates and stashes ?next= (same-site path only) in the OAuth-state session before redirecting to Okta, so a deep link an assignment email carried (X1/CR-011/CR-014) survives the round trip instead of always landing on /index.html. - okta_callback(): reads that stashed next= back (re-validated on the way out too - belt and suspenders against a crafted value) and redirects there on success. The two failure paths that used to raise a raw HTTPException - OAuthError (sign-in cancelled/failed) and a locally-disabled account - now redirect to /login.html?error=... instead: this route is reached by a full page browser navigation from Okta, not a fetch() call, so a JSON error body just looks like a broken page to whoever is signing in. html/login.html + html/login.js: rebuilt as a single "Sign in with Okta" link, replacing the username/password form and the forgot/reset-password views (gone entirely - no local password exists to reset, per D15/D16/T10.4). Kept the accessible error/ok banner pattern (role=alert / role=status) byte-for- byte, since CLAUDE.md names this file as the reference other pages copy for that pattern. login.js reads ?next= off its own URL (auth-guard.js's goToLogin() already builds this, unchanged) and forwards it to /api/auth/okta/login, and shows a plain-language message for ?error=disabled / ?error=cancelled, clearing the code from the address bar once shown. Sign- out (auth-guard.js's wpLogout()) already redirected to login.html - untouched, already satisfied "lands back on the app's own login page." Uses a real <a href> rather than a JS-driven navigation, so it's a working link even before login.js runs, and needs no keyboard/touch handling beyond what a link gets for free (C1 accessibility). Also fixed in passing (not a separate commit - this is what exposed it): _safe_next_path() on the server and safeNext() in login.js enforce the exact same rule (same-site path only, reject '//' and scheme URLs) so a crafted ?next= can't become an open redirect through a real Okta sign-in. Verified: a fake-Okta-client round trip against the real app (SessionMiddleware fix from the prior commit) confirms next= is honored end to end, a malicious next= is rejected and falls back to /index.html, OAuthError redirects to ?error=cancelled, and a disabled account redirects to ?error=disabled. The JS-side safeNext() was checked against the same cases directly in Node and matches the server's validation exactly. login.js passes `node --check`; login.html parses cleanly. Live 390px/1440px screenshots were NOT captured this session - the environment's browser pane isn't signed in to view a published preview of it, so that check needs to happen when this branch is actually run and opened by a signed-in browser; the layout risk is low since .card/.brand/.error/.ok/.foot are unchanged from the already-shipped file and the only new CSS is one simple full-width block link. wave-10.md T10.5 / D15 / D16
This commit is contained in:
@@ -680,11 +680,27 @@ def logout(response: Response):
|
||||
# can complete authorize_redirect at all. No app-side group/claim check is layered on
|
||||
# top here — see okta_auth.py's docstring and wave-10.md T10.2 for why.
|
||||
|
||||
def _safe_next_path(raw: str) -> str:
|
||||
"""A same-site path only — same rule login.js's own nextTarget() enforces
|
||||
client-side. Rejects absolute/scheme URLs ('//evil.com', 'https://evil.com')
|
||||
so a crafted ?next= can't turn a real Okta sign-in into an open redirect."""
|
||||
raw = (raw or "").strip()
|
||||
if raw and raw.startswith("/") and not raw.startswith("//"):
|
||||
return raw
|
||||
return ""
|
||||
|
||||
|
||||
@app.get("/api/auth/okta/login")
|
||||
async def okta_login(request: Request):
|
||||
"""Send the browser to Okta's authorize endpoint."""
|
||||
"""Send the browser to Okta's authorize endpoint. Where to land afterward
|
||||
(?next=, e.g. from a deep link an assignment email carried — X1/CR-011/CR-014)
|
||||
rides in the OAuth-state session cookie alongside Authlib's own state/nonce,
|
||||
since nothing else survives the round trip to Okta and back."""
|
||||
if not okta_auth.oauth:
|
||||
raise HTTPException(status_code=503, detail="Sign-in is temporarily unavailable. Contact IT.")
|
||||
next_path = _safe_next_path(request.query_params.get("next", ""))
|
||||
if next_path:
|
||||
request.session["post_login_redirect"] = next_path
|
||||
return await okta_auth.oauth.okta.authorize_redirect(request, okta_auth.REDIRECT_URI)
|
||||
|
||||
|
||||
@@ -692,14 +708,19 @@ async def okta_login(request: Request):
|
||||
async def okta_callback(request: Request, db: Session = Depends(get_db)):
|
||||
"""Exchange the authorization code for tokens, validate the ID token, and sign the
|
||||
person in. T10.3: matches the identity claim to a local account, or JIT-provisions
|
||||
one, then issues the same session cookie login() does today."""
|
||||
one, then issues the same session cookie login() does today.
|
||||
|
||||
Failure paths land back on the login page with a plain-language ?error= instead
|
||||
of a raw HTTPException — this route is reached by a full-page browser navigation
|
||||
from Okta, not a fetch() call, so a JSON error body is just a broken-looking page
|
||||
to whoever is sitting at the keyboard (T10.5)."""
|
||||
if not okta_auth.oauth:
|
||||
raise HTTPException(status_code=503, detail="Sign-in is temporarily unavailable. Contact IT.")
|
||||
try:
|
||||
token = await okta_auth.oauth.okta.authorize_access_token(request)
|
||||
except OAuthError as exc:
|
||||
log.warning("Okta callback rejected: %s", exc)
|
||||
raise HTTPException(status_code=401, detail="Sign-in was not completed.")
|
||||
return RedirectResponse(url="/login.html?error=cancelled", status_code=303)
|
||||
claims = token.get("userinfo") or {}
|
||||
identity = (claims.get(okta_auth.IDENTITY_CLAIM) or "").strip()
|
||||
if not identity:
|
||||
@@ -727,9 +748,10 @@ async def okta_callback(request: Request, db: Session = Depends(get_db)):
|
||||
detail={"role": user.role, "via": "okta_jit"})
|
||||
elif not user.is_active:
|
||||
# Deprovisioning stays local (D15's "roles stay local"): Okta letting someone
|
||||
# through does not override an account this app has disabled. Same rule and
|
||||
# same message login() enforces today.
|
||||
raise HTTPException(status_code=403, detail="Account is disabled")
|
||||
# through does not override an account this app has disabled. Same rule
|
||||
# login() enforced today, now surfaced as a login-page banner (T10.5)
|
||||
# instead of a raw 403 body, for the reason in this route's docstring.
|
||||
return RedirectResponse(url="/login.html?error=disabled", status_code=303)
|
||||
|
||||
user.failed_attempts = 0
|
||||
user.locked_until = None
|
||||
@@ -738,7 +760,8 @@ async def okta_callback(request: Request, db: Session = Depends(get_db)):
|
||||
db.refresh(user)
|
||||
|
||||
tok = auth.create_token(user)
|
||||
redirect = RedirectResponse(url="/index.html", status_code=303)
|
||||
target = _safe_next_path(request.session.pop("post_login_redirect", "")) or "/index.html"
|
||||
redirect = RedirectResponse(url=target, status_code=303)
|
||||
auth.set_session_cookie(redirect, request, tok)
|
||||
return redirect
|
||||
|
||||
|
||||
Reference in New Issue
Block a user