nextauthjs / next-auth

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

Outdated Dgraph adapter: encoding, id overlap `DgraphClientError: unknown field` and edge runtime. #11273

Open adriangalilea opened 2 months ago

adriangalilea commented 2 months ago

Adapter type

@auth/dgraph-adapter

Environment

  System:
    OS: macOS 14.4.1
    CPU: (10) arm64 Apple M2 Pro
    Memory: 461.52 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.1.0 - /opt/homebrew/bin/node
    npm: 10.8.1 - /opt/homebrew/bin/npm
    pnpm: 8.6.7 - /opt/homebrew/bin/pnpm
    bun: 1.1.7 - /opt/homebrew/bin/bun
  Browsers:
    Brave Browser: 123.1.64.122
    Chrome: 126.0.6478.116
    Safari: 17.4.1
  npmPackages:
    @auth/dgraph-adapter: ^2.4.1 => 2.4.1 
    next: 14.1.4 => 14.1.4 
    next-auth: ^5.0.0-beta.19 => 5.0.0-beta.19 
    react: ^18.2.0 => 18.3.1 

Reproduction URL

https://github.com/adriangalilea/next-auth-example

Describe the issue

There are several issues with the Dgraph adapter:

1. JWT session and @auth directive

update

Fixed it and pushed it on the pull request.

original

Dgraph only works with HS256 or RS256 algorithms.

This is no longer true.

So this area of the docs can be removed for simplicity, once tested.

2. DgraphClientError: unknown field

update 2

Dgraph has

The @id directive is used for external identifiers, such as email addresses.

So the right solution may be doing this, I never quite understood why next-auth insist in handling id's itself.

However I'll continue overriding it and respecting Dgraph internal id's as I suspect it'd be better for performance.

update

Patched the Dgraph adapter so that it respects dgraph internal id.

repo branch

original

I spent way too many hours debugging this until I realised that it's the fact that Dgraph doesn't accept external id's.

Currently there is an open PR

This error completely prevents the use of the Dgraph adapter.

3. The edge runtime does not support Node.js 'crypto' module.

update:

I managed to fix the edge runtime issue by following Guides/Edge Compatibility

Fix can be seen here

original message:

 ⨯ Error: The edge runtime does not support Node.js 'crypto' module.
Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime
    at <unknown> (webpack-internal:///(middleware)/./node_modules/next/dist/esm/server/web/globals.js:33)
    at Object.get (webpack-internal:///(middleware)/./node_modules/next/dist/esm/server/web/globals.js:33:19)
    at module.exports [as sign] (webpack-internal:///(middleware)/./node_modules/jsonwebtoken/sign.js:115:58)
    at client (webpack-internal:///(middleware)/./node_modules/@auth/dgraph-adapter/lib/client.js:28:73)
    at DgraphAdapter (webpack-internal:///(middleware)/./node_modules/@auth/dgraph-adapter/index.js:29:69)
    at eval (webpack-internal:///(middleware)/./auth.ts:39:81)
    at (middleware)/./auth.ts (....next/server/middleware.js:84:1)
    at __webpack_require__ (..../.next/server/edge-runtime-webpack.js:37:33)
    at fn (.../.next/server/edge-runtime-webpack.js:285:21)
    at eval (webpack-internal:///(middleware)/./middleware.ts:7:63)
    at (middleware)/./middleware.ts (.../.next/server/middleware.js:95:1) {
  middleware: true
}

I haven't identified specifically what part of the adapter is causing this, but it's def not up to date to be ran on the edge, thus can't check if user is logged in on the middleware, I know there's been some debate around edge runtime and auth, but I think it should be consistent with the rest of auth.js regardless.

How to reproduce

  1. Install Dgraph adapter
  2. Nothing works.

Expected behavior

Working adapter.

adriangalilea commented 2 months ago

Updated 3. as I found the solution following the docs and seems not related to Dgraph adapter.

adriangalilea commented 2 months ago

Updated 2. as I managed to patch the dgraph-adapter so that it doesn't use id on user

repo

adriangalilea commented 2 months ago

Update 2 on 2.

Dgraph has introduces since this adapter:

The @id directive is used for external identifiers, such as email addresses.

So the right solution may be doing this, I never quite understood why next-auth insist in using his own id's rather than allow internal db id's.

Seems to be intentional and never answered by @balazsorban44

adriangalilea commented 2 months ago

Also updated JWT to HS512, removed the section of the docs, documented the code and respect backwards compatibility.