diff --git a/package.json b/package.json index 3bfb3aa02a..27dda7899b 100644 --- a/package.json +++ b/package.json @@ -147,7 +147,6 @@ "passport-headerapikey": "^1.2.2", "passport-jwt": "^4.0.1", "passport-local": "^1.0.0", - "passport-local-mongoose": "^7.1.2", "path-browserify": "^1.0.1", "pino": "^6.4.1", "pino-pretty": "^9.1.1", @@ -174,6 +173,7 @@ "sass": "^1.57.1", "sass-loader": "^12.6.0", "scheduler": "^0.23.0", + "scmp": "^2.1.0", "sharp": "^0.31.3", "slate": "^0.91.4", "slate-history": "^0.86.0", @@ -299,4 +299,4 @@ "publishConfig": { "registry": "https://registry.npmjs.org/" } -} +} \ No newline at end of file diff --git a/src/auth/baseFields/auth.ts b/src/auth/baseFields/auth.ts index c17ba9ae78..0415dca2c8 100644 --- a/src/auth/baseFields/auth.ts +++ b/src/auth/baseFields/auth.ts @@ -4,7 +4,7 @@ import { extractTranslations } from '../../translations/extractTranslations'; const labels = extractTranslations(['general:email']); -export default [ +const baseAuthFields: Field[] = [ { name: 'email', label: labels['general:email'], @@ -27,4 +27,16 @@ export default [ type: 'date', hidden: true, }, -] as Field[]; + { + name: 'salt', + type: 'text', + hidden: true, + }, + { + name: 'hash', + type: 'text', + hidden: true, + } +]; + +export default baseAuthFields diff --git a/src/auth/operations/login.ts b/src/auth/operations/login.ts index 1cede21a11..2d6c11d042 100644 --- a/src/auth/operations/login.ts +++ b/src/auth/operations/login.ts @@ -11,6 +11,8 @@ import { User } from '../types'; import { Collection } from '../../collections/config/types'; import { afterRead } from '../../fields/hooks/afterRead'; import unlock from './unlock'; +import { incrementLoginAttempts } from '../strategies/local/incrementLoginAttempts'; +import { authenticateLocalStrategy } from '../strategies/local/authenticate'; export type Result = { user?: User, @@ -77,7 +79,9 @@ async function login( // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore Improper typing in library, additional args should be optional - const userDoc = await Model.findByUsername(email); + const userDoc = await Model.findOne({ email }).lean(); + + let user = JSON.parse(JSON.stringify(userDoc)); if (!userDoc || (args.collection.config.auth.verify && userDoc._verified === false)) { throw new AuthenticationError(req.t); @@ -87,12 +91,21 @@ async function login( throw new LockedAuth(req.t); } - const authResult = await userDoc.authenticate(password); + const authResult = await authenticateLocalStrategy({ password, doc: user }); + + user = sanitizeInternalFields(user); const maxLoginAttemptsEnabled = args.collection.config.auth.maxLoginAttempts > 0; - if (!authResult.user) { - if (maxLoginAttemptsEnabled) await userDoc.incLoginAttempts(); + if (!authResult) { + if (maxLoginAttemptsEnabled) { + await incrementLoginAttempts({ + payload: req.payload, + doc: user, + collection: collectionConfig, + }); + } + throw new AuthenticationError(req.t); } @@ -108,10 +121,6 @@ async function login( }); } - let user = userDoc.toJSON({ virtuals: true }); - user = JSON.parse(JSON.stringify(user)); - user = sanitizeInternalFields(user); - const fieldsToSign = collectionConfig.fields.reduce((signedFields, field: Field) => { const result = { ...signedFields, diff --git a/src/auth/operations/resetPassword.ts b/src/auth/operations/resetPassword.ts index 083abaadca..452288b9ff 100644 --- a/src/auth/operations/resetPassword.ts +++ b/src/auth/operations/resetPassword.ts @@ -6,6 +6,8 @@ import getCookieExpiration from '../../utilities/getCookieExpiration'; import { UserDocument } from '../types'; import { fieldAffectsData } from '../../fields/config/types'; import { PayloadRequest } from '../../express/types'; +import { authenticateLocalStrategy } from '../strategies/local/authenticate'; +import { generatePasswordSaltHash } from '../strategies/local/generatePasswordSaltHash'; export type Result = { token: string @@ -49,14 +51,20 @@ async function resetPassword(args: Arguments): Promise { // Reset Password // ///////////////////////////////////// - const user = await Model.findOne({ + let user = await Model.findOne({ resetPasswordToken: data.token, resetPasswordExpiration: { $gt: Date.now() }, - }) as UserDocument; + }).lean(); + + user = JSON.parse(JSON.stringify(user)); if (!user) throw new APIError('Token is either invalid or has expired.'); - await user.setPassword(data.password); + // TODO: replace this method + const { salt, hash } = await generatePasswordSaltHash({ password: data.password }) + + user.salt = salt; + user.hash = hash; user.resetPasswordExpiration = Date.now(); @@ -64,9 +72,11 @@ async function resetPassword(args: Arguments): Promise { user._verified = true; } - await user.save(); + let doc = await Model.findByIdAndUpdate({ _id: user.id }, user, { new: true }).lean() - await user.authenticate(data.password); + doc = JSON.parse(JSON.stringify(doc)) + + await authenticateLocalStrategy({ password: data.password, doc }); const fieldsToSign = collectionConfig.fields.reduce((signedFields, field) => { if (fieldAffectsData(field) && field.saveToJWT) { diff --git a/src/auth/operations/unlock.ts b/src/auth/operations/unlock.ts index 97b4f05f12..f532c5018d 100644 --- a/src/auth/operations/unlock.ts +++ b/src/auth/operations/unlock.ts @@ -2,6 +2,7 @@ import { APIError } from '../../errors'; import executeAccess from '../executeAccess'; import { Collection } from '../../collections/config/types'; import { PayloadRequest } from '../../express/types'; +import { resetLoginAttempts } from '../strategies/local/resetLoginAttempts'; export type Args = { collection: Collection @@ -46,7 +47,11 @@ async function unlock(args: Args): Promise { if (!user) return null; - await user.resetLoginAttempts(); + await resetLoginAttempts({ + payload: req.payload, + collection: collectionConfig, + doc: user, + }); return true; } diff --git a/src/auth/strategies/local/authenticate.ts b/src/auth/strategies/local/authenticate.ts new file mode 100644 index 0000000000..f551c88066 --- /dev/null +++ b/src/auth/strategies/local/authenticate.ts @@ -0,0 +1,41 @@ +import crypto from 'crypto' +import scmp from 'scmp' +import { TypeWithID } from "../../../collections/config/types" +import { AuthenticationError } from "../../../errors" + +type Doc = TypeWithID & Record + +type Args = { + doc: Doc + password: string +} + +export const authenticateLocalStrategy = async ({ + doc, + password, +}: Args): Promise => { + try { + const salt = doc.salt + const hash = doc.hash + + if (typeof salt === 'string' && typeof hash === 'string') { + const res = await new Promise((resolve, reject) => { + crypto.pbkdf2(password, salt, 25000, 512, 'sha256', (e, hashBuffer) => { + if (e) reject(null) + + if (scmp(hashBuffer, Buffer.from(hash, 'hex'))) { + resolve(doc) + } else { + reject(null) + } + }); + }) + + return res + } + + return null + } catch (err) { + return null + } +} \ No newline at end of file diff --git a/src/auth/strategies/local/generatePasswordSaltHash.ts b/src/auth/strategies/local/generatePasswordSaltHash.ts new file mode 100644 index 0000000000..01ef75ae6b --- /dev/null +++ b/src/auth/strategies/local/generatePasswordSaltHash.ts @@ -0,0 +1,39 @@ +import crypto from 'crypto' +import { ValidationError } from "../../../errors" + +const defaultPasswordValidator = (password: string): string | true => { + if (!password) return 'No password was given' + if (password.length < 3) return 'Password must be at least 3 characters' + + return true +} + +function randomBytes(): Promise { + return new Promise((resolve, reject) => crypto.randomBytes(32, (err, saltBuffer) => (err ? reject(err) : resolve(saltBuffer)))); +} + +function pbkdf2Promisified(password: string, salt: string): Promise { + return new Promise((resolve, reject) => crypto.pbkdf2(password, salt, 25000, 512, 'sha256', (err, hashRaw) => (err ? reject(err) : resolve(hashRaw)))); +} + +type Args = { + password: string +} + +export const generatePasswordSaltHash = async ({ + password, +}: Args): Promise<{ salt: string, hash: string }> => { + const validationResult = defaultPasswordValidator(password) + + if (typeof validationResult === 'string') { + throw new ValidationError([{ message: validationResult, field: 'password' }]) + } + + const saltBuffer = await randomBytes() + const salt = saltBuffer.toString('hex') + + const hashRaw = await pbkdf2Promisified(password, salt) + const hash = hashRaw.toString('hex') + + return { salt, hash } +} \ No newline at end of file diff --git a/src/auth/strategies/local/incrementLoginAttempts.ts b/src/auth/strategies/local/incrementLoginAttempts.ts new file mode 100644 index 0000000000..0e12fbff38 --- /dev/null +++ b/src/auth/strategies/local/incrementLoginAttempts.ts @@ -0,0 +1,56 @@ +import { Payload } from "../../.." +import { SanitizedCollectionConfig, TypeWithID } from "../../../collections/config/types" + +type Args = { + payload: Payload + doc: TypeWithID & Record + collection: SanitizedCollectionConfig +} + +export const incrementLoginAttempts = async ({ + payload, + doc, + collection, +}: Args): Promise => { + const { + auth: { + maxLoginAttempts, + lockTime, + } + } = collection + + if ('lockUntil' in doc && typeof doc.lockUntil === 'string') { + const lockUntil = Math.floor(new Date(doc.lockUntil).getTime() / 1000) + + // Expired lock, restart count at 1 + if (lockUntil < Date.now()) { + await payload.update({ + collection: collection.slug, + id: doc.id, + data: { + loginAttempts: 1, + lockUntil: null, + } + }) + } + + return + } + + const data: Record = { + loginAttempts: Number(doc.loginAttempts) + 1, + } + + // Lock the account if at max attempts and not already locked + if (typeof doc.loginAttempts === 'number' && doc.loginAttempts + 1 >= maxLoginAttempts) { + const lockUntil = new Date((Date.now() + lockTime)) + data.lockUntil = lockUntil + } + + + await payload.update({ + collection: collection.slug, + id: doc.id, + data, + }) +} \ No newline at end of file diff --git a/src/auth/strategies/local/register.ts b/src/auth/strategies/local/register.ts new file mode 100644 index 0000000000..14310d57c0 --- /dev/null +++ b/src/auth/strategies/local/register.ts @@ -0,0 +1,43 @@ +import { ValidationError } from "../../../errors" +import { Payload } from '../../..' +import { SanitizedCollectionConfig } from '../../../collections/config/types' +import { generatePasswordSaltHash } from './generatePasswordSaltHash'; + +type Args = { + collection: SanitizedCollectionConfig + doc: Record + password: string + payload: Payload +} + +export const registerLocalStrategy = async ({ + collection, + doc, + password, + payload, +}: Args): Promise> => { + const existingUser = await payload.find({ + collection: collection.slug, + depth: 0, + where: { + email: { + equals: doc.email, + } + } + }) + + if (existingUser.docs.length > 0) { + throw new ValidationError([{ message: 'A user with the given email is already registered', field: 'email' }]) + } + + const { salt, hash } = await generatePasswordSaltHash({ password }) + + const result = await payload.collections[collection.slug].Model.create({ + ...doc, + salt, + hash + }) + + return result +} + diff --git a/src/auth/strategies/local/resetLoginAttempts.ts b/src/auth/strategies/local/resetLoginAttempts.ts new file mode 100644 index 0000000000..f2d7045c55 --- /dev/null +++ b/src/auth/strategies/local/resetLoginAttempts.ts @@ -0,0 +1,23 @@ +import { Payload } from "../../.." +import { SanitizedCollectionConfig, TypeWithID } from "../../../collections/config/types" + +type Args = { + payload: Payload + doc: TypeWithID & Record + collection: SanitizedCollectionConfig +} + +export const resetLoginAttempts = async ({ + payload, + doc, + collection, +}: Args): Promise => { + await payload.update({ + collection: collection.slug, + id: doc.id, + data: { + loginAttempts: 0, + lockUntil: null, + }, + }) +} \ No newline at end of file diff --git a/src/collections/initLocal.ts b/src/collections/initLocal.ts index 2d606af354..cc65fb66f7 100644 --- a/src/collections/initLocal.ts +++ b/src/collections/initLocal.ts @@ -1,6 +1,5 @@ -import mongoose, { UpdateAggregationStage, UpdateQuery } from 'mongoose'; +import mongoose from 'mongoose'; import paginate from 'mongoose-paginate-v2'; -import passportLocalMongoose from 'passport-local-mongoose'; import mongooseAggregatePaginate from 'mongoose-aggregate-paginate-v2'; import { buildVersionCollectionFields } from '../versions/buildCollectionFields'; import getBuildQueryPlugin from '../mongoose/buildQuery'; @@ -16,52 +15,6 @@ export default function initCollectionsLocal(ctx: Payload): void { const schema = buildCollectionSchema(formattedCollection, ctx.config); - if (collection.auth && !collection.auth.disableLocalStrategy) { - schema.plugin(passportLocalMongoose, { - usernameField: 'email', - errorMessages: { - UserExistsError: 'A user with the given email is already registered', - }, - }); - - - const { maxLoginAttempts, lockTime } = collection.auth; - - if (maxLoginAttempts > 0) { - type LoginSchema = { - loginAttempts: number - lockUntil: number - isLocked: boolean - }; - - // eslint-disable-next-line func-names - schema.methods.incLoginAttempts = function (this: mongoose.Document & LoginSchema, cb) { - // Expired lock, restart count at 1 - if (this.lockUntil && this.lockUntil < Date.now()) { - return this.updateOne({ - $set: { loginAttempts: 1 }, - $unset: { lockUntil: 1 }, - }, cb); - } - - const updates: UpdateQuery = { $inc: { loginAttempts: 1 } }; - // Lock the account if at max attempts and not already locked - if (this.loginAttempts + 1 >= maxLoginAttempts) { - updates.$set = { lockUntil: Date.now() + lockTime }; - } - return this.updateOne(updates as UpdateAggregationStage, cb); - }; - - // eslint-disable-next-line func-names - schema.methods.resetLoginAttempts = function (cb) { - return this.updateOne({ - $set: { loginAttempts: 0 }, - $unset: { lockUntil: 1 }, - }, cb); - }; - } - } - if (collection.versions) { const versionModelName = getVersionsModelName(collection); diff --git a/src/collections/operations/create.ts b/src/collections/operations/create.ts index f1a44d0f8e..101f3299db 100644 --- a/src/collections/operations/create.ts +++ b/src/collections/operations/create.ts @@ -22,6 +22,7 @@ import { afterRead } from '../../fields/hooks/afterRead'; import { generateFileData } from '../../uploads/generateFileData'; import { saveVersion } from '../../versions/saveVersion'; import { mapAsync } from '../../utilities/mapAsync'; +import { registerLocalStrategy } from '../../auth/strategies/local/register'; const unlinkFile = promisify(fs.unlink); @@ -197,15 +198,12 @@ async function create( resultWithLocales._verificationToken = crypto.randomBytes(20).toString('hex'); } - try { - doc = await Model.register(resultWithLocales, data.password as string); - } catch (error) { - // Handle user already exists from passport-local-mongoose - if (error.name === 'UserExistsError') { - throw new ValidationError([{ message: error.message, field: 'email' }], req.t); - } - throw error; - } + doc = await registerLocalStrategy({ + collection: collectionConfig, + doc: resultWithLocales, + payload: req.payload, + password: data.password as string, + }) } else { try { doc = await Model.create(resultWithLocales); diff --git a/src/collections/operations/updateByID.ts b/src/collections/operations/updateByID.ts index c5c1b4ae96..87f9334fcc 100644 --- a/src/collections/operations/updateByID.ts +++ b/src/collections/operations/updateByID.ts @@ -18,6 +18,7 @@ import { generateFileData } from '../../uploads/generateFileData'; import { getLatestCollectionVersion } from '../../versions/getLatestCollectionVersion'; import { deleteAssociatedFiles } from '../../uploads/deleteAssociatedFiles'; import { unlinkTempFiles } from '../../uploads/unlinkTempFiles'; +import { generatePasswordSaltHash } from '../../auth/strategies/local/generatePasswordSaltHash'; export type Arguments = { collection: Collection @@ -223,9 +224,12 @@ async function updateByID( // Handle potential password update // ///////////////////////////////////// - if (shouldSavePassword) { - await doc.setPassword(password); - await doc.save(); + const dataToUpdate: Record = { ...result } + + if (shouldSavePassword && typeof password === 'string') { + const { hash, salt } = await generatePasswordSaltHash({ password }) + dataToUpdate.salt = salt + dataToUpdate.hash = hash delete data.password; delete result.password; } @@ -238,7 +242,7 @@ async function updateByID( try { result = await Model.findByIdAndUpdate( { _id: id }, - result, + dataToUpdate, { new: true }, ); } catch (error) { diff --git a/test/auth/int.spec.ts b/test/auth/int.spec.ts index 852fd4f6ed..adaa1638aa 100644 --- a/test/auth/int.spec.ts +++ b/test/auth/int.spec.ts @@ -290,7 +290,7 @@ describe('Auth', () => { const { loginAttempts, lockUntil } = userResult; expect(loginAttempts).toBe(0); - expect(lockUntil).toBeUndefined(); + expect(lockUntil).toBeNull(); }); }); }); diff --git a/test/collections-graphql/int.spec.ts b/test/collections-graphql/int.spec.ts index c7c94b4c98..09759e79c3 100644 --- a/test/collections-graphql/int.spec.ts +++ b/test/collections-graphql/int.spec.ts @@ -472,10 +472,10 @@ describe('collections-graphql', () => { }); expect(Array.isArray(error.response.errors)).toBe(true); - expect(error.response.errors[0].message).toEqual('No password was given'); + expect(error.response.errors[0].message).toEqual('The following field is invalid: password'); expect(Array.isArray(error.response.errors[0].locations)).toEqual(true); expect(error.response.errors[0].path[0]).toEqual('test2'); - expect(error.response.errors[0].extensions.name).toEqual('MissingPasswordError'); + expect(error.response.errors[0].extensions.name).toEqual('ValidationError'); expect(error.response.errors[1].message).toEqual('The following field is invalid: email'); expect(error.response.errors[1].path[0]).toEqual('test3'); diff --git a/yarn.lock b/yarn.lock index c7f4221e51..fc4a2f71e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10650,7 +10650,7 @@ schema-utils@^4.0.0: scmp@^2.1.0: version "2.1.0" - resolved "https://registry.yarnpkg.com/scmp/-/scmp-2.1.0.tgz#37b8e197c425bdeb570ab91cc356b311a11f9c9a" + resolved "https://registry.npmjs.org/scmp/-/scmp-2.1.0.tgz#37b8e197c425bdeb570ab91cc356b311a11f9c9a" integrity sha512-o/mRQGk9Rcer/jEEw/yw4mwo3EU/NvYvp577/Btqrym9Qy5/MdWGBqipbALgd2lrdWTJ5/gqDusxfnQBxOxT2Q== scroll-into-view-if-needed@^2.2.20: