Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
},
"dependencies": {
"@stoplight/json": "^3.0.1",
"@stoplight/types": "^9.1.2",
"@stoplight/types": "^11.0.0",
"@types/swagger-schema-official": "~2.0.18",
"@types/urijs": "~1.19.1",
"lodash": "^4.17.11",
Expand Down
111 changes: 96 additions & 15 deletions src/oas2/__tests__/accessors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,90 @@ const securityDefinitionsFixture: Dictionary<Security> = {
};

describe('accessors', () => {
const securityFixture = [
const securityFixture: Array<Dictionary<string[], string>> = [
{
api_key: [],
petstore_auth: ['write:pets', 'read:pets'],
},
];

describe('relation between schemes', () => {
describe('when all of the given schemes are expected to be validated against', () => {
const securityFixtureWithAndRelation = securityFixture;

it('returns an array containing multiple elements', () => {
expect(
getSecurities(
{
securityDefinitions: securityDefinitionsFixture,
security: [],
},
securityFixtureWithAndRelation,
),
).toEqual([
[
{
in: 'header',
name: 'api_key',
type: 'apiKey',
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason you can think of that we might need the security array's key here? Currently there is nothing tying this oauth2 object back to the security array. Does that make sense?

Suggested change
{
{
securityKey: 'petstore_auth',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lottamus, I think that given https://github.com/stoplightio/http-spec/pull/15/files#diff-f5ac86d0dd4d4fa1390aca28ac1fbc59L107-L112 it's OK to just not add this securityKey? If it was not a problem before, it shouldn't be a problem now. We can address it later if it turns out to be an issue

authorizationUrl: 'http://swagger.io/api/oauth/dialog',
flow: 'implicit',
scopes: {
'read:pets': 'read your pets',
'write:pets': 'modify pets in your account',
},
type: 'oauth2',
},
],
]);
});
});

describe('when one of the given schemes is expected to be validated against', () => {
it('returns arrays containing one element each', () => {
const securityFixtureWithOrRelation: Array<Dictionary<string[], string>> = [
{
petstore_auth: ['write:pets', 'read:pets'],
},
{
api_key: [],
},
];

expect(
getSecurities(
{
securityDefinitions: securityDefinitionsFixture,
security: [],
},
securityFixtureWithOrRelation,
),
).toEqual([
[
{
authorizationUrl: 'http://swagger.io/api/oauth/dialog',
flow: 'implicit',
scopes: {
'read:pets': 'read your pets',
'write:pets': 'modify pets in your account',
},
type: 'oauth2',
},
],
[
{
in: 'header',
name: 'api_key',
type: 'apiKey',
},
],
]);
});
});
});

describe('getSecurities', () => {
test('given no security definitions should return empty array', () => {
expect(
Expand Down Expand Up @@ -87,7 +164,7 @@ describe('accessors', () => {
},

Choose a reason for hiding this comment

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

Looks good but needs some more tests: something that can prove we support both AND & OR cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

@chris-miaskowski @lag-of-death is there anything left to address here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lottamus, no, the test was added:
#15 (comment)

],
),
).toEqual([{ in: 'header', name: 'api_key', type: 'apiKey' }]);
).toEqual([[{ in: 'header', name: 'api_key', type: 'apiKey' }]]);
});

test('given security with custom scopes should override global definition', () => {
Expand All @@ -104,12 +181,14 @@ describe('accessors', () => {
],
),
).toEqual([
{
authorizationUrl: 'http://swagger.io/api/oauth/dialog',
flow: 'implicit',
scopes: { 'read:pets': 'read your pets', 'write:pets': 'modify pets in your account' },
type: 'oauth2',
},
[
{
authorizationUrl: 'http://swagger.io/api/oauth/dialog',
flow: 'implicit',
scopes: { 'read:pets': 'read your pets', 'write:pets': 'modify pets in your account' },
type: 'oauth2',
},
],
]);
});

Expand All @@ -123,13 +202,15 @@ describe('accessors', () => {
securityFixture,
),
).toEqual([
{ in: 'header', name: 'api_key', type: 'apiKey' },
{
authorizationUrl: 'http://swagger.io/api/oauth/dialog',
flow: 'implicit',
scopes: { 'write:pets': 'modify pets in your account', 'read:pets': 'read your pets' },
type: 'oauth2',
},
[
{ in: 'header', name: 'api_key', type: 'apiKey' },
{
authorizationUrl: 'http://swagger.io/api/oauth/dialog',
flow: 'implicit',
scopes: { 'write:pets': 'modify pets in your account', 'read:pets': 'read your pets' },
type: 'oauth2',
},
],
]);
});
});
Expand Down
38 changes: 20 additions & 18 deletions src/oas2/accessors.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import { Dictionary } from '@stoplight/types';
import { compact, flatten, get, map, merge } from 'lodash';
import { compact, get, isEmpty, map, merge } from 'lodash';
import { negate } from 'lodash/fp';
import { Operation, Security, Spec } from 'swagger-schema-official';

export function getSecurities(
spec: Partial<Spec>,
operationSecurity: Array<Dictionary<string[], string>> | undefined,
): Security[] {
): Security[][] {
const globalSecurities = getSecurity(spec.security, spec.securityDefinitions || {});
const operationSecurities = getSecurity(operationSecurity, spec.securityDefinitions || {});
return !!operationSecurity ? operationSecurities : globalSecurities;

const securities = !!operationSecurity ? operationSecurities : globalSecurities;

return securities.filter(negate(isEmpty));
}

export function getProduces(spec: Partial<Spec>, operation: Partial<Operation>) {
Expand All @@ -22,21 +26,19 @@ export function getConsumes(spec: Partial<Spec>, operation: Partial<Operation>)
function getSecurity(
security: Array<Dictionary<string[], string>> | undefined,
definitions: Dictionary<Security, string>,
): Security[] {
return flatten(
map(security, sec => {
return compact(
map(Object.keys(sec), (key: string) => {
const def = definitions[key];
if (def) {
const defCopy = merge<Object, Security>({}, def);
return defCopy;
}
return null;
}),
);
}),
);
): Security[][] {
return map(security, sec => {
return compact(
map(Object.keys(sec), (key: string) => {
const def = definitions[key];
if (def) {
const defCopy = merge<Object, Security>({}, def);
return defCopy;
}
return null;
}),
);
});
}

function getProducesOrConsumes(which: 'produces' | 'consumes', spec: Partial<Spec>, operation: Partial<Operation>) {
Expand Down
Loading