Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace bcrypt and workFactor options to password field with generic kdf option #9471

Merged
merged 2 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/polite-colts-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@keystone-6/auth": major
"@keystone-6/core": major
---

Replace `bcrypt` and `workFactor` options for `password` field with new generic `kdf` option
4 changes: 1 addition & 3 deletions docs/content/docs/fields/password.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ Options:
- `validation.match.regex`: The regular expression
- `validation.match.explanation` (default: `${fieldLabel} must match ${validation.match.regex}`): A message shown in the Admin when a value doesn't match the regex and returned as a validation error from the GraphQL API
- `validation.rejectCommon` (default: `false`): Rejects passwords from a list of commonly used passwords.
- `bcrypt` (default: `require('bcryptjs')`): A module which implements the same interface as the [`bcryptjs`](https://www.npmjs.com/package/bcryptjs) package, such as the native [`bcrypt`](https://www.npmjs.com/package/bcrypt) package.
This module will be used for all encryption routines in the `password` field.
- `kdf` (default: `{ hash: (secret) => bcryptjs.hash(secret, 10), compare: (secret, hash) => bcryptjs.compare(secret, hash) }`): An object with `hash` and `compare` functions for hashing and comparing passwords.

```typescript
import { config, list } from '@keystone-6/core';
Expand All @@ -36,7 +35,6 @@ export default config({
isRequired: true,
rejectCommon: true,
},
bcrypt: require('bcrypt'),
}),
/* ... */
},
Expand Down
15 changes: 10 additions & 5 deletions packages/auth/src/gql/getBaseAuthSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import type {
KeystoneContext
} from '@keystone-6/core/types'
import { g } from '@keystone-6/core'
import { getPasswordFieldKDF } from '@keystone-6/core/fields/types/password'
import type {
AuthGqlNames,
SecretFieldImpl,
} from '../types'

const AUTHENTICATION_FAILURE = {
Expand All @@ -18,16 +18,21 @@ export function getBaseAuthSchema<I extends string, S extends string> ({
identityField,
secretField,
gqlNames,
secretFieldImpl,
base,
}: {
listKey: string
identityField: I
secretField: S
gqlNames: AuthGqlNames
secretFieldImpl: SecretFieldImpl
base: g.BaseSchemaMeta
}) {
const kdf = getPasswordFieldKDF(base.schema, listKey, secretField)
if (!kdf) {
throw new Error(
`${listKey}.${secretField} is not a valid password field.`
)
}

const ItemAuthenticationWithPasswordSuccess = g.object<{
sessionToken: string
item: BaseItem
Expand Down Expand Up @@ -94,11 +99,11 @@ export function getBaseAuthSchema<I extends string, S extends string> ({
})

if ((typeof item?.[secretField] !== 'string')) {
await secretFieldImpl.generateHash('simulated-password-to-counter-timing-attack')
await kdf.hash('simulated-password-to-counter-timing-attack')
return AUTHENTICATION_FAILURE
}

const equal = await secretFieldImpl.compare(secret, item[secretField])
const equal = await kdf.compare(secret, item[secretField])
if (!equal) return AUTHENTICATION_FAILURE

const sessionToken = await context.sessionStrategy.start({
Expand Down
28 changes: 0 additions & 28 deletions packages/auth/src/schema.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import {
type GraphQLSchema,
assertObjectType,
assertInputObjectType,
GraphQLString,
GraphQLID,
Expand All @@ -14,35 +12,10 @@ import type {
AuthGqlNames,
AuthTokenTypeConfig,
InitFirstItemConfig,
SecretFieldImpl,
} from './types'
import { getBaseAuthSchema } from './gql/getBaseAuthSchema'
import { getInitFirstItemSchema } from './gql/getInitFirstItemSchema'

function assertSecretFieldImpl (
impl: any,
listKey: string,
secretField: string
): asserts impl is SecretFieldImpl {
if (
!impl ||
typeof impl.compare !== 'function' ||
impl.compare.length < 2 ||
typeof impl.generateHash !== 'function'
) {
const s = JSON.stringify(secretField)
const msg = `A createAuth() invocation for the "${listKey}" list specifies ${s} as its secretField, but the field type doesn't implement the required functionality.`
throw new Error(msg)
}
}

export function getSecretFieldImpl (schema: GraphQLSchema, listKey: string, fieldKey: string) {
const gqlOutputType = assertObjectType(schema.getType(listKey))
const secretFieldImpl = gqlOutputType.getFields()?.[fieldKey].extensions?.keystoneSecretField
assertSecretFieldImpl(secretFieldImpl, listKey, fieldKey)
return secretFieldImpl
}

export const getSchemaExtension = ({
identityField,
listKey,
Expand Down Expand Up @@ -81,7 +54,6 @@ export const getSchemaExtension = ({
listKey,
secretField,
gqlNames,
secretFieldImpl: getSecretFieldImpl(base.schema, listKey, secretField),
base,
})

Expand Down
4 changes: 4 additions & 0 deletions packages/core/fields/types/password/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"main": "dist/keystone-6-core-fields-types-password.cjs.js",
"module": "dist/keystone-6-core-fields-types-password.esm.js"
}
6 changes: 6 additions & 0 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@
"module": "./admin-ui/components/dist/keystone-6-core-admin-ui-components.esm.js",
"default": "./admin-ui/components/dist/keystone-6-core-admin-ui-components.cjs.js"
},
"./fields/types/password": {
"types": "./fields/types/password/dist/keystone-6-core-fields-types-password.cjs.js",
"module": "./fields/types/password/dist/keystone-6-core-fields-types-password.esm.js",
"default": "./fields/types/password/dist/keystone-6-core-fields-types-password.cjs.js"
},
"./fields/types/file/views": {
"types": "./fields/types/file/views/dist/keystone-6-core-fields-types-file-views.cjs.js",
"module": "./fields/types/file/views/dist/keystone-6-core-fields-types-file-views.esm.js",
Expand Down Expand Up @@ -305,6 +310,7 @@
"admin-ui/image.tsx",
"admin-ui/utils/index.ts",
"fields/index.ts",
"fields/types/password/index.ts",
"fields/types/*/views/index.tsx",
"fields/types/{image,file}/utils.ts",
"types/index.ts"
Expand Down
45 changes: 23 additions & 22 deletions packages/core/src/fields/types/password/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@ import { g } from '../../..'
import { type PasswordFieldMeta } from './views'
import { makeValidateHook } from '../../non-null-graphql'
import { mergeFieldHooks } from '../../resolve-hooks'
import { isObjectType, type GraphQLSchema } from 'graphql'

export type PasswordFieldConfig<ListTypeInfo extends BaseListTypeInfo> =
CommonFieldConfig<ListTypeInfo> & {
/**
* @default 10
*/
workFactor?: number
validation?: {
isRequired?: boolean
rejectCommon?: boolean
Expand All @@ -34,9 +31,14 @@ export type PasswordFieldConfig<ListTypeInfo extends BaseListTypeInfo> =
map?: string
extendPrismaSchema?: (field: string) => string
}
bcrypt?: Pick<typeof bcryptjs, 'compare' | 'hash'>
kdf?: KDF
}

type KDF = {
compare(preImage: string, hash: string): Promise<boolean>
hash(preImage: string): Promise<string>
}

const PasswordState = g.object<{ isSet: boolean }>()({
name: 'PasswordState',
fields: {
Expand All @@ -51,12 +53,12 @@ const PasswordFilter = g.inputObject({
},
})

const bcryptHashRegex = /^\$2[aby]?\$\d{1,2}\$[./A-Za-z0-9]{53}$/

export function password <ListTypeInfo extends BaseListTypeInfo> (config: PasswordFieldConfig<ListTypeInfo> = {}): FieldTypeFunc<ListTypeInfo> {
const {
bcrypt = bcryptjs, // TODO: rename to kdf in breaking change
workFactor = 10, // TODO: remove in breaking change, use a custom KDF
kdf = {
hash: (secret) => bcryptjs.hash(secret, 10),
compare: (secret, hash) => bcryptjs.compare(secret, hash),
},
validation = {},
} = config
const {
Expand Down Expand Up @@ -92,13 +94,10 @@ export function password <ListTypeInfo extends BaseListTypeInfo> (config: Passwo
) {
throw new Error(`${meta.listKey}.${meta.fieldKey} specifies a validation.length.max that is less than the validation.length.min, and therefore has no valid options`)
}
if (workFactor < 6 || workFactor > 31 || !Number.isInteger(workFactor)) {
throw new Error(`${meta.listKey}.${meta.fieldKey}: workFactor must be an integer between 6 and 31`)
}

function inputResolver (val: string | null | undefined) {
if (val == null) return val
return bcrypt.hash(val, workFactor)
return kdf.hash(val)
}

const hasAdditionalValidation = match || rejectCommon || min !== undefined || max !== undefined
Expand Down Expand Up @@ -185,19 +184,21 @@ export function password <ListTypeInfo extends BaseListTypeInfo> (config: Passwo
output: g.field({
type: PasswordState,
resolve (val) {
return { isSet: val.value !== null && bcryptHashRegex.test(val.value) }
return { isSet: val.value !== null }
},
extensions: {
keystoneSecretField: {
generateHash: async (secret: string) => {
return bcrypt.hash(secret, workFactor)
},
compare: (secret: string, hash: string) => {
return bcrypt.compare(secret, hash)
},
},
keystoneKDF: kdf,
},
}),
})
}
}


export function getPasswordFieldKDF (schema: GraphQLSchema, listKey: string, fieldKey: string): KDF | null {
const gqlOutputType = schema.getType(listKey)
if (!isObjectType(gqlOutputType)) return null
const passwordField = gqlOutputType.getFields()[fieldKey]
if (!passwordField?.extensions.keystoneKDF) return null
return passwordField.extensions.keystoneKDF as KDF
}