OpenSaasAU / keystone-nextjs-auth

A package that add social auth to Keystone-6 (https://keystonejs.com/) by using next-authjs (https://next-auth.js.org/)
MIT License
75 stars 11 forks source link

Can't get Google Auth working #14

Closed jordie23 closed 3 years ago

jordie23 commented 3 years ago

For some reason I just can't get the Google provider to work. I was able to debug some (as per the two PRs I sent) but I'm kinda stumped now, I think I just don't have enough knowledge of nextjs-auth or keystone to debug it properly.

Basically I can get it to pop up the google auth screen, I can choose my google account but when it lands on the callback page /api/auth/callback/google?state=.... I'm redirected to an error page /api/auth/error?error=AccessDenied.

I installed keystone-next at version 22 and couldn't get it working. I tried updating latest keystone (23.0.2 as of writing) and am getting a typescript error now when using withAuth by the way.

I used the setup where keystone generates a User list and a Post list, and installed using a normal email/password setup. I tried to add this package afterward. That setup did not have a subjectId field, so I have since added that, but hasn't seemed to help.

This is my keystone.ts file, maybe I'm doing something wrong with my config?

import { config } from '@keystone-next/keystone/schema'
import { statelessSessions } from '@keystone-next/keystone/session'
// original keystone auth
// import { createAuth } from '@keystone-next/auth'
import { lists } from './schema'
import {
  createAuth,
  nextAuthProviders as Providers,
} from '@opensaas/keystone-nextjs-auth'

let sessionSecret = process.env.SESSION_SECRET

if (!sessionSecret) {
  if (process.env.NODE_ENV === 'production') {
    throw new Error(
      'The SESSION_SECRET environment variable must be set in production'
    )
  } else {
    sessionSecret =
      '-- DEV SECRET - The bodies are buried under the binary tree --'
  }
}

let sessionMaxAge = 60 * 60 * 24 * 30 // 30 days

export const providers = [
  Providers.Google({
    clientId: process.env.GOOGLE_CLIENT_ID,
    clientSecret: process.env.GOOGLE_CLIENT_SECRET
  }),
]

const auth = createAuth({
  listKey: 'User',
  identityField: 'subjectId',
  sessionData: `id name email`,
  autoCreate: false,
  userMap: { subjectId: 'id', name: 'name' },
  accountMap: {},
  profileMap: { email: 'email' },
})

// Original keystone auth config
// const { withAuth } = createAuth({
//   listKey: 'User',
//   identityField: 'email',
//   secretField: 'password',
//   sessionData: 'name',
//   initFirstItem: {
//     fields: ['name', 'email', 'password'],
//   },
// })

const session = statelessSessions({
  maxAge: sessionMaxAge,
  secret: sessionSecret,
})

const getDbUrl = () => {
  if (
    process.env.DATABASE_URL &&
    typeof process.env.DATABASE_URL === 'string' &&
    process.env.DATABASE_URL !== 'undefined' // for some reason this can end up as a string of 'undefined'
  ) {
    return process.env.DATABASE_URL
  }

  if (
    process.env.DB_NAME &&
    process.env.DB_PASS &&
    process.env.DB_USER &&
    process.env.DB_HOST
  ) {
    return `postgres://${process.env.DB_USER}:${process.env.DB_PASS}@${process.env.DB_HOST}/${process.env.DB_NAME}`
  }

  // local dev default
  return 'postgres://postgres:example@127.0.0.1/example'
}

export default auth.withAuth(
  config({
    experimental: {
      generateNodeAPI: true,
    },
    db: {
      adapter: 'prisma_postgresql',
      url: getDbUrl(),
    },
    lists,
    session,
  })
)

Any ideas on where I should be looking to start debugging?

borisno2 commented 3 years ago

Hey @jordie23 Thanks for the couple PRs...

I notice you have autoCreate to false have you created the user in your DB with the correct subjectId? if autoCreate if false and the subjectId doesn't exist in the DB it will give an access denied

The code does

 if (!result.success) {
          if (!autoCreate) {
            console.log('False');
            return false;
          }
          console.log('Create User');

so you should see a False in your console where keystone in running if it can't get a user with a matching subjectId I suggest setting on autoCreate to true to see if it can get past that bit...

Let me know how you go...

borisno2 commented 3 years ago

Also FYI, just updated keystone packages #15

jordie23 commented 3 years ago

Thanks for the response, I appreciate it.

It was indeed logging False.

When I tried setting autoCreate to true I just got thrown to /api/auth/error?error=You%20attempted%20to%20perform%20an%20invalid%20mutation and a similar error logged to terminal ValidationFailureError: You attempted to perform an invalid mutation.

After some debugging I realised that it was because of how I initially installed the app using a keystone blog default. It means the password field on the User list was set to be required (and the create user obviously wasn't setting that field). When I set that as not required, I didn't get the mutation error anymore and I could login with a new users and it set the user fields correctly. Maybe worth a note in the readme regarding required fields in a User list. I can make a PR if you'd like.

However... I don't wish to have autoCreate set to true as this will just be an internal tool for our company. It'd be great if I could create a user in the User list, and then have them be able to login. The problem here is I won't know what their subjectId would be to do this.

So I'm thinking, could we expand the ability of identityField so we can specify the provider field in addition to the keystone field? At the moment it's hardcoded to check the id value from the provider. https://github.com/OpenSaasAU/keystone-nextjs-auth/blob/main/packages/keystone-nextjs-auth/src/pages/NextAuthPage.tsx#L40

Perhaps it could work like the other *Map fields, with a keystone field being the key and a provider field being the value. Maybe in order to prevent breaking existing users can create a new one called identityFieldMap. This way I could set it to match on email, say using identityFieldMap: { email: 'email' } then all I need to do is create users with a specific email, and they'll be allowed to log in.

Also btw "sign out" doesn't appear to work for me, I have to manually clear the cookies from my browser.

borisno2 commented 3 years ago

Hey @jordie23 Glad you got it working...

Happy for you to do a PR for the README, good feedback, thanks!

Good point re the identity field, I have been thinking from a Auth0 perspective where email is not a reliable identifier especially when you have multiple signin methods it is possible for someone to signup with the same email and therefore steal your account. But, can see that in your situation it is probably good to make it configurable - I generally setup roles that don't allow users to do much until they have a role.

Is this something you would be happy to do a PR for? The other thing to think of is then the initFirstItem which isn't accounted for if autoCreate is set to false. There is some chat over at #1 regarding this.

Thanks!

jordie23 commented 3 years ago

Happy for you to do a PR for the README, good feedback, thanks!

Cool I'll jump on that one shortly.

I have been thinking from a Auth0 perspective where email is not a reliable identifier especially when you have multiple signin methods it is possible for someone to signup with the same email and therefore steal your account.

I hadn't thought of that, interesting! (and scary)

Is this something you would be happy to do a PR for?

I'll see if I get some time for a PR, it's definitely something we're very keen on having for the long term, so there's motivation. I'll do what I can. For the short term we can manage with manually adding users (we won't have many to start), I found a way to get someone to get their google account ID that I can set into a subject ID field. https://gist.github.com/msafi/b1cb05bfab5b897c57e6

borisno2 commented 3 years ago

Thanks @jordie23.. let me know, if it something you want to do and if you need a hand.

I have also created #18 for the signout issue, and should have a fix for that shortly.

borisno2 commented 3 years ago

Just pushed new packages that map the Providers to the keystone publicPages - involves a config change #21 (also updates keystone to latest..)

jordie23 commented 3 years ago

Thanks for the update.

I don't think I'll get that PR regarding the custom identifier map done anytime soon, I did get a start but I've just got a lot on my plate atm!