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.

Sign up for GitHub

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

Jump to bottom

[jwt] fix typo in plugin options name #3426

Merged
merged 5 commits into from
Nov 5, 2024
Merged

[jwt] fix typo in plugin options name #3426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
049107e
[jwt] fix docs
EmrysMyrddin Sep 3, 2024
9260059
[jwt] fix typo in option name
EmrysMyrddin Sep 3, 2024
e82b8af
fix last other typos
EmrysMyrddin Sep 3, 2024
d737143
make backward compatible option optional
EmrysMyrddin Sep 3, 2024
edc3824
fix: retro-compatibility and add test to confirm it
EmrysMyrddin Nov 5, 2024
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
5 changes: 5 additions & 0 deletions .changeset/slow-ads-dress.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-yoga/plugin-jwt': patch
---

Fix typo of the option `singingKeyProviders` => `signingKeyProviders`.
82 changes: 61 additions & 21 deletions packages/plugins/jwt/src/__tests__/jwt.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ I3OrgFkoqk03cpX4AL2GYC2ejytAqboL6pFTfmTgg2UtvKIeaTyF
describe('jwt plugin', () => {
test('incoming http request is reject when auth token is not present', async () => {
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
signingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
});
const response = await test.queryWithoutAuth();
expect(response.status).toBe(401);
Expand All @@ -62,7 +62,7 @@ describe('jwt plugin', () => {

test('should allow to continue if reject.missingToken is set to false', async () => {
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
signingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
reject: {
missingToken: false,
invalidToken: true,
Expand All @@ -75,7 +75,7 @@ describe('jwt plugin', () => {
test('any prefix is supported when strict prefix validation is not configured', async () => {
const secret = 'topsecret';
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
tokenLookupLocations: [
extractFromHeader({
name: 'Authorization',
Expand All @@ -89,7 +89,7 @@ describe('jwt plugin', () => {

test('incoming http has a token but prefix does not match or missing', async () => {
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
signingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
});
// does not match prefix
let response = await test.queryWithAuth('Basic 123');
Expand Down Expand Up @@ -120,7 +120,7 @@ describe('jwt plugin', () => {

test('token provided but jwt token is not valid for decoding', async () => {
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
signingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
});
const response = await test.queryWithAuth('Bearer BadJwt');
expect(response.status).toBe(400);
Expand All @@ -137,7 +137,7 @@ describe('jwt plugin', () => {

test('invalid token can be accepted when reject.invalidToken=false is set', async () => {
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
signingKeyProviders: [createInlineSigningKeyProvider('topsecret')],
reject: {
invalidToken: false,
},
Expand All @@ -149,7 +149,7 @@ describe('jwt plugin', () => {
it('should not allow non matching issuer', async () => {
const secret = 'topsecret';
const server = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
tokenVerification: {
issuer: ['http://yoga'],
},
Expand All @@ -168,7 +168,7 @@ describe('jwt plugin', () => {
it('should allow matching issuer', async () => {
const secret = 'topsecret';
const server = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
tokenVerification: {
issuer: ['http://yoga'],
},
Expand All @@ -182,7 +182,7 @@ describe('jwt plugin', () => {
it('should not allow non matching audience', async () => {
const secret = 'topsecret';
const server = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
tokenVerification: {
audience: 'my.app',
},
Expand All @@ -203,7 +203,7 @@ describe('jwt plugin', () => {
it('should allow matching audience', async () => {
const secret = 'topsecret';
const server = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
tokenVerification: {
audience: 'my.app',
},
Expand Down Expand Up @@ -234,7 +234,7 @@ describe('jwt plugin', () => {

try {
const server = createTestServer({
singingKeyProviders: [
signingKeyProviders: [
createRemoteJwksSigningKeyProvider({
jwksUri: `http://localhost:${(jwksServer.address() as any).port}`,
}),
Expand All @@ -259,7 +259,7 @@ describe('jwt plugin', () => {
it('should not accept token without algorithm', async () => {
const secret = 'topsecret';
const server = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
});

const response = await server.queryWithAuth(buildJWTWithoutAlg());
Expand All @@ -276,7 +276,7 @@ describe('jwt plugin', () => {
test('valid token is injected to the GraphQL context', async () => {
const secret = 'topsecret';
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
});
const token = buildJWT({ sub: '123', scopes: ['users.read'] }, { key: secret });
const response = await test.queryWithAuth(token);
Expand All @@ -302,7 +302,7 @@ describe('jwt plugin', () => {
test('valid token is injected to the GraphQL context (custom field)', async () => {
const secret = 'topsecret';
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
extendContext: 'my_jwt',
});
const token = buildJWT({ sub: '123', scopes: ['users.read'] }, { key: secret });
Expand All @@ -327,6 +327,31 @@ describe('jwt plugin', () => {
});

test('auth is passing when token is valid (HS256)', async () => {
const secret = 'topsecret';
const test = createTestServer({
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
});
const token = buildJWT({ sub: '123' }, { key: secret });
const response = await test.queryWithAuth(token);
expect(response.status).toBe(200);
expect(await response.json()).toMatchObject({
data: {
ctx: {
jwt: {
payload: {
sub: '123',
},
token: {
prefix: 'Bearer',
value: expect.any(String),
},
},
},
},
});
});

test('should allow to use deprecated `singingKeyProviders` option', async () => {
const secret = 'topsecret';
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
Expand All @@ -351,9 +376,24 @@ describe('jwt plugin', () => {
});
});

test('should not allow to use both deprecated `singingKeyProviders` option and its replacement', async () => {
const secret = 'topsecret';
try {
// @ts-expect-error This should not be allowed by TS
createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
});
} catch (err) {
expect((err as Error).message).toBe(
'You are using both deprecated `singingKeyProviders` and its new replacement `signingKeyProviders` configuration. Please use only `signingKeyProviders`',
);
}
});

test('auth is passing when token is valid (RS256)', async () => {
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(JWKS_RSA512_PRIVATE_PEM)],
signingKeyProviders: [createInlineSigningKeyProvider(JWKS_RSA512_PRIVATE_PEM)],
});
const token = buildJWT({ sub: '123' }, { key: JWKS_RSA512_PRIVATE_PEM, algorithm: 'RS256' });
const response = await test.queryWithAuth(token);
Expand Down Expand Up @@ -397,7 +437,7 @@ describe('jwt plugin', () => {

try {
const test = createTestServer({
singingKeyProviders: [
signingKeyProviders: [
createRemoteJwksSigningKeyProvider({
jwksUri: `http://localhost:${(jwksServer.address() as any).port}`,
}),
Expand Down Expand Up @@ -438,7 +478,7 @@ describe('jwt plugin', () => {

try {
const test = createTestServer({
singingKeyProviders: [
signingKeyProviders: [
// Remote, invalid
createRemoteJwksSigningKeyProvider({
jwksUri: `http://localhost:${(jwksServer.address() as any).port}`,
Expand Down Expand Up @@ -476,7 +516,7 @@ describe('jwt plugin', () => {
test('should throw when lookup is configured for cookie but no cookie store available', async () => {
const secret = 'topsecret';
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
tokenLookupLocations: [extractFromCookie({ name: 'auth' })],
});
const token = buildJWT({ sub: '123' }, { key: secret });
Expand All @@ -495,7 +535,7 @@ describe('jwt plugin', () => {
const secret = 'topsecret';
const test = createTestServer(
{
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
tokenLookupLocations: [extractFromCookie({ name: 'auth' })],
},
[useCookies<any>()],
Expand All @@ -508,7 +548,7 @@ describe('jwt plugin', () => {
test('custom getToken functiFailed to verify authentication token. Verifon', async () => {
const secret = 'topsecret';
const test = createTestServer({
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
tokenLookupLocations: [
async payload => {
expect(payload.request).toBeDefined();
Expand Down Expand Up @@ -539,7 +579,7 @@ describe('jwt plugin', () => {
const secret = 'topsecret';
const test = createTestServer(
{
singingKeyProviders: [createInlineSigningKeyProvider(secret)],
signingKeyProviders: [createInlineSigningKeyProvider(secret)],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a test for the backward compatibility to prevent future regressions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean you want the old misspelled option should continue to work and we should have a test for this ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, Until we completely remove it, let's make sure it works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh i just realized this change would break the existing users because it needs signing one as required while the old one is optional now. A union type would be better no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aaaah you're right ! I will fix this and add test

tokenLookupLocations: [
extractFromHeader({
name: 'Authorization',
Expand Down
52 changes: 37 additions & 15 deletions packages/plugins/jwt/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,30 @@ export type ExtractTokenFunction = (params: {

export type GetSigningKeyFunction = (kid?: string) => Promise<string> | string;

export type JwtPluginOptions = {
/**
* List of configurations for the signin-key providers. You can configure multiple signin-key providers to allow for key rotation, fallbacks, etc.
*
* In addition, you can use the `remote` variant and configure [`jwks-rsa`'s JWKS client](https://github.com/auth0/node-jwks-rsa/tree/master).
*
* The plugin will try to fetch the keys from the providers in the order they are defined in this array.
*
* If the first provider fails to fetch the keys, the plugin will try the next provider in the list.
*
*/
singingKeyProviders: AtleastOneItem<GetSigningKeyFunction>;
export type JwtPluginOptions = (
| {
/**
* List of configurations for the signin-key providers. You can configure multiple signin-key providers to allow for key rotation, fallbacks, etc.
*
* In addition, you can use the `remote` variant and configure [`jwks-rsa`'s JWKS client](https://github.com/auth0/node-jwks-rsa/tree/master).
*
* The plugin will try to fetch the keys from the providers in the order they are defined in this array.
*
* If the first provider fails to fetch the keys, the plugin will try the next provider in the list.
*
*/
signingKeyProviders: AtleastOneItem<GetSigningKeyFunction>;
singingKeyProviders?: never;
}
| {
/**
* @deprecated: please use `signingKeyProviders` instead.
*/

singingKeyProviders: AtleastOneItem<GetSigningKeyFunction>;
signingKeyProviders?: never;
}
) & {
/**
* List of locations to look for the token in the incoming request.
*
Expand Down Expand Up @@ -77,9 +89,19 @@ export type JwtPluginOptions = {
};

export function normalizeConfig(input: JwtPluginOptions) {
if (input.singingKeyProviders.length === 0) {
// TODO: remove this on next major version.
if (input.singingKeyProviders) {
if (input.signingKeyProviders) {
throw new TypeError(
'You are using both deprecated `singingKeyProviders` and its new replacement `signingKeyProviders` configuration. Please use only `signingKeyProviders`',
);
}
(input.signingKeyProviders as unknown) = input.singingKeyProviders;
}

if (!input.signingKeyProviders) {
throw new TypeError(
'You must provide at least one signing key provider. Please verify your `singingKeyProviders` configuration.',
'You must provide at least one signing key provider. Please verify your `signingKeyProviders` configuration.',
);
}

Expand All @@ -102,7 +124,7 @@ export function normalizeConfig(input: JwtPluginOptions) {
}

return {
singingKeyProviders: input.singingKeyProviders,
signingKeyProviders: input.signingKeyProviders,
tokenLookupLocations,
tokenVerification: input.tokenVerification ?? {
algorithms: ['RS256', 'HS256'],
Expand Down
2 changes: 1 addition & 1 deletion packages/plugins/jwt/src/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function useJWT(options: JwtPluginOptions): Plugin<{
};

const getSigningKey = async (kid?: string) => {
for (const provider of normalizedOptions.singingKeyProviders) {
for (const provider of normalizedOptions.signingKeyProviders) {
try {
const key = await provider(kid);

Expand Down
8 changes: 4 additions & 4 deletions website/src/pages/docs/features/jwt.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const yoga = createYoga({
plugins: [
useJWT({
// Configure your signing providers: either a local signing-key or a remote JWKS are supported.
singingKeyProviders: [
signingKeyProviders: [
createInlineSigningKeyProvider(signingKey),
createRemoteJwksSigningKeyProvider({ jwksUri: 'https://example.com/.well-known/jwks.json' })
]
Expand Down Expand Up @@ -230,7 +230,7 @@ const yoga = createYoga({
// ...
plugins: [
useJWT({
singingKeyProviders: [createInlineSigningKeyProvider(process.env.MY_JWT_SECRET)]
signingKeyProviders: [createInlineSigningKeyProvider(process.env.MY_JWT_SECRET)]
})
]
})
Expand All @@ -253,7 +253,7 @@ const yoga = createYoga({
// ...
plugins: [
useJWT({
singingKeyProviders: [
signingKeyProviders: [
createRemoteJwksSigningKeyProvider({
jwksUri: 'https://example.com/.well-known/jwks.json'
})
Expand All @@ -278,7 +278,7 @@ const yoga = createYoga({
// ...
plugins: [
useJWT({
singingKeyProviders: [
signingKeyProviders: [
// In case your remote provider is not available, the plugin will try use the inline provider.
createRemoteJwksSigningKeyProvider({
jwksUri: 'https://example.com/.well-known/jwks.json'
Expand Down
Loading