Skip to content

Commit

Permalink
cleanup and refactor (#749)
Browse files Browse the repository at this point in the history
* add checks for active user

* add source ip address and log violations

* remove logging

* remove express-async-errors dependency

* add comments to clarify nature of the collate command

* populate user
  • Loading branch information
dsclassen authored Jan 7, 2025
1 parent 628fc14 commit bf0d4fa
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 27 deletions.
10 changes: 0 additions & 10 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
"dotenv": "^16.4.7",
"eslint-plugin-jest": "^28.10.0",
"express": "~4.21.2",
"express-async-errors": "^3.1.1",
"express-rate-limit": "^7.5.0",
"express-winston": "^4.2.0",
"fs-extra": "^11.2.0",
Expand Down
15 changes: 12 additions & 3 deletions src/controllers/authController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,21 @@ interface RefreshToken {
const otp = async (req: Request, res: Response) => {
try {
const { otp: code } = req.body
logger.info(`Received OTP: ${code}`)

if (!code) res.status(400).json({ message: 'OTP required.' })
if (!code) {
res.status(400).json({ message: 'OTP required.' })
return
}

const user: IUser | null = await User.findOne({ 'otp.code': code })

if (user) {
if (!user.active) {
logger.warn('User is not active')
res.status(401).json({ message: 'Unauthorized - User is not active`' })
return
}
logger.debug(`User found: ${user.username}`)
// logger.info({ level: 'info', message: 'hello' })
logger.info(`OTP login for user: ${user.username} email: ${user.email}`)
Expand Down Expand Up @@ -120,7 +129,7 @@ const refresh = async (req: Request, res: Response) => {
try {
const foundUser = await User.findOne({ email: decoded.email }).exec()
// console.log('foundUser --->', foundUser)
if (!foundUser) {
if (!foundUser || !foundUser.active) {
res.status(401).json({ message: 'Unauthorized' })
return
}
Expand Down Expand Up @@ -152,7 +161,7 @@ const refresh = async (req: Request, res: Response) => {
const logout = (req: Request, res: Response) => {
const cookies = req.cookies
if (!cookies?.jwt) res.sendStatus(204) //No content
res.clearCookie('jwt', { httpOnly: true, sameSite: 'none', secure: true })
res.clearCookie('jwt', { httpOnly: true, sameSite: 'none', secure: true }) // May need to adjust to match cookie settings from above
res.json({ message: 'Cookie cleared' })
}

Expand Down
13 changes: 8 additions & 5 deletions src/controllers/jobsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ const getAllJobs = async (req: Request, res: Response) => {

// Fetch jobs from both Job and MultiJob collections
const [DBjobs, DBmultiJobs] = await Promise.all([
Job.find(jobFilter).lean<IJob[]>().exec(),
MultiJob.find(jobFilter).lean<IMultiJob[]>().exec()
Job.find(jobFilter).populate('user').lean<IJob[]>().exec(),
MultiJob.find(jobFilter).populate('user').lean<IMultiJob[]>().exec()
])

// Combine both job types
Expand All @@ -86,19 +86,22 @@ const getAllJobs = async (req: Request, res: Response) => {
// Process and format jobs
const formattedJobs = await Promise.all(
allJobs.map(async (mongo) => {
const user = await User.findById(mongo.user).lean().exec()

let bullmq = null
if (['BilboMd', 'BilboMdAuto'].includes(mongo.__t)) {
bullmq = await getBullMQJob(mongo.uuid)
} else if (mongo.__t === 'BilboMdScoper') {
bullmq = await getBullMQScoperJob(mongo.uuid)
}

// Manually assign the id field from _id
if (mongo.user && mongo.user._id) {
mongo.user.id = mongo.user._id.toString()
}

return {
mongo,
bullmq,
username: user?.username
username: mongo.user.username
}
})
)
Expand Down
6 changes: 3 additions & 3 deletions src/controllers/registerController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const handleNewUser = async (req: Request, res: Response) => {

// check for duplicate username in the db
const duplicateUser = await User.findOne({ username: user })
.collation({ locale: 'en', strength: 2 })
.collation({ locale: 'en', strength: 2 }) // provide case-insensitive search
.lean()
.exec()
if (duplicateUser) {
Expand All @@ -30,7 +30,7 @@ const handleNewUser = async (req: Request, res: Response) => {

// Check for duplicate email in the email field
const duplicateEmail = await User.findOne({ email: email })
.collation({ locale: 'en', strength: 2 })
.collation({ locale: 'en', strength: 2 }) // provide case-insensitive search
.lean()
.exec()

Expand All @@ -44,7 +44,7 @@ const handleNewUser = async (req: Request, res: Response) => {
const duplicatePreviousEmail = await User.findOne({
previousEmails: { $in: [email] }
})
.collation({ locale: 'en', strength: 2 })
.collation({ locale: 'en', strength: 2 }) // provide case-insensitive search
.lean()
.exec()

Expand Down
2 changes: 1 addition & 1 deletion src/controllers/usersController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const updateUser = async (req: Request, res: Response): Promise<void> => {
}

const duplicate = await User.findOne({ username })
.collation({ locale: 'en', strength: 2 })
.collation({ locale: 'en', strength: 2 }) // provide case-insensitive search
.lean()
.exec()

Expand Down
9 changes: 8 additions & 1 deletion src/middleware/loginLimiter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import rateLimit, { Options } from 'express-rate-limit'
import { Request, Response, NextFunction } from 'express'
import { logger } from './loggers.js'

const loginLimiter = rateLimit({
windowMs: 60 * 1000, // 1 minute
Expand All @@ -9,7 +10,13 @@ const loginLimiter = rateLimit({
'Too many login attempts from this IP, please try again after a 60 second pause'
},
handler: (req: Request, res: Response, next: NextFunction, options: Options) => {
// logEvents(`Too Many Requests: ${options.message.message}\t${req.method}\t${req.url}\t${req.headers.origin}`, 'errLog.log')
const clientIp = (req.ip ?? '').includes('::ffff:')
? (req.ip ?? '').split('::ffff:')[1]
: req.ip ?? ''
logger.error(
`Too Many Requests: ${options.message.message}\t${req.method}\t${req.url}\t${req.headers.origin}\t${clientIp}`,
'errLog.log'
)
res.status(options.statusCode).send(options.message)
},
standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
Expand Down
3 changes: 0 additions & 3 deletions src/middleware/verifyJWT.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import jwt from 'jsonwebtoken'
import { Request, Response, NextFunction } from 'express'
// import { logger } from './loggers.js'

interface DecodedJWT {
UserInfo: {
Expand Down Expand Up @@ -34,13 +33,11 @@ const verifyJWT = (req: Request, res: Response, next: NextFunction) => {
}

const userInfo = (decoded as DecodedJWT).UserInfo
// logger.info(`Decoded JWT: ${JSON.stringify(userInfo)}`)

if (userInfo) {
req.user = userInfo.username
req.roles = userInfo.roles
req.email = userInfo.email
// logger.info(`User ${userInfo.username} has roles ${userInfo.roles}`)
next()
} else {
res.status(403).json({ message: 'Forbidden - no userInfo' })
Expand Down

0 comments on commit bf0d4fa

Please sign in to comment.