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 API Key auth #5399

Merged
merged 3 commits into from
Nov 10, 2022
Merged

Add API Key auth #5399

merged 3 commits into from
Nov 10, 2022

Conversation

Asamsig
Copy link
Contributor

@Asamsig Asamsig commented Nov 8, 2022

Supports header / query parameter in both webrequest and websocket

changelog(Improvements): Added API Key Authentication method for HTTP and WebSocket requests

@Asamsig
Copy link
Contributor Author

Asamsig commented Nov 8, 2022

Amended the commit, had an unused import.

@Asamsig
Copy link
Contributor Author

Asamsig commented Nov 8, 2022

Added support for cookie as well.

I started to look into updating the importers (specifically the OpenApi 3 importer), and it seems pretty straight forward, except it hits a limitation in the sense that we can only support ONE securityScheme at a time, but this is common for all authentication types in Insomnia.
I.e. if an endpoint support several security schemes, the importer will have to just pick one, it already does this for other auth types, but currently for API Key auth it actually supports several since just places them in the header, query params, and cookie tabs that supports lists, which the auth tab does not.

I guess it might be seen as a downgrade for some people, if we change the importer in that way, any thoughts/input?

Copy link
Member

@filfreire filfreire left a comment

Choose a reason for hiding this comment

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

@Asamsig I tested this locally, LGTM, thank you for your contribution!

Regarding your comment above:

  • I think the default behavior could be kept the OpenAPI importer, meaning there's 1 or more api keys on the security schema, when requests are generated, the api keys go into headers like before
  • For Postman importer I've taken the liberty of pushing and adding it to this PR, hope you don't mind.

From my side I think this is in mergeable state, waiting for someone else from @Kong/team-insomnia to also give the 👍

@filfreire filfreire requested a review from a team November 9, 2022 17:26
@Asamsig
Copy link
Contributor Author

Asamsig commented Nov 9, 2022

@filfreire Yeah, so if there is only one security scheme, we could use the new auth, but otherwise preserve the current behaviour, that seems reasonable.
I'll take a look over the next few days, and create a separate PR for that.

Awesome that you already updated the Postman importer!

Copy link
Contributor

@gatzjames gatzjames left a comment

Choose a reason for hiding this comment

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

👍 Good one! Thanks for the PR!

@filfreire filfreire merged commit 5095c60 into Kong:develop Nov 10, 2022
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