From ee8a51498204cd7de4998af0bb05871301bfb26a Mon Sep 17 00:00:00 2001 From: Mitchell Hamilton Date: Mon, 26 Jul 2021 09:09:39 +1000 Subject: [PATCH] Require the `identityField` passed to `createAuth` has `isUnique: true` (#6179) --- .changeset/cyan-tigers-tell.md | 5 +++ examples-staging/roles/schema.graphql | 1 + examples-staging/roles/schema.prisma | 2 +- examples-staging/roles/schema.ts | 2 +- packages/auth/src/lib/findMatchingIdentity.ts | 21 ++++------ packages/auth/src/schema.ts | 22 +++++++++- tests/api-tests/access-control/utils.ts | 9 ++-- tests/api-tests/auth-header.test.ts | 42 ++++++++++++++++--- 8 files changed, 77 insertions(+), 27 deletions(-) create mode 100644 .changeset/cyan-tigers-tell.md diff --git a/.changeset/cyan-tigers-tell.md b/.changeset/cyan-tigers-tell.md new file mode 100644 index 00000000000..0b1f6d3f79d --- /dev/null +++ b/.changeset/cyan-tigers-tell.md @@ -0,0 +1,5 @@ +--- +'@keystone-next/auth': major +--- + +The field passed to the `identityField` option is now required to have `isUnique: true`. diff --git a/examples-staging/roles/schema.graphql b/examples-staging/roles/schema.graphql index f8971e6488f..06ce7ac0d3d 100644 --- a/examples-staging/roles/schema.graphql +++ b/examples-staging/roles/schema.graphql @@ -159,6 +159,7 @@ input PersonWhereInput { input PersonWhereUniqueInput { id: ID + email: String } enum SortPeopleBy { diff --git a/examples-staging/roles/schema.prisma b/examples-staging/roles/schema.prisma index 8121d6034f1..cf5f2e67c2d 100644 --- a/examples-staging/roles/schema.prisma +++ b/examples-staging/roles/schema.prisma @@ -22,7 +22,7 @@ model Todo { model Person { id String @id @default(cuid()) name String? - email String? + email String? @unique password String? role Role? @relation("Person_role", fields: [roleId], references: [id]) roleId String? @map("role") diff --git a/examples-staging/roles/schema.ts b/examples-staging/roles/schema.ts index 11cedbc8666..fcb7d1ec8de 100644 --- a/examples-staging/roles/schema.ts +++ b/examples-staging/roles/schema.ts @@ -105,7 +105,7 @@ export const lists = createSchema({ /* The name of the user */ name: text({ isRequired: true }), /* The email of the user, used to sign in */ - email: text({ isRequired: true }), + email: text({ isRequired: true, isUnique: true }), /* The password of the user */ password: password({ isRequired: true, diff --git a/packages/auth/src/lib/findMatchingIdentity.ts b/packages/auth/src/lib/findMatchingIdentity.ts index 5c39b867b23..3297aacf5a8 100644 --- a/packages/auth/src/lib/findMatchingIdentity.ts +++ b/packages/auth/src/lib/findMatchingIdentity.ts @@ -10,18 +10,13 @@ export async function findMatchingIdentity( | { success: false; code: AuthTokenRequestErrorCode } | { success: true; item: { id: any; [prop: string]: any } } > { - const items = await dbItemAPI.findMany({ where: { [identityField]: identity } }); - - // Identity failures with helpful errors - let code: AuthTokenRequestErrorCode | undefined; - if (items.length === 0) { - code = 'IDENTITY_NOT_FOUND'; - } else if (items.length > 1) { - code = 'MULTIPLE_IDENTITY_MATCHES'; - } - if (code) { - return { success: false, code }; - } else { - return { success: true, item: items[0] as any }; + try { + const item = await dbItemAPI.findOne({ where: { [identityField]: identity } }); + return { success: true, item }; + } catch (err) { + if (err.message === 'You do not have access to this resource') { + return { success: false, code: 'IDENTITY_NOT_FOUND' }; + } + throw err; } } diff --git a/packages/auth/src/schema.ts b/packages/auth/src/schema.ts index 3a7c9caa447..bc78ec3fd19 100644 --- a/packages/auth/src/schema.ts +++ b/packages/auth/src/schema.ts @@ -1,7 +1,13 @@ import { mergeSchemas } from '@graphql-tools/merge'; import { ExtendGraphqlSchema } from '@keystone-next/types'; -import { assertObjectType, GraphQLSchema } from 'graphql'; +import { + assertObjectType, + GraphQLSchema, + assertInputObjectType, + GraphQLString, + GraphQLID, +} from 'graphql'; import { AuthGqlNames, AuthTokenTypeConfig, InitFirstItemConfig, SecretFieldImpl } from './types'; import { getBaseAuthSchema } from './gql/getBaseAuthSchema'; import { getInitFirstItemSchema } from './gql/getInitFirstItemSchema'; @@ -53,6 +59,20 @@ export const getSchemaExtension = magicAuthLink?: AuthTokenTypeConfig; }): ExtendGraphqlSchema => schema => { + const uniqueWhereInputType = assertInputObjectType( + schema.getType(`${listKey}WhereUniqueInput`) + ); + const identityFieldOnUniqueWhere = uniqueWhereInputType.getFields()[identityField]; + if ( + identityFieldOnUniqueWhere?.type !== GraphQLString && + identityFieldOnUniqueWhere?.type !== GraphQLID + ) { + throw new Error( + `createAuth was called with an identityField of ${identityField} on the list ${listKey} ` + + `but that field doesn't allow being searched uniquely with a String or ID. ` + + `You should likely add \`isUnique: true\` to the field at ${listKey}.${identityField}` + ); + } return [ getBaseAuthSchema({ identityField, diff --git a/tests/api-tests/access-control/utils.ts b/tests/api-tests/access-control/utils.ts index ab0ae005956..012a038f17c 100644 --- a/tests/api-tests/access-control/utils.ts +++ b/tests/api-tests/access-control/utils.ts @@ -2,7 +2,6 @@ import { text, password } from '@keystone-next/fields'; import { createSchema, list } from '@keystone-next/keystone/schema'; import { statelessSessions } from '@keystone-next/keystone/session'; import { createAuth } from '@keystone-next/auth'; -import type { KeystoneConfig } from '@keystone-next/types'; import { apiTestConfig } from '../utils'; const FAKE_ID = { postgresql: 'cdsfasfafafadfasdf', sqlite: 'cdsfasfafafadfasdf' } as const; @@ -93,7 +92,7 @@ const lists = createSchema({ User: list({ fields: { name: text(), - email: text(), + email: text({ isUnique: true }), password: password(), noRead: text({ access: { read: () => false } }), yesRead: text({ access: { read: () => true } }), @@ -143,11 +142,11 @@ const auth = createAuth({ sessionData: 'id', }); -const config = apiTestConfig( - auth.withAuth({ +const config = auth.withAuth( + apiTestConfig({ lists, session: statelessSessions({ secret: COOKIE_SECRET }), - } as KeystoneConfig) + }) ); export { diff --git a/tests/api-tests/auth-header.test.ts b/tests/api-tests/auth-header.test.ts index 376b86ef138..fa634ee382a 100644 --- a/tests/api-tests/auth-header.test.ts +++ b/tests/api-tests/auth-header.test.ts @@ -2,8 +2,8 @@ import { text, timestamp, password } from '@keystone-next/fields'; import { createSchema, list } from '@keystone-next/keystone/schema'; import { statelessSessions } from '@keystone-next/keystone/session'; import { createAuth } from '@keystone-next/auth'; -import type { KeystoneContext, KeystoneConfig } from '@keystone-next/types'; -import { setupTestRunner, TestArgs } from '@keystone-next/testing'; +import type { KeystoneContext } from '@keystone-next/types'; +import { setupTestRunner, TestArgs, setupTestEnv } from '@keystone-next/testing'; import { apiTestConfig, expectAccessDenied } from './utils'; const initialData = { @@ -36,8 +36,8 @@ const auth = createAuth({ }); const runner = setupTestRunner({ - config: apiTestConfig( - auth.withAuth({ + config: auth.withAuth( + apiTestConfig({ lists: createSchema({ Post: list({ fields: { @@ -48,7 +48,7 @@ const runner = setupTestRunner({ User: list({ fields: { name: text(), - email: text(), + email: text({ isUnique: true }), password: password(), }, access: { @@ -60,7 +60,7 @@ const runner = setupTestRunner({ }), }), session: statelessSessions({ secret: COOKIE_SECRET }), - } as KeystoneConfig) + }) ), }); @@ -99,6 +99,36 @@ describe('Auth testing', () => { }) ); + test('Fails with useful error when identity field is not unique', async () => { + const auth = createAuth({ + listKey: 'User', + identityField: 'email', + secretField: 'password', + sessionData: 'id', + }); + await expect( + setupTestEnv({ + config: auth.withAuth( + apiTestConfig({ + lists: createSchema({ + User: list({ + fields: { + name: text(), + email: text(), + password: password(), + }, + }), + }), + + session: statelessSessions({ secret: COOKIE_SECRET }), + }) + ), + }) + ).rejects.toMatchInlineSnapshot( + `[Error: createAuth was called with an identityField of email on the list User but that field doesn't allow being searched uniquely with a String or ID. You should likely add \`isUnique: true\` to the field at User.email]` + ); + }); + describe('logged in', () => { // eslint-disable-next-line jest/no-disabled-tests test.skip(