nextauthjs / next-auth

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

[TypeORM] Typings for TypeORM options and add changed casing to migration docs #3826

Closed xndyz closed 2 years ago

xndyz commented 3 years ago

In V3 next-auth was using snake-case for database columns, but now the typeorm legacy next adapter references camelcase version. This is not mentioned in the migration docs, which could break some projects.

xndyz commented 3 years ago

After doing some digging it looks like the default is supposed to be snake case?

https://github.com/nextauthjs/adapters/blob/1f980baaa3dec4cf5f047bb19ace8406d86e32b5/packages/typeorm-legacy/src/lib/transform.js#L10

But for some reason, the Google provider is giving me this error:

[next-auth][error][OAUTH_CALLBACK_HANDLER_ERROR] 
https://next-auth.js.org/errors#oauth_callback_handler_error column AccountEntity.userId does not exist QueryFailedError: column AccountEntity.userId does not exist
    at QueryFailedError.TypeORMError [as constructor] (/Users/fang/Projects/mammal-app/node_modules/typeorm/error/TypeORMError.js:9:28)
    at new QueryFailedError (/Users/fang/Projects/mammal-app/node_modules/typeorm/error/QueryFailedError.js:13:28)
    at PostgresQueryRunner.<anonymous> (/Users/fang/Projects/mammal-app/node_modules/typeorm/driver/postgres/PostgresQueryRunner.js:277:31)
    at step (/Users/fang/Projects/mammal-app/node_modules/typeorm/node_modules/tslib/tslib.js:143:27)
    at Object.throw (/Users/fang/Projects/mammal-app/node_modules/typeorm/node_modules/tslib/tslib.js:124:57)
    at rejected (/Users/fang/Projects/mammal-app/node_modules/typeorm/node_modules/tslib/tslib.js:115:69)
    at processTicksAndRejections (internal/process/task_queues.js:95:5) {
  name: 'GetUserByAccountError'
}

Which makes me think it's either a bug in the adapter, or in the Google provider in next-auth@next.

xndyz commented 3 years ago

Looking at the docs maybe this is intentional to have inconsistent naming?

https://next-auth.js.org/adapters/models image

Would be great to have a SQL schema gen code like with V3 in the docs so it's more clear.

xndyz commented 3 years ago

Ah, upon further inspection it looks like you can actually customize the schema by passing in entities as a second argument to the adapter.

balazsorban44 commented 3 years ago

the casing has changed in such a way so that we can easily forward certain things as is to the adapter. For example the shape of Account is determined partially by our new underlying OAuth library, which uses snake case for certain fields, following the naming in the OAuth spec.

In my opinion the v3 adapters were trying to be too much, and they were overly complicated. We reduced their size significantly to make it also easier to people who want to have a look at their source code.

As you mentioned, we still give you the option to tap into naming/extra fields through entities which seems to be the default recommended way in TypeORM anyway.

We were discussing this with @ndom91, and we haven't seen a compelling enough reason to enforce a consistent casing, given the extra maintenance overhead/complexity.

For example the typeorm adapter went from 11 source files to 3, where the main file - containing the necessary TS types as well - went to 187 LOC from 320. Even with potentially used functionality, I'm confident that anyone with custom requirements could easily create their own custom adapter instead, if necessary. I believe most of users won't really care about naming or miss any of the previously existing/now gone features anyway.

balazsorban44 commented 3 years ago

@ndom91 we should probably mention the schema changes in the adapter section of the migration docs

https://next-auth.js.org/getting-started/upgrade-v4#adapters

UPDATE:

Taking some of the advise, I created this PR https://github.com/nextauthjs/docs/pull/31

xndyz commented 3 years ago

@balazsorban44 on the topic of schema documentation, I noticed that next-auth enforces certain naming conventions like plural for database names. Ideally there should be a way to rename this (with the schema definitions, or adapter options). Right now the onboarding's a pain because you have to find this out yourself by breaking the auth flow.

balazsorban44 commented 3 years ago

You can rename database names:

https://next-auth.js.org/adapters/typeorm#custom-models

TypeORM is puzzling for me as well, but the way it works is that it uses Entities https://typeorm.io/#/entities

It uses experimental JS features (decorators) through which you can add more info about the Entity.

To choose the name of the table, you pass it as the name option to the @Entity. Honestly, I hate TypeORM, because it feels very complicated, especially for someone who hasn't worked with databases before. That's why I hope we can get rid of TypeORM in its entirety, just create specialized adapters for single flavors of SQL. This requires more energy than the migration we have done on TypeORM at the moment though, so it has to wait unless someone from the community steps in to help.

xndyz commented 3 years ago

You can rename database names:

https://next-auth.js.org/adapters/typeorm#custom-models

TypeORM is puzzling for me as well, but the way it works is that it uses Entities https://typeorm.io/#/entities

It uses experimental JS features (decorators) through which you can add more info about the Entity.

To choose the name of the table, you pass it as the name option to the @Entity. Honestly, I hate TypeORM, because it feels very complicated, especially for someone who hasn't worked with databases before. That's why I hope we can get rid of TypeORM in its entirety, just create specialised adapters for single flavours of SQL. This requires more energy than the migration we have done on TypeORM at the moment though.

Ah, I assumed you wouldn't be able to rename tables since you can't rename columns. And I agree with TypeORM being convoluted. Personally I'm only using it because it seems easier to get started with Postgres, but will play around with the Prisma adapter to see if it's a simpler alternative. In the docs it does appear to be better supported.

The Prisma schema example doesn't show how to rename tables ~ is this possible?

balazsorban44 commented 3 years ago

I :heart: Prisma, it is so much easier! I think you could, but we tried to minimize the API surface and renaming tables seemed like not something many would do, so there was no real focus on this kind of customizability.

The Prisma adapter is only 47 lines in its entirety, so even if something isn't possible, it takes almost no effort to just copy and customize it: https://github.com/nextauthjs/adapters/blob/next/packages/prisma/src/index.ts

Prisma generates its TypeScript from the schema and we kind of "assume" tables to have a certain name, but I think something like this would work:

const prisma = new PrismaClient()

export default NextAuth({
  adapter: PrismaAdapter({
    user: prisma.users,
    account: prisma.accounts,
    session: prisma.sessions,
    verificationToken: prisma.verificationTokens
  }),
  ...
})

So basically you remap tables to the expected name, and I ASSUME it would work. I haven't tested it though.

jcaryk commented 3 years ago

has anyone found a way around this? It seems like the TypeORMLegacyAdapter is bugged in the beta version. I tried multiple providers (google and github) both with custom entities and the default entities (using the default postgres schema), and I always get the same error you listed above. I tried this by doing npx create-next-app and then installing next-auth according to the docs, so it should be reproducible with a clean instance.

[next-auth][error][OAUTH_CALLBACK_HANDLER_ERROR] 
https://next-auth.js.org/errors#oauth_callback_handler_error column AccountEntity.userId does not exist QueryFailedError: column AccountEntity.userId does not exist
    at QueryFailedError.TypeORMError [as constructor] (/Users/justincaryk/Documents/repos/next-auth-test/node_modules/typeorm/error/TypeORMError.js:9:28)
    at new QueryFailedError (/Users/justincaryk/Documents/repos/next-auth-test/node_modules/typeorm/error/QueryFailedError.js:13:28)
    at PostgresQueryRunner.<anonymous> (/Users/justincaryk/Documents/repos/next-auth-test/node_modules/typeorm/driver/postgres/PostgresQueryRunner.js:277:31)
    at step (/Users/justincaryk/Documents/repos/next-auth-test/node_modules/typeorm/node_modules/tslib/tslib.js:143:27)
    at Object.throw (/Users/justincaryk/Documents/repos/next-auth-test/node_modules/typeorm/node_modules/tslib/tslib.js:124:57)
    at rejected (/Users/justincaryk/Documents/repos/next-auth-test/node_modules/typeorm/node_modules/tslib/tslib.js:115:69)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  name: 'GetUserByAccountError'
}
ndom91 commented 3 years ago

@jcaryk so the default typeorm schema (https://next-auth.js.org/adapters/typeorm#custom-models) definitely have an accounts entity field called userId.

Are you sure you seeded your db or enabled the synchronize: true option with typeorm?

If so, can you paste in some examples of your code and/or provide a reproduction?

Thanks!

jcaryk commented 3 years ago

@ndom91 I didn't use the synchronize true flag because I'll need it to work in production and I already seeded the db.

[...next-auth].ts

import { TypeORMLegacyAdapter } from '@next-auth/typeorm-legacy-adapter'
export default NextAuth({
  jwt: {
    secret: graphile.jwtSecret,
  },
  secret: graphile.jwtSecret,
  session: {
    jwt: true,
  },
  providers: [
    GoogleProvider({
      clientId: process.env.GOOGLE_CLIENT_ID,
      clientSecret: process.env.GOOGLE_CLIENT_SECRET
    }),
    GitHubProvider({
      clientId: process.env.GITHUB_CLIENT_ID,
      clientSecret: process.env.GITHUB_CLIENT_SECRET
    }),
  ],
  adapter: TypeORMLegacyAdapter(process.env.DATABASE_URL)
})

I used this to seed the db...

CREATE TABLE accounts
  (
    id                   SERIAL,
    compound_id          VARCHAR(255) NOT NULL,
    user_id              INTEGER NOT NULL,
    provider_type        VARCHAR(255) NOT NULL,
    provider_id          VARCHAR(255) NOT NULL,
    provider_account_id  VARCHAR(255) NOT NULL,
    refresh_token        TEXT,
    access_token         TEXT,
    access_token_expires TIMESTAMPTZ,
    created_at           TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_at           TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
    PRIMARY KEY (id)
  );

CREATE TABLE sessions
  (
    id            SERIAL,
    user_id       INTEGER NOT NULL,
    expires       TIMESTAMPTZ NOT NULL,
    session_token VARCHAR(255) NOT NULL,
    access_token  VARCHAR(255) NOT NULL,
    created_at    TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_at    TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
    PRIMARY KEY (id)
  );

CREATE TABLE users
  (
    id             SERIAL,
    name           VARCHAR(255),
    email          VARCHAR(255),
    email_verified TIMESTAMPTZ,
    status         app_public.auth_person_status default 'applied',
    auth_level     app_public.auth_level default 'user',
    image          TEXT,
    created_at     TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_at     TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
    PRIMARY KEY (id)
  );

CREATE TABLE verification_requests
  (
    id         SERIAL,
    identifier VARCHAR(255) NOT NULL,
    token      VARCHAR(255) NOT NULL,
    expires    TIMESTAMPTZ NOT NULL,
    created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
    PRIMARY KEY (id)
  );

CREATE UNIQUE INDEX compound_id
  ON accounts(compound_id);

CREATE INDEX provider_account_id
  ON accounts(provider_account_id);

CREATE INDEX provider_id
  ON accounts(provider_id);

CREATE INDEX user_id
  ON accounts(user_id);

CREATE UNIQUE INDEX session_token
  ON sessions(session_token);

CREATE UNIQUE INDEX access_token
  ON sessions(access_token);

CREATE UNIQUE INDEX email
  ON users(email);

CREATE UNIQUE INDEX token
  ON verification_requests(token);

I also tried installing v3 and downgrading the code to be v3 compatible, which worked.

ndom91 commented 3 years ago

@jcaryk so it looks like you manually seeded the db using the schema from v3. This resulted in the field being called accounts.user_id instead of accounts.userId like v4 is looking for.

After doing some more research this is clearly a place where we need to improve in the documentation. We've moved to using native typeorm entities in this adapter, instead of manual schemas for each db type (mysql, postgres, etc.).

This means that the correct way to seed the db and setup the required tables and columns would be to either use the cli or run it with synchronise: true at least one time at first so that typeorm can translate the entities into correct tables and columns for your chosen DB type and create them.

The typeorm cli has a command called schema:sync which would apply the next-auth entity definitions to your db as well.

i.e.

./node_modules/typeorm/cli.js schema:sync
npx typeorm schema:sync
ndom91 commented 3 years ago

I'm going to try to work this into a more user friendly file / command / way of using. But here's a typeorm entity config file that can be used to apply the v4 schema to any typeorm supported db:

const EntitySchema = require('typeorm').EntitySchema

const entities = [
  new EntitySchema({
    name: "Account",
    columns: {
      id: {
        // This property has `objectId: true` instead of `type: int` in MongoDB
        primary: true,
        type: "int",
        generated: true,
      },
      compoundId: {
        // The compound ID ensures that there there is only one instance of an
        // OAuth account in a way that works across different databases.
        // It is not used for anything else.
        type: "varchar",
        unique: true,
      },
      userId: {
        // This property is set to `type: objectId` on MongoDB databases
        type: "int",
      },
      providerType: {
        type: "varchar",
      },
      providerId: {
        type: "varchar",
      },
      providerAccountId: {
        type: "varchar",
      },
      refreshToken: {
        type: "text",
        nullable: true,
      },
      accessToken: {
        // AccessTokens are not (yet) automatically rotated by NextAuth.js
        // You can update it using the refreshToken and the accessTokenUrl endpoint for the provider
        type: "text",
        nullable: true,
      },
      accessTokenExpires: {
        // AccessTokens expiry times are not (yet) updated by NextAuth.js
        // You can update it using the refreshToken and the accessTokenUrl endpoint for the provider
        type: "timestamp",
        nullable: true,
      },
      createdAt: {
        type: "timestamp",
        createDate: true,
      },
      updatedAt: {
        type: "timestamp",
        updateDate: true,
      },
    },
    indices: [
      {
        name: "userId",
        columns: ["userId"],
      },
      {
        name: "providerId",
        columns: ["providerId"],
      },
      {
        name: "providerAccountId",
        columns: ["providerAccountId"],
      },
    ],
  }),
  new EntitySchema({
    name: "User",
    columns: {
      id: {
        // This property has `objectId: true` instead of `type: int` in MongoDB
        primary: true,
        type: "int",
        generated: true,
      },
      name: {
        type: "varchar",
        nullable: true,
      },
      email: {
        // This is inherited from the one in the OAuth provider profile on
        // initial sign in, if one is specified in that profile.
        type: "varchar",
        unique: true,
        nullable: true,
      },
      emailVerified: {
        // Contains a timestamp of the last time an action was performed that
        // confirmed this email address was active and used by the user (e.g.
        // when an email sign in link is clicked on and verified). Is null
        // if the email address specified has never been verified.
        type: "timestamp",
        nullable: true,
      },
      image: {
        // A URL that points to an avatar to use for the user.
        // This is inherited from the one in the OAuth provider profile on
        // initial sign in, if one is specified in that profile.
        type: "varchar",
        nullable: true,
      },
      createdAt: {
        type: "timestamp",
        createDate: true,
      },
      updatedAt: {
        type: "timestamp",
        updateDate: true,
      },
    },
  }),
  new EntitySchema({
    name: "Session",
    columns: {
      id: {
        // This property has `objectId: true` instead of `type: int` in MongoDB
        primary: true,
        type: "int",
        generated: true,
      },
      userId: {
        // This property is set to `type: objectId` on MongoDB databases
        type: "int",
      },
      expires: {
        // The date the session expires (is updated when a session is active)
        type: "timestamp",
      },
      sessionToken: {
        // The sessionToken should never be exposed to client side JavaScript
        type: "varchar",
        unique: true,
      },
      accessToken: {
        // The accessToken can be safely exposed to client side JavaScript to
        // to identify the owner of a session without exposing the sessionToken
        type: "varchar",
        unique: true,
      },
      createdAt: {
        type: "timestamp",
        createDate: true,
      },
      updatedAt: {
        type: "timestamp",
        updateDate: true,
      },
    },
  }
  ),
  new EntitySchema({
    name: "VerificationRequest",
    columns: {
      id: {
        // This property has `objectId: true` instead of `type: int` in MongoDB
        primary: true,
        type: "int",
        generated: true,
      },
      identifier: {
        // An email address, phone number, username or other unique identifier
        // associated with the request (used to track who it was on behalf of)
        type: "varchar",
      },
      token: {
        // The token used verify the request (maybe hashed or encrypted)
        type: "varchar",
        unique: true,
      },
      expires: {
        // After this time, the request will no longer ve valid
        type: "timestamp",
      },
      createdAt: {
        type: "timestamp",
        createDate: true,
      },
      updatedAt: {
        type: "timestamp",
        updateDate: true,
      },
    },
  }),
]

const config = {
   type: "mysql",
   host: "localhost",
   port: 3306,
   username: "root",
   password: "password",
   database: "next-auth",
   entities
}

module.exports = config

Obviously don't forget to change the connection settings and potentially db type in the config object at the bottom.

This can then be called like this: npx typeorm schema:sync -f ./this/config/file/ormconfig.js

jcaryk commented 3 years ago

@ndom91 thank you - i will give this a whirl.

jcaryk commented 3 years ago

@ndom91 can you provide an example of how to use the synchronize: true flag in v4? It doesn't look like it makes a difference, but we're using postgresql.

in v3 it looks like:

adapter: Adapters.TypeORM.Adapter({  type: "sqlite",  database: ":memory:",  synchronize: true,})

but in version 4, it looks like the adapter only accepts string | ConnectionOptions from typeorm as the first param and an object containing entities as the only option for the second param.

balazsorban44 commented 3 years ago

Same should work for v4 adapter as well:

adapter: TypeORMLegacyAdapter({
  type: "sqlite",
  database: ":memory:",
  synchronize: true
})

Running tests like that here:

https://github.com/nextauthjs/adapters/blob/c42bfd0fc31a3cd6a811958991da3c4674d826f5/packages/typeorm-legacy/tests/sqlite/index.test.ts#L5-L13

ndom91 commented 3 years ago

Yeah so it depends on if yuo're using the string or connectionsoptions version, but the string version would just get a ?synchronize=true param at the end of it and the connection options as balazs pointed out above ^^

jcaryk commented 3 years ago

thank you all for the help!

ndom91 commented 2 years ago

nextauthjs/next-auth has been migrated to a monorepository. The adapters code can now be found there under packages/adapter-*. Thanks for your interest in the project!

balazsorban44 commented 2 years ago

Hi, we have included a section in the docs specific to TypeORM about naming conventions:

https://next-auth.js.org/adapters/typeorm#naming-conventions

We also have a database migration section in our Upgrade to v4 docs:

https://next-auth.js.org/getting-started/upgrade-v4#database-migration