lucia-auth / lucia

Authentication, simple and clean
https://lucia-auth.com
MIT License
9.19k stars 467 forks source link

[Bug]: custom sessionId does not work with Drizzle adapter #1660

Open BryanBerger98 opened 1 month ago

BryanBerger98 commented 1 month ago

Package

​@lucia-auth/session-drizzle

Describe the bug

Hi!

When I create a new session with lucia.createSession for a user who already has an existing session, a duplication key error is thrown by the database:

{
  severity: 'ERROR',
  code: '23505',
  detail: 'Key (id)=(1) already exists.',
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: 'public',
  table: 'session',
  column: undefined,
  dataType: undefined,
  constraint: 'session_pkey',
  file: 'nbtinsert.c',
  line: '666',
  routine: '_bt_check_unique',
  sourceError: undefined
}

For information, the userId used here is the integer 1. It seems that the userId is always used by default to set the sessionId. The sessionId is not auto incremented.

To fix this, I wanted to set a custom sessionId this way:

const randomId = generateRandomString(64, alphabet('a-z', 'A-Z', '0-9'));
const session = await lucia.createSession(user.id, user, { sessionId: randomId });

But it does not work. It seems that lucia.createSession does not care about my custom sessionId. It still insert a 1 as the sessionId.

If I try to do it with an other user, the same thing happen. For example, I try with a user with an id 2. So the sessionId was using the userId and the session was inserted with the id 2. If I try to create another session for the same user, the same error will be thrown.

So finaly I tried to do it manually, by skipping the usage of lucia.createSession and directly using the DB client:

const [ session ] = await db
    .insert(sessionSchema)
    .values({
        id: randomId,
        userId: user.id,
        expiresAt: createDate(new TimeSpan(30, 'd')),
    })
    .returning(getTableColumns(sessionSchema));

And it works ! So this way I am sure that the problem does not come from my table schema or any database configuration.

Maybe I could help finding the origin of the problem and fix it ?

Thank you in advance for your help !

pilcrowOnPaper commented 1 month ago

It's because you're passing user to createSession(). I'm not sure why you're passing it, but user.id is overriding the session ID

BryanBerger98 commented 1 month ago

Oh my god I'm so stupid! I thought I needed to add user attributes to the session object but it is useless actually. And I didn't know that the attributes, including an id key, will be overriding the session ID. Thank you for your help!

BryanBerger98 commented 1 month ago

I am thinking about this: Maybe we could modify the code of the Drizzle adapter to avoid the possibility of this overriding. I think that when we set a custom session ID, this custom ID should be used even if an id key exists inside the attributes. Otherwise, we can be confused as I was.

The following code can be a solution.

public async setSession(session: DatabaseSession): Promise<void> {
    await this.db.insert(this.sessionTable).values({
                ...session.attributes,
        id: session.id,
        userId: session.userId,
        expiresAt: session.expiresAt
    });
}

If you want to keep the possibility to set an id by the attributes argument, we can do it by modifying the createSession method inside the core.ts file of the lucia package, this way:

const sessionId = options?.sessionId ?? attributes.id ?? generateIdFromEntropySize(25);

This way, if a custom ID exists, it has priority over the id inside attributes.

Another option could be removing the options argument and use the attributes argument to set a custom ID.

What do you think about this idea ?

pilcrowOnPaper commented 1 month ago

Yeah I didn't close the issue :D

BryanBerger98 commented 1 month ago

Do you want me to contribute by proposing the solution I described above in a pull request ?