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

Added ability to define some OpenAPI Security in Config #15659

Merged
merged 1 commit into from
Mar 16, 2021
Merged

Added ability to define some OpenAPI Security in Config #15659

merged 1 commit into from
Mar 16, 2021

Conversation

phillip-kruger
Copy link
Member

As discussed in the OpenAPI Quarkus Insights session.

This allows adding basic, JWT, OIDC and Implicit flow OAuth2 security via config.

Signed-off-by:Phillip Kruger phillip.kruger@gmail.com

@geoand
Copy link
Contributor

geoand commented Mar 12, 2021

Very nice!

I can't say I know much about this stuff, so maybe best if @sberyozkin takes a look?

@sberyozkin
Copy link
Member

@phillip-kruger agree with @geoand it looks good; perhaps they can be made mutually exclusive ? I.e, one should only see one of those security schemes as opposed to for ex Basic and OIDC ? (though we actually have a pending issue where Basic is preferred for a specific resource while OIDC - for the rest of the space but for now we don't support such a resource specific authentication)

Base automatically changed from master to main March 12, 2021 15:55
@phillip-kruger
Copy link
Member Author

@sberyozkin - how do you mean ? Should we only allow one type at a time in config ? In OpenAPI you can define multiple security schemes, in fact, you can even for instance have multiple basic schemes (That is not possible in this PR, as I thinks that is not used that much)

This only adds the definition(s), the developer will still have to annotate the individual methods (operations) with the Security Requirement.

Let me know, happy to make any changes you suggest.

@sberyozkin
Copy link
Member

@phillip-kruger I only meant that even if OpenApi allows it (multiple sec schemes) we can't back it up at the Quarkus level at a per-method level, for ex, let's say a user typed Basic and OIDC or Basic and JWT - we can't support one for one method and another one for another method - combining the schemes works but not a per-method/per-resource level.
So my only suggestion was to check if more than one scheme is declared and if yes then warn or probably fail...
Sorry if I'm missing something

@phillip-kruger
Copy link
Member Author

Hi @sberyozkin - as discussed, I changed this to only allow one of the types (mutually exclusive). Please can you review.
Thanks :)

Signed-off-by: Phillip Kruger <phillip.kruger@gmail.com>
@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Mar 16, 2021
@phillip-kruger phillip-kruger merged commit 71b5a6b into quarkusio:main Mar 16, 2021
@quarkus-bot quarkus-bot bot added this to the 1.13 - main milestone Mar 16, 2021
@phillip-kruger phillip-kruger deleted the openapi-security-by-config branch March 16, 2021 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/openapi area/smallrye triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants