auth0 / express-jwt

connect/express middleware that validates a JsonWebToken (JWT) and set the req.user with the attributes
MIT License
4.49k stars 444 forks source link

expressjwt middleware shouldn't throw error if credentialsRequired is falsed #314

Closed phatify closed 1 year ago

phatify commented 1 year ago

Description

The default behavior shouldn't throw an error if the token is invalid incredentialsRequired: false optional

Reproduction

return [
    expressJwt({
        secret: secretKey,
        algorithms: ['HS256'],
        requestProperty: 'user_data',
        credentialsRequired: false,
        getToken: authToken,
    }),
    (err, req, res, next) => {
        if (err && err.name === 'UnauthorizedError') {    // err wouldn't throw here because it no necessary
            return res.status(401).send('Wrong authentication token!')
        }
        next(err);
    },
];

Environment

Please provide the following:

micheledallerive commented 1 year ago

The problem with your code is that credentialsRequired: true goes to the next middleware if there is no token, not if the token verification fails: if there is a token that is invalid, the verification will happen anyways. I think this behaviour is intended, since in the README the author says:

credentialsRequired?: boolean (optional): If its false, continue to the next middleware if the request does not contain a token instead of failing, defaults to true.

I am not sure if this behaviour should be changed or not.

phatify commented 1 year ago

credentialsRequired

@micheledallerive Sure, maybe I should do next(new ErrorInstance(401, Wrong authentication token!)) instead of return res.status(401).send('Wrong authentication token!') because the error will be forwarded to the error middleware for final handle.

micheledallerive commented 1 year ago

credentialsRequired

@micheledallerive Sure, maybe I should do next(new ErrorInstance(401, Wrong authentication token!)) instead of return res.status(401).send('Wrong authentication token!') because the error will be forwarded to the error middleware for final handle.

I still don't get why you are using the middleware if you don't want the authentication to be handled. Could you provide more details on where you use that middleware?

phatify commented 1 year ago

@micheledallerive In some case, you want to get the payload in the request object with requestPropertyto check logged in status of user (example: verify you are author of the public post regardless of whether you are logged in or not). So in that case, I need to disable credentialsRequired flag but still working with user_data in requestProperty. the error exception in this middleware is not really necessary.

    (err, req, res, next) => {
        // the err variable should be null or undefined when enabling credentialsRequired flag
        next(err);
    },

I am using middleware to test whether err exist or not in case: credentialsRequired: false

Do you know what I mean?

jfromaniello commented 1 year ago

Do you have another layer of reverse proxy or api gateway doing the actual validarion of the token? In that case please do not use this middleware, it is not intended for the solely purpose of base64 decoding.. you could build a much simpler one, even use the jwt-decode module