Account Switching

[ekzyis]

Closes #489

This PR implements account switching:

Implementation details

If a user wants to add an account, they are redirected to /login with multiAuth param set:

components/switch-account.js:

const AddAccount = () => {
  const router = useRouter()
  return (
    <div className='d-flex flex-column me-2 my-1 text-center'>
      <Image
        width='135' height='135' src='https://imgs.search.brave.com/t8qv-83e1m_kaajLJoJ0GNID5ch0WvBGmy7Pkyr4kQY/rs:fit:860:0:0/g:ce/aHR0cHM6Ly91cGxv/YWQud2lraW1lZGlh/Lm9yZy93aWtpcGVk/aWEvY29tbW9ucy84/Lzg5L1BvcnRyYWl0/X1BsYWNlaG9sZGVy/LnBuZw' style={{ cursor: 'pointer' }} onClick={() => {
          router.push({
            pathname: '/login',
            query: { callbackUrl: window.location.origin + router.asPath, multiAuth: true }
          })
        }}
      />
      <div className='fst-italic'>+ add account</div>
    </div>
  )
}

With this param, a login flow within a session can be initiated. This param is checked in the backend to only set cookies without actually switching the user (return token instead of return user):

if (multiAuth) {
  // we want to add a new account to 'switch accounts'
  const secret = process.env.NEXTAUTH_SECRET
  // default expiration for next-auth JWTs is in 1 month
  const expiresAt = datePivot(new Date(), { months: 1 })
  const cookieOptions = {
    path: '/',
    httpOnly: true,
    secure: true,
    sameSite: 'lax',
    expires: expiresAt
  }
  const userJWT = await encodeJWT({
    token: {
      id: user.id,
      name: user.name,
      email: user.email
    },
    secret
  })
  const me = await prisma.user.findUnique({ where: { id: token.id } })
  const tokenJWT = await encodeJWT({ token, secret })
  // NOTE: why can't I put this in a function with a for loop?!
  res.appendHeader('Set-Cookie', cookie.serialize(`multi_auth.${user.id}`, userJWT, cookieOptions))
  res.appendHeader('Set-Cookie', cookie.serialize(`multi_auth.${me.id}`, tokenJWT, cookieOptions))
  res.appendHeader('Set-Cookie',
    cookie.serialize('multi_auth',
      JSON.stringify([
        { id: user.id, name: user.name, photoId: user.photoId },
        { id: me.id, name: me.name, photoId: me.photoId }
      ]),
      { ...cookieOptions, httpOnly: false }))
  // don't switch accounts, we only want to add. switching is done in client via "pointer cookie"
  return token
}
return null

With these multi_auth.* cookies set, we can now show accounts to the user to which they can switch. The multi_auth
cookie is not HTTP only since we want to access it from JS. This shouldn't be a problem since this is only for reading which accounts can be switched to and render this view:

On click, another cookie multi_auth.user-id is created via JS. This cookie is read inside a middleware in the backend to replace the session cookie that will be used to determine the user in the backend:

middleware.js:

const multiAuthMiddleware = (request) => {
  // switch next-auth session cookie with multi_auth cookie if cookie pointer present
  const userId = request.cookies?.get('multi_auth.user-id')?.value
  const sessionCookieName = '__Secure-next-auth.session-token'
  const hasSession = request.cookies?.has(sessionCookieName)
  if (userId && hasSession) {
    const userJWT = request.cookies.get(`multi_auth.${userId}`)?.value
    if (userJWT) request.cookies.set(sessionCookieName, userJWT)
  }
  const response = NextResponse.next({ request })
  return response
}

Showcase

2023-11-19.02-25-56.mp4

TODO

• switching to anon
• UI to go from anon back to session
• add multiAuth param to all login methods
  • lightning auth
  • nostr auth
  • email auth (decided to remove this auth method, see Account Switching #644 (comment))
  • github auth (decided to remove this auth method, see Account Switching #644 (comment))
  • twitter auth (decided to remove this auth method, see Account Switching #644 (comment))
• testing
• replace hardcoded link for add account image
• handle case where account is added which doesn't exist yet (this will simply create a new account now but current account will never be updated if multiAuth is used)
• also refresh JWTs in multi_auth.* (?) (how?)
• think more about if using this "cookie pointer" approach is secure (found an article which also mentions "cookie pointers" - and I thought I invented that term, lol)
• middleware is now applied to all routes ... performance impact? (nah, i think it's okay)
• fix bug when looking at item and switching between anon and user ("fixed" in 0c8893c)
• on logout, only delete session token for current logged in account and move user to next available account

[ekzyis]

There is a bug where you can't switch between anon and a user if you're currently looking at an item:

https://files.ekzyis.com/public/sn/account_switching_item_bug.mp4

I'll probably need to rewrite the corresponding code which calls hooks conditionally (depending on me)

edit: "fixed" in 0c8893c

[ekzyis]

Current state: https://files.ekzyis.com/public/sn/account_switching_002.mp4

Missing (see PR description):

• JWT refresh: Figuring out if I can hook into whatever NextAuth calls to refresh the session token.
• Multi auth support for Github, Email, Twitter: Going to be hard since I can't really test (maybe github) and their flows are very different. But let's see
• some more testing
• replace hardcoded link for add account image

[ekzyis]

Mhh, okay, can't find how to access a response to set the required multi_auth.* cookies on successful authentication for the Github, Twitter and Email provider. Looking at the source also didn't help. I decided to just remove these providers from the available auth methods if multiAuth is set in 77887ae.

Same problem with refreshing JWTs. For some reason, the jwt callback only has access to request and response on login/signup.

Putting this out of draft since everything else works, so I think we can ship as is.

---

Update: Unfortunately, this doesn't improve anon UX a lot since you can't pay from your account balance yet. And the cookies are shared between tabs so if you switch in one tab, you also switch in the other tab (obviously). Switching to anon in one tab also currently breaks the other tab since the state is not properly updated in the other tab (educated guess). The other tab then thinks you're completely signed out.

Keeping this as ready for review since I think this is still ready for review (lol)

One upside is that with this feature, I can easily keep track of replies to @hn.

[ekzyis]

I hope this doesn't break anything but afaict, it doesn't. It just does unnecessary requests since anon has fixed fees. But removing this line is required to not run hooks conditionally. When switching to anon, this line changed the hook order and thus broke this rule:

Don’t call Hooks inside loops, conditions, or nested functions. Instead, always use Hooks at the top level of your React function, before any early returns. By following this rule, you ensure that Hooks are called in the same order each time a component renders. That’s what allows React to correctly preserve the state of Hooks between multiple useState and useEffect calls. (If you’re curious, we’ll explain this in depth below.)

[huumn]

This isn't a hook though. It's a function that returns a hook

[ekzyis]

It's a function that returns a hook

Yes but conditionally

[huumn]

User reported this hook error in prod on telegram. I'm not sure if it's related but I believe this is already conditional

[ekzyis]

but I believe this is already conditional

Yes, it is. That's what my change is trying to fix since the bug would manifest when switching to anon. The user report shows that the bug already manifests in some cases.

Example:

This is the full function:

export function postCommentUseRemoteLineItems ({ parentId, me } = {}) {
  if (!me) return () => {}
  const query = parentId
    ? gql`{ itemRepetition(parentId: "${parentId}") }`
    : gql`{ itemRepetition }`

  return function useRemoteLineItems () {
    const [line, setLine] = useState({})

    const { data } = useQuery(query, SSR ? {} : { pollInterval: 1000, nextFetchPolicy: 'cache-and-network' })

    useEffect(() => {
      const repetition = data?.itemRepetition
      if (!repetition) return setLine({})
      setLine({
        itemRepetition: {
          term: <>x 10<sup>{repetition}</sup></>,
          label: <>{repetition} {parentId ? 'repeat or self replies' : 'posts'} in 10m</>,
          modifier: (cost) => cost * Math.pow(10, repetition)
        }
      })
    }, [data?.itemRepetition])

    return line
  }
}

return function useRemoteLineItems () is a hook which is only returned if we don't already return () => {}. That's what I meant with "but conditionally":

It's a function that returns a hook

Yes but conditionally

So yes, this is conditional. That the user reported it means that the bug actually manifests already, too. Not only with my changes, assuming I wouldn't remove this line: if (!me) return () => {} (which what this conversation here is about)

[ekzyis]

Sorry for the detailed and thus maybe snarky answer, I feel like we've been talking past each other for several replies already now, lol :)

[ekzyis]

Reminder for myself: check if there is still an error (there probably is) if you're on /settings and then you switch to anon

[huumn]

Looks great!

I added some high level remarks. After we talk/work through those, I'll do another more detailed pass.

[ekzyis]

I'll fix the conflicts, give me a sec

[huumn]

No rush. This won't until I ship territories first

[ekzyis]

This is going to be great for my new puppet account @oracle 👀

[huumn]

For some reason this doesn't work for me:

1. can't switch to anon
2. when I go to add account only some providers are listed

This might need migration logic to be added for stackers that are already logged in if that wasn't tested.

broken.mov

[ekzyis]

For some reason this doesn't work for me:

1. can't switch to anon

Mhh, I had this error before. I think I fixed it by clearing all my session cookies since it's related to a (new) cookie being HTTP only when it should not have been set as HTTP only for some reason iirc. I think I also noticed that sometimes, cookies are not overridden but duplicated. This should only happen if the cookie path is different - it defaults to the current path if no path is specified (which should no longer be the case). I don't remember 100% though.

Can you show me your cookies when this bug happens to you?

2. when I go to add account only some providers are listed

I wasn't able to hook into the OAuth login process and Email auth (and was hard to test locally). So I thought - at least for the initial version - we can provide only Login with Lightning and Nostr if people want to use account switching.

This might need migration logic to be added for stackers that are already logged in if that wasn't tested.

Mhh, I didn't think too much about this but I think this should be 100% backwards compatible. But I will test (again).

[ekzyis]

Did a full test (afaict) again.

https://files.ekzyis.com/public/sn/account_switching_full_test.mp4

1. 00:00 - 00:15 -- Login sets multi_auth cookie which is a base64 encoded list of all available accounts and is accessible from JS (HTTP only set to false) and multi_auth.<userid> cookie with HTTP only set to true containing the JWT for account switching to this user.

2. 00:27 - 00:32 -- Switching to anon set the pointer cookie multi_auth.user-id to anonymous. This tells the backend to not use any JWT.

3. 00:33 - 00:56 -- adding a new account updates multi_auth cookie and adds a new multi_auth.<userid> cookie for that user.

4. 00:59 - 01:06 -- switching between accounts (anon or not anon)

5. 01:07 - 01:13 -- logging out switches to next available account and deletes multi_auth.<userid> cookie of completely logs you out (same as before)

6. 01:26 - 02:28 -- testing backwards compatibility by simulating a user that logged in without account switching deployed and then uses account switching

7. 02:28 -- ok there is indeed a bug lol. new account does not show up in the list even though cookies seem to be set correctly at first glance.

[ekzyis]

Putting in draft until bug is fixed

Edit: mhh, can't find how to put in draft in the Github app

[ekzyis]

Sessions that existed before we deployed account switching should now also be able to use account switching immediately. We now sync the cookie from their session if there is no multi_auth cookie. See 6839066

2023-12-20.16-21-43.mp4

[huumn]

So this is out of draft now?

[ekzyis]

So this is out of draft now?

Going to fix conflicts now, just had a call with someone from Alby

btw, regarding this:

can't switch to anon

I noticed that sometimes, it just takes some time (max 5 seconds) for the switch to anon to complete. Not sure if it's because of dev mode or network latency or something else (bug). But yes, in your video, it didn't look like anything is going to happen, no matter how long you wait. Would be interesting to see if multi_auth.user-id is set to anonymous in that case.

update: testing currently again after fixing conflicts

[ekzyis]

Okay, found out why this doesn't work on localhost (big duh moment in hindsight, lol)

With 1845db2, I actually completely broke login on non-HTTPS sites since I am forcing the session cookie to have the Secure attribute set. Additionally, I am setting the__Secure- cookie prefix so the browser can't even read the cookie:

The __Secure- prefix makes a cookie accessible from HTTPS sites only. A HTTP site can not read or update a cookie if the name starts with __Secure-. This protects against the attack we earlier described, where an attacker uses a forged insecure site to overwrite a secure cookie.

-- https://www.sjoerdlangkemper.nl/2017/02/09/cookie-prefixes/

I am also setting Secure on all cookies required for multi authentication. So this explains a lot.

I am going to see how this could work on localhost / HTTP without introducing vulnerabilities. Going to check how Next-Auth does it.

update: Seems like all they do is to check if the protocol is http:https://. Then they don't use secure cookies:

This option defaults to false on URLs that start with http:https:// (e.g. http:https://localhost:3000) for developer convenience.

-- https://next-auth.js.org/configuration/options#usesecurecookies

[ekzyis]

Also works on HTTP now:

2024-01-07.00-43-55.mp4

[ekzyis]

Account switching could also be useful for local development

Need to rebase the code and see if I can remove the code in the middleware and replace it with code similar to #915