From 83f77d3a05d246262dba7b25af7e04ea25c9a763 Mon Sep 17 00:00:00 2001 From: Elliot DeNolf Date: Sat, 26 Sep 2020 15:48:08 -0400 Subject: [PATCH] some adjustments to how locking and verification work --- demo/collections/Admin.js | 2 ++ demo/collections/PublicUsers.js | 2 ++ demo/payload.config.js | 2 -- payload.d.ts | 4 ++-- src/auth/operations/login.js | 9 +++++---- .../components/views/collections/Edit/Auth/index.js | 11 ++++++++++- .../components/views/collections/Edit/Default.js | 2 ++ src/collections/init.js | 9 ++++----- src/collections/sanitize.js | 8 +++++++- src/express/middleware/index.js | 5 +++++ src/index.js | 9 ++------- src/utilities/sanitizeConfig.js | 3 --- 12 files changed, 41 insertions(+), 25 deletions(-) diff --git a/demo/collections/Admin.js b/demo/collections/Admin.js index a2f3c53d08..77438c87cc 100644 --- a/demo/collections/Admin.js +++ b/demo/collections/Admin.js @@ -22,6 +22,8 @@ module.exports = { auth: { tokenExpiration: 7200, emailVerification: false, + maxLoginAttempts: 5, + lockTime: 600 * 1000, // lock time in ms useAPIKey: true, cookies: { secure: process.env.NODE_ENV === 'production', diff --git a/demo/collections/PublicUsers.js b/demo/collections/PublicUsers.js index b0b5f441e8..56c00ab21f 100644 --- a/demo/collections/PublicUsers.js +++ b/demo/collections/PublicUsers.js @@ -33,6 +33,8 @@ module.exports = { auth: { tokenExpiration: 300, emailVerification: true, + maxLoginAttempts: 5, + lockTime: 600 * 1000, // lock time in ms generateVerificationUrl: (req, token) => `http://localhost:3000/api/verify?token=${token}`, cookies: { secure: process.env.NODE_ENV === 'production', diff --git a/demo/payload.config.js b/demo/payload.config.js index 9737d8dc4b..a9c722b088 100644 --- a/demo/payload.config.js +++ b/demo/payload.config.js @@ -29,8 +29,6 @@ module.exports = { admin: { user: 'admins', // indexHTML: 'custom-index.html', - maxLoginAttempts: 3, - lockTime: 600 * 1000, // lock time in ms meta: { titleSuffix: '- Payload Demo', // ogImage: '/static/find-image-here.jpg', diff --git a/payload.d.ts b/payload.d.ts index ac9920137b..3a8ae7ee80 100644 --- a/payload.d.ts +++ b/payload.d.ts @@ -143,6 +143,8 @@ declare module "@payloadcms/payload/types" { auth?: { tokenExpiration?: number; emailVerification?: boolean; + maxLoginAttempts?: number; + lockTime?: number; useAPIKey?: boolean; cookies?: { secure?: boolean; @@ -169,8 +171,6 @@ declare module "@payloadcms/payload/types" { export interface PayloadConfig { admin?: { user?: string; - maxLoginAttempts?: number; - lockTime?: number; meta?: { titleSuffix?: string; }, diff --git a/src/auth/operations/login.js b/src/auth/operations/login.js index 5183e1a2bf..1eae6170c0 100644 --- a/src/auth/operations/login.js +++ b/src/auth/operations/login.js @@ -40,13 +40,14 @@ async function login(args) { } const authResult = await userDoc.authenticate(password); - if (authResult.user) { - await authResult.user.resetLoginAttempts(); - } else { - await userDoc.incLoginAttempts(); + const maxLoginAttemptsEnabled = args.collection.config.auth.maxLoginAttempts > 0; + if (!authResult.user) { + if (maxLoginAttemptsEnabled) await userDoc.incLoginAttempts(); throw new AuthenticationError(); } + if (maxLoginAttemptsEnabled) await authResult.user.resetLoginAttempts(); + const userQuery = await operations.collections.find({ where: { email: { diff --git a/src/client/components/views/collections/Edit/Auth/index.js b/src/client/components/views/collections/Edit/Auth/index.js index 7cebcaccec..79bc5fbaf8 100644 --- a/src/client/components/views/collections/Edit/Auth/index.js +++ b/src/client/components/views/collections/Edit/Auth/index.js @@ -13,7 +13,7 @@ import './index.scss'; const baseClass = 'auth-fields'; const Auth = (props) => { - const { useAPIKey, requirePassword } = props; + const { useAPIKey, requirePassword, emailVerification } = props; const [changingPassword, setChangingPassword] = useState(requirePassword); const { getField } = useFormFields(); const modified = useFormModified(); @@ -74,6 +74,13 @@ const Auth = (props) => { )} )} + {emailVerification && ( + + )} ); }; @@ -81,11 +88,13 @@ const Auth = (props) => { Auth.defaultProps = { useAPIKey: false, requirePassword: false, + emailVerification: false, }; Auth.propTypes = { useAPIKey: PropTypes.bool, requirePassword: PropTypes.bool, + emailVerification: PropTypes.bool, }; export default Auth; diff --git a/src/client/components/views/collections/Edit/Default.js b/src/client/components/views/collections/Edit/Default.js index 0db135560b..9d11121704 100644 --- a/src/client/components/views/collections/Edit/Default.js +++ b/src/client/components/views/collections/Edit/Default.js @@ -91,6 +91,7 @@ const DefaultEditView = (props) => { )} {upload && ( @@ -233,6 +234,7 @@ DefaultEditView.propTypes = { timestamps: PropTypes.bool, auth: PropTypes.shape({ useAPIKey: PropTypes.bool, + emailVerification: PropTypes.bool, }), upload: PropTypes.shape({}), }).isRequired, diff --git a/src/collections/init.js b/src/collections/init.js index 38072d339c..238843f393 100644 --- a/src/collections/init.js +++ b/src/collections/init.js @@ -24,15 +24,14 @@ function registerCollections() { usernameField: 'email', }); - // Check if collection is the admin user set in config - if (collection.slug === this.config.admin.user) { + + const { maxLoginAttempts, lockTime } = collection.auth; + + if (maxLoginAttempts > 0) { schema.add({ loginAttempts: { type: Number, hide: true, default: 0 } }); schema.add({ lockUntil: { type: Date, hide: true } }); - schema.virtual('isLocked').get(() => !!(this.lockUntil && this.lockUntil > Date.now())); - const { maxLoginAttempts, lockTime } = this.config.admin; - // eslint-disable-next-line func-names schema.methods.incLoginAttempts = function (cb) { // Expired lock, restart count at 1 diff --git a/src/collections/sanitize.js b/src/collections/sanitize.js index 8f69679794..36b677fe12 100644 --- a/src/collections/sanitize.js +++ b/src/collections/sanitize.js @@ -241,13 +241,19 @@ const sanitizeCollection = (collections, collection) => { authFields.push({ name: '_verified', type: 'checkbox', - hidden: true, + access: { + create: () => false, + update: () => false, + }, admin: { disabled: true, }, }); } + sanitized.auth.maxLoginAttempts = typeof sanitized.auth.maxLoginAttempts === 'undefined' ? 5 : sanitized.auth.maxLoginAttempts; + sanitized.auth.lockTime = sanitized.auth.lockTime || 600000; // 10 minutes + if (!sanitized.auth.tokenExpiration) sanitized.auth.tokenExpiration = 7200; if (!sanitized.auth.cookies) sanitized.auth.cookies = {}; diff --git a/src/express/middleware/index.js b/src/express/middleware/index.js index ee47ca2642..5e3b895f7d 100644 --- a/src/express/middleware/index.js +++ b/src/express/middleware/index.js @@ -5,11 +5,16 @@ const bodyParser = require('body-parser'); const methodOverride = require('method-override'); const qsMiddleware = require('qs-middleware'); const fileUpload = require('express-fileupload'); +const rateLimit = require('express-rate-limit'); const localizationMiddleware = require('../../localization/middleware'); const authenticate = require('./authenticate'); const identifyAPI = require('./identifyAPI'); const middleware = (payload) => [ + rateLimit({ + windowMs: payload.config.rateLimit.window, + max: payload.config.rateLimit.max, + }), passport.initialize(), identifyAPI('REST'), authenticate(payload.config), diff --git a/src/index.js b/src/index.js index 7ee056f4b9..86450a9bf1 100644 --- a/src/index.js +++ b/src/index.js @@ -3,7 +3,6 @@ require('isomorphic-fetch'); const express = require('express'); const graphQLPlayground = require('graphql-playground-middleware-express').default; -const rateLimit = require('express-rate-limit'); const logger = require('./utilities/logger')(); const bindOperations = require('./init/bindOperations'); const bindRequestHandlers = require('./init/bindRequestHandlers'); @@ -99,12 +98,8 @@ class Payload { }, })); - const apiLimiter = rateLimit({ - windowMs: this.config.rateLimit.window, - max: this.config.rateLimit.max, - }); - // Bind router to API and add rate limiter - this.express.use(this.config.routes.api, apiLimiter, this.router); + // Bind router to API + this.express.use(this.config.routes.api, this.router); // Enable static routes for all collections permitting upload this.initStatic(); diff --git a/src/utilities/sanitizeConfig.js b/src/utilities/sanitizeConfig.js index 72955a936f..ad17eda3b5 100644 --- a/src/utilities/sanitizeConfig.js +++ b/src/utilities/sanitizeConfig.js @@ -29,9 +29,6 @@ const sanitizeConfig = (config) => { sanitizedConfig.collections.push(defaultUser); } - sanitizedConfig.maxLoginAttempts = sanitizedConfig.maxLoginAttempts || 3; - sanitizedConfig.lockTime = sanitizedConfig.lockTime || 600000; // 10 minutes - sanitizedConfig.email = config.email || {}; sanitizedConfig.email.fromName = sanitizedConfig.email.fromName || 'Payload'; sanitizedConfig.email.fromAddress = sanitizedConfig.email.fromName || 'hello@payloadcms.com';