From 72b10283fcd15f228035ee73881bedb017a15cc6 Mon Sep 17 00:00:00 2001 From: Matt Mabrey Date: Thu, 3 Sep 2026 11:27:26 -0700 Subject: [PATCH] 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 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 --- html/login.html | 93 +++------------------ html/login.js | 218 +++++++----------------------------------------- server/app.py | 37 ++++++-- 3 files changed, 70 insertions(+), 278 deletions(-) diff --git a/html/login.html b/html/login.html index 5a91149..9ec7fd8 100644 --- a/html/login.html +++ b/html/login.html @@ -31,34 +31,23 @@ margin-bottom: 1.5rem; } .brand img { height: 36px; width: auto; } - .brand .name { font-weight: 700; font-size: 0.95rem; color: var(--cds-text-primary); } h1 { font-size: 1.5rem; margin-bottom: 0.25rem; } .sub { color: var(--cds-text-secondary); font-size: 0.875rem; margin-bottom: 1.75rem; } - label { display: block; font-size: 0.75rem; color: var(--cds-text-secondary); margin-bottom: 0.375rem; } - .field { margin-bottom: 1.25rem; } - input[type=text], input[type=password] { - width: 100%; - padding: 0.75rem; - font-size: 1rem; - background: var(--cds-field); - border: none; - border-bottom: 1px solid var(--cds-border-strong); - outline: 2px solid transparent; - outline-offset: -2px; - } - input:focus { outline: 2px solid var(--cds-focus); background: var(--cds-field-hover); } - button { + .btn { + display: block; width: 100%; padding: 0.875rem 1rem; font-size: 1rem; font-weight: 600; + text-align: center; + text-decoration: none; color: var(--cds-text-on-color); background: var(--cds-button-primary); border: none; transition: background 0.15s; } - button:hover:not(:disabled) { background: var(--cds-hover-primary); } - button:disabled { background: var(--cds-disabled-02); cursor: not-allowed; } + .btn:hover { background: var(--cds-hover-primary); } + .btn:focus-visible { outline: 2px solid var(--cds-focus); outline-offset: 2px; } .error { display: none; background: var(--wp-status-error-bg); @@ -69,7 +58,6 @@ margin-bottom: 1.25rem; } .error.show { display: block; } - .foot { margin-top: 1.5rem; font-size: 0.75rem; color: var(--cds-text-helper); text-align: center; } .ok { display: none; background: var(--wp-status-success-bg); @@ -80,15 +68,7 @@ margin-bottom: 1.25rem; } .ok.show { display: block; } - .note { - font-size: 0.8125rem; color: var(--cds-text-secondary); - background: var(--cds-layer-accent); border-left: 3px solid var(--cds-link-primary); - padding: 0.75rem; margin-bottom: 1.25rem; - } - .hint { font-size: 0.75rem; color: var(--cds-text-helper); margin-top: -0.75rem; margin-bottom: 1.25rem; } - a.link { color: var(--cds-link-primary); text-decoration: none; font-size: 0.8125rem; } - a.link:hover { text-decoration: underline; } - .center { text-align: center; margin-top: 1.25rem; } + .foot { margin-top: 1.5rem; font-size: 0.75rem; color: var(--cds-text-helper); text-align: center; } @@ -99,61 +79,10 @@
- -
-

Sign in

-

Work Package Suite

-
-
- - -
-
- - -
- -
-

Forgot password?

-
- - - - - - +

Sign in

+

Work Package Suite uses your organization's Okta sign-in. Select the + button below and follow the prompts there.

+ Sign in with Okta

Authorized use only · BTG / Pilot

diff --git a/html/login.js b/html/login.js index f95e8f3..c4577d3 100644 --- a/html/login.js +++ b/html/login.js @@ -1,26 +1,16 @@ /* Login page logic for the Work Package Suite. - Three views on one page: - • sign in posts to /api/auth/login. On success the server sets an - HttpOnly session cookie (not readable here — that's the - point) and we redirect to ?next= or the home page. - • forgot password posts to /api/auth/forgot-password, which emails a - single-use link. Only offered when the server reports - email is actually configured (/api/auth/reset-available); - otherwise we say to ask an admin. - • set a new password shown when the page is opened as login.html?reset= - from that email. Posts to /api/auth/reset-password. - - The reset token stays in the URL only until it's used; on success we strip it - from the address bar so it isn't left in history or copied out of the bar. */ + One action: sign in with Okta. There is no local password anymore (D15/D16, + T10.4) — this page's only job is building the link to /api/auth/okta/login + (carrying ?next=, if there was one) and showing a plain-language message for + the failure states server/app.py's okta_callback() sends back here (T10.5). */ (function () { 'use strict'; var errorBox = document.getElementById('error'); var okBox = document.getElementById('ok'); + var signinLink = document.getElementById('okta-signin'); - function show(el) { if (el) el.style.display = ''; } - function hide(el) { if (el) el.style.display = 'none'; } function byId(id) { return document.getElementById(id); } function showError(msg) { @@ -28,188 +18,38 @@ errorBox.textContent = msg; errorBox.classList.add('show'); } - function showOk(msg) { - errorBox.classList.remove('show'); - okBox.textContent = msg; - okBox.classList.add('show'); - } - function clearBanners() { - errorBox.classList.remove('show'); - okBox.classList.remove('show'); - } - // Where to go after signing in: the ?next= param if it's a safe same-site - // path, otherwise the home page. (Reject absolute/scheme URLs to avoid an - // open-redirect.) - function nextTarget() { + // Same-site path only — mirrors the check server/app.py's _safe_next_path() + // makes again on the way back, so a crafted ?next= can't become an open + // redirect even if this client-side check were somehow bypassed. + function safeNext() { try { var next = new URLSearchParams(location.search).get('next') || ''; if (next && next.charAt(0) === '/' && next.charAt(1) !== '/') return next; } catch (e) {} - return 'index.html'; + return ''; } - function resetToken() { - try { return new URLSearchParams(location.search).get('reset') || ''; } catch (e) { return ''; } + if (signinLink) { + var next = safeNext(); + if (next) signinLink.href = '/api/auth/okta/login?next=' + encodeURIComponent(next); } - function postJson(url, payload) { - return fetch(url, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(payload) - }).then(function (r) { - return r.json().catch(function () { return null; }).then(function (j) { - return { status: r.status, ok: r.ok, json: j }; - }); - }); - } + var ERROR_MESSAGES = { + disabled: 'Your account has been disabled. Contact an administrator.', + cancelled: 'Sign-in was not completed. Select the button below to try again.' + }; - function detail(res, fallback) { - var d = res && res.json && res.json.detail; - return (typeof d === 'string' && d) ? d : fallback; - } - - function view(which) { - clearBanners(); - ['login', 'forgot', 'reset'].forEach(function (v) { - (which === v ? show : hide)(byId('view-' + v)); - }); - } - - // ── sign in ──────────────────────────────────────────────────────────────── - var form = byId('login-form'); - var submitBtn = byId('submit'); - // Guarded because a cached older login.html may not have the reset views; an - // unguarded addEventListener on null would break sign-in itself. - if (!form || !submitBtn) return; - form.addEventListener('submit', function (e) { - e.preventDefault(); - clearBanners(); - var username = byId('username').value.trim(); - var password = byId('password').value; - if (!username || !password) { showError('Enter your username and password.'); return; } - - submitBtn.disabled = true; - submitBtn.textContent = 'Signing in…'; - postJson('/api/auth/login', { username: username, password: password }) - .then(function (res) { - if (res.ok) { location.replace(nextTarget()); return; } - if (res.status === 401) showError('Invalid username or password.'); - else if (res.status === 403) showError(detail(res, 'Your account is disabled.')); - else if (res.status === 429) showError(detail(res, 'Too many failed attempts. Try again later.')); - else showError(detail(res, 'Sign-in failed (HTTP ' + res.status + ').')); - submitBtn.disabled = false; - submitBtn.textContent = 'Sign in'; - }) - .catch(function () { - showError('Could not reach the server. Check your connection and try again.'); - submitBtn.disabled = false; - submitBtn.textContent = 'Sign in'; - }); - }); - - // ── forgot password ──────────────────────────────────────────────────────── - var resetAvailable = null; // null = not checked yet - - function checkResetAvailable() { - if (resetAvailable !== null) return Promise.resolve(resetAvailable); - return fetch('/api/auth/reset-available') - .then(function (r) { return r.ok ? r.json() : null; }) - .then(function (j) { resetAvailable = !!(j && j.enabled); return resetAvailable; }) - .catch(function () { resetAvailable = false; return false; }); - } - - (byId('forgot-link') || {addEventListener: function(){}}).addEventListener('click', function (e) { - e.preventDefault(); - view('forgot'); - // Prefill from the sign-in box so nobody types their username twice. - var u = byId('username').value.trim(); - if (u) byId('forgot-username').value = u; - checkResetAvailable().then(function (enabled) { - // With email off there's nothing to submit — say so and hide the form. - (enabled ? hide : show)(byId('forgot-unavailable')); - (enabled ? show : hide)(byId('forgot-form')); - if (enabled) byId('forgot-username').focus(); - }); - }); - - (byId('back-to-login') || {addEventListener: function(){}}).addEventListener('click', function (e) { - e.preventDefault(); - view('login'); - }); - - var forgotForm = byId('forgot-form') || document.createElement('form'); - var forgotBtn = byId('forgot-submit') || document.createElement('button'); - forgotForm.addEventListener('submit', function (e) { - e.preventDefault(); - clearBanners(); - var who = byId('forgot-username').value.trim(); - if (!who) { showError('Enter your username or email.'); return; } - forgotBtn.disabled = true; - forgotBtn.textContent = 'Sending…'; - postJson('/api/auth/forgot-password', { username: who }) - .then(function (res) { - if (res.status === 503) { - showError(detail(res, "Password reset by email isn't available. Ask an administrator.")); - } else if (res.ok) { - // Deliberately the same message whether or not the account exists. - showOk('If that account exists, a reset link is on its way. The link expires in an hour.'); - hide(forgotForm); - } else { - showError(detail(res, 'Could not send the reset email (HTTP ' + res.status + ').')); - } - forgotBtn.disabled = false; - forgotBtn.textContent = 'Email me a reset link'; - }) - .catch(function () { - showError('Could not reach the server. Check your connection and try again.'); - forgotBtn.disabled = false; - forgotBtn.textContent = 'Email me a reset link'; - }); - }); - - // ── set a new password (from the emailed link) ────────────────────────────── - (byId('reset-to-login') || {addEventListener: function(){}}).addEventListener('click', function (e) { - e.preventDefault(); - view('login'); - }); - - var resetForm = byId('reset-form') || document.createElement('form'); - var resetBtn = byId('reset-submit') || document.createElement('button'); - resetForm.addEventListener('submit', function (e) { - e.preventDefault(); - clearBanners(); - var token = resetToken(); - var pw = byId('new-password').value; - var pw2 = byId('new-password2').value; - if (!token) { showError('This reset link is incomplete. Request a new one.'); return; } - if (pw !== pw2) { showError('The two passwords do not match.'); return; } - if (pw.length < 12) { showError('Password must be at least 12 characters.'); return; } - - resetBtn.disabled = true; - resetBtn.textContent = 'Saving…'; - postJson('/api/auth/reset-password', { token: token, new_password: pw }) - .then(function (res) { - if (res.ok) { - // Take the token out of the URL before anything else — it's spent. - try { history.replaceState(null, '', 'login.html'); } catch (err) {} - view('login'); - showOk('Password updated. Sign in with your new password.'); - byId('username').focus(); - return; - } - showError(detail(res, 'Could not set your password (HTTP ' + res.status + ').')); - resetBtn.disabled = false; - resetBtn.textContent = 'Set password & sign in'; - }) - .catch(function () { - showError('Could not reach the server. Check your connection and try again.'); - resetBtn.disabled = false; - resetBtn.textContent = 'Set password & sign in'; - }); - }); - - // Arriving from the reset email opens straight into the new-password view. - if (resetToken()) view('reset'); + (function showErrorFromQuery() { + try { + var code = new URLSearchParams(location.search).get('error') || ''; + if (!code) return; + showError(ERROR_MESSAGES[code] || 'Sign-in was not completed. Select the button below to try again.'); + // Out of the address bar once shown — an error code has no reason to + // survive a refresh or get copied along with the link. + var url = new URL(location.href); + url.searchParams.delete('error'); + history.replaceState(null, '', url.pathname + url.search); + } catch (e) {} + })(); })(); diff --git a/server/app.py b/server/app.py index f45b7a1..e7774b5 100644 --- a/server/app.py +++ b/server/app.py @@ -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