Skip to content

Commit

Permalink
Require the identityField passed to createAuth has `isUnique: tru…
Browse files Browse the repository at this point in the history
…e` (#6179)
  • Loading branch information
emmatown authored Jul 25, 2021
1 parent f3c0edc commit ee8a514
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/cyan-tigers-tell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-next/auth': major
---

The field passed to the `identityField` option is now required to have `isUnique: true`.
1 change: 1 addition & 0 deletions examples-staging/roles/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ input PersonWhereInput {

input PersonWhereUniqueInput {
id: ID
email: String
}

enum SortPeopleBy {
Expand Down
2 changes: 1 addition & 1 deletion examples-staging/roles/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion examples-staging/roles/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 8 additions & 13 deletions packages/auth/src/lib/findMatchingIdentity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
22 changes: 21 additions & 1 deletion packages/auth/src/schema.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 4 additions & 5 deletions tests/api-tests/access-control/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 } }),
Expand Down Expand Up @@ -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 {
Expand Down
42 changes: 36 additions & 6 deletions tests/api-tests/auth-header.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -36,8 +36,8 @@ const auth = createAuth({
});

const runner = setupTestRunner({
config: apiTestConfig(
auth.withAuth({
config: auth.withAuth(
apiTestConfig({
lists: createSchema({
Post: list({
fields: {
Expand All @@ -48,7 +48,7 @@ const runner = setupTestRunner({
User: list({
fields: {
name: text(),
email: text(),
email: text({ isUnique: true }),
password: password(),
},
access: {
Expand All @@ -60,7 +60,7 @@ const runner = setupTestRunner({
}),
}),
session: statelessSessions({ secret: COOKIE_SECRET }),
} as KeystoneConfig)
})
),
});

Expand Down Expand Up @@ -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(
Expand Down

1 comment on commit ee8a514

@vercel
Copy link

@vercel vercel bot commented on ee8a514 Jul 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.