From 4423f0a5edf6c23aabd77dd167a088a3895eaa01 Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Sun, 31 May 2026 13:02:59 +0200 Subject: [PATCH] fix(api-client): generalize 401 redirect from /account/* to all 4 personas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Yesterday's redirectIfCustomerAreaUnauthorized was scoped to /account/* only. Admin, store, and merchant pages still hit the same UX gap when an AJAX call returned 401 on token expiry: apiClient cleared tokens and threw, leaving the page in a broken state with whatever generic error UI the caller had wired up — no redirect, no `?next=` round-trip, identical bug to the customer flicker we fixed in `b04b36a2` / `6564f138`. Rename and dispatch by path: - /account/* (not /account/login) → /account/login?next=… - /admin/* (not /admin/login) → /admin/login?next=… - /merchants/* (not /merchants/login) → /merchants/login?next=… - /store/{code}/* (not /store/{code}/login) → /store/{code}/login?next=… - anything else → return false (caller throws) Store paths include the per-store code, so the helper does a small regex to extract `{code}` from the current pathname and builds the persona's login URL with the right prefix. All three 401 handlers in apiClient (request, requestFormData, getBlob) already wrap this with the `return new Promise(() => {})` pattern from 6564f138, so the caller's `.finally(() => loading = false)` doesn't fire before navigation completes — kills the wrong-state UI flash on every persona, not just customer. Login pages updated to honour `?next=` precedence over the existing `*_last_visited_page` localStorage fallback, with persona-specific safety checks (must start with /admin/, /merchants/, /store/{code}/ respectively; must not be a login or onboarding URL). The store login also normalises the basePath because the store-code path prefix can flip between subdomain (/store/{code}/...) and dev/path-based (/platforms/{platform}/store/{code}/...) modes. Customer login already honoured `?next=` from bbb481aa; left unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/modules/core/static/admin/js/login.js | 20 +++++-- app/modules/core/static/merchant/js/login.js | 16 ++++-- app/modules/tenancy/static/store/js/login.js | 23 ++++++-- static/shared/js/api-client.js | 56 ++++++++++++++------ 4 files changed, 87 insertions(+), 28 deletions(-) diff --git a/app/modules/core/static/admin/js/login.js b/app/modules/core/static/admin/js/login.js index 5b01a8fa..96e73071 100644 --- a/app/modules/core/static/admin/js/login.js +++ b/app/modules/core/static/admin/js/login.js @@ -236,13 +236,25 @@ function adminLogin() { // Super admin or single platform - proceed to dashboard this.success = 'Login successful! Redirecting...'; - // Check for last visited page (saved before logout) + // Decide where to land after login. Precedence: + // 1. ?next= — set by apiClient on 401 mid-session; + // lands the user back on the page they were on. + // 2. admin_last_visited_page localStorage — fallback for + // organic logins where no `?next=` is supplied. + // 3. /admin/dashboard — last-resort default. + // Path safety check on (1) and (2): must start with /admin/ + // and not be a login / select-platform URL. + const nextParam = new URLSearchParams(window.location.search).get('next'); const lastPage = localStorage.getItem('admin_last_visited_page'); - const redirectTo = (lastPage && lastPage.startsWith('/admin/') && !lastPage.includes('/login') && !lastPage.includes('/select-platform')) - ? lastPage - : '/admin/dashboard'; + const isSafeAdminUrl = (u) => + u && u.startsWith('/admin/') && !u.includes('/login') && !u.includes('/select-platform'); + const redirectTo = + isSafeAdminUrl(nextParam) ? nextParam : + isSafeAdminUrl(lastPage) ? lastPage : + '/admin/dashboard'; loginLog.info('=== EXECUTING REDIRECT ==='); + loginLog.debug('next param:', nextParam); loginLog.debug('Last visited page:', lastPage); loginLog.debug('Target URL:', redirectTo); diff --git a/app/modules/core/static/merchant/js/login.js b/app/modules/core/static/merchant/js/login.js index 2b640122..06c0bb0a 100644 --- a/app/modules/core/static/merchant/js/login.js +++ b/app/modules/core/static/merchant/js/login.js @@ -131,13 +131,21 @@ function merchantLogin() { // Show success message this.success = 'Login successful! Redirecting...'; - // Check for last visited page + // Decide where to land after login. Precedence: + // 1. ?next= — set by apiClient on 401 mid-session. + // 2. merchant_last_visited_page localStorage fallback. + // 3. /merchants/dashboard default. + const nextParam = new URLSearchParams(window.location.search).get('next'); const lastPage = localStorage.getItem('merchant_last_visited_page'); - const redirectTo = (lastPage && lastPage.startsWith('/merchants/') && !lastPage.includes('/login')) - ? lastPage - : '/merchants/dashboard'; + const isSafeMerchantUrl = (u) => + u && u.startsWith('/merchants/') && !u.includes('/login'); + const redirectTo = + isSafeMerchantUrl(nextParam) ? nextParam : + isSafeMerchantUrl(lastPage) ? lastPage : + '/merchants/dashboard'; loginLog.info('Redirecting to:', redirectTo); + loginLog.debug('next param:', nextParam, '| lastPage:', lastPage); window.location.href = redirectTo; } catch (error) { diff --git a/app/modules/tenancy/static/store/js/login.js b/app/modules/tenancy/static/store/js/login.js index 663e4f1c..3b7c0c50 100644 --- a/app/modules/tenancy/static/store/js/login.js +++ b/app/modules/tenancy/static/store/js/login.js @@ -157,18 +157,33 @@ function storeLogin() { ? `/platforms/${platformCode}/store/${this.storeCode}` : `/store/${this.storeCode}`; - // Check for last visited page (saved before logout) + // Decide where to land after login. Precedence: + // 1. ?next= — set by apiClient on 401 mid-session; + // must be a /store/{code}/... URL that doesn't loop back + // to login or onboarding. + // 2. store_last_visited_page localStorage — fallback for + // organic logins (preserves the store-relative sub-path + // even if the basePath prefix changed, e.g. domain swap). + // 3. {basePath}/dashboard — last-resort default. + const nextParam = new URLSearchParams(window.location.search).get('next'); const lastPage = localStorage.getItem('store_last_visited_page'); - let redirectTo = `${basePath}/dashboard`; + const isSafeStoreUrl = (u) => + u && u.includes(`/store/${this.storeCode}/`) && + !u.includes('/login') && !u.includes('/onboarding'); - if (lastPage && !lastPage.includes('/login') && !lastPage.includes('/onboarding')) { - // Extract the store-relative path (strip any existing prefix) + let redirectTo = `${basePath}/dashboard`; + if (isSafeStoreUrl(nextParam)) { + redirectTo = nextParam; + } else if (lastPage && !lastPage.includes('/login') && !lastPage.includes('/onboarding')) { + // Preserve only the store-relative sub-path; basePath + // may have changed (subdomain ↔ /platforms/... etc.) const storePathMatch = lastPage.match(/\/store\/[^/]+(\/.*)/); if (storePathMatch) { redirectTo = `${basePath}${storePathMatch[1]}`; } } + storeLoginLog.info('next param:', nextParam); storeLoginLog.info('Last visited page:', lastPage); storeLoginLog.info('Redirecting to:', redirectTo); diff --git a/static/shared/js/api-client.js b/static/shared/js/api-client.js index ec412246..d74489d1 100644 --- a/static/shared/js/api-client.js +++ b/static/shared/js/api-client.js @@ -155,7 +155,7 @@ class APIClient { apiLog.debug('Error details:', data); apiLog.info('Clearing authentication tokens'); this.clearTokens(); - if (this.redirectIfCustomerAreaUnauthorized()) { + if (this.redirectIfUnauthorized()) { // Page is navigating away to /account/login. Return a // never-resolving promise so the caller's await never // returns and any `.finally(() => loading = false)` @@ -312,7 +312,7 @@ class APIClient { if (response.status === 401) { apiLog.warn('401 Unauthorized - Authentication failed'); this.clearTokens(); - if (this.redirectIfCustomerAreaUnauthorized()) { + if (this.redirectIfUnauthorized()) { return new Promise(() => {}); } throw new Error(data.message || data.detail || 'Unauthorized'); @@ -355,7 +355,7 @@ class APIClient { if (response.status === 401) { apiLog.warn('401 Unauthorized - Authentication failed'); this.clearTokens(); - if (this.redirectIfCustomerAreaUnauthorized()) { + if (this.redirectIfUnauthorized()) { return new Promise(() => {}); } throw new Error('Unauthorized'); @@ -451,24 +451,48 @@ class APIClient { } /** - * If the user is on a customer-area page (/account/*) and gets 401, - * send them to the login page with a return URL so they land back - * here after re-authenticating. + * If the user is on a protected page (customer / admin / store / + * merchant area) and gets a 401, send them to the persona's login + * page with a `?next=` return URL so they land back here after + * re-authenticating. * - * No-op for admin/store/merchant areas — those callers handle 401 - * their own way. Also no-op if already on the login page (avoids - * a redirect loop). + * Dispatches by path: + * /account/* (not /account/login) → /account/login?next=... + * /admin/* (not /admin/login) → /admin/login?next=... + * /store/{code}/* (not /store/{code}/login) → /store/{code}/login?next=... + * /merchants/* (not /merchants/login) → /merchants/login?next=... * - * Returns true if a redirect was scheduled (caller should suppress - * its own error UI since the page is about to navigate away). + * Returns true if a redirect was scheduled (caller should return a + * never-resolving promise so its `.finally(() => loading = false)` + * doesn't fire mid-redirect and flash a wrong UI state). + * Returns false for unknown paths or login pages — caller throws as + * usual. */ - redirectIfCustomerAreaUnauthorized() { + redirectIfUnauthorized() { const path = window.location.pathname; - const onCustomerArea = path.startsWith('/account/') && path !== '/account/login'; - if (!onCustomerArea) return false; + let loginUrl = null; + + if (path.startsWith('/account/') && path !== '/account/login') { + loginUrl = '/account/login'; + } else if (path.startsWith('/admin/') && path !== '/admin/login') { + loginUrl = '/admin/login'; + } else if (path.startsWith('/merchants/') && path !== '/merchants/login') { + loginUrl = '/merchants/login'; + } else if (path.startsWith('/store/')) { + // Store paths include the store code: /store/{code}/. + // Login URL is /store/{code}/login. Skip if already on it. + const m = path.match(/^\/store\/([^/]+)\//); + if (m) { + const candidate = `/store/${m[1]}/login`; + if (path !== candidate) loginUrl = candidate; + } + } + + if (!loginUrl) return false; + const next = encodeURIComponent(path + window.location.search); - apiLog.info('Redirecting to /account/login (session expired), next=' + next); - window.location.href = `/account/login?next=${next}`; + apiLog.info(`Redirecting to ${loginUrl} (session expired), next=${next}`); + window.location.href = `${loginUrl}?next=${next}`; return true; }