Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions frontend/src/features/accounts/components/oauth-dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ const browserPendingState = {
errorMessage: null,
};

const browserStartingState = {
...browserPendingState,
status: "starting" as const,
};

const successState = {
...idleState,
status: "success" as const,
Expand Down Expand Up @@ -162,4 +167,85 @@ describe("OauthDialog", () => {
"http://localhost:1455/auth/callback?code=abc&state=expected",
);
});

it("refreshes the browser authorization link without leaving the dialog", async () => {
const user = userEvent.setup();
const onStart = vi.fn().mockResolvedValue(undefined);

render(
<OauthDialog
open
state={browserPendingState}
onOpenChange={vi.fn()}
onStart={onStart}
onComplete={vi.fn().mockResolvedValue(undefined)}
onManualCallback={vi.fn().mockResolvedValue(undefined)}
onReset={vi.fn()}
/>,
);

await user.click(screen.getByRole("button", { name: "Refresh link" }));

expect(onStart).toHaveBeenCalledWith("browser");
});

it("renders a disabled loading refresh state while generating a fresh browser link", () => {
render(
<OauthDialog
open
state={browserStartingState}
onOpenChange={vi.fn()}
onStart={vi.fn().mockResolvedValue(undefined)}
onComplete={vi.fn().mockResolvedValue(undefined)}
onManualCallback={vi.fn().mockResolvedValue(undefined)}
onReset={vi.fn()}
/>,
);

expect(screen.getByRole("button", { name: "Refreshing..." })).toBeDisabled();
expect(screen.getByRole("button", { name: "Change method" })).toBeDisabled();
expect(screen.getByText("Generating a fresh sign-in link...")).toBeInTheDocument();
expect(screen.queryByRole("button", { name: "Copy" })).not.toBeInTheDocument();
expect(screen.queryByRole("link", { name: "Open sign-in page" })).not.toBeInTheDocument();
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});

it("clears the pasted callback input when browser refresh disables the form", async () => {
const user = userEvent.setup();
const { rerender } = render(
<OauthDialog
open
state={browserPendingState}
onOpenChange={vi.fn()}
onStart={vi.fn().mockResolvedValue(undefined)}
onComplete={vi.fn().mockResolvedValue(undefined)}
onManualCallback={vi.fn().mockResolvedValue(undefined)}
onReset={vi.fn()}
/>,
);

const callbackInput = screen.getByPlaceholderText(
"http://localhost:1455/auth/callback?code=...&state=...",
);
await user.type(callbackInput, "http://localhost:1455/auth/callback?code=abc&state=expected");
expect(callbackInput).toHaveValue(
"http://localhost:1455/auth/callback?code=abc&state=expected",
);

rerender(
<OauthDialog
open
state={browserStartingState}
onOpenChange={vi.fn()}
onStart={vi.fn().mockResolvedValue(undefined)}
onComplete={vi.fn().mockResolvedValue(undefined)}
onManualCallback={vi.fn().mockResolvedValue(undefined)}
onReset={vi.fn()}
/>,
);

expect(callbackInput).toHaveValue("");
expect(callbackInput).toBeDisabled();
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
});
129 changes: 107 additions & 22 deletions frontend/src/features/accounts/components/oauth-dialog.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Check, CircleAlert, Copy, ExternalLink, Loader2 } from "lucide-react";
import { Check, CircleAlert, Copy, ExternalLink, Loader2, RefreshCw } from "lucide-react";
import { useCallback, useEffect, useRef, useState } from "react";

import { Button } from "@/components/ui/button";
Expand Down Expand Up @@ -38,7 +38,7 @@ function CopyButton({ text }: { text: string }) {
type="button"
size="sm"
variant="ghost"
className="h-7 gap-1 px-2 text-xs"
className="h-7 cursor-pointer gap-1 px-2 text-xs disabled:cursor-not-allowed"
onClick={() => void handleCopy()}
>
{copied ? (
Expand All @@ -58,12 +58,20 @@ function CopyButton({ text }: { text: string }) {

function ManualCallbackInput({
onSubmit,
disabled = false,
}: {
onSubmit: (callbackUrl: string) => Promise<void>;
disabled?: boolean;
}) {
const [callbackUrl, setCallbackUrl] = useState("");
const [submitting, setSubmitting] = useState(false);

useEffect(() => {
if (disabled) {
setCallbackUrl("");
}
}, [disabled]);

const handleSubmit = useCallback(async () => {
if (!callbackUrl.trim()) return;
setSubmitting(true);
Expand All @@ -87,14 +95,15 @@ function ManualCallbackInput({
type="text"
value={callbackUrl}
onChange={(e) => setCallbackUrl(e.target.value)}
disabled={disabled}
placeholder="http://localhost:1455/auth/callback?code=...&state=..."
className="flex-1 rounded-lg border bg-muted/20 px-3 py-2 font-mono text-xs outline-none focus:ring-1 focus:ring-primary"
className="flex-1 rounded-lg border bg-muted/20 px-3 py-2 font-mono text-xs outline-none focus:ring-1 focus:ring-primary disabled:cursor-not-allowed disabled:opacity-60"
/>
<Button
type="button"
size="sm"
className="h-8 px-3 text-xs"
disabled={!callbackUrl.trim() || submitting}
className="h-8 cursor-pointer px-3 text-xs disabled:cursor-not-allowed"
disabled={disabled || !callbackUrl.trim() || submitting}
onClick={() => void handleSubmit()}
>
{submitting ? "Submitting..." : "Submit"}
Expand Down Expand Up @@ -126,6 +135,7 @@ export function OauthDialog({
const [selectedMethod, setSelectedMethod] = useState<"browser" | "device">("browser");
const stage = getStage(state);
const completedRef = useRef(false);
const browserRefreshInProgress = stage === "browser" && state.status === "starting";

useEffect(() => {
if (stage === "success" && !completedRef.current) {
Expand All @@ -149,6 +159,10 @@ export function OauthDialog({
void onStart(selectedMethod);
};

const handleRefreshBrowserLink = () => {
void onStart("browser");
};

const handleChangeMethod = () => {
onReset();
};
Expand All @@ -172,7 +186,7 @@ export function OauthDialog({
type="button"
onClick={() => setSelectedMethod("browser")}
className={cn(
"w-full rounded-lg border p-3 text-left transition-colors",
"w-full cursor-pointer rounded-lg border p-3 text-left transition-colors",
selectedMethod === "browser"
? "border-primary bg-primary/5"
: "hover:bg-muted/50",
Expand All @@ -187,7 +201,7 @@ export function OauthDialog({
type="button"
onClick={() => setSelectedMethod("device")}
className={cn(
"w-full rounded-lg border p-3 text-left transition-colors",
"w-full cursor-pointer rounded-lg border p-3 text-left transition-colors",
selectedMethod === "device"
? "border-primary bg-primary/5"
: "hover:bg-muted/50",
Expand All @@ -204,16 +218,46 @@ export function OauthDialog({
{/* Browser stage */}
{stage === "browser" ? (
<div className="min-w-0 space-y-3 text-sm">
{state.authorizationUrl ? (
<div className="space-y-1.5">
<div className="space-y-1.5">
<div className="flex items-center justify-between gap-2">
<p className="text-xs font-medium text-muted-foreground">Authorization URL</p>
<Button
type="button"
size="sm"
variant="ghost"
className="h-7 cursor-pointer gap-1 px-2 text-xs disabled:cursor-not-allowed"
disabled={browserRefreshInProgress}
onClick={handleRefreshBrowserLink}
>
{browserRefreshInProgress ? (
<>
<Loader2 className="h-3 w-3 animate-spin" />
Refreshing...
</>
) : (
<>
<RefreshCw className="h-3 w-3" />
Refresh link
</>
)}
</Button>
</div>
{browserRefreshInProgress ? (
<div className="flex items-center gap-2 rounded-lg border bg-muted/20 px-3 py-2 text-xs text-muted-foreground">
<Loader2 className="h-3.5 w-3.5 animate-spin" />
<span>Generating a fresh sign-in link...</span>
</div>
) : state.authorizationUrl ? (
<div className="flex min-w-0 items-center gap-2 rounded-lg border bg-muted/20 px-3 py-2">
<p className="min-w-0 flex-1 truncate font-mono text-xs">{state.authorizationUrl}</p>
<CopyButton text={state.authorizationUrl} />
</div>
</div>
) : null}
<ManualCallbackInput onSubmit={onManualCallback} />
) : null}
<p className="text-xs text-muted-foreground">
Refresh the link if the current sign-in page has already been used.
</p>
</div>
<ManualCallbackInput onSubmit={onManualCallback} disabled={browserRefreshInProgress} />
<div className="flex items-center gap-2 text-xs text-muted-foreground">
<Loader2 className="h-3.5 w-3.5 animate-spin" />
<span>Waiting for authorization to complete...</span>
Expand Down Expand Up @@ -281,22 +325,41 @@ export function OauthDialog({
<DialogFooter>
{stage === "intro" ? (
<>
<Button type="button" variant="outline" onClick={() => close(false)}>
<Button
type="button"
variant="outline"
className="cursor-pointer disabled:cursor-not-allowed"
onClick={() => close(false)}
>
Cancel
</Button>
<Button type="button" onClick={handleStart}>
<Button
type="button"
className="cursor-pointer disabled:cursor-not-allowed"
onClick={handleStart}
>
Start sign-in
</Button>
</>
) : null}

{stage === "browser" ? (
<>
<Button type="button" variant="outline" onClick={handleChangeMethod}>
<Button
type="button"
variant="outline"
className="cursor-pointer disabled:cursor-not-allowed"
disabled={browserRefreshInProgress}
onClick={handleChangeMethod}
>
Change method
</Button>
{state.authorizationUrl ? (
<Button type="button" asChild>
{state.authorizationUrl && !browserRefreshInProgress ? (
<Button
type="button"
className="cursor-pointer disabled:cursor-not-allowed"
asChild
>
<a href={state.authorizationUrl} target="_blank" rel="noreferrer">
<ExternalLink className="mr-1.5 h-3.5 w-3.5" />
Open sign-in page
Expand All @@ -308,11 +371,20 @@ export function OauthDialog({

{stage === "device" ? (
<>
<Button type="button" variant="outline" onClick={handleChangeMethod}>
<Button
type="button"
variant="outline"
className="cursor-pointer disabled:cursor-not-allowed"
onClick={handleChangeMethod}
>
Change method
</Button>
{state.verificationUrl ? (
<Button type="button" asChild>
<Button
type="button"
className="cursor-pointer disabled:cursor-not-allowed"
asChild
>
<a href={state.verificationUrl} target="_blank" rel="noreferrer">
<ExternalLink className="mr-1.5 h-3.5 w-3.5" />
Open link
Expand All @@ -323,17 +395,30 @@ export function OauthDialog({
) : null}

{stage === "success" ? (
<Button type="button" onClick={() => close(false)}>
<Button
type="button"
className="cursor-pointer disabled:cursor-not-allowed"
onClick={() => close(false)}
>
Done
</Button>
) : null}

{stage === "error" ? (
<>
<Button type="button" variant="outline" onClick={handleChangeMethod}>
<Button
type="button"
variant="outline"
className="cursor-pointer disabled:cursor-not-allowed"
onClick={handleChangeMethod}
>
Try again
</Button>
<Button type="button" onClick={() => close(false)}>
<Button
type="button"
className="cursor-pointer disabled:cursor-not-allowed"
onClick={() => close(false)}
>
Close
</Button>
</>
Expand Down
12 changes: 12 additions & 0 deletions openspec/changes/refresh-browser-oauth-link/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
## Why
The Accounts page OAuth dialog currently generates a browser PKCE authorization link only when the user first clicks `Start sign-in`. Because the link is single-use, operators who want to sign in multiple accounts in sequence must leave the current browser step and start over to mint another link. That adds unnecessary friction to a common dashboard workflow.

## What Changes
- Add an explicit refresh action to the browser PKCE stage of the Accounts OAuth dialog.
- Reuse the existing browser OAuth start flow so refreshing creates a fresh authorization URL without forcing the user to leave the dialog.
- Cover the refreshed-link behavior with frontend tests.

## Impact
- Affects the Accounts page OAuth dialog in the frontend.
- Reuses the existing `/api/oauth/start` browser flow and its PKCE/state regeneration.
- No API contract changes are required.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
## MODIFIED Requirements

### Requirement: Accounts page

The Accounts page SHALL display a two-column layout: left panel with searchable account list, import button, and add account button; right panel with selected account details including usage, token info, and actions (pause/resume/delete/re-authenticate).

#### Scenario: Account selection

- **WHEN** a user clicks an account in the list
- **THEN** the right panel shows the selected account's details

#### Scenario: Account import

- **WHEN** a user clicks the import button and uploads an auth.json file
- **THEN** the app calls `POST /api/accounts/import` and refreshes the account list on success

#### Scenario: Ambiguous duplicate identity import conflict

- **WHEN** `importWithoutOverwrite` was previously enabled and duplicate accounts with the same email exist
- **AND** overwrite mode is enabled again
- **AND** a new import matches multiple existing accounts by email without an exact ID match
- **THEN** `POST /api/accounts/import` returns `409` with `error.code=duplicate_identity_conflict`
- **AND** no existing account is modified

#### Scenario: OAuth add account

- **WHEN** a user clicks the add account button
- **THEN** an OAuth dialog opens with browser and device code flow options

#### Scenario: Browser OAuth link refresh

- **WHEN** a user is on the browser PKCE step of the OAuth dialog
- **AND** the current authorization URL has already been used or needs to be replaced
- **THEN** the dialog offers a refresh action that starts the browser OAuth flow again without leaving the dialog
- **AND** the dialog updates to the newly generated authorization URL

#### Scenario: Account actions

- **WHEN** a user clicks pause/resume/delete on an account
- **THEN** the corresponding API is called and the account list is refreshed
Loading