Document decomposeJwt

[ottokruse]

Issue #, if available: #82

Description of changes: Document the decomposeJwt function

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[hakanson]

Even thought I like the decomposeUnverifiedJwt name better, I don't love the breaking change requiring a major version bump. Thoughts on leaving decomposeJwt and marking as @deprecated then having it call into decomposeUnverifiedJwt ? I found https://github.com/gund/eslint-plugin-deprecation, but not sure how many toolchains look at @deprecated in JSDoc comments.

Should we update verifyJwt calls to take either a jwt: string or a decomposedJwt: DecomposedJwt to avoid double parsing, or does it not really help peformance since we would need to validate the DecomposedJwt parts were valid anyway?

Also, should we have a top level import like JwtParser (next to JwtRsaVerifier and CognitoJwtVerifier). That would have a method decomposeUntrusted(jwt: string): DecomposedJwt to be the primary interface? Supporting issue #82 will probably require this pre-parsing and using some value of the payload to find the correct issuer for verification. This could come in a later PR or major version.

[ottokruse]

Thoughts on leaving decomposeJwt and marking as @deprecated then having it call into decomposeUnverifiedJwt ?

Valid idea for sure. We would eventually remove decomposeJwt, so this would just push that major version update to the future. Perhaps we'd have other breaking changes then, and we combine several in a new major version. OTOH I think we should be liberal with new major versions in those cases where it's needed and better for the library's design--and I think this is such a case. Avoiding it adds maintenance burden on us. We are in awslabs after all. Also, this renaming isn't a total revamp of the lib, it will be easy for users to change their code and I suspect 99% of our current users do not use decomposeJwt as it was undocumented.

Should we update verifyJwt calls to take either a jwt: string or a decomposedJwt: DecomposedJwt to avoid double parsing, or does it not really help peformance since we would need to validate the DecomposedJwt parts were valid anyway?

Mmmm good thought. It would shave a few cycles off, but only synchronous work. You're gonna have a hard time even measuring that improvement I think. And It would be at the expense of making the interface more complicated. Think I'm not inclined to it.

Also, should we have a top level import like JwtParser (next to JwtRsaVerifier and CognitoJwtVerifier). That would have a method decomposeUntrusted(jwt: string): DecomposedJwt to be the primary interface? Supporting issue #82 will probably require this pre-parsing and using some value of the payload to find the correct issuer for verification. This could come in a later PR or major version.

Interesting idea! Would also make it easier to support verifying ALB JWTs.
We'd have to play around with the idea and do some sample implementations and see what works nicely.

[hakanson]

I'm fine with the major version and keeping the interface less complicated. We can roll the other ideas into the next minor release.