nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
24.35k stars 3.41k forks source link

Prisma throws on attempted to delete record that does not exist but next-auth core does not expect that #4495

Closed cymen closed 2 years ago

cymen commented 2 years ago

Adapter type

@next-auth/prisma-adapter

Environment

System: OS: macOS 12.3.1 CPU: (8) x64 Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz Memory: 81.57 MB / 32.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 16.14.2 - ~/Library/Caches/fnm_multishells/75640_1651423362851/bin/node Yarn: 1.22.18 - ~/Library/Caches/fnm_multishells/75640_1651423362851/bin/yarn npm: 8.5.0 - ~/Library/Caches/fnm_multishells/75640_1651423362851/bin/npm Browsers: Chrome: 100.0.4896.127 Firefox: 99.0 Safari: 15.4 npmPackages: next: ^12 => 12.1.5 next-auth: ^4.1.2 => 4.3.3 react: 18 => 18.0.0

npmPackages: @next-auth/prisma-adapter: ^1.0.1 => 1.0.3

Reproduction URL

n/a

Describe the issue

Prisma throws an exception if delete is called for a record which does not exist. The adapter does not handle that exception so it breaks the login. Here is where the session is being deleted in the adapter:

https://github.com/nextauthjs/next-auth/blob/main/packages/adapter-prisma/src/index.ts#L33-L34

Prisma has some surprising/rough edges and this is one of them -- ongoing issue about it here:

https://github.com/prisma/prisma/issues/4072

How to reproduce

Logs:

https://next-auth.js.org/errors#callback_email_error An operation failed because it depends on one or more records that were required but not found. Record to delete does not exist. Error: An operation failed because it depends on one or more records that were required but not found. Record to delete does not exist.
    at Object.request (/var/sites/fasmussen/node_modules/@prisma/client/runtime/index.js:45578:15)
    at async PrismaClient._request (/var/sites/fasmussen/node_modules/@prisma/client/runtime/index.js:46405:18) {
  name: 'DeleteSessionError',
  code: 'P2025'
}

Expected behavior

The Prisma adapter should factor in the odd Prisma API that throws on attempting to delete a record that does not exist. I worked around it for now by changing:

https://github.com/nextauthjs/next-auth/blob/main/packages/adapter-prisma/src/index.ts#L33-L34

    deleteSession: (sessionToken) =>
      p.session.delete({ where: { sessionToken } }),

To:

    deleteSession: (sessionToken) => {
      try {
        return p.session.delete({ where: { sessionToken } });
      } catch (e) {
        console.error("Failed to delete session", e);
        return null;
      }
    },
cymen commented 2 years ago

It's difficult to create a recreation of this because it relies on the state on the browser side to get the non-existent session token id. Happy to try to provide a recreation just not sure about that aspect of it.

ThangHuuVu commented 2 years ago

I wonder if this is the only problematic method. What about deleteUser, updateSession, etc...? 🤔

balazsorban44 commented 2 years ago

This is factored in in useVerificationToken, because the user might click the same link multiple times, but why would we not expect to throw an error in case of manually manipulating the accounts? There is a deleteUser method (given it's not implemented in core so you should write code to use it in your app) that should properly clean up any user, together with their accounts and sessions, thanks to cascading deletes: https://next-auth.js.org/adapters/prisma#create-the-prisma-schema

cymen commented 2 years ago

@balazsorban44 There could be other reasons to want to remove sessions while not deleting the user. This is the problem I ran into. I'm using my own Prisma adapter however Prisma escalating some actions to be exceptions is surprising and the Nextjs adapter not handling those exceptions is surprising.

Yes, you can argue there are other ways to handle removing the sessions and .... But the fact is you don't find out that the basic approach fails until those users whose sessions were removed can't login is painful. The whole experience here is not developer friendly.

So I'd suggest you reconsider but at this point I'm using my forked adapter.

cymen commented 2 years ago

To add one more example, say my database crashes and I need to restore from backup. If I do that and the code is as it is now, every single user that logged in before my restore time will not be able to login to my site/service because the session is not present in the database.

This behavior is broken.

jacquesh82 commented 1 year ago

I've the same problem but only with mondodb (not psql)

I'll fix it like this : deleteSession: (sessionToken) => p.session.deleteMany({ where: { sessionToken } }),

do you plan to fix it in next release ?

Regards

benevbright commented 8 months ago

should be reopened.

jeffrey-lunaon commented 6 months ago

Agreed, seeing that the latest code has not fixed this problem at present, I directly inherited the Adapter Class in the business code and replaced the deleteSession function to fix it

import { PrismaAdapter } from "@auth/prisma-adapter";
import { PrismaClient } from "@prisma/client";
import { Adapter } from "next-auth/adapters";

export function CustomPrismaAdapter(p: PrismaClient): Adapter {
  let origin = PrismaAdapter(p);
  return {
    ...origin,
    // fix: Record to delete does not exist. https://github.com/nextauthjs/next-auth/issues/4495
    deleteSession: async (sessionToken: any) => {
      try {
        return await p.session.delete({ where: { sessionToken } });
      } catch (e) {
        console.error("Failed to delete session", e);
        return null;
      }
    },
  } as unknown as Adapter;
}
alejandrobalderas commented 4 months ago

Agreed, seeing that the latest code has not fixed this problem at present, I directly inherited the Adapter Class in the business code and replaced the deleteSession function to fix it

import { PrismaAdapter } from "@auth/prisma-adapter";
import { PrismaClient } from "@prisma/client";
import { Adapter } from "next-auth/adapters";

export function CustomPrismaAdapter(p: PrismaClient): Adapter {
  let origin = PrismaAdapter(p);
  return {
    ...origin,
    // fix: Record to delete does not exist. https://github.com/nextauthjs/next-auth/issues/4495
    deleteSession: async (sessionToken: any) => {
      try {
        return await p.session.delete({ where: { sessionToken } });
      } catch (e) {
        console.error("Failed to delete session", e);
        return null;
      }
    },
  } as unknown as Adapter;
}

How is this not a thing already built into the PrismaAdapter?

I was building a functionality for my app where I only allow one sign in per account. I deleted the previous session if you try to sign in again so that users can not share their accounts but without this, the authentication flow was completely broken.

ndamulelonemakh commented 1 month ago

Still a problem in 2024 for Magic Link sign in with Prisma And AuthJs 5.0.0-beta.20. But thanks to @jeffrey-lunaon 's solution, I was able to move past it

magoz commented 1 week ago

Same issue here with Resend. This needs to be reopened @balazsorban44

balazsorban44 commented 1 week ago

Care to open a PR?