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

Add extra API keys as headers or query params #759

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

webholik
Copy link
Contributor

@webholik webholik commented Sep 4, 2023

If we encounter extra API keys in the security rule set we will add them as extra header/query params in the request.

@webholik webholik changed the title [IMPORT-820] Add extra API keys as headers or query params Add extra API keys as headers or query params Sep 4, 2023
Copy link
Member

@VShingala VShingala left a comment

Choose a reason for hiding this comment

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

If we make changes into collection generation flow, we also have to take care of this in validateTransactions flow, otherwise users will have issues in that flow where these headers will be removed in suggestions.

if (_.isObject(security) && _.isEmpty(security)) {
helper = {
authHelper = {
Copy link
Member

Choose a reason for hiding this comment

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

Why are we using authHelper here specifically if it will anyways get assigned helper in line no. 1300.

if (_.get(apiKey, 'apikey.2.value') === 'header') {
headers.push(keyValuePair);
}
else {
Copy link
Member

Choose a reason for hiding this comment

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

let's use else if ( === query ) { condition specifically to represent this better here. As API key can be mentioned inside cookie as well.

const securitySet = _.get(openapi, 'security', []);
let count = 0;

_.forEach(securitySet, (security) => {
Copy link
Member

Choose a reason for hiding this comment

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

This is not how you should iterate for multiple API keys.

Take a look at this article: there are combinations of AND and OR in case when multiple keys are mentioned.

https://swagger.io/docs/specification/authentication/api-keys/?sbsearch=API%20Keys

@VShingala
Copy link
Member

@webholik Let's address the comments and make the related changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants