Skip to content

Comments

fix: preserve page context via cookie after Bluesky redirect#930

Merged
danielroe merged 12 commits intonpmx-dev:mainfrom
NandkishorJadoun:fix/preserve-current-page-context
Feb 5, 2026
Merged

fix: preserve page context via cookie after Bluesky redirect#930
danielroe merged 12 commits intonpmx-dev:mainfrom
NandkishorJadoun:fix/preserve-current-page-context

Conversation

@NandkishorJadoun
Copy link
Contributor

fixes #901

Previously, users logging in via Bluesky were redirected to the homepage regardless of which page they started from.

@vercel
Copy link

vercel bot commented Feb 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 5, 2026 1:38pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 5, 2026 1:38pm
npmx-lunaria Ignored Ignored Feb 5, 2026 1:38pm

Request Review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

Client components now capture the current route via useRoute() and pass redirectTo (route.fullPath) when initiating Bluesky sign‑in or create‑account flows. authRedirect was changed to accept an AuthRedirectOptions object (supports create and redirectTo) and builds the OAuth query accordingly. The server endpoint validates a returnTo query against the client URI, stores a safe relative path in an auth_return_to cookie (maxAge 300s, httpOnly, secure toggled for development), and reads/deletes that cookie after callback to perform the final redirect.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description directly references the linked issue #901 and explains the bug fix being implemented.
Linked Issues check ✅ Passed The code changes successfully preserve page context via cookie during Bluesky OAuth redirect, meeting the issue #901 requirement to return users to their original page after authentication.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the page context preservation feature for Bluesky authentication as specified in issue #901.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/components/Header/AuthModal.client.vue (1)

26-36: ⚠️ Potential issue | 🟠 Major

Missing returnTo parameter in handleLogin creates inconsistent redirect behaviour.

The handleLogin() function for manual handle input does not include the returnTo query parameter, whilst handleBlueskySignIn() and handleCreateAccount() do. Users logging in with a custom handle will be redirected to the homepage instead of their original page.

🐛 Proposed fix to add returnTo parameter
 async function handleLogin() {
   if (handleInput.value) {
     await navigateTo(
       {
         path: '/api/auth/atproto',
-        query: { handle: handleInput.value },
+        query: { handle: handleInput.value, returnTo: route.fullPath },
       },
       { external: true },
     )
   }
 }

@danielroe
Copy link
Member

would you mind resolving the conflicts? 🙏

/* Individual symbol articles */
.docs-content .docs-symbol {
@apply mb-10 pb-10 border-b border-border/30 last:border-0;
word-break: break-word;
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it have to do with the described fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was working on another issue and forgot to switch branches. Please ignore that commit for now

@NandkishorJadoun
Copy link
Contributor Author

yeah, working on it

Copy link
Contributor

@wojtekmaj wojtekmaj left a comment

Choose a reason for hiding this comment

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

Listen to the rabbit 🐰

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@wojtekmaj wojtekmaj left a comment

Choose a reason for hiding this comment

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

Aside from the style change that sneaked in, LGTM.

@codecov
Copy link

codecov bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/utils/atproto/helpers.ts 0.00% 2 Missing and 2 partials ⚠️
app/components/Header/AuthModal.client.vue 33.33% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/components/Header/AuthModal.client.vue (1)

29-33: ⚠️ Potential issue | 🟠 Major

Custom handle login does not preserve page context.

The handleLogin function uses authRedirect(handleInput.value) without passing the returnTo parameter. Users who enter a custom handle will be redirected to the homepage after authentication, inconsistent with the other two login flows (handleBlueskySignIn and handleCreateAccount) which both pass returnTo: route.fullPath.

The authRedirect function currently accepts only identifier and optional create parameters, and does not include returnTo in its query object. Update the function signature to accept an optional returnTo parameter:

-export async function authRedirect(identifier: string, create: boolean = false) {
+export async function authRedirect(identifier: string, create: boolean = false, returnTo?: string) {
   let query: LocationQueryRaw = { handle: identifier }
   if (create) {
     query = { ...query, create: 'true' }
   }
+  if (returnTo) {
+    query = { ...query, returnTo }
+  }
   await navigateTo(
     {
       path: '/api/auth/atproto',
       query,
     },
     { external: true },
   )
 }

Then call it with the return path in handleLogin:

 async function handleLogin() {
   if (handleInput.value) {
-    await authRedirect(handleInput.value)
+    await authRedirect(handleInput.value, false, route.fullPath)
   }
 }

Copy link
Contributor

@fatfingers23 fatfingers23 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for adding it! Was a big ask we got

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/components/Header/AuthModal.client.vue (1)

17-21: ⚠️ Potential issue | 🟡 Minor

Missing redirectTo in manual handle login flow.

The handleLogin function does not pass redirectTo: route.fullPath, unlike the other authentication flows. Users who log in by entering their handle manually will still be redirected to the homepage instead of their original page, which is inconsistent with the PR objective.

Proposed fix
 async function handleLogin() {
   if (handleInput.value) {
-    await authRedirect(handleInput.value)
+    await authRedirect(handleInput.value, { redirectTo: route.fullPath })
   }
 }

@danielroe danielroe added this pull request to the merge queue Feb 5, 2026
Merged via the queue into npmx-dev:main with commit 796e330 Feb 5, 2026
15 of 16 checks passed
@NandkishorJadoun NandkishorJadoun deleted the fix/preserve-current-page-context branch February 10, 2026 04:00
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.

[Bug] Login via Atmosphere account redirects to Homepage instead of preserving current page context

4 participants