Skip to content

initial auth structure --wip-- - #106

Open
switz wants to merge 1 commit into
timber-migration-v1from
auth
Open

switz wants to merge 1 commit into
timber-migration-v1from
auth

Conversation

@switz

@switz switz commented Aug 26, 2026

Copy link
Copy Markdown
Member

No description provided.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7011f4b250

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/oidcLogin.ts
Comment on lines +20 to +21
// somewhere off-site based on a value we round-tripped through storage.
export function isRelativePath(path: string): boolean {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Validate return targets by resolved origin

A crafted return_to such as /\attacker.example passes this check because it starts with one forward slash, but browsers resolve that path against an HTTPS origin as https://attacker.example/. Since /auth/login accepts arbitrary query parameters and the callback later redirects to the stored value, an attacker can turn the completed sign-in flow into an open redirect; resolve the value against the Relisten origin and require the resulting origin to match, or reject backslashes.

Useful? React with 👍 / 👎.

Comment thread src/lib/refreshSession.ts
Comment on lines +26 to +30
const response = await ky
.post(`${ACCOUNTS_AUTH_URL}/connect/token`, {
body: new URLSearchParams({
grant_type: 'refresh_token',
refresh_token: tokens.refreshToken,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize refresh-token rotation

When two requests arrive within the refresh window—such as concurrent tabs or parallel navigations—both read the same cookie and submit the same rotating refresh token here. One rotation succeeds while the other is treated as reuse; the failure path then clears the cookie and may cause the backend to revoke the newly issued token family, producing intermittent forced sign-outs. Coalesce or otherwise serialize refreshes per session instead of independently rotating in every middleware request.

Useful? React with 👍 / 👎.

RELISTEN
</Link>
<SecondaryNavBar artistName={artistName} />
{ACCOUNTS_FEATURE_ENABLED && <AccountMenu session={session} />}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add account actions to mobile navigation

On viewports below the lg breakpoint, this account menu is inside the preceding max-lg:hidden container, while the mobile branch only renders the Re link and the existing popover Menu contains no account entry. Consequently, once accounts are enabled, mobile users have no visible way to sign in or sign out.

Useful? React with 👍 / 👎.

Comment on lines +12 to +13
const loginHref = (provider: LoginProvider) =>
`/auth/login?provider=${provider}&return_to=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the query string across login

usePathname() excludes the current query string, so signing in from URLs that use repository query state—such as playback source/t, browse slug, or Today month/day—returns the user to the path with all of that state discarded. Include the current search parameters in the encoded return_to value.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants