Skip to content

Commit

Permalink
fix: properly send label with unified snippet (#960)
Browse files Browse the repository at this point in the history
With the unified snippet work we weren't properly getting the label from
the keys array. This makes getGroupIdByApiKey more generic to return the
label as well.
  • Loading branch information
mjcuva authored Feb 7, 2024
1 parent 326e953 commit 751a8a4
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 30 deletions.
10 changes: 5 additions & 5 deletions packages/node/src/lib/ReadMe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { NextFunction, Request, Response } from 'express';
import readmeSdk from '../.api/apis/developers';

import findAPIKey from './find-api-key';
import { getGroupIdByApiKey } from './get-group-id';
import { getGroupByApiKey } from './get-group-id';
import { log } from './log';
import { buildSetupView } from './setup-readme-view';
import { testVerifyWebhook } from './test-verify-webhook';
Expand Down Expand Up @@ -189,8 +189,8 @@ export default class ReadMe {
const user = await userFunction(req, getUser);
if (!user || !Object.keys(user).length || options.disableMetrics) return next();

const groupId = getGroupIdByApiKey(user, requestAPIKey);
if (!groupId) {
const group = getGroupByApiKey(user, requestAPIKey);
if (!group) {
console.error(
usingManualAPIKey
? 'The API key you passed in via `manualAPIKey` could not be found in the user object you provided.'
Expand All @@ -204,8 +204,8 @@ export default class ReadMe {
req,
res,
{
apiKey: groupId,
label: user.label ? user.label : user.name,
apiKey: group.id,
label: group.label,
email: user.email,
},
options,
Expand Down
9 changes: 6 additions & 3 deletions packages/node/src/lib/get-group-id.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const getGroupFromKeysBySecuritySchemes = (keys: ApiKey[] = [], securitySchemes:
/**
* Gets the group ID from the keys array for a given apiKey
*/
const getGroupFromKeysByApiKey = (keys: ApiKey[] = [], apiKey = ''): string | undefined => {
const getGroupFromKeysByApiKey = (keys: ApiKey[] = [], apiKey = '') => {
if (!keys || !Array.isArray(keys)) {
return undefined;
}
Expand All @@ -109,7 +109,10 @@ const getGroupFromKeysByApiKey = (keys: ApiKey[] = [], apiKey = ''): string | un
return undefined;
}

return get(relevantKey, groupKey) as string;
return {
id: get(relevantKey, groupKey) as string,
label: get(relevantKey, 'label') || get(relevantKey, 'name'),
};
};

/**
Expand Down Expand Up @@ -141,7 +144,7 @@ const getFromUser = (user: GroupingObject, securitySchemes: string[]): string |
* 5. The key "label"
* 6. The key "name"
*/
export const getGroupIdByApiKey = (user: GroupingObject, apiKey: string) => {
export const getGroupByApiKey = (user: GroupingObject, apiKey: string) => {
if (!user) {
return undefined;
}
Expand Down
59 changes: 37 additions & 22 deletions packages/node/test/lib/get-group-id.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { Operation } from 'oas';

import { describe, expect, it, beforeEach } from 'vitest';

import { getGroupIdByApiKey, getGroupIdByOperation } from '../../src/lib/get-group-id';
import { getGroupByApiKey, getGroupIdByOperation } from '../../src/lib/get-group-id';

const MOCK_USER = {
id: 'user-id',
Expand Down Expand Up @@ -184,27 +184,27 @@ describe('getGroupId', () => {

describe('byApiKey', () => {
it('returns undefined when no user is passed', () => {
const groupId = getGroupIdByApiKey(undefined, 'requestApiKey');
const groupId = getGroupByApiKey(undefined, 'requestApiKey');
expect(groupId).toBeUndefined();
});

it('returns undefined for a user without a keys array', () => {
const groupId = getGroupIdByApiKey({} as GroupingObject, 'requestApiKey');
const groupId = getGroupByApiKey({} as GroupingObject, 'requestApiKey');
expect(groupId).toBeUndefined();
});

it('returns undefined for a user with a null keys array', () => {
const groupId = getGroupIdByApiKey(mockUser(null), 'requestApiKey');
const groupId = getGroupByApiKey(mockUser(null), 'requestApiKey');
expect(groupId).toBeUndefined();
});

it('returns undefined for a user with an object as the keys array', () => {
const groupId = getGroupIdByApiKey({ keys: {} } as GroupingObject, 'requestApiKey');
const groupId = getGroupByApiKey({ keys: {} } as GroupingObject, 'requestApiKey');
expect(groupId).toBeUndefined();
});

it('returns undefined for a user with a string as the keys array', () => {
const groupId = getGroupIdByApiKey({ keys: 'broken' as unknown } as GroupingObject, 'requestApiKey');
const groupId = getGroupByApiKey({ keys: 'broken' as unknown } as GroupingObject, 'requestApiKey');
expect(groupId).toBeUndefined();
});

Expand All @@ -218,16 +218,16 @@ describe('getGroupId', () => {
otherField: 'requestApiKey',
},
]);
const groupId = getGroupIdByApiKey(user, 'requestApiKey');
const groupId = getGroupByApiKey(user, 'requestApiKey');

expect(groupId).toBe('key-2-id');
expect(groupId.id).toBe('key-2-id');
});

it('returns the id of the key as first priority', () => {
const user = mockUser([{ id: 'key-1-id', name: 'key-1-name', otherField: 'requestApiKey' }]);
const groupId = getGroupIdByApiKey(user, 'requestApiKey');
const groupId = getGroupByApiKey(user, 'requestApiKey');

expect(groupId).toBe('key-1-id');
expect(groupId.id).toBe('key-1-id');
});

it('returns the apiKey of the key as second priority', () => {
Expand All @@ -238,37 +238,37 @@ describe('getGroupId', () => {
apiKey: 'key-1-apiKey',
},
]);
const groupId = getGroupIdByApiKey(user, 'requestApiKey');
const groupId = getGroupByApiKey(user, 'requestApiKey');

expect(groupId).toBe('key-1-apiKey');
expect(groupId.id).toBe('key-1-apiKey');
});

it('returns the value of the matching apiKey as the third priority', () => {
const user = mockUser([{ otherField: 'requestApiKey' }]);
const groupId = getGroupIdByApiKey(user, 'requestApiKey');
const groupId = getGroupByApiKey(user, 'requestApiKey');

expect(groupId).toBe('requestApiKey');
expect(groupId.id).toBe('requestApiKey');
});

it('returns the basic user as the fourth priority', () => {
const user = mockUser([{ user: 'basic-user', pass: 'basic-pass' }]);
const groupId = getGroupIdByApiKey(user, 'requestApiKey');
const groupId = getGroupByApiKey(user, 'requestApiKey');

expect(groupId).toBe('basic-user');
expect(groupId.id).toBe('basic-user');
});

it('returns the name of the key as fifth priority', () => {
const user = mockUser([{ name: 'key-1-name' }]);
const groupId = getGroupIdByApiKey(user, 'requestApiKey');
const groupId = getGroupByApiKey(user, 'requestApiKey');

expect(groupId).toBe('key-1-name');
expect(groupId.id).toBe('key-1-name');
});

it('supports having nested basic auth', () => {
const user = mockUser([{ notRelevant: 'foo' }, { basic: { user: 'basic-user', pass: 'basic-pass' } }]);
const groupId = getGroupIdByApiKey(user, 'basic-user');
const groupId = getGroupByApiKey(user, 'basic-user');

expect(groupId).toBe('basic-user');
expect(groupId.id).toBe('basic-user');
});
});

Expand All @@ -281,9 +281,24 @@ describe('getGroupId', () => {
name: 'key-2-name',
},
]);
const groupId = getGroupIdByApiKey(user, 'requestApiKey');
const groupId = getGroupByApiKey(user, 'requestApiKey');

expect(groupId.id).toBe('key-1-id');
});
});

describe('return the label from the keys array', () => {
it('returns the label', () => {
const user = mockUser([
{ id: 'key-1-id', label: 'key-1-name' },
{
id: 'key-2-id',
label: 'key-2-name',
},
]);
const groupId = getGroupByApiKey(user, 'requestApiKey');

expect(groupId).toBe('key-1-id');
expect(groupId.label).toBe('key-1-name');
});
});
});
Expand Down

0 comments on commit 751a8a4

Please sign in to comment.