Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: types-compabitility for express-jwt @ 7 #301

Merged
merged 3 commits into from
May 6, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions index.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { SecretCallback, SecretCallbackLong } from 'express-jwt';
import { Agent as HttpAgent } from 'http';
import { Agent as HttpsAgent } from 'https';
import type {Jwt, Secret} from 'jsonwebtoken'
Copy link

@aaronatbissell aaronatbissell May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes jwks-rsa only compatible with Typescript 3.8+ as the type-only imports and exports feature was release with TS 3.8. For one of my projects, we are on TS 3.7.3, which means this breaks our compatibility

Also - this line is missing a semicolon :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, my bad.

Internally, I'd say update typescript on the project.

If that's not possible, then you may want to open a merge request to address that issue

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem - we just attempted to upgrade and ran into a couple of other issues. We will be putting up a pull request soon!


declare function JwksRsa(options: JwksRsa.Options): JwksRsa.JwksClient;

Expand Down Expand Up @@ -55,7 +55,24 @@ declare namespace JwksRsa {

type SigningKey = CertSigningKey | RsaSigningKey;

function expressJwtSecret(options: ExpressJwtOptions): SecretCallbackLong;
/**
* Types are duplicated from express-jwt@6/7
* due to numerous breaking changes in the lib's types
* whilst this lib supportd both <=6 & >=7 implementations
*
* express-jwt's installed version (or its @types)
* will be the types used at transpilation time
*/

/** Types from express-jwt@<=6 */
type secretType = string|Buffer;
type SecretCallbackLong = (req: Express.Request, header: any, payload: any, done: (err: any, secret?: secretType) => void) => void;
type SecretCallback = (req: Express.Request, payload: any, done: (err: any, secret?: secretType) => void) => void;

/** Types from express-jwt@>=7 */
type GetVerificationKey = (req: Express.Request, token: Jwt | undefined) => Secret | Promise<Secret>;

function expressJwtSecret(options: ExpressJwtOptions): SecretCallbackLong|GetVerificationKey;

function passportJwtSecret(options: ExpressJwtOptions): SecretCallback;

Expand Down
160 changes: 99 additions & 61 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@
"node": ">=10 < 13 || >=14"
},
"dependencies": {
"@types/express-jwt": "0.0.42",
"@types/express": "^4.17.13",
"debug": "^4.3.4",
"jose": "^2.0.5",
"limiter": "^1.1.5",
"lru-memoizer": "^2.1.4"
},
"devDependencies": {
"@types/chai": "^4.2.11",
"@types/express-jwt": "^6.0.4",
"@types/jsonwebtoken": "^8.5.8",
"@types/mocha": "^5.2.7",
"@types/nock": "^11.0.0",
"@types/node": "^14.14.12",
Expand Down
22 changes: 21 additions & 1 deletion tests/ts-definitions.tests.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import * as jwksRsaType from '../index';
import {expect} from 'chai';
import expressjwt6 from "express-jwt";
import { expressjwt as expressjwt7, GetVerificationKey } from "express-jwt-v7";
const { jwksEndpoint } = require('../tests/mocks/jwks');
const { publicKey } = require('../tests/mocks/keys');
const jwksRsa: typeof jwksRsaType = require('../src');
Expand All @@ -24,7 +26,7 @@ describe('typescript definition', () => {
});
});

describe('getKeysInterceptor', async () => {
it('getKeysInterceptor', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting this, looks like you've uncovered a failing test.

Could you replace keySetResponse with https://github.com/auth0/node-jwks-rsa/blob/master/tests/keys.js#L2 to fix

const keySetResponse = {
keys: [
{
Expand All @@ -44,4 +46,22 @@ describe('typescript definition', () => {
const key = await client.getSigningKey('NkFCNEE1NDFDNTQ5RTQ5OTE1QzRBMjYyMzY0NEJCQTJBMjJBQkZCMA');
expect(key.kid).to.equal('NkFCNEE1NDFDNTQ5RTQ5OTE1QzRBMjYyMzY0NEJCQTJBMjJBQkZCMA');
});

it.skip('Types-Only Validation with express-jwt', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it.skip('Types-Only Validation with express-jwt', () => {
it('Types-Only Validation with express-jwt', () => {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a skip marker here as I was only testing the transpilation, which was the issue at hand.

I've removed the skip marker in c98ac1f
Now, the test may fail at transpilation due to mismatched types.
Or it may fail if something throws at initialization.

I've purposely not duplicated the test suite from tests/express.es5.tests.js as I didn't see any gains from doing so

expressjwt6({
algorithms: ["RS256"],
secret: jwksRsa.expressJwtSecret({
cache: true,
jwksUri: `https://my-authz-server/.well-known/jwks.json`
})
});

expressjwt7({
algorithms: ['RS256'],
secret: jwksRsa.expressJwtSecret({
cache: true,
jwksUri: `https://my-authz-server/.well-known/jwks.json`
}) as GetVerificationKey
});
})
});