mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-23 19:06:33 +00:00
feat(web): signed-in account chip on CLI auth approval + switch-accounts (TASK-836) (#271)
* feat(web): show signed-in account chip on CLI auth approval page (TASK-836)
The CLI auth approval page (/auth/cli/{code}) previously showed only an
"Approve" button with no indication of WHICH account was about to grant
the CLI access. For OAuth users on Pad Cloud — most of whom have
multiple GitHub/Google accounts — wrong-account approval was a silent
footgun, recoverable only by revoking the CLI token after the fact.
This change renders an account chip above the Approve button when the
session is pending, showing:
- The user's avatar (when avatar_url is present)
- Display name (or username fallback if name is empty)
- Email
Below the chip, a "I'm not <Name> — switch accounts" link button calls
api.auth.logout() and navigates to /login?redirect=/auth/cli/{code}, so
after re-login the user lands back on this same approval page (the
login page already validates relative-only redirects to prevent open
redirects).
Graceful degradation: api.auth.me() is wrapped in its own try/catch.
If it fails, currentUser stays null and the chip simply doesn't
render — the Approve flow still works. The Approve button is also
disabled while a switch-accounts call is in flight to avoid
double-action races.
Works for both email/password (self-hosted) and OAuth (Cloud)
sessions because api.auth.me() and api.auth.logout() operate on the
unified pad session regardless of how it was established.
Parent: PLAN-833. Source: IDEA-831 issue #3.
* fix(web): plumb redirect through OAuth login + surface logout failures
Codex round-1 findings on TASK-836:
- MEDIUM: The login page already preserved ?redirect= for password and
2FA login but the GitHub/Google OAuth buttons were plain anchors with
hardcoded hrefs. A user clicking "Switch accounts" on the CLI auth
approval page and then signing in via OAuth would land at /console
instead of back at /auth/cli/{code}. Added a $derived oauthRedirectQuery
rune that reuses the existing getRedirectTarget() validation and
appends ?redirect=<encoded> to both OAuth links when the redirect is
non-default. Whether pad-cloud's /auth/github and /auth/google handlers
honor the redirect param is an out-of-tree concern and tracked
separately if needed; the client side now consistently passes it.
- LOW: handleSwitchAccount silently swallowed logout failures and then
navigated to /login. If the server didn't actually invalidate the
session cookie (network/CSRF), login's onMount would see an
authenticated session and bounce the user right back to the approval
page — making "switch accounts" appear to be a no-op. The handler now
surfaces the error in the page error slot and stays on the approval
page, giving the user a clear next step (retry or close the tab) and
also resets switchingAccount so the UI isn't stuck in a "Switching..."
state.
A defensive code check was also added to handleSwitchAccount to mirror
handleApprove's "Missing CLI session code" guard, even though the button
only renders when status === 'pending'.
Parent: PLAN-833.
* fix(web): tighten redirect validation + cover OAuth banner buttons
Codex round-2 findings on TASK-836:
- MEDIUM: getRedirectTarget() accepted protocol-relative URLs (`//host`
and `/\host`) because the bare `startsWith('/')` check passes for
both. Browsers and most server-side redirect handlers treat those as
cross-origin destinations, so a crafted `?redirect=//evil.example`
could become an open redirect once forwarded through the OAuth
handler. Now also rejects strings that start with `//` or `/\`. This
was a pre-existing bug in the password/2FA redirect path; the OAuth
link change made the surface area worth tightening.
- LOW: The "Use a different GitHub/Google account" banner buttons that
appear on `oauth_provider_not_linked` errors hardcoded `?force=1` and
dropped the redirect target. Added a sibling `oauthRedirectAmpQuery`
derived value (`&redirect=...`) so those links compose properly with
`?force=1`. When the redirect is the default `/console` it stays
empty so we don't add redundant query noise.
Both changes live in cmd/pad/... no, in web/src/routes/login/+page.svelte
and don't affect the password / 2FA paths beyond the validation
tightening (which they were already passing through silently).
Parent: PLAN-833.
This commit is contained in:
xarmian
@@ -3,10 +3,13 @@
|
||||
import { page } from '$app/stores';
|
||||
import { goto } from '$app/navigation';
|
||||
import { api } from '$lib/api/client';
|
||||
import type { User } from '$lib/types';
|
||||
|
||||
let status = $state<'loading' | 'pending' | 'approved' | 'expired' | 'error' | 'success' | 'already_approved'>('loading');
|
||||
let error = $state('');
|
||||
let approving = $state(false);
|
||||
let switchingAccount = $state(false);
|
||||
let currentUser = $state<User | null>(null);
|
||||
|
||||
onMount(async () => {
|
||||
const code = $page.params.code;
|
||||
@@ -36,6 +39,15 @@
|
||||
return;
|
||||
}
|
||||
|
||||
// Best-effort load of the current user so we can show which account
|
||||
// the CLI session is about to be linked to. If this fails, the chip
|
||||
// just won't render but the Approve flow still works.
|
||||
try {
|
||||
currentUser = await api.auth.me();
|
||||
} catch {
|
||||
currentUser = null;
|
||||
}
|
||||
|
||||
status = 'pending';
|
||||
} catch {
|
||||
status = 'error';
|
||||
@@ -65,6 +77,32 @@
|
||||
approving = false;
|
||||
}
|
||||
}
|
||||
|
||||
async function handleSwitchAccount() {
|
||||
const code = $page.params.code;
|
||||
if (!code) {
|
||||
error = 'Missing CLI session code.';
|
||||
return;
|
||||
}
|
||||
switchingAccount = true;
|
||||
error = '';
|
||||
try {
|
||||
await api.auth.logout();
|
||||
} catch (err: unknown) {
|
||||
// Surface the failure rather than swallowing it: if the server
|
||||
// did not invalidate the session cookie, navigating to /login
|
||||
// would bounce straight back here authenticated and "switch
|
||||
// accounts" would appear to be a no-op. Showing the error
|
||||
// gives the user a clear next step (retry / close the tab).
|
||||
switchingAccount = false;
|
||||
error =
|
||||
err instanceof Error && err.message
|
||||
? `Couldn't sign you out: ${err.message}`
|
||||
: "Couldn't sign you out. Please try again or close this tab.";
|
||||
return;
|
||||
}
|
||||
goto(`/login?redirect=/auth/cli/${code}`, { replaceState: true });
|
||||
}
|
||||
</script>
|
||||
|
||||
<div class="login-page">
|
||||
@@ -89,11 +127,41 @@
|
||||
A CLI session is requesting access to your account. Approve this request to sign in from the terminal.
|
||||
</p>
|
||||
|
||||
{#if currentUser}
|
||||
<div class="account-chip">
|
||||
{#if currentUser.avatar_url}
|
||||
<img
|
||||
class="avatar"
|
||||
src={currentUser.avatar_url}
|
||||
alt=""
|
||||
width="32"
|
||||
height="32"
|
||||
/>
|
||||
{/if}
|
||||
<div class="account-info">
|
||||
<div class="account-name">{currentUser.name || currentUser.username}</div>
|
||||
<div class="account-email">{currentUser.email}</div>
|
||||
</div>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
class="link-button"
|
||||
onclick={handleSwitchAccount}
|
||||
disabled={switchingAccount || approving}
|
||||
>
|
||||
{#if switchingAccount}
|
||||
Switching...
|
||||
{:else}
|
||||
I'm not {currentUser.name || currentUser.username} — switch accounts
|
||||
{/if}
|
||||
</button>
|
||||
{/if}
|
||||
|
||||
{#if error}
|
||||
<p class="error">{error}</p>
|
||||
{/if}
|
||||
|
||||
<button onclick={handleApprove} disabled={approving}>
|
||||
<button onclick={handleApprove} disabled={approving || switchingAccount}>
|
||||
{#if approving}
|
||||
Approving...
|
||||
{:else}
|
||||
@@ -190,4 +258,78 @@
|
||||
opacity: 0.6;
|
||||
cursor: not-allowed;
|
||||
}
|
||||
|
||||
.account-chip {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: var(--space-3);
|
||||
width: 100%;
|
||||
padding: var(--space-3);
|
||||
background: var(--bg-primary);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius);
|
||||
margin-bottom: var(--space-3);
|
||||
text-align: left;
|
||||
}
|
||||
|
||||
.avatar {
|
||||
width: 32px;
|
||||
height: 32px;
|
||||
border-radius: 50%;
|
||||
object-fit: cover;
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
.account-info {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
min-width: 0;
|
||||
flex: 1;
|
||||
}
|
||||
|
||||
.account-name {
|
||||
color: var(--text-primary);
|
||||
font-size: 0.9rem;
|
||||
font-weight: 500;
|
||||
line-height: 1.3;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.account-email {
|
||||
color: var(--text-muted);
|
||||
font-size: 0.8rem;
|
||||
line-height: 1.3;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
button.link-button {
|
||||
width: 100%;
|
||||
padding: 0;
|
||||
background: none;
|
||||
color: var(--text-muted);
|
||||
border: none;
|
||||
border-radius: 0;
|
||||
font-size: 0.8rem;
|
||||
font-weight: 400;
|
||||
font-family: var(--font-ui);
|
||||
text-align: center;
|
||||
cursor: pointer;
|
||||
margin-bottom: var(--space-5);
|
||||
transition: color 0.15s;
|
||||
}
|
||||
|
||||
button.link-button:hover:not(:disabled) {
|
||||
color: var(--text-primary);
|
||||
opacity: 1;
|
||||
text-decoration: underline;
|
||||
}
|
||||
|
||||
button.link-button:disabled {
|
||||
opacity: 0.6;
|
||||
cursor: not-allowed;
|
||||
}
|
||||
</style>
|
||||
|
||||
@@ -32,13 +32,38 @@
|
||||
|
||||
function getRedirectTarget(): string {
|
||||
const redirect = $page.url.searchParams.get('redirect');
|
||||
// Only allow relative redirects (prevent open redirect)
|
||||
if (redirect && redirect.startsWith('/')) {
|
||||
// Only allow relative redirects (prevent open redirect). A bare
|
||||
// `startsWith('/')` is NOT enough: protocol-relative URLs like
|
||||
// `//evil.example` and `/\evil.example` pass that check but are
|
||||
// treated by browsers (and most server-side redirect handlers) as
|
||||
// cross-origin destinations. Reject those explicitly.
|
||||
if (
|
||||
redirect &&
|
||||
redirect.startsWith('/') &&
|
||||
!redirect.startsWith('//') &&
|
||||
!redirect.startsWith('/\\')
|
||||
) {
|
||||
return redirect;
|
||||
}
|
||||
return '/console';
|
||||
}
|
||||
|
||||
const oauthRedirectQuery = $derived.by(() => {
|
||||
const target = getRedirectTarget();
|
||||
if (target === '/console') return '';
|
||||
return `?redirect=${encodeURIComponent(target)}`;
|
||||
});
|
||||
|
||||
// Same target appended with `&` so it composes with `?force=1` on
|
||||
// the "Use a different <provider> account" banner buttons. Empty
|
||||
// when redirect is the default destination so we don't append a
|
||||
// redundant `&redirect=%2Fconsole`.
|
||||
const oauthRedirectAmpQuery = $derived.by(() => {
|
||||
const target = getRedirectTarget();
|
||||
if (target === '/console') return '';
|
||||
return `&redirect=${encodeURIComponent(target)}`;
|
||||
});
|
||||
|
||||
// Map pad-cloud's /login?error=... redirect codes to a friendly banner
|
||||
// plus optional CTAs. Kept near onMount so the full list of codes the
|
||||
// frontend handles is easy to audit against pad-cloud/oauth.go.
|
||||
@@ -234,7 +259,7 @@
|
||||
<div class="oauth-banner-actions">
|
||||
{#if oauthBanner.provider === 'github' || oauthBanner.provider === null}
|
||||
<a
|
||||
href="/auth/github?force=1"
|
||||
href="/auth/github?force=1{oauthRedirectAmpQuery}"
|
||||
data-sveltekit-reload
|
||||
class="oauth-banner-btn"
|
||||
>
|
||||
@@ -243,7 +268,7 @@
|
||||
{/if}
|
||||
{#if oauthBanner.provider === 'google' || oauthBanner.provider === null}
|
||||
<a
|
||||
href="/auth/google?force=1"
|
||||
href="/auth/google?force=1{oauthRedirectAmpQuery}"
|
||||
data-sveltekit-reload
|
||||
class="oauth-banner-btn"
|
||||
>
|
||||
@@ -335,11 +360,11 @@
|
||||
</div>
|
||||
|
||||
<div class="oauth-buttons">
|
||||
<a href="/auth/github" data-sveltekit-reload class="oauth-btn oauth-github">
|
||||
<a href="/auth/github{oauthRedirectQuery}" data-sveltekit-reload class="oauth-btn oauth-github">
|
||||
<svg width="18" height="18" viewBox="0 0 16 16" fill="currentColor"><path d="M8 0C3.58 0 0 3.58 0 8c0 3.54 2.29 6.53 5.47 7.59.4.07.55-.17.55-.38 0-.19-.01-.82-.01-1.49-2.01.37-2.53-.49-2.69-.94-.09-.23-.48-.94-.82-1.13-.28-.15-.68-.52-.01-.53.63-.01 1.08.58 1.23.82.72 1.21 1.87.87 2.33.66.07-.52.28-.87.51-1.07-1.78-.2-3.64-.89-3.64-3.95 0-.87.31-1.59.82-2.15-.08-.2-.36-1.02.08-2.12 0 0 .67-.21 2.2.82.64-.18 1.32-.27 2-.27.68 0 1.36.09 2 .27 1.53-1.04 2.2-.82 2.2-.82.44 1.1.16 1.92.08 2.12.51.56.82 1.27.82 2.15 0 3.07-1.87 3.75-3.65 3.95.29.25.54.73.54 1.48 0 1.07-.01 1.93-.01 2.2 0 .21.15.46.55.38A8.013 8.013 0 0016 8c0-4.42-3.58-8-8-8z"/></svg>
|
||||
Continue with GitHub
|
||||
</a>
|
||||
<a href="/auth/google" data-sveltekit-reload class="oauth-btn oauth-google">
|
||||
<a href="/auth/google{oauthRedirectQuery}" data-sveltekit-reload class="oauth-btn oauth-google">
|
||||
<svg width="18" height="18" viewBox="0 0 18 18" fill="none"><path d="M17.64 9.2c0-.637-.057-1.251-.164-1.84H9v3.481h4.844a4.14 4.14 0 01-1.796 2.716v2.259h2.908c1.702-1.567 2.684-3.875 2.684-6.615z" fill="#4285F4"/><path d="M9 18c2.43 0 4.467-.806 5.956-2.18l-2.908-2.259c-.806.54-1.837.86-3.048.86-2.344 0-4.328-1.584-5.036-3.711H.957v2.332A8.997 8.997 0 009 18z" fill="#34A853"/><path d="M3.964 10.71A5.41 5.41 0 013.682 9c0-.593.102-1.17.282-1.71V4.958H.957A8.996 8.996 0 000 9c0 1.452.348 2.827.957 4.042l3.007-2.332z" fill="#FBBC05"/><path d="M9 3.58c1.321 0 2.508.454 3.44 1.345l2.582-2.58C13.463.891 11.426 0 9 0A8.997 8.997 0 00.957 4.958L3.964 7.29C4.672 5.163 6.656 3.58 9 3.58z" fill="#EA4335"/></svg>
|
||||
Continue with Google
|
||||
</a>
|
||||
|
||||
Reference in New Issue
Block a user