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

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

Open
BryanBerger98 opened this issue Aug 14, 2024 · 5 comments
Open

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

BryanBerger98 opened this issue Aug 14, 2024 · 5 comments
Labels
bug Something ain't right...

Comments

@BryanBerger98
Copy link

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 !

@BryanBerger98 BryanBerger98 added the bug Something ain't right... label Aug 14, 2024
@pilcrowonpaper
Copy link
Member

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
Copy link
Author

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
Copy link
Author

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 ?

@BryanBerger98 BryanBerger98 reopened this Aug 15, 2024
@pilcrowonpaper
Copy link
Member

Yeah I didn't close the issue :D

@BryanBerger98
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something ain't right...
Projects
None yet
Development

No branches or pull requests

2 participants