From 6104fe5011fd1dbfe4fa2b48899a657361e87be4 Mon Sep 17 00:00:00 2001 From: Dan Ribbens Date: Tue, 3 Dec 2024 09:52:23 -0500 Subject: [PATCH] feat: disableLocalStrategy with auth fields still enabled (#9579) Adds configuration options to `auth.disableLocalStrategy` to allow customization of how payload treats an auth enabled collection. Two new properties have been added to `disableLocalStrategy`: - `enableFields` Include auth fields on the collection even though the local strategy is disabled. Useful when you do not want the database or types to vary depending on the auth configuration used. - `optionalPassword`: makes the password field not required --- .../graphql/src/schema/initCollections.ts | 12 +- packages/payload/src/auth/getAuthFields.ts | 6 +- .../src/auth/operations/forgotPassword.ts | 9 +- packages/payload/src/auth/operations/login.ts | 5 + .../src/auth/operations/registerFirstUser.ts | 4 + .../src/auth/operations/resetPassword.ts | 20 ++-- .../payload/src/auth/operations/unlock.ts | 4 + .../src/auth/operations/verifyEmail.ts | 6 +- packages/payload/src/auth/types.ts | 11 +- .../src/utilities/configToJSONSchema.ts | 8 +- packages/ui/src/views/Edit/Auth/types.ts | 2 +- test/auth/config.ts | 27 ++++- test/auth/e2e.spec.ts | 1 - test/auth/int.spec.ts | 74 +++++++++++- test/auth/payload-types.ts | 106 ++++++++++++++---- test/auth/shared.ts | 2 + 16 files changed, 256 insertions(+), 41 deletions(-) diff --git a/packages/graphql/src/schema/initCollections.ts b/packages/graphql/src/schema/initCollections.ts index 8c7d51019..3f08e1ce7 100644 --- a/packages/graphql/src/schema/initCollections.ts +++ b/packages/graphql/src/schema/initCollections.ts @@ -126,12 +126,20 @@ export function initCollections({ config, graphqlResult }: InitCollectionsGraphQ const mutationInputFields = [...fields] - if (collectionConfig.auth && !collectionConfig.auth.disableLocalStrategy) { + if ( + collectionConfig.auth && + (!collectionConfig.auth.disableLocalStrategy || + (typeof collectionConfig.auth.disableLocalStrategy === 'object' && + collectionConfig.auth.disableLocalStrategy.optionalPassword)) + ) { mutationInputFields.push({ name: 'password', type: 'text', label: 'Password', - required: true, + required: !( + typeof collectionConfig.auth.disableLocalStrategy === 'object' && + collectionConfig.auth.disableLocalStrategy.optionalPassword + ), }) } diff --git a/packages/payload/src/auth/getAuthFields.ts b/packages/payload/src/auth/getAuthFields.ts index 122c43984..90e621dd4 100644 --- a/packages/payload/src/auth/getAuthFields.ts +++ b/packages/payload/src/auth/getAuthFields.ts @@ -15,7 +15,11 @@ export const getBaseAuthFields = (authConfig: IncomingAuthType): Field[] => { authFields.push(...apiKeyFields) } - if (!authConfig.disableLocalStrategy) { + if ( + !authConfig.disableLocalStrategy || + (typeof authConfig.disableLocalStrategy === 'object' && + authConfig.disableLocalStrategy.enableFields) + ) { const emailField = { ...emailFieldConfig } let usernameField: TextField | undefined diff --git a/packages/payload/src/auth/operations/forgotPassword.ts b/packages/payload/src/auth/operations/forgotPassword.ts index 082bef0bd..f49c771fa 100644 --- a/packages/payload/src/auth/operations/forgotPassword.ts +++ b/packages/payload/src/auth/operations/forgotPassword.ts @@ -11,6 +11,7 @@ import type { PayloadRequest, Where } from '../../types/index.js' import { buildAfterOperation } from '../../collections/operations/utils.js' import { APIError } from '../../errors/index.js' +import { Forbidden } from '../../index.js' import { commitTransaction } from '../../utilities/commitTransaction.js' import { initTransaction } from '../../utilities/initTransaction.js' import { killTransaction } from '../../utilities/killTransaction.js' @@ -43,6 +44,11 @@ export const forgotPasswordOperation = async ( ? data.username.toLowerCase().trim() : null + let args = incomingArgs + + if (incomingArgs.collection.config.auth.disableLocalStrategy) { + throw new Forbidden(incomingArgs.req.t) + } if (!sanitizedEmail && !sanitizedUsername) { throw new APIError( `Missing ${loginWithUsername ? 'username' : 'email'}.`, @@ -50,8 +56,6 @@ export const forgotPasswordOperation = async ( ) } - let args = incomingArgs - try { const shouldCommit = await initTransaction(args.req) @@ -74,7 +78,6 @@ export const forgotPasswordOperation = async ( const { collection: { config: collectionConfig }, - data, disableEmail, expiration, req: { diff --git a/packages/payload/src/auth/operations/login.ts b/packages/payload/src/auth/operations/login.ts index 97d2ae986..aa08987d7 100644 --- a/packages/payload/src/auth/operations/login.ts +++ b/packages/payload/src/auth/operations/login.ts @@ -10,6 +10,7 @@ import type { User } from '../types.js' import { buildAfterOperation } from '../../collections/operations/utils.js' import { AuthenticationError, LockedAuth, ValidationError } from '../../errors/index.js' import { afterRead } from '../../fields/hooks/afterRead/index.js' +import { Forbidden } from '../../index.js' import { killTransaction } from '../../utilities/killTransaction.js' import sanitizeInternalFields from '../../utilities/sanitizeInternalFields.js' import { getFieldsToSign } from '../getFieldsToSign.js' @@ -40,6 +41,10 @@ export const loginOperation = async ( ): Promise<{ user: DataFromCollectionSlug } & Result> => { let args = incomingArgs + if (args.collection.config.auth.disableLocalStrategy) { + throw new Forbidden(args.req.t) + } + try { // ///////////////////////////////////// // beforeOperation - Collection diff --git a/packages/payload/src/auth/operations/registerFirstUser.ts b/packages/payload/src/auth/operations/registerFirstUser.ts index 2030726e3..2db43a843 100644 --- a/packages/payload/src/auth/operations/registerFirstUser.ts +++ b/packages/payload/src/auth/operations/registerFirstUser.ts @@ -42,6 +42,10 @@ export const registerFirstUserOperation = async ( req: { payload }, } = args + if (config.auth.disableLocalStrategy) { + throw new Forbidden(req.t) + } + try { const shouldCommit = await initTransaction(req) diff --git a/packages/payload/src/auth/operations/resetPassword.ts b/packages/payload/src/auth/operations/resetPassword.ts index 8bf59f829..56b8b8322 100644 --- a/packages/payload/src/auth/operations/resetPassword.ts +++ b/packages/payload/src/auth/operations/resetPassword.ts @@ -3,7 +3,7 @@ import httpStatus from 'http-status' import type { Collection } from '../../collections/config/types.js' import type { PayloadRequest } from '../../types/index.js' -import { APIError } from '../../errors/index.js' +import { APIError, Forbidden } from '../../errors/index.js' import { commitTransaction } from '../../utilities/commitTransaction.js' import { initTransaction } from '../../utilities/initTransaction.js' import { killTransaction } from '../../utilities/killTransaction.js' @@ -29,13 +29,6 @@ export type Arguments = { } export const resetPasswordOperation = async (args: Arguments): Promise => { - if ( - !Object.prototype.hasOwnProperty.call(args.data, 'token') || - !Object.prototype.hasOwnProperty.call(args.data, 'password') - ) { - throw new APIError('Missing required data.', httpStatus.BAD_REQUEST) - } - const { collection: { config: collectionConfig }, data, @@ -48,6 +41,17 @@ export const resetPasswordOperation = async (args: Arguments): Promise = req, } = args + if ( + !Object.prototype.hasOwnProperty.call(data, 'token') || + !Object.prototype.hasOwnProperty.call(data, 'password') + ) { + throw new APIError('Missing required data.', httpStatus.BAD_REQUEST) + } + + if (collectionConfig.auth.disableLocalStrategy) { + throw new Forbidden(req.t) + } + try { const shouldCommit = await initTransaction(req) diff --git a/packages/payload/src/auth/operations/unlock.ts b/packages/payload/src/auth/operations/unlock.ts index a6b6e44ee..044e6a91d 100644 --- a/packages/payload/src/auth/operations/unlock.ts +++ b/packages/payload/src/auth/operations/unlock.ts @@ -8,6 +8,7 @@ import type { CollectionSlug } from '../../index.js' import type { PayloadRequest, Where } from '../../types/index.js' import { APIError } from '../../errors/index.js' +import { Forbidden } from '../../index.js' import { commitTransaction } from '../../utilities/commitTransaction.js' import { initTransaction } from '../../utilities/initTransaction.js' import { killTransaction } from '../../utilities/killTransaction.js' @@ -44,6 +45,9 @@ export const unlockOperation = async ( args.data.username.toLowerCase().trim()) || null + if (collectionConfig.auth.disableLocalStrategy) { + throw new Forbidden(req.t) + } if (!sanitizedEmail && !sanitizedUsername) { throw new APIError( `Missing ${collectionConfig.auth.loginWithUsername ? 'username' : 'email'}.`, diff --git a/packages/payload/src/auth/operations/verifyEmail.ts b/packages/payload/src/auth/operations/verifyEmail.ts index fff5d842e..45052b183 100644 --- a/packages/payload/src/auth/operations/verifyEmail.ts +++ b/packages/payload/src/auth/operations/verifyEmail.ts @@ -3,7 +3,7 @@ import httpStatus from 'http-status' import type { Collection } from '../../collections/config/types.js' import type { PayloadRequest } from '../../types/index.js' -import { APIError } from '../../errors/index.js' +import { APIError, Forbidden } from '../../errors/index.js' import { commitTransaction } from '../../utilities/commitTransaction.js' import { initTransaction } from '../../utilities/initTransaction.js' import { killTransaction } from '../../utilities/killTransaction.js' @@ -16,6 +16,10 @@ export type Args = { export const verifyEmailOperation = async (args: Args): Promise => { const { collection, req, token } = args + + if (collection.config.auth.disableLocalStrategy) { + throw new Forbidden(req.t) + } if (!Object.prototype.hasOwnProperty.call(args, 'token')) { throw new APIError('Missing required data.', httpStatus.BAD_REQUEST) } diff --git a/packages/payload/src/auth/types.ts b/packages/payload/src/auth/types.ts index 2bc0162bb..f087fa2e1 100644 --- a/packages/payload/src/auth/types.ts +++ b/packages/payload/src/auth/types.ts @@ -206,7 +206,16 @@ export interface IncomingAuthType { /** * Advanced - disable Payload's built-in local auth strategy. Only use this property if you have replaced Payload's auth mechanisms with your own. */ - disableLocalStrategy?: true + disableLocalStrategy?: + | { + /** + * Include auth fields on the collection even though the local strategy is disabled. + * Useful when you do not want the database or types to vary depending on the auth configuration. + */ + enableFields?: true + optionalPassword?: true + } + | true /** * Customize the way that the forgotPassword operation functions. * @link https://payloadcms.com/docs/authentication/email#forgot-password diff --git a/packages/payload/src/utilities/configToJSONSchema.ts b/packages/payload/src/utilities/configToJSONSchema.ts index 2501f2ea8..9edb6e5e0 100644 --- a/packages/payload/src/utilities/configToJSONSchema.ts +++ b/packages/payload/src/utilities/configToJSONSchema.ts @@ -617,7 +617,13 @@ export function entityToJSONSchema( }) } - if ('auth' in entity && entity.auth && !entity.auth?.disableLocalStrategy) { + if ( + 'auth' in entity && + entity.auth && + (!entity.auth?.disableLocalStrategy || + (typeof entity.auth?.disableLocalStrategy === 'object' && + entity.auth.disableLocalStrategy.enableFields)) + ) { entity.flattenedFields.push({ name: 'password', type: 'text', diff --git a/packages/ui/src/views/Edit/Auth/types.ts b/packages/ui/src/views/Edit/Auth/types.ts index 766f8896b..de3b198fd 100644 --- a/packages/ui/src/views/Edit/Auth/types.ts +++ b/packages/ui/src/views/Edit/Auth/types.ts @@ -3,7 +3,7 @@ import type { SanitizedCollectionConfig } from 'payload' export type Props = { className?: string collectionSlug: SanitizedCollectionConfig['slug'] - disableLocalStrategy?: boolean + disableLocalStrategy?: SanitizedCollectionConfig['auth']['disableLocalStrategy'] email: string loginWithUsername: SanitizedCollectionConfig['auth']['loginWithUsername'] operation: 'create' | 'update' diff --git a/test/auth/config.ts b/test/auth/config.ts index b459db879..66b349fab 100644 --- a/test/auth/config.ts +++ b/test/auth/config.ts @@ -6,7 +6,13 @@ import { v4 as uuid } from 'uuid' import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js' import { devUser } from '../credentials.js' -import { apiKeysSlug, namedSaveToJWTValue, saveToJWTKey, slug } from './shared.js' +import { + apiKeysSlug, + namedSaveToJWTValue, + partialDisableLocaleStrategiesSlug, + saveToJWTKey, + slug, +} from './shared.js' export default buildConfigWithDefaults({ admin: { @@ -174,6 +180,25 @@ export default buildConfigWithDefaults({ }, ], }, + { + slug: partialDisableLocaleStrategiesSlug, + auth: { + disableLocalStrategy: { + // optionalPassword: true, + enableFields: true, + }, + }, + fields: [ + // with `enableFields: true`, the following DB columns will be created: + // email + // reset_password_token + // reset_password_expiration + // salt + // hash + // login_attempts + // lock_until + ], + }, { slug: apiKeysSlug, access: { diff --git a/test/auth/e2e.spec.ts b/test/auth/e2e.spec.ts index 8a8280ca4..4e4a594e1 100644 --- a/test/auth/e2e.spec.ts +++ b/test/auth/e2e.spec.ts @@ -4,7 +4,6 @@ import type { SanitizedConfig } from 'payload' import { expect, test } from '@playwright/test' import { devUser } from 'credentials.js' import path from 'path' -import { wait } from 'payload/shared' import { fileURLToPath } from 'url' import { v4 as uuid } from 'uuid' diff --git a/test/auth/int.spec.ts b/test/auth/int.spec.ts index 5144969f2..9d969dc79 100644 --- a/test/auth/int.spec.ts +++ b/test/auth/int.spec.ts @@ -1,4 +1,4 @@ -import type { Payload, User } from 'payload' +import type { FieldAffectingData, Payload, User } from 'payload' import { jwtDecode } from 'jwt-decode' import path from 'path' @@ -9,7 +9,13 @@ import type { NextRESTClient } from '../helpers/NextRESTClient.js' import { devUser } from '../credentials.js' import { initPayloadInt } from '../helpers/initPayloadInt.js' -import { apiKeysSlug, namedSaveToJWTValue, saveToJWTKey, slug } from './shared.js' +import { + apiKeysSlug, + namedSaveToJWTValue, + partialDisableLocaleStrategiesSlug, + saveToJWTKey, + slug, +} from './shared.js' let restClient: NextRESTClient let payload: Payload @@ -709,6 +715,70 @@ describe('Auth', () => { }) }) + describe('disableLocalStrategy', () => { + it('should allow create of a user with disableLocalStrategy', async () => { + const email = 'test@example.com' + const user = await payload.create({ + collection: partialDisableLocaleStrategiesSlug, + data: { + email, + // password is not required + }, + }) + expect(user.email).toStrictEqual(email) + }) + + it('should retain fields when auth.disableLocalStrategy.enableFields is true', () => { + const authFields = payload.collections[partialDisableLocaleStrategiesSlug].config.fields + // eslint-disable-next-line jest/no-conditional-in-test + .filter((field) => 'name' in field && field.name) + .map((field) => (field as FieldAffectingData).name) + + expect(authFields).toMatchObject([ + 'updatedAt', + 'createdAt', + 'email', + 'resetPasswordToken', + 'resetPasswordExpiration', + 'salt', + 'hash', + 'loginAttempts', + 'lockUntil', + ]) + }) + + it('should prevent login of user with disableLocalStrategy.', async () => { + await payload.create({ + collection: partialDisableLocaleStrategiesSlug, + data: { + email: devUser.email, + password: devUser.password, + }, + }) + + await expect(async () => { + await payload.login({ + collection: partialDisableLocaleStrategiesSlug, + data: { + email: devUser.email, + password: devUser.password, + }, + }) + }).rejects.toThrow('You are not allowed to perform this action.') + }) + + it('rest - should prevent login', async () => { + const response = await restClient.POST(`/${partialDisableLocaleStrategiesSlug}/login`, { + body: JSON.stringify({ + email, + password, + }), + }) + + expect(response.status).toBe(403) + }) + }) + describe('API Key', () => { it('should authenticate via the correct API key user', async () => { const usersQuery = await payload.find({ diff --git a/test/auth/payload-types.ts b/test/auth/payload-types.ts index 57dea6d90..a2cdc14fa 100644 --- a/test/auth/payload-types.ts +++ b/test/auth/payload-types.ts @@ -9,11 +9,13 @@ export interface Config { auth: { users: UserAuthOperations; + 'partial-disable-locale-strategies': PartialDisableLocaleStrategyAuthOperations; 'api-keys': ApiKeyAuthOperations; 'public-users': PublicUserAuthOperations; }; collections: { users: User; + 'partial-disable-locale-strategies': PartialDisableLocaleStrategy; 'api-keys': ApiKey; 'public-users': PublicUser; relationsCollection: RelationsCollection; @@ -24,6 +26,7 @@ export interface Config { collectionsJoins: {}; collectionsSelect: { users: UsersSelect | UsersSelect; + 'partial-disable-locale-strategies': PartialDisableLocaleStrategiesSelect | PartialDisableLocaleStrategiesSelect; 'api-keys': ApiKeysSelect | ApiKeysSelect; 'public-users': PublicUsersSelect | PublicUsersSelect; relationsCollection: RelationsCollectionSelect | RelationsCollectionSelect; @@ -32,7 +35,7 @@ export interface Config { 'payload-migrations': PayloadMigrationsSelect | PayloadMigrationsSelect; }; db: { - defaultIDType: string; + defaultIDType: number; }; globals: {}; globalsSelect: {}; @@ -41,6 +44,9 @@ export interface Config { | (User & { collection: 'users'; }) + | (PartialDisableLocaleStrategy & { + collection: 'partial-disable-locale-strategies'; + }) | (ApiKey & { collection: 'api-keys'; }) @@ -70,6 +76,24 @@ export interface UserAuthOperations { password: string; }; } +export interface PartialDisableLocaleStrategyAuthOperations { + forgotPassword: { + email: string; + password: string; + }; + login: { + email: string; + password: string; + }; + registerFirstUser: { + email: string; + password: string; + }; + unlock: { + email: string; + password: string; + }; +} export interface ApiKeyAuthOperations { forgotPassword: { email: string; @@ -111,7 +135,7 @@ export interface PublicUserAuthOperations { * via the `definition` "users". */ export interface User { - id: string; + id: number; adminOnlyField?: string | null; roles: ('admin' | 'editor' | 'moderator' | 'user' | 'viewer')[]; namedSaveToJWT?: string | null; @@ -146,12 +170,29 @@ export interface User { lockUntil?: string | null; password?: string | null; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "partial-disable-locale-strategies". + */ +export interface PartialDisableLocaleStrategy { + id: number; + updatedAt: string; + createdAt: string; + email: string; + resetPasswordToken?: string | null; + resetPasswordExpiration?: string | null; + salt?: string | null; + hash?: string | null; + loginAttempts?: number | null; + lockUntil?: string | null; + password?: string | null; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "api-keys". */ export interface ApiKey { - id: string; + id: number; updatedAt: string; createdAt: string; enableAPIKey?: boolean | null; @@ -163,7 +204,7 @@ export interface ApiKey { * via the `definition` "public-users". */ export interface PublicUser { - id: string; + id: number; updatedAt: string; createdAt: string; email: string; @@ -182,8 +223,8 @@ export interface PublicUser { * via the `definition` "relationsCollection". */ export interface RelationsCollection { - id: string; - rel?: (string | null) | User; + id: number; + rel?: (number | null) | User; text?: string | null; updatedAt: string; createdAt: string; @@ -193,37 +234,45 @@ export interface RelationsCollection { * via the `definition` "payload-locked-documents". */ export interface PayloadLockedDocument { - id: string; + id: number; document?: | ({ relationTo: 'users'; - value: string | User; + value: number | User; + } | null) + | ({ + relationTo: 'partial-disable-locale-strategies'; + value: number | PartialDisableLocaleStrategy; } | null) | ({ relationTo: 'api-keys'; - value: string | ApiKey; + value: number | ApiKey; } | null) | ({ relationTo: 'public-users'; - value: string | PublicUser; + value: number | PublicUser; } | null) | ({ relationTo: 'relationsCollection'; - value: string | RelationsCollection; + value: number | RelationsCollection; } | null); globalSlug?: string | null; user: | { relationTo: 'users'; - value: string | User; + value: number | User; + } + | { + relationTo: 'partial-disable-locale-strategies'; + value: number | PartialDisableLocaleStrategy; } | { relationTo: 'api-keys'; - value: string | ApiKey; + value: number | ApiKey; } | { relationTo: 'public-users'; - value: string | PublicUser; + value: number | PublicUser; }; updatedAt: string; createdAt: string; @@ -233,19 +282,23 @@ export interface PayloadLockedDocument { * via the `definition` "payload-preferences". */ export interface PayloadPreference { - id: string; + id: number; user: | { relationTo: 'users'; - value: string | User; + value: number | User; + } + | { + relationTo: 'partial-disable-locale-strategies'; + value: number | PartialDisableLocaleStrategy; } | { relationTo: 'api-keys'; - value: string | ApiKey; + value: number | ApiKey; } | { relationTo: 'public-users'; - value: string | PublicUser; + value: number | PublicUser; }; key?: string | null; value?: @@ -265,7 +318,7 @@ export interface PayloadPreference { * via the `definition` "payload-migrations". */ export interface PayloadMigration { - id: string; + id: number; name?: string | null; batch?: number | null; updatedAt: string; @@ -317,6 +370,21 @@ export interface UsersSelect { loginAttempts?: T; lockUntil?: T; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "partial-disable-locale-strategies_select". + */ +export interface PartialDisableLocaleStrategiesSelect { + updatedAt?: T; + createdAt?: T; + email?: T; + resetPasswordToken?: T; + resetPasswordExpiration?: T; + salt?: T; + hash?: T; + loginAttempts?: T; + lockUntil?: T; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "api-keys_select". diff --git a/test/auth/shared.ts b/test/auth/shared.ts index 73c67b3cf..f099f66ee 100644 --- a/test/auth/shared.ts +++ b/test/auth/shared.ts @@ -1,6 +1,8 @@ export const slug = 'users' export const apiKeysSlug = 'api-keys' +export const partialDisableLocaleStrategiesSlug = 'partial-disable-locale-strategies' + export const namedSaveToJWTValue = 'namedSaveToJWT value' export const saveToJWTKey = 'x-custom-jwt-property-name'