BE Auth Service Crypto package#1
Conversation
KailasMahavarkar
commented
Feb 11, 2026
KailasMahavarkar
commented
- added crypto package
| const { hash } = ALGORITHM_MAP[this.algorithm]; | ||
|
|
||
| // Parse payload to get iat for issuedBefore checks | ||
| // This is a non-crypto operation, safe to do before verification |
There was a problem hiding this comment.
The verifySignature method parses the payload JSON before verifying the cryptographic signature, but parsing untrusted, unverified JSON is a security anti-pattern in crypto libraries. A crafted payload could exploit JSON parser quirks. Consider restructuring to verify the signature first, then parse claims..This is not a critical vulnerability but it is and crypto anitpattern
| } | ||
|
|
||
| // Import ERROR_MESSAGES for validateExpiration | ||
| import { ERROR_MESSAGES } from './constants'; |
There was a problem hiding this comment.
Would moving this import at the top be more conventional method of importing in typescript
| onJti?: (jti: string) => boolean | Promise<boolean>; | ||
| } | ||
|
|
||
| export type JWTVerifyResult<T> = { valid: true; payload: T } | { valid: false; error: string }; No newline at end of file |
There was a problem hiding this comment.
JWTVerifyResult only has { valid: false; error: string } a plain string error. But JWKSVerifyResult has { valid: false; error: string; code: JWKSErrorCode } ....a more structured error code enum. This is an inconsistent API surface within the same package, Consumers using JWTService cannot programmatically handle error cases without parsing strings
adding an error code enum to JWTVerifyResult would be better for parity with the JWKS service
| .setProtectedHeader({ | ||
| alg: this.config.algorithm, | ||
| typ: JWT_HEADER.TYPE, | ||
| kid: this.primaryKid!, |
There was a problem hiding this comment.
oth sign methods use this.primaryKid! (non-null assertion) without a guard-->
While the constructor always sets this through fromPEM/fromKMS, the field is typed as string | null. If someone extends or modifies the class, this will throw a runtime error deep in JWT signing. Adding an explicit check or change the type to be non-nullable after construction
| }; | ||
| } | ||
|
|
||
| export interface VaultConfig { |
There was a problem hiding this comment.
VaultConfig declares token?: string (optional), but VaultKMSProvider's constructor immediately throws if token is missing (falling back to process.env.VAULT_TOKEN). The type signature is misleading....it suggests you can create a VaultConfig without a token, but that only works if the env var is set
Either documenting this in the type or making the type more explicit would be better
| } | ||
|
|
||
| const { payload, protectedHeader } = successfulResult; | ||
|
|
There was a problem hiding this comment.
The maxPayloadSize check happens after full signature verification (jwtVerify has already parsed and verified)...By this point, the expensive crypto verification is already done. If DoS prevention is the goal, consider checking raw token segment sizes before calling jwtVerify
|
reviewed |
2 participants
