From bbc37c02021e33b8a3ec3bc1a7cf9cd14da4dc98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Salgado?= Date: Fri, 16 Apr 2021 05:53:22 +0100 Subject: [PATCH] fix: multiple small fixes across auth --- lib/new-admin/admin-server.js | 5 +- .../graphql/modules/authentication.js | 74 +++++++++++-------- .../graphql/resolvers/users.resolver.js | 14 ++-- lib/new-admin/graphql/types/users.type.js | 2 +- lib/new-admin/services/login.js | 2 +- lib/users.js | 44 +++-------- .../src/components/inputs/base/CodeInput.js | 10 ++- .../inputs/base/CodeInput.styles.js | 10 +-- .../src/components/layout/Header.js | 20 +---- .../src/components/typography/styles.js | 9 ++- new-lamassu-admin/src/lamassu/App.js | 64 ++++++++++------ .../src/pages/Authentication/Input2FAState.js | 53 ++++++------- .../src/pages/Authentication/LoginCard.js | 71 ++++-------------- .../src/pages/Authentication/LoginState.js | 53 +++++++------ .../src/pages/Authentication/Register.js | 41 +++++----- .../src/pages/Authentication/Setup2FAState.js | 45 +++++++---- .../src/pages/Authentication/states.js | 7 ++ .../SessionManagement/SessionManagement.js | 5 +- new-lamassu-admin/src/routing/routes.js | 24 +----- new-lamassu-admin/src/routing/utils.js | 11 ++- new-lamassu-admin/src/styling/variables.js | 2 + new-lamassu-admin/src/utils/apollo.js | 21 +++++- 22 files changed, 296 insertions(+), 291 deletions(-) create mode 100644 new-lamassu-admin/src/pages/Authentication/states.js diff --git a/lib/new-admin/admin-server.js b/lib/new-admin/admin-server.js index e28c0795..c2d6f69b 100644 --- a/lib/new-admin/admin-server.js +++ b/lib/new-admin/admin-server.js @@ -51,7 +51,7 @@ const apolloServer = new ApolloServer({ console.log(error) return error }, - context: async ({ req }) => { + context: async ({ req, res }) => { if (!req.session.user) return { req } const user = await users.verifyAndUpdateUser( @@ -67,6 +67,9 @@ const apolloServer = new ApolloServer({ req.session.user.id = user.id req.session.user.role = user.role + res.set('role', user.role) + res.set('Access-Control-Expose-Headers', 'role') + return { req } } }) diff --git a/lib/new-admin/graphql/modules/authentication.js b/lib/new-admin/graphql/modules/authentication.js index f54ff0d9..cc9c55d5 100644 --- a/lib/new-admin/graphql/modules/authentication.js +++ b/lib/new-admin/graphql/modules/authentication.js @@ -21,8 +21,9 @@ function authenticateUser(username, password) { if (!isMatch) throw new authErrors.InvalidCredentialsError() return loginHelper.validateUser(username, hashedPassword) }) - .catch(e => { - console.error(e) + .then(user => { + if (!user) throw new authErrors.InvalidCredentialsError() + return user }) } @@ -70,7 +71,7 @@ const validateRegisterLink = token => { const validateResetPasswordLink = token => { if (!token) throw new authErrors.InvalidUrlError() - return users.validatePasswordResetToken(token) + return users.validateAuthToken(token, 'reset_password') .then(r => { if (!r.success) throw new authErrors.InvalidUrlError() return { id: r.userID } @@ -80,7 +81,7 @@ const validateResetPasswordLink = token => { const validateReset2FALink = token => { if (!token) throw new authErrors.InvalidUrlError() - return users.validate2FAResetToken(token) + return users.validateAuthToken(token, 'reset_twofa') .then(r => { if (!r.success) throw new authErrors.InvalidUrlError() return users.getUserById(r.userID) @@ -111,10 +112,12 @@ const login = (username, password) => { } const input2FA = (username, password, rememberMe, code, context) => { - return authenticateUser(username, password).then(user => { - if (!user) throw new authErrors.InvalidCredentialsError() - - return users.getUserById(user.id).then(user => { + return authenticateUser(username, password) + .then(user => { + if (!user) throw new authErrors.InvalidCredentialsError() + return users.getUserById(user.id) + }) + .then(user => { const secret = user.twofa_code const isCodeValid = otplib.authenticator.verify({ token: code, secret: secret }) if (!isCodeValid) throw new authErrors.InvalidTwoFactorError() @@ -125,25 +128,32 @@ const input2FA = (username, password, rememberMe, code, context) => { return true }) - }) } -const setup2FA = (username, password, secret, codeConfirmation) => { - return authenticateUser(username, password).then(user => { - if (!user || !secret) throw new authErrors.InvalidCredentialsError() +const setup2FA = (username, password, rememberMe, secret, codeConfirmation, context) => { + return authenticateUser(username, password) + .then(user => { + if (!user || !secret) throw new authErrors.InvalidCredentialsError() - const isCodeValid = otplib.authenticator.verify({ token: codeConfirmation, secret: secret }) - if (!isCodeValid) throw new authErrors.InvalidTwoFactorError() + const isCodeValid = otplib.authenticator.verify({ token: codeConfirmation, secret: secret }) + if (!isCodeValid) throw new authErrors.InvalidTwoFactorError() - return users.save2FASecret(user.id, secret).then(() => true) - }) + return users.getUserById(user.id) + }) + .then(user => { + const finalUser = { id: user.id, username: user.username, role: user.role } + context.req.session.user = finalUser + if (rememberMe) context.req.session.cookie.maxAge = REMEMBER_ME_AGE + + return true + }) } const createResetPasswordToken = userID => { return users.getUserById(userID) .then(user => { if (!user) throw new authErrors.InvalidCredentialsError() - return users.createResetPasswordToken(user.id) + return users.createAuthToken(user.id, 'reset_password') }) .catch(err => console.error(err)) } @@ -152,7 +162,7 @@ const createReset2FAToken = userID => { return users.getUserById(userID) .then(user => { if (!user) throw new authErrors.InvalidCredentialsError() - return users.createReset2FAToken(user.id) + return users.createAuthToken(user.id, 'reset_twofa') }) .catch(err => console.error(err)) } @@ -177,20 +187,26 @@ const register = (token, username, password, role) => { } const resetPassword = (token, userID, newPassword, context) => { - return users.getUserById(userID).then(user => { - if (!user) throw new authErrors.InvalidCredentialsError() - if (context.req.session.user && user.id === context.req.session.user.id) context.req.session.destroy() - return users.updatePassword(token, user.id, newPassword) - }).then(() => true).catch(err => console.error(err)) + return users.getUserById(userID) + .then(user => { + if (!user) throw new authErrors.InvalidCredentialsError() + if (context.req.session.user && user.id === context.req.session.user.id) context.req.session.destroy() + return users.updatePassword(token, user.id, newPassword) + }) + .then(() => true) + .catch(err => console.error(err)) } const reset2FA = (token, userID, code, secret, context) => { - return users.getUserById(userID).then(user => { - const isCodeValid = otplib.authenticator.verify({ token: code, secret }) - if (!isCodeValid) throw new authErrors.InvalidTwoFactorError() - if (context.req.session.user && user.id === context.req.session.user.id) context.req.session.destroy() - return users.reset2FASecret(token, user.id, secret).then(() => true) - }).catch(err => console.error(err)) + const isCodeValid = otplib.authenticator.verify({ token: code, secret }) + if (!isCodeValid) throw new authErrors.InvalidTwoFactorError() + + return users.getUserById(userID) + .then(user => { + if (context.req.session.user && user.id === context.req.session.user.id) context.req.session.destroy() + return users.reset2FASecret(token, user.id, secret).then(() => true) + }) + .catch(err => console.error(err)) } module.exports = { diff --git a/lib/new-admin/graphql/resolvers/users.resolver.js b/lib/new-admin/graphql/resolvers/users.resolver.js index ad7fa0a7..2f344d58 100644 --- a/lib/new-admin/graphql/resolvers/users.resolver.js +++ b/lib/new-admin/graphql/resolvers/users.resolver.js @@ -7,9 +7,9 @@ const resolver = { users: () => users.getUsers(), sessions: () => sessionManager.getSessions(), userSessions: (...[, { username }]) => sessionManager.getSessionsByUsername(username), - userData: (root, args, context, info) => authentication.getUserData(context), + userData: (...[, {}, context]) => authentication.getUserData(context), get2FASecret: (...[, { username, password }]) => authentication.get2FASecret(username, password), - confirm2FA: (root, args, context, info) => authentication.confirm2FA(args.code, context), + confirm2FA: (...[, { code }, context]) => authentication.confirm2FA(code, context), validateRegisterLink: (...[, { token }]) => authentication.validateRegisterLink(token), validateResetPasswordLink: (...[, { token }]) => authentication.validateResetPasswordLink(token), validateReset2FALink: (...[, { token }]) => authentication.validateReset2FALink(token) @@ -17,18 +17,18 @@ const resolver = { Mutation: { enableUser: (...[, { id }]) => users.enableUser(id), disableUser: (...[, { id }]) => users.disableUser(id), - deleteSession: (root, args, context, info) => authentication.deleteSession(args.sid, context), + deleteSession: (...[, { sid }, context]) => authentication.deleteSession(sid, context), deleteUserSessions: (...[, { username }]) => sessionManager.deleteSessionsByUsername(username), changeUserRole: (...[, { id, newRole }]) => users.changeUserRole(id, newRole), login: (...[, { username, password }]) => authentication.login(username, password), - input2FA: (root, args, context, info) => authentication.input2FA(args.username, args.password, args.rememberMe, args.code, context), - setup2FA: (...[, { username, password, secret, codeConfirmation }]) => authentication.setup2FA(username, password, secret, codeConfirmation), + input2FA: (...[, { username, password, rememberMe, code }, context]) => authentication.input2FA(username, password, rememberMe, code, context), + setup2FA: (...[, { username, password, rememberMe, secret, codeConfirmation }, context]) => authentication.setup2FA(username, password, rememberMe, secret, codeConfirmation, context), createResetPasswordToken: (...[, { userID }]) => authentication.createResetPasswordToken(userID), createReset2FAToken: (...[, { userID }]) => authentication.createReset2FAToken(userID), createRegisterToken: (...[, { username, role }]) => authentication.createRegisterToken(username, role), register: (...[, { token, username, password, role }]) => authentication.register(token, username, password, role), - resetPassword: (root, args, context, info) => authentication.resetPassword(args.token, args.userID, args.newPassword, context), - reset2FA: (root, args, context, info) => authentication.reset2FA(args.token, args.userID, args.code, args.secret, context) + resetPassword: (...[, { token, userID, newPassword }, context]) => authentication.resetPassword(token, userID, newPassword, context), + reset2FA: (...[, { token, userID, code, secret }, context]) => authentication.reset2FA(token, userID, code, secret, context) } } diff --git a/lib/new-admin/graphql/types/users.type.js b/lib/new-admin/graphql/types/users.type.js index 9ccc5a59..fc92683f 100644 --- a/lib/new-admin/graphql/types/users.type.js +++ b/lib/new-admin/graphql/types/users.type.js @@ -65,7 +65,7 @@ const typeDef = ` toggleUserEnable(id: ID!): User @auth(requires: [SUPERUSER]) login(username: String!, password: String!): String input2FA(username: String!, password: String!, code: String!, rememberMe: Boolean!): Boolean - setup2FA(username: String!, password: String!, secret: String!, codeConfirmation: String!): Boolean + setup2FA(username: String!, password: String!, rememberMe: Boolean!, secret: String!, codeConfirmation: String!): Boolean createResetPasswordToken(userID: ID!): ResetToken @auth(requires: [SUPERUSER]) createReset2FAToken(userID: ID!): ResetToken @auth(requires: [SUPERUSER]) createRegisterToken(username: String!, role: String!): RegistrationToken @auth(requires: [SUPERUSER]) diff --git a/lib/new-admin/services/login.js b/lib/new-admin/services/login.js index 2c8ca03e..6d43462a 100644 --- a/lib/new-admin/services/login.js +++ b/lib/new-admin/services/login.js @@ -4,7 +4,7 @@ function validateUser (username, password) { const sql = 'SELECT id, username FROM users WHERE username=$1 AND password=$2' const sqlUpdateLastAccessed = 'UPDATE users SET last_accessed = now() WHERE username=$1' - return db.oneOrNone(sql, [username, password]) + return db.one(sql, [username, password]) .then(user => { return db.none(sqlUpdateLastAccessed, [user.username]) .then(() => user) diff --git a/lib/users.js b/lib/users.js index bd323027..bfb29c48 100644 --- a/lib/users.js +++ b/lib/users.js @@ -72,16 +72,16 @@ function save2FASecret (id, secret) { }) } -function validate2FAResetToken (token) { +function validateAuthToken (token, type) { const sql = `SELECT user_id, now() < expire AS success FROM auth_tokens - WHERE token=$1 AND type='reset_twofa'` + WHERE token=$1 AND type=$2` - return db.one(sql, [token]) + return db.one(sql, [token, type]) .then(res => ({ userID: res.user_id, success: res.success })) } function reset2FASecret (token, id, secret) { - return validate2FAResetToken(token).then(res => { + return validateAuthToken(token, 'reset_twofa').then(res => { if (!res.success) throw new Error('Failed to verify 2FA reset token') return db.tx(t => { const q1 = t.none('UPDATE users SET twofa_code=$1 WHERE id=$2', [secret, id]) @@ -92,23 +92,15 @@ function reset2FASecret (token, id, secret) { }) } -function createReset2FAToken (userID) { +function createAuthToken (userID, type) { const token = crypto.randomBytes(32).toString('hex') - const sql = `INSERT INTO auth_tokens (token, type, user_id) VALUES ($1, 'reset_twofa', $2) ON CONFLICT (user_id, type) DO UPDATE SET token=$1, expire=now() + interval '30 minutes' RETURNING *` + const sql = `INSERT INTO auth_tokens (token, type, user_id) VALUES ($1, $2, $3) ON CONFLICT (user_id, type) DO UPDATE SET token=$1, expire=now() + interval '30 minutes' RETURNING *` - return db.one(sql, [token, userID]) -} - -function validatePasswordResetToken (token) { - const sql = `SELECT user_id, now() < expire AS success FROM auth_tokens - WHERE token=$1 AND type='reset_password'` - - return db.one(sql, [token]) - .then(res => ({ userID: res.user_id, success: res.success })) + return db.one(sql, [token, type, userID]) } function updatePassword (token, id, password) { - return validatePasswordResetToken(token).then(res => { + return validateAuthToken(token, 'reset_password').then(res => { if (!res.success) throw new Error('Failed to verify password reset token') return bcrypt.hash(password, 12).then(function (hash) { return db.tx(t => { @@ -121,13 +113,6 @@ function updatePassword (token, id, password) { }) } -function createResetPasswordToken (userID) { - const token = crypto.randomBytes(32).toString('hex') - const sql = `INSERT INTO auth_tokens (token, type, user_id) VALUES ($1, 'reset_password', $2) ON CONFLICT (user_id, type) DO UPDATE SET token=$1, expire=now() + interval '30 minutes' RETURNING *` - - return db.one(sql, [token, userID]) -} - function createUserRegistrationToken (username, role) { const token = crypto.randomBytes(32).toString('hex') const sql = `INSERT INTO user_register_tokens (token, username, role) VALUES ($1, $2, $3) ON CONFLICT (username) @@ -162,11 +147,8 @@ function changeUserRole (id, newRole) { } function enableUser (id) { - return db.tx(t => { - const q1 = t.none(`UPDATE users SET enabled=true WHERE id=$1`, [id]) - const q2 = t.none(`DELETE FROM user_sessions WHERE sess -> 'user' ->> 'id'=$1`, [id]) - return t.batch([q1, q2]) - }) + const sql = `UPDATE users SET enabled=true WHERE id=$1` + return db.none(sql, [id]) } function disableUser (id) { @@ -187,10 +169,8 @@ module.exports = { updatePassword, save2FASecret, reset2FASecret, - validate2FAResetToken, - createReset2FAToken, - validatePasswordResetToken, - createResetPasswordToken, + validateAuthToken, + createAuthToken, createUserRegistrationToken, validateUserRegistrationToken, register, diff --git a/new-lamassu-admin/src/components/inputs/base/CodeInput.js b/new-lamassu-admin/src/components/inputs/base/CodeInput.js index 88847a74..a2d0f09d 100644 --- a/new-lamassu-admin/src/components/inputs/base/CodeInput.js +++ b/new-lamassu-admin/src/components/inputs/base/CodeInput.js @@ -3,9 +3,12 @@ import classnames from 'classnames' import React from 'react' import OtpInput from 'react-otp-input' +import typographyStyles from 'src/components/typography/styles' + import styles from './CodeInput.styles' const useStyles = makeStyles(styles) +const useTypographyStyles = makeStyles(typographyStyles) const CodeInput = ({ name, @@ -18,6 +21,7 @@ const CodeInput = ({ ...props }) => { const classes = useStyles() + const typographyClasses = useTypographyStyles() return ( } containerStyle={classnames(containerStyle, classes.container)} - inputStyle={classnames(inputStyle, classes.input)} + inputStyle={classnames( + inputStyle, + classes.input, + typographyClasses.confirmationCode + )} focusStyle={classes.focus} errorStyle={classes.error} hasErrored={error} diff --git a/new-lamassu-admin/src/components/inputs/base/CodeInput.styles.js b/new-lamassu-admin/src/components/inputs/base/CodeInput.styles.js index f1bee93f..189add4c 100644 --- a/new-lamassu-admin/src/components/inputs/base/CodeInput.styles.js +++ b/new-lamassu-admin/src/components/inputs/base/CodeInput.styles.js @@ -1,17 +1,9 @@ -import { - fontPrimary, - primaryColor, - zircon, - errorColor -} from 'src/styling/variables' +import { primaryColor, zircon, errorColor } from 'src/styling/variables' const styles = { input: { width: '3.5rem !important', height: '5rem', - fontFamily: fontPrimary, - fontSize: 35, - color: primaryColor, border: '2px solid', borderColor: zircon, borderRadius: '4px' diff --git a/new-lamassu-admin/src/components/layout/Header.js b/new-lamassu-admin/src/components/layout/Header.js index d1716e1a..1620a36e 100644 --- a/new-lamassu-admin/src/components/layout/Header.js +++ b/new-lamassu-admin/src/components/layout/Header.js @@ -36,15 +36,7 @@ const Subheader = ({ item, classes, user }) => {