Skip to content

Code refactoring for auth/index.js #54

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.

Sign up for GitHub

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

Open
Mehulantony wants to merge 1 commit into
base: main
Choose a base branch
from
+98 −72
Open

Code refactoring for auth/index.js #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 97 additions & 71 deletions auth/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,43 +1,95 @@
import { auth } from 'express-oauth2-jwt-bearer'
import config from '../config/index.js'

/**
* Parse and decode the JWT payload from the Authorization header.
* @param {Object} req - Express request object
* @returns {Object} decoded JWT payload
*/
const parseAuthHeaderPayload = (req) => {
const token = req.header('authorization').split(' ')[1]
const payload = token.split('.')[1]
return JSON.parse(Buffer.from(payload, 'base64').toString())
}

Copy link
Copy Markdown
Collaborator

@allenbakki allenbakki Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Missing or invalid header handling
    Currently there is no guard for a missing or malformed authorization header. If the header is undefined, not a string containing a space, or not a valid 3-part JWT, this can throw a TypeError during parsing.

Previously, the inline try/catch in the call sites implicitly protected against this. Since parsing is now centralized in this helper, it would be safer for the helper itself to defensively validate the header before attempting to decode it.

  1. Base64 variant handling
    JWTs use URL-safe Base64 (- and _) rather than standard Base64. While Buffer.from(payload, 'base64') in Node often works, it can sometimes be brittle due to padding or character differences. It may be safer to normalize the Base64 string or explicitly support Base64URL decoding.

  2. validation clarity
    This function currently only decodes the token and does not validate the signature or expiration. That is fine if it is always used after auth() has verified the token, but _tokenError is explicitly handling invalid or expired tokens.

Suggested change
const parseAuthHeaderPayload = (req) => {
const authHeader = req.header('authorization')
if (!authHeader || typeof authHeader !== 'string') {
throw new Error('Missing or invalid authorization header')
}
const [, token] = authHeader.split(' ')
if (!token) {
throw new Error('Malformed authorization header')
}
const parts = token.split('.')
if (parts.length < 2) {
throw new Error('Malformed JWT token')
}
const base64 = parts[1].replace(/-/g, '+').replace(/_/g, '/')
const payloadJson = Buffer.from(base64, 'base64').toString()
return JSON.parse(payloadJson)
}

/**
* Request a token object from Auth0 using the provided form payload.
* @param {Object} form - Auth0 token request payload
* @returns {Object} token response object or error object
*/
const requestTokenFromAuth0 = async (form) => {
return await fetch('https://cubap.auth0.com/oauth/token', {
Copy link
Copy Markdown
Collaborator

@allenbakki allenbakki Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using async + await while also chaining .then() and .catch() is redundant. Prefer one style.

method: 'POST',
Copy link
Copy Markdown
Collaborator

@allenbakki allenbakki Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

    try {
        const resp = await fetch('https://cubap.auth0.com/oauth/token', {
            method: 'POST',
            headers: { 'Content-Type': 'application/json' },
            body: JSON.stringify(form)
        })

        const json = await resp.json()
        return json
    }
    catch (err) {
        console.error(err)
        return {
            error: true,
            error_description: err && err.message ? err.message : String(err)
        }
    }
}

headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify(form)
})
.then(resp => resp.json())
.catch(err => {
console.error(err)
return { error: true, error_description: err }
})
}

/**
* Handles invalid token errors from express-oauth2-jwt-bearer.
* If the error is not an invalid_token, passes it to next.
* Otherwise, checks if the user is a bot; if so, allows the request.
* If not, passes the error to next.
* @param {Error} err - The error from the auth middleware.
* @param {Object} req - The request object.
* @param {Object} res - Unused response object.
* @param {Function} next - The next middleware function.
*/
const _tokenError = function (err, req, res, next) {
if(!err.code || err.code !== "invalid_token"){
if (!err.code || err.code !== 'invalid_token') {
next(err)
return
}
try{
let user = JSON.parse(Buffer.from(req.header("authorization").split(" ")[1].split('.')[1], 'base64').toString())
if(isBot(user)){
console.log("Request allowed via bot check")

try {
const user = parseAuthHeaderPayload(req)
Copy link
Copy Markdown
Collaborator

@allenbakki allenbakki Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If parseAuthHeaderPayload throws for a missing/invalid header, you’ll set 401 with a RERUM-specific message. That’s likely acceptable, but be aware you’re masking actual error details.

Consider using a known custom error or e.code to disambiguate the cause if you need different responses for different failures.

if (isBot(user)) {
console.log('Request allowed via bot check')
next()
return
}
}
catch(e){
e.message = e.statusMessage = `This token did not contain a known RERUM agent.`
catch (e) {
e.message = e.statusMessage = 'This token did not contain a known RERUM agent.'
e.status = 401
e.statusCode = 401
next(e)
return
}

next(err)
}

/**
* Extracts the user object from the JWT in the Authorization header.
* Parses the JWT payload and sets req.user.
* If parsing fails, passes an error to next.
* @param {Object} req - The request object.
* @param {Object} res - Unused response object.
* @param {Function} next - The next middleware function.
*/
const _extractUser = (req, res, next) => {
try{
req.user = JSON.parse(Buffer.from(req.header("authorization").split(" ")[1].split('.')[1], 'base64').toString())
try {
req.user = parseAuthHeaderPayload(req)
next()
}
catch(e){
e.message = e.statusMessage = `This token did not contain a known RERUM agent.}`
catch (e) {
e.message = e.statusMessage = 'This token did not contain a known RERUM agent.'
e.status = 401
e.statusCode = 401
next(e)
}
}

/**
* Use like:
* Use like:
* app.get('/api/private', checkJwt, function(req, res) {
* // do authorized things
* });
Expand All @@ -51,90 +103,64 @@
* @param {ExpressResponse} res to return the new token.
*/
const generateNewAccessToken = async (req, res, next) => {
console.log("RERUM v1 is generating a proxy access token.")
console.log('RERUM v1 is generating a proxy access token.')

const form = {
grant_type: 'refresh_token',
client_id: config.CLIENT_ID,
client_secret: config.CLIENT_SECRET,
refresh_token: req.body.refresh_token,
redirect_uri: config.RERUM_PREFIX
}
try{
// Successful responses from auth 0 look like {"refresh_token":"BLAHBLAH", "access_token":"BLAHBLAH"}
// Error responses come back as successful, but they look like {"error":"blahblah", "error_description": "this is why"}
const tokenObj = await fetch('https://cubap.auth0.com/oauth/token',
{
method: 'POST',
headers: {
"Content-Type": "application/json"
},
body:JSON.stringify(form)
})
.then(resp => resp.json())
.catch(err => {
// Mock Auth0 error object
console.error(err)
return {"error": true, "error_description":err}
})
// Here we need to check if this is an Auth0 success object or an Auth0 error object
if(tokenObj.error){

try {
const tokenObj = await requestTokenFromAuth0(form)

if (tokenObj.error) {
console.error(tokenObj.error_description)
res.status(500).send(tokenObj.error_description)
Copy link
Copy Markdown
Collaborator

@allenbakki allenbakki Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status code semantics:
For Auth0 errors, returning 500 may not be ideal. many of these are client or auth errors (e.g. invalid grant) and could be 400/401. This is pre-existing behavior though, if you want to preserve behavior, leave as is.

}
else{
res.status(200).send(tokenObj)
else {
res.status(200).send(tokenObj)
}
}
catch (e) {
console.error(e.response ? e.response.body : e.message ? e.message : e)
res.status(500).send(e)
Copy link
Copy Markdown
Collaborator

@allenbakki allenbakki Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error body:
On catch, you res.status(500).send(e) which might leak internal error structure or stack. Consider sending a minimal message instead and logging full details.

}
}
}

/**
* Used by RERUM to renew the refresh token upon user request.
* @param {ExpressRequest} req from registered server application.
* @param {ExpressResponse} res to return the new token.

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.
*/
const generateNewRefreshToken = async (req, res, next) => {
console.log("RERUM v1 is generating a new refresh token.")
console.log('RERUM v1 is generating a new refresh token.')

const form = {
grant_type: 'authorization_code',
client_id: config.CLIENT_ID,
client_secret: config.CLIENT_SECRET,
code: req.body.authorization_code,
redirect_uri: config.RERUM_PREFIX
}

try {
// Successful responses from auth 0 look like {"refresh_token":"BLAHBLAH", "access_token":"BLAHBLAH"}
// Error responses come back as successful, but they look like {"error":"blahblah", "error_description": "this is why"}
const tokenObj = await fetch('https://cubap.auth0.com/oauth/token',
{
method: 'POST',
headers: {
"Content-Type": "application/json"
},
body:JSON.stringify(form)
})
.then(resp => resp.json())
.catch(err => {
// Mock Auth0 error object
console.error(err)
return {"error": true, "error_description":err}
})
// Here we need to check if this is an Auth0 success object or an Auth0 error object
if(tokenObj.error){
const tokenObj = await requestTokenFromAuth0(form)

if (tokenObj.error) {
console.error(tokenObj.error_description)
res.status(500).send(tokenObj.error_description)
}
else{
res.status(200).send(tokenObj)
else {
res.status(200).send(tokenObj)
}
}
catch (e) {
}
catch (e) {
console.error(e.response ? e.response.body : e.message ? e.message : e)
res.status(500).send(e)
}
}
}

/**
Expand All @@ -146,39 +172,39 @@
const verifyAccess = (secret) => {
return jwt({
secret,
audience: `http://rerum.io/api`,
issuer: `https://rerum.io/`,
audience: 'http://rerum.io/api',
issuer: 'https://rerum.io/',
algorithms: ['RS256']
})
}

/**
*
*
* @param {Object} obj RERUM database entry
* @param {Object} User object discerned from token
* @param {Object} userObj User object discerned from token
* @returns Boolean match between encoded Generator Agent and obj generator
*/
const isGenerator = (obj, userObj) => {
return userObj[config.RERUM_AGENT_CLAIM] === obj.__rerum.generatedBy
}

/**
* Even expired tokens may be accepted if the Agent is a known bot. This is a
* Even expired tokens may be accepted if the Agent is a known bot. This is a
* dangerous thing to include, but may be a useful convenience.
* @param {Object} User object discerned from token
* @param {Object} userObj User object discerned from token
* @returns Boolean for matching ID.
*/
const isBot = (userObj) => {
return config.BOT_AGENT === userObj[config.RERUM_AGENT_CLAIM]
}

function READONLY(req, res, next) {
if(config.READONLY=="true"){
res.status(503).json({"message":"RERUM v1 is read only at this time. We apologize for the inconvenience. Try again later."})
if (config.READONLY === 'true') {
res.status(503).json({ message: 'RERUM v1 is read only at this time. We apologize for the inconvenience. Try again later.' })
return
}
next()
return
}

next()
}

export default {
Expand All @@ -189,4 +215,4 @@
isBot,
isGenerator,
READONLY
}
}
2 changes: 1 addition & 1 deletion routes/__tests__/since.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ it("'/since/:id' route functions", async () => {
expect(response.headers["allow"]).toBeTruthy()
expect(response.headers["link"]).toBeTruthy()
expect(Array.isArray(response.body)).toBe(true)
})
}, 20000)
Loading