Skip to content

Code refactoring for auth/index.js#54

Open
Mehulantony wants to merge 1 commit intomainfrom
code-refactor
Open

Code refactoring for auth/index.js#54
Mehulantony wants to merge 1 commit intomainfrom
code-refactor

Conversation

@Mehulantony
Copy link
Copy Markdown
Collaborator

Performed code refactoring for the auth/index.js file and introduced the following changes for cleanup and maintainability:

  1. Extracted JWT payload parsing into one helper function parseAuthHeaderPayload(req)
  2. Fixed typo issues in _extractUser
  3. Used consistent quote style and spacing
  4. Extracted repeated Auth0 fetch logic into one helper function requestTokenFromAuth0

const payload = token.split('.')[1]
return JSON.parse(Buffer.from(payload, 'base64').toString())
}

Copy link
Copy Markdown
Collaborator

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)
}

console.log("Request allowed via bot check")

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

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.

* @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

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.

*/
const requestTokenFromAuth0 = async (form) => {
return await fetch('https://cubap.auth0.com/oauth/token', {
method: 'POST',
Copy link
Copy Markdown
Collaborator

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)
        }
    }
}


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

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.

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants