lamassu/lamassu-server
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: security flaw on auth tokens, error handling

Browse source
This commit is contained in:
Sérgio Salgado 2021-04-06 19:12:36 +01:00 • committed by Josh Harvey
parent 40974dd501
commit c00249586d
12 changed files with 185 additions and 144 deletions
Download patch file Download diff file Expand all files Collapse all files

39
lib/new-admin/graphql/modules/authentication.js
View file

@ -10,8 +10,9 @@ const authErrors = require('../errors/authentication')
const REMEMBER_ME_AGE = 90 * T.day
function authenticateUser(username, password) {
return loginHelper.checkUser(username)
.then(hashedPassword => {
return users.getUserByUsername(username)
.then(user => {
const hashedPassword = user.password
if (!hashedPassword) throw new authErrors.InvalidCredentialsError()
return Promise.all([bcrypt.compare(password, hashedPassword), hashedPassword])
})
@ -47,7 +48,7 @@ const confirm2FA = (token, context) => {
if (!requestingUser) throw new authErrors.InvalidCredentialsError()
return users.get2FASecret(requestingUser.id).then(user => {
return users.getUserById(requestingUser.id).then(user => {
const secret = user.twofa_code
const isCodeValid = otplib.authenticator.verify({ token, secret })
@ -81,7 +82,7 @@ const validateReset2FALink = token => {
return users.validate2FAResetToken(token)
.then(r => {
if (!r.success) throw new authErrors.InvalidUrlError()
return users.findById(r.userID)
return users.getUserById(r.userID)
})
.then(user => {
const secret = otplib.authenticator.generateSecret()
@ -101,7 +102,7 @@ const deleteSession = (sessionID, context) => {
const login = (username, password) => {
return authenticateUser(username, password).then(user => {
if (!user) throw new authErrors.InvalidCredentialsError()
return users.get2FASecret(user.id).then(user => {
return users.getUserById(user.id).then(user => {
const twoFASecret = user.twofa_code
return twoFASecret ? 'INPUT2FA' : 'SETUP2FA'
})
@ -112,7 +113,7 @@ const input2FA = (username, password, rememberMe, code, context) => {
return authenticateUser(username, password).then(user => {
if (!user) throw new authErrors.InvalidCredentialsError()
return users.get2FASecret(user.id).then(user => {
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()
@ -138,7 +139,7 @@ const setup2FA = (username, password, secret, codeConfirmation) => {
}
const createResetPasswordToken = userID => {
return users.findById(userID)
return users.getUserById(userID)
.then(user => {
if (!user) throw new authErrors.InvalidCredentialsError()
return users.createResetPasswordToken(user.id)
@ -147,7 +148,7 @@ const createResetPasswordToken = userID => {
}
const createReset2FAToken = userID => {
return users.findById(userID)
return users.getUserById(userID)
.then(user => {
if (!user) throw new authErrors.InvalidCredentialsError()
return users.createReset2FAToken(user.id)
@ -156,7 +157,7 @@ const createReset2FAToken = userID => {
}
const createRegisterToken = (username, role) => {
return users.getByName(username)
return users.getUserByUsername(username)
.then(user => {
if (user) throw new authErrors.UserAlreadyExistsError()
@ -165,29 +166,29 @@ const createRegisterToken = (username, role) => {
.catch(err => console.error(err))
}
const register = (username, password, role) => {
return users.getByName(username)
const register = (token, username, password, role) => {
return users.getUserByUsername(username)
.then(user => {
if (user) throw new authErrors.UserAlreadyExistsError()
return users.createUser(username, password, role).then(() => true)
return users.register(token, username, password, role).then(() => true)
})
.catch(err => console.error(err))
}
const resetPassword = (userID, newPassword, context) => {
return users.findById(userID).then(user => {
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(user.id, newPassword)
return users.updatePassword(token, user.id, newPassword)
}).then(() => true).catch(err => console.error(err))
}
const reset2FA = (userID, token, secret, context) => {
return users.findById(userID).then(user => {
const isCodeValid = otplib.authenticator.verify({ token, secret })
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.save2FASecret(user.id, secret).then(() => true)
return users.reset2FASecret(token, user.id, secret).then(() => true)
}).catch(err => console.error(err))
}

10
lib/new-admin/graphql/resolvers/users.resolver.js
View file

@ -15,20 +15,20 @@ const resolver = {
validateReset2FALink: (...[, { token }]) => authentication.validateReset2FALink(token)
},
Mutation: {
deleteUser: (...[, { id }]) => users.deleteUser(id),
enableUser: (...[, { id }]) => users.enableUser(id),
disableUser: (...[, { id }]) => users.disableUser(id),
deleteSession: (root, args, context, info) => authentication.deleteSession(args.sid, context),
deleteUserSessions: (...[, { username }]) => sessionManager.deleteSessionsByUsername(username),
changeUserRole: (...[, { id, newRole }]) => users.changeUserRole(id, newRole),
toggleUserEnable: (...[, { id }]) => users.toggleUserEnable(id),
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),
createResetPasswordToken: (...[, { userID }]) => authentication.createResetPasswordToken(userID),
createReset2FAToken: (...[, { userID }]) => authentication.createReset2FAToken(userID),
createRegisterToken: (...[, { username, role }]) => authentication.createRegisterToken(username, role),
register: (...[, { username, password, role }]) => authentication.register(username, password, role),
resetPassword: (root, args, context, info) => authentication.resetPassword(args.userID, args.newPassword, context),
reset2FA: (root, args, context, info) => authentication.reset2FA(args.userID, args.code, args.secret, context)
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)
}
}

9
lib/new-admin/graphql/types/users.type.js
View file

@ -57,7 +57,8 @@ const typeDef = `
}
type Mutation {
deleteUser(id: ID!): User @auth(requires: [SUPERUSER])
enableUser(id: ID!): User @auth(requires: [SUPERUSER])
disableUser(id: ID!): User @auth(requires: [SUPERUSER])
deleteSession(sid: String!): UserSession @auth(requires: [SUPERUSER])
deleteUserSessions(username: String!): [UserSession] @auth(requires: [SUPERUSER])
changeUserRole(id: ID!, newRole: String!): User @auth(requires: [SUPERUSER])
@ -68,9 +69,9 @@ const typeDef = `
createResetPasswordToken(userID: ID!): ResetToken @auth(requires: [SUPERUSER])
createReset2FAToken(userID: ID!): ResetToken @auth(requires: [SUPERUSER])
createRegisterToken(username: String!, role: String!): RegistrationToken @auth(requires: [SUPERUSER])
register(username: String!, password: String!, role: String!): Boolean
resetPassword(userID: ID!, newPassword: String!): Boolean
reset2FA(userID: ID!, secret: String!, code: String!): Boolean
register(token: String!, username: String!, password: String!, role: String!): Boolean
resetPassword(token: String!, userID: ID!, newPassword: String!): Boolean
reset2FA(token: String!, userID: ID!, secret: String!, code: String!): Boolean
}
`

6
lib/new-admin/services/login.js
View file

@ -1,10 +1,5 @@
const db = require('../../db')
function checkUser (username) {
const sql = 'SELECT * FROM users WHERE username=$1'
return db.oneOrNone(sql, [username]).then(value => { return value.password }).catch(() => false)
}
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'
@ -18,6 +13,5 @@ function validateUser (username, password) {
}
module.exports = {
checkUser,
validateUser
}

133
lib/users.js
View file

@ -37,16 +37,21 @@ function getByIds (ids) {
return db.any(sql, [idList])
}
function getUserById (id) {
const sql = `SELECT * FROM users WHERE id=$1`
return db.oneOrNone(sql, [id])
}
function getUserByUsername (username) {
const sql = `SELECT * FROM users WHERE username=$1`
return db.oneOrNone(sql, [username])
}
function getUsers () {
const sql = `SELECT id, username, role, enabled, last_accessed, last_accessed_from, last_accessed_address FROM users ORDER BY username`
return db.any(sql)
}
function getByName (username) {
const sql = `SELECT id, username, role, last_accessed FROM users WHERE username=$1 limit 1`
return db.oneOrNone(sql, [username])
}
function verifyAndUpdateUser (id, ua, ip) {
const sql = `SELECT id, username, role, enabled FROM users WHERE id=$1 limit 1`
return db.oneOrNone(sql, [id]).then(user => {
@ -59,83 +64,66 @@ function verifyAndUpdateUser (id, ua, ip) {
})
}
function createUser (username, password, role) {
const sql = `INSERT INTO users (id, username, password, role) VALUES ($1, $2, $3, $4)`
return bcrypt.hash(password, 12).then(function (hash) {
return db.none(sql, [uuid.v4(), username, hash, role])
})
}
// TO DELETE
function deleteUser (id) {
const sql = `DELETE FROM users WHERE id=$1`
const sql2 = `DELETE FROM user_sessions WHERE sess -> 'user' ->> 'id'=$1`
return db.none(sql, [id]).then(() => db.none(sql2, [id]))
}
function findById (id) {
const sql = 'SELECT id, username FROM users WHERE id=$1'
return db.oneOrNone(sql, [id])
}
function get2FASecret (id) {
const sql = 'SELECT id, username, twofa_code, role FROM users WHERE id=$1'
return db.oneOrNone(sql, [id])
}
function save2FASecret (id, secret) {
return db.tx(t => {
const q1 = t.none('UPDATE users SET twofa_code=$1 WHERE id=$2', [secret, 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 twofa_code=$1 WHERE id=$2'
// const sql2 = `DELETE FROM user_sessions WHERE sess -> 'user' ->> 'id'=$1`
// return db.none(sql, [secret, id]).then(() => db.none(sql2, [id]))
})
}
function validate2FAResetToken (token) {
const sql = `DELETE FROM reset_twofa
WHERE token=$1
RETURNING user_id, now() < expire AS success`
const sql = `SELECT user_id, now() < expire AS success FROM auth_tokens
WHERE token=$1 AND type='reset_twofa'`
return db.one(sql, [token])
.then(res => ({ userID: res.user_id, success: res.success }))
}
function reset2FASecret (token, id, secret) {
return validate2FAResetToken(token).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])
const q2 = t.none(`DELETE FROM user_sessions WHERE sess -> 'user' ->> 'id'=$1`, [id])
const q3 = t.none(`DELETE FROM auth_tokens WHERE token=$1 and type='reset_password'`, [token])
return t.batch([q1, q2, q3])
})
})
}
function createReset2FAToken (userID) {
const token = crypto.randomBytes(32).toString('hex')
const sql = `INSERT INTO reset_twofa (token, user_id) VALUES ($1, $2) ON CONFLICT (user_id) DO UPDATE SET token=$1, expire=now() + interval '30 minutes' RETURNING *`
const sql = `INSERT INTO auth_tokens (token, type, user_id) VALUES ($1, 'reset_twofa', $2) ON CONFLICT (user_id) DO UPDATE SET token=$1, expire=now() + interval '30 minutes' RETURNING *`
return db.one(sql, [token, userID])
}
function updatePassword (id, password) {
bcrypt.hash(password, 12).then(function (hash) {
return db.tx(t => {
const q1 = t.none(`UPDATE users SET password=$1 WHERE id=$2`, [hash, 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 password=$1 WHERE id=$2`
// const sql2 = `DELETE FROM user_sessions WHERE sess -> 'user' ->> 'id'=$1`
// return db.none(sql, [hash, id]).then(() => db.none(sql2, [id]))
})
}
function validatePasswordResetToken (token) {
const sql = `DELETE FROM reset_password
WHERE token=$1
RETURNING user_id, now() < expire AS success`
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 }))
}
function updatePassword (token, id, password) {
return validatePasswordResetToken(token).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 => {
const q1 = t.none(`UPDATE users SET password=$1 WHERE id=$2`, [hash, id])
const q2 = t.none(`DELETE FROM user_sessions WHERE sess -> 'user' ->> 'id'=$1`, [id])
const q3 = t.none(`DELETE FROM auth_tokens WHERE token=$1 and type='reset_password'`, [token])
return t.batch([q1, q2, q3])
})
})
})
}
function createResetPasswordToken (userID) {
const token = crypto.randomBytes(32).toString('hex')
const sql = `INSERT INTO reset_password (token, user_id) VALUES ($1, $2) ON CONFLICT (user_id) DO UPDATE SET token=$1, expire=now() + interval '30 minutes' RETURNING *`
const sql = `INSERT INTO auth_tokens (token, type, user_id) VALUES ($1, 'reset_password', $2) ON CONFLICT (user_id) DO UPDATE SET token=$1, expire=now() + interval '30 minutes' RETURNING *`
return db.one(sql, [token, userID])
}
@ -149,20 +137,37 @@ function createUserRegistrationToken (username, role) {
}
function validateUserRegistrationToken (token) {
const sql = `DELETE FROM user_register_tokens WHERE token=$1
RETURNING username, role, now() < expire AS success`
const sql = `SELECT username, role, now() < expire AS success FROM user_register_tokens WHERE token=$1`
return db.one(sql, [token])
.then(res => ({ username: res.username, role: res.role, success: res.success }))
}
function register (token, username, password, role) {
return validateUserRegistrationToken(token).then(res => {
if (!res.success) throw new Error('Failed to verify registration token')
return bcrypt.hash(password, 12).then(hash => {
return db.tx(t => {
const q1 = t.none(`INSERT INTO users (id, username, password, role) VALUES ($1, $2, $3, $4)`, [uuid.v4(), username, hash, role])
const q2 = t.none(`DELETE FROM user_register_tokens WHERE token=$1`, [token])
return t.batch([q1, q2])
})
})
})
}
function changeUserRole (id, newRole) {
const sql = `UPDATE users SET role=$1 WHERE id=$2`
return db.none(sql, [newRole, id])
}
function toggleUserEnable (id) {
const sql = `UPDATE users SET enabled=not enabled WHERE id=$1`
function enableUser (id) {
const sql = `UPDATE users SET enabled=true WHERE id=$1`
return db.none(sql, [id])
}
function disableUser (id) {
const sql = `UPDATE users SET enabled=false WHERE id=$1`
return db.none(sql, [id])
}
@ -170,20 +175,20 @@ module.exports = {
get,
getByIds,
getUsers,
getByName,
getUserById,
getUserByUsername,
verifyAndUpdateUser,
createUser,
deleteUser,
findById,
updatePassword,
get2FASecret,
save2FASecret,
reset2FASecret,
validate2FAResetToken,
createReset2FAToken,
validatePasswordResetToken,
createResetPasswordToken,
createUserRegistrationToken,
validateUserRegistrationToken,
register,
changeUserRole,
toggleUserEnable
enableUser,
disableUser
}

11
migrations/1605181184453-users.js
View file

@ -21,18 +21,13 @@ exports.up = function (next) {
WITH (OIDS=FALSE)`,
`ALTER TABLE "user_sessions" ADD CONSTRAINT "session_pkey" PRIMARY KEY ("sid") NOT DEFERRABLE INITIALLY IMMEDIATE`,
`CREATE INDEX "IDX_session_expire" ON "user_sessions" ("expire")`,
`CREATE TABLE reset_password (
`CREATE TYPE auth_token_type AS ENUM('reset_password', 'reset_twofa')`,
`CREATE TABLE auth_tokens (
token TEXT NOT NULL PRIMARY KEY,
type auth_token_type NOT NULL,
user_id UUID REFERENCES users(id) ON DELETE CASCADE UNIQUE,
expire TIMESTAMPTZ NOT NULL DEFAULT now() + interval '30 minutes'
)`,
`CREATE INDEX "idx_reset_pw_expire" ON "reset_password" ("expire")`,
`CREATE TABLE reset_twofa (
token TEXT NOT NULL PRIMARY KEY,
user_id UUID REFERENCES users(id) ON DELETE CASCADE UNIQUE,
expire TIMESTAMPTZ NOT NULL DEFAULT now() + interval '30 minutes'
)`,
`CREATE INDEX "idx_reset_twofa_expire" ON "reset_twofa" ("expire")`,
`CREATE TABLE user_register_tokens (
token TEXT NOT NULL PRIMARY KEY,
username TEXT NOT NULL UNIQUE,

19
new-lamassu-admin/src/components/layout/Header.js
View file

@ -27,7 +27,7 @@ const HAS_UNREAD = gql`
}
`
const Subheader = ({ item, classes }) => {
const Subheader = ({ item, classes, user }) => {
const [prev, setPrev] = useState(null)
return (
@ -35,7 +35,17 @@ const Subheader = ({ item, classes }) => {
<div className={classes.content}>
<nav>
<ul className={classes.subheaderUl}>
{item.children.map((it, idx) => (
{item.children.map((it, idx) => {
if (
!R.includes(
user.role,
it.allowedRoles.map(v => {
return v.key
})
)
)
return <></>
return (
<li key={idx} className={classes.subheaderLi}>
<NavLink
to={{ pathname: it.route, state: { prev } }}
@ -49,7 +59,8 @@ const Subheader = ({ item, classes }) => {
{it.label}
</NavLink>
</li>
))}
)
})}
</ul>
</nav>
</div>
@ -193,7 +204,7 @@ const Header = memo(({ tree, user }) => {
</div>
</div>
{active && active.children && (
<Subheader item={active} classes={classes} />
<Subheader item={active} classes={classes} user={user} />
)}
{open && <AddMachine close={() => setOpen(false)} onPaired={onPaired} />}
</header>

15
new-lamassu-admin/src/pages/Authentication/Register.js
View file

@ -27,8 +27,18 @@ const VALIDATE_REGISTER_LINK = gql`
`
const REGISTER = gql`
mutation register($username: String!, $password: String!, $role: String!) {
register(username: $username, password: $password, role: $role)
mutation register(
$token: String!
$username: String!
$password: String!
$role: String!
) {
register(
token: $token
username: $username
password: $password
role: $role
)
}
`
@ -117,6 +127,7 @@ const Register = () => {
onSubmit={values => {
register({
variables: {
token: token,
username: username,
password: values.password,
role: role

10
new-lamassu-admin/src/pages/Authentication/Reset2FA.js
View file

@ -28,8 +28,13 @@ const VALIDATE_RESET_2FA_LINK = gql`
`
const RESET_2FA = gql`
mutation reset2FA($userID: ID!, $secret: String!, $code: String!) {
reset2FA(userID: $userID, secret: $secret, code: $code)
mutation reset2FA(
$token: String!
$userID: ID!
$secret: String!
$code: String!
) {
reset2FA(token: $token, userID: $userID, secret: $secret, code: $code)
}
`
@ -154,6 +159,7 @@ const Reset2FA = () => {
}
reset2FA({
variables: {
token: token,
userID: userID,
secret: secret,
code: twoFAConfirmation

5
new-lamassu-admin/src/pages/Authentication/ResetPassword.js
View file

@ -26,8 +26,8 @@ const VALIDATE_RESET_PASSWORD_LINK = gql`
`
const RESET_PASSWORD = gql`
mutation resetPassword($userID: ID!, $newPassword: String!) {
resetPassword(userID: $userID, newPassword: $newPassword)
mutation resetPassword($token: String!, $userID: ID!, $newPassword: String!) {
resetPassword(token: $token, userID: $userID, newPassword: $newPassword)
}
`
@ -114,6 +114,7 @@ const ResetPassword = () => {
onSubmit={values => {
resetPassword({
variables: {
token: token,
userID: userID,
newPassword: values.confirmPassword
}

22
new-lamassu-admin/src/pages/UserManagement/UserManagement.js
View file

@ -42,9 +42,17 @@ const CHANGE_USER_ROLE = gql`
}
`
const TOGGLE_USER_ENABLE = gql`
mutation toggleUserEnable($id: ID!) {
toggleUserEnable(id: $id) {
const ENABLE_USER = gql`
mutation enableUser($id: ID!) {
enableUser(id: $id) {
id
}
}
`
const DISABLE_USER = gql`
mutation disableUser($id: ID!) {
disableUser(id: $id) {
id
}
}
@ -81,7 +89,11 @@ const Users = () => {
refetchQueries: () => ['users']
})
const [toggleUserEnable] = useMutation(TOGGLE_USER_ENABLE, {
const [enableUser] = useMutation(ENABLE_USER, {
refetchQueries: () => ['users']
})
const [disableUser] = useMutation(DISABLE_USER, {
refetchQueries: () => ['users']
})
@ -307,7 +319,7 @@ const Users = () => {
showModal={showEnableUserModal}
toggleModal={toggleEnableUserModal}
user={userInfo}
confirm={toggleUserEnable}
confirm={userInfo?.enabled ? disableUser : enableUser}
inputConfirmToggle={toggleInputConfirmModal}
setAction={setAction}
/>

24
new-lamassu-admin/src/utils/apollo.js
View file

@ -19,7 +19,12 @@ const getClient = (history, location, setUserData) =>
onError(({ graphQLErrors, networkError }) => {
if (graphQLErrors)
graphQLErrors.forEach(({ message, locations, path, extensions }) => {
handle(extensions?.code, history, location, setUserData)
handle(
{ message, locations, path, extensions },
history,
location,
setUserData
)
console.log(
`[GraphQL error]: Message: ${message}, Location: ${locations}, Path: ${path}`
)
@ -47,21 +52,20 @@ const getClient = (history, location, setUserData) =>
}
})
const handle = (type, ...args) => {
const handle = (apolloError, ...args) => {
const handler = {
UNAUTHENTICATED: ({ history, location, setUserData }) => {
UNAUTHENTICATED: (...args) => {
const history = args[0]
const location = args[1]
const setUserData = args[2]
setUserData(null)
if (location.pathname !== '/login') history.push('/login')
},
INVALID_CREDENTIALS: () => {},
INVALID_TWO_FACTOR_CODE: () => {},
INVALID_URL_TOKEN: () => {},
USER_ALREADY_EXISTS: () => {}
}
}
if (!R.has(type, handler)) throw new Error('Unknown error code.')
if (!R.has(apolloError.extensions?.code, handler)) return apolloError
return handler[type](...args)
return handler[apolloError.extensions?.code](...args)
}
const Provider = ({ children }) => {