-
-
Notifications
You must be signed in to change notification settings - Fork 97
Refactor permission management #2090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Changes from all commits
1e20d9b
f411f17
8f5886d
a492c2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,8 @@ | |
|
|
||
| // Load in all of our node modules. Their uses are explained below as they are called. | ||
| const express = require('express'); | ||
| const cron = require('node-cron'); | ||
| const fetch = require('node-fetch'); | ||
| // const cron = require('node-cron'); | ||
| // const fetch = require('node-fetch'); | ||
| const morgan = require('morgan'); | ||
| const cookieParser = require('cookie-parser'); | ||
|
|
||
|
|
@@ -52,14 +52,18 @@ app.use(cookieParser()); | |
| app.use(morgan('dev')); | ||
|
|
||
| // WORKERS | ||
| const runOpenCheckinWorker = require('./workers/openCheckins')(cron, fetch); | ||
| const runCloseCheckinWorker = require('./workers/closeCheckins')(cron, fetch); | ||
| // const runOpenCheckinWorker = require('./workers/openCheckins')(cron, fetch); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these commected out? |
||
| // const runCloseCheckinWorker = require('./workers/closeCheckins')(cron, fetch); | ||
|
|
||
| const { createRecurringEvents } = require('./workers/createRecurringEvents'); | ||
| const runCreateRecurringEventsWorker = createRecurringEvents(cron, fetch); | ||
| // const { createRecurringEvents } = require('./workers/createRecurringEvents'); | ||
| // const runCreateRecurringEventsWorker = createRecurringEvents(cron, fetch); | ||
|
|
||
| // const runSlackBot = require("./workers/slackbot")(fetch); | ||
|
|
||
| // Run cleanup expired refresh token(s) on startup | ||
| const { cleanupExpiredTokens } = require('./workers/tokenCleanup'); | ||
| cleanupExpiredTokens(); | ||
|
|
||
| // MIDDLEWARE | ||
| const errorhandler = require('./middleware/errorhandler.middleware'); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| /*eslint-disable */ | ||
| module.exports = { | ||
| SECRET: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should not be part of the code, this should be as the name implies a secret. This should be a env var. |
||
| 'c0d7d0716e4cecffe9dcc77ff90476d98f5aace08ea40f5516bd982b06401021191f0f24cd6759f7d8ca41b64f68d0b3ad19417453bddfd1dbe8fcb197245079', | ||
| CUSTOM_REQUEST_HEADER: process.env.CUSTOM_REQUEST_HEADER, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling an env variable like this, if the var does not exist will cause a silent failure returning undefined. |
||
| TOKEN_EXPIRATION_SEC: 900, | ||
| // 15 minutes | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is the difference between these two ACCESS_TOKEN_EXPIRATION, and ACCESS_TOKEN_EXPIRATION_MS? |
||
| ACCESS_TOKEN_EXPIRATION: '15m', | ||
| ACCESS_TOKEN_EXPIRATION_MS: 15 * 60 * 1000, | ||
| // 30 days | ||
| REFRESH_TOKEN_EXPIRATION_MS: 30 * 24 * 60 * 60 * 1000, | ||
| }; | ||
| /* eslint-enable */ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,28 +1,156 @@ | ||
| const jwt = require('jsonwebtoken'); | ||
| const { CONFIG_AUTH } = require('../config'); | ||
|
|
||
| function verifyToken(req, res, next) { | ||
| // Allow users to set token | ||
| // eslint-disable-next-line dot-notation | ||
| let token = req.headers['x-access-token'] || req.headers['authorization']; | ||
| if (token.startsWith('Bearer ')) { | ||
| // Remove Bearer from string | ||
| token = token.slice(7, token.length); | ||
| const { RefreshToken, User } = require('../models'); | ||
| const crypto = require('crypto'); | ||
| const AuthUtils = require('../../shared/authorizationUtils'); | ||
|
|
||
| const SECRET_KEY = process.env.JWT_SECRET; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should avoid calling these blindly, they will silently fail if they do not exist. |
||
|
|
||
| // Utility functions | ||
|
|
||
| function generateAccessToken(user, auth_origin) { | ||
| return jwt.sign( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is there no experation time on the signature of the access token? |
||
| { | ||
| id: user._id, | ||
| email: user.email, | ||
| role: user.accessLevel, | ||
| accessLevel: user.accessLevel, | ||
| auth_origin: auth_origin, | ||
| }, | ||
| SECRET_KEY, | ||
| { expiresIn: CONFIG_AUTH.ACCESS_TOKEN_EXPIRATION }, | ||
| ); | ||
| } | ||
|
|
||
| function generateRefreshToken() { | ||
| return crypto.randomBytes(32).toString('hex'); | ||
| } | ||
|
|
||
| function hashToken(token) { | ||
| return crypto.createHash('sha256').update(token).digest('hex'); | ||
| } | ||
|
|
||
| function getClientIp(req) { | ||
| // Check X-Forwarded-For header (most common) | ||
| const forwarded = req.headers['x-forwarded-for']; | ||
| if (forwarded) { | ||
| // Takes the first IP if there are multiple | ||
| return forwarded.split(',')[0].trim(); | ||
| } | ||
| if (!token) { | ||
| return res.sendStatus(403); | ||
|
|
||
| // Check other common headers | ||
| return ( | ||
| req.headers['x-real-ip'] || req.connection.remoteAddress || req.socket.remoteAddress || req.ip | ||
| ); | ||
| } | ||
|
|
||
| async function authenticateAccessToken(req, res, next) { | ||
| try { | ||
| // Extract token from Authorization header | ||
| let accessToken = | ||
| req.cookies.token || req.headers['x-access-token'] || req.headers['authorization']; | ||
|
|
||
| if (!accessToken) { | ||
| return res.status(401).json({ error: 'Access token required' }); | ||
| } | ||
|
|
||
| if (accessToken.startsWith('Bearer ')) { | ||
| accessToken = accessToken.slice(7, accessToken.length); | ||
| } | ||
|
|
||
| const decoded = jwt.verify(accessToken, SECRET_KEY); | ||
| // Attach user info to request | ||
| req.user = decoded; | ||
|
|
||
| next(); | ||
| } catch (error) { | ||
| if (error.name === 'TokenExpiredError') { | ||
| return res.status(401).json({ error: 'Token expired' }); | ||
| } | ||
|
|
||
| if (error.name === 'JsonWebTokenError') { | ||
| return res.status(401).json({ error: 'Invalid token' }); | ||
| } | ||
|
|
||
| return res.status(401).json({ error: 'Authentication failed' }); | ||
| } | ||
| } | ||
|
|
||
| // shorthand for authenticateAccessToken | ||
| const authUser = authenticateAccessToken; | ||
|
|
||
| async function authenticateRefreshToken(req, res, next) { | ||
| try { | ||
| const decoded = jwt.verify(token, CONFIG_AUTH.SECRET); | ||
| res.cookie('token', token, { httpOnly: true }); | ||
| req.userId = decoded.id; | ||
| return next(); | ||
| } catch (err) { | ||
| return res.sendStatus(401); | ||
| const refreshToken = req.cookies?.refresh_token; | ||
|
|
||
| if (!refreshToken) { | ||
| return res.status(401).json({ error: 'Refresh token required' }); | ||
| } | ||
|
|
||
| const tokenHash = hashToken(refreshToken); | ||
|
|
||
| const tokenDoc = await RefreshToken.findOne({ | ||
| hash: tokenHash, | ||
| expiresAt: { $gt: new Date() }, | ||
| }); | ||
|
|
||
| if (!tokenDoc) { | ||
| return res.status(401).json({ error: 'Invalid or expired refresh token' }); | ||
| } | ||
|
|
||
| const user = await User.findById(tokenDoc.userId); | ||
| if (!user) { | ||
| return res.status(401).json({ error: 'User not found for this token' }); | ||
| } | ||
|
|
||
| // Attach user & refresh token to request for downstream handlers | ||
| req.user = user; | ||
| req.refreshToken = tokenDoc; | ||
|
|
||
| next(); | ||
| } catch (error) { | ||
| console.error('Refresh token validation error:', error); | ||
| return res.status(401).json({ error: 'Authentication failed' }); | ||
| } | ||
| } | ||
|
|
||
| function requireRole(...roles) { | ||
| return (req, res, next) => { | ||
| if (!req.user) { | ||
| return res.status(401).json({ error: 'Authentication required' }); | ||
| } | ||
|
|
||
| if (!AuthUtils.hasAnyRole(req.user, roles)) { | ||
| return res.status(403).json({ | ||
| error: 'Insufficient permissions', | ||
| required_role: roles, | ||
| your_role: req.user.accessLevel, | ||
| }); | ||
| } | ||
|
|
||
| next(); | ||
| }; | ||
| } | ||
|
|
||
| function requireMinimumRole(role) { | ||
| return (req, res, next) => { | ||
| if (!req.user) { | ||
| return res.status(401).json({ error: 'Authentication required' }); | ||
| } | ||
|
|
||
| const user = req.user; | ||
| if (!AuthUtils.hasMinimumRole(user, role)) { | ||
| return res.status(403).json({ | ||
| error: 'Insufficient permissions', | ||
| required_minimum_role: role, | ||
| your_role: req.user.accessLevel, | ||
| }); | ||
| } | ||
| next(); | ||
| }; | ||
| } | ||
|
|
||
| function verifyCookie(req, res, next) { | ||
| jwt.verify(req.cookies.token, CONFIG_AUTH.SECRET, (err, decoded) => { | ||
| if (err) { | ||
|
|
@@ -35,8 +163,15 @@ function verifyCookie(req, res, next) { | |
| }); | ||
| } | ||
|
|
||
| const AuthUtil = { | ||
| verifyToken, | ||
| module.exports = { | ||
| authenticateAccessToken, | ||
| authUser, | ||
| authenticateRefreshToken, | ||
| requireRole, | ||
| requireMinimumRole, | ||
| generateAccessToken, | ||
| generateRefreshToken, | ||
| getClientIp, | ||
| hashToken, | ||
| verifyCookie, | ||
| }; | ||
| module.exports = AuthUtil; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,7 @@ | ||
| const AuthUtil = require('./auth.middleware'); | ||
| const Auth = require('./auth.middleware'); | ||
| const verifyUser = require('./user.middleware'); | ||
| const verifyToken = require('./token.middleware'); | ||
|
|
||
| module.exports = { | ||
| AuthUtil, | ||
| Auth, | ||
| verifyUser, | ||
| verifyToken, | ||
| }; |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these commented out?