auth0 / node-jsonwebtoken

JsonWebToken implementation for node.js http://self-issued.info/docs/draft-ietf-oauth-json-web-token.html
MIT License
17.68k stars 1.23k forks source link

TypeScript: verify callback typings can't assume types #757

Open Sir-hennihau opened 3 years ago

Sir-hennihau commented 3 years ago

jwt.verify(token, secret, (error, data) => { const { userName } = data })

throws

Property 'userName' does not exist on type '{}'.

Environment

"@types/jsonwebtoken": "^8.5.0", "jsonwebtoken": "^8.5.1",

This is the current typings for VerifyCallback

export type VerifyCallback = (
    err: VerifyErrors | null,
    decoded: object | undefined,
) => void; 

Maybe a generic could be added to extend decoded.

Or someone else has a better idea how to improve TypeScript behaviour here?

Sir-hennihau commented 3 years ago

My suggestion would be something like

export function verify<Decoded>(
    token: string,
    secretOrPublicKey: Secret | GetPublicKeyOrSecret,
    callback?: VerifyCallback<Decoded>,
): void;
export type VerifyCallback<Decoded> = (
    err: VerifyErrors | null,
    decoded: Decoded | undefined ,
) => void;
erdzan12 commented 3 years ago

Any suggestions on this?

ThisIsMissEm commented 3 years ago

You can't actually type the decoded value, as it comes from an untrusted source; typing it as anything other than Record<string, unknown> would be giving you a false sense of security in your typesafety.

Even if you think the decoded token will include a given property, it may not actually, JWTs don't validate properties beyond a limit set (those you can validate using the options argument).

There's nothing that guarantees the returned decoded value will contain given properties — only those the library explicitly checks (based on the options argument) could be "known" types. decoded would be better represented as Record<string, unknown>

So if you need a specific property, you should actually check if the decoded JWT contains that property.

Example:

const token = jwt.sign({ foo: "fooValue"}, "secret");

jwt.verify<{ bar: boolean }>(token, "secret", (err, decoded) => {
  // decoded is: { foo: "fooValue" }, so if `decoded` was cast as `{ bar: boolean }` then 
  // you'd think `decoded.bar` is always present. As the signer of the token's payload 
  // didn't include the `bar` property, then it won't be there.
})

If you want to guarantee the shape of the decoded object, then you'd need to pass decoded through a validation library like Joi or Yup, or just manually assert properties exist.

This issue can be a particular gotcha for people consuming JWTs from other services/servers.

In fact, in the typings for jws.decode which this module uses, payload of jws.Signature is typed as any.

There's actually no code in jwt.decode that guarantees the decoded value will be an object: https://github.com/auth0/node-jsonwebtoken/blob/master/decode.js#L7-L17

Nor in jwt.verifypayload isn't guaranteed to a parsed JSON Object (it comes from jwt.decode): https://github.com/auth0/node-jsonwebtoken/blob/master/verify.js#L136

ThisIsMissEm commented 3 years ago

To clarify with working code:

const jws = require('jws');
const jwt = require('jsonwebtoken')

const token = jws.sign({ header: { "alg": "HS256" }, payload: true, secret: 'secret'})
// => 'eyJhbGciOiJIUzI1NiJ9.dHJ1ZQ.tNWhzx87CKMgFxL2x9cqZJkzixGWWAkRNbHmbn6d5Dk'

jwt.decode(token, { complete: true })
/* {
  header: { alg: 'HS256' },
  payload: 'true',
  signature: 'tNWhzx87CKMgFxL2x9cqZJkzixGWWAkRNbHmbn6d5Dk'
} */

jwt.verify(token, "secret")
// => 'true'

This is also why you should be really protective over your jsonwebtoken secret/key data, if that gets compromised, then someone can create all manner of random crap to exploit bugs in your app.

Sir-hennihau commented 2 years ago

Since this post got a lot of upvotes and there is no satisfying typescript solution yet, I'm gonna link this stackeroverflow here. Casting seems like the best solution for now.

let isJWTValid = false;

jwt.verify(token, "mysecret", (error, data) => {
    if (error) {
        return;
    }

    isJWTValid = true;
});

if (!isJWTValid) {
    return "invalid token";
}

const decodedToken = jwt.decode(token) as { myValue: string }; // <--- Cast here for Typescript

const { myValue } = decodedToken;

console.log("myValue", myValue);
jossowski commented 2 years ago

I stumbled about the same issue and removed the additional call to jwt.decode

jwt.verify(token, "mysecret", (error, data) => {
    if (error) {
        return;
    }

    const decodedToken = data as { myValue: string }; // <--- Cast here for Typescript
    const { myValue } = decodedToken;

    console.log("myValue", myValue);
});
ThisIsMissEm commented 2 years ago

@jossowski that'd give you a false sense of security: the token may contain anything, you'd need to first check does myValue exist and is it a string, if those pass, then yes, you could cast like this, but otherwise you'd need to reject or ignore the token.

Perhaps doable using type predicates (formerly guards) — https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates

jossowski commented 2 years ago

@ThisIsMissEm thank you for pointing it out. My take on the previous example was simple to get rid of the decode call. The checks are needed in any case.

LuanNVVnB commented 1 year ago

interface CustomRequest extends Request { token: JwtPayload; }

const authenToken = (req: Request, res: Response, next: NextFunction) => { try { const token = req.header('Authorization')?.replace('Bearer ', '');

    if (!token) {
        throw new Error();
    }

    jwt.verify(token, signingKey as string, (err: VerifyErrors | null, payload) => {
        if (err || payload === undefined) {
            return next(err);
        }

        (req as CustomRequest).token = payload as JwtPayload;
        next();
    });

} catch (err) {
    res.status(401).send('Please authenticate');
}

};