Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handling when session exists but user doesn't exist in db #507

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

caprolactam
Copy link
Contributor

change handling from authenticator.logout() to logout().

Why
This process is intended to remove invalid session about userId on both cookie and db sides, when userId which get from session doesn't exists in db.
We manage userId in authSessionStorage now. On the other hand, authenticator handle connection, which is stored in connectionSessionStorage. Therefore, even after authenticator.logout(), the information about the userId remains. This can cause infinite redirect. I think this is left over from the process when remix-auth and remix-auth-form were used for authentication.
Replace authenticator.logout() with logout(), but if necessary, I will add the process to destory connectionSession manually.
For example:

import { connectionSessionStorage } from '#app/utils/connections.server.ts'
// in action code.
const connectioSession = await connectionSessionStorage.getSession(
  request.headers.get('cookie'),
)
const responseInit = {
  headers: { 'set-cookie': connectionSessionStorage.destroySession(connectioSession) }
}
await logout({ request }, responseInit)

Test Plan

Checklist

  • Tests updated
  • Docs updated

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Wow! Thank you for finding this and fixing it!

I think your analysis is correct. This is left over code. I'm glad you've fixed it.

@kentcdodds kentcdodds merged commit c88db93 into epicweb-dev:main Oct 31, 2023
5 checks passed
@caprolactam caprolactam deleted the pr/handle-invalid-session branch October 31, 2023 12:21
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