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

Investigate how to have all Quarkus Security path properties prepended with quarkus.http.root-path #25154

Open
sberyozkin opened this issue Apr 25, 2022 · 8 comments
Labels
area/security kind/enhancement New feature or request

Comments

@sberyozkin
Copy link
Member

Description

vertx-http, oidc, keycloak-authorization and possibly other security extensions have some endpoint paths configured, security policy paths, various OIDC code flow paths, etc.
When these paths are checked, quarkus.http.root-path is not taken into consideration internally.
We should try to find the best approach how to do it.

Currently the users can do it by prepending ${quarkus.http.root-path} to the specific path, for example:

quarkus.http.root-path=/root
quarkus.oidc.logout.logout-path=${quarkus.http.root-path}/logout

Implementation ideas

  1. Just go ahead and have every related property checked and its value auto-prepended - migration risk has to be evaluated.
  2. Each extension has a root property such as quarkus.oidc.root set to quarkus.http.root-path by default and users can override it if necessary
  3. Or stay with something like
quarkus.http.root-path=/root
quarkus.oidc.logout.logout-path=${quarkus.http.root-path}/logout
@sberyozkin
Copy link
Member Author

sberyozkin commented Apr 25, 2022

Well, we also have quarkus.servlet.context-path, not sure anyone is using it but since we have an Undertow extension and it is a doc-ed property, I guess we can't assume for sure no one is using it.

@sberyozkin
Copy link
Member Author

Comment from @knutwannheden re option 1 is here. Indeed it was a breaking change in 2.8.1 so good it was reverted for 2.8.2 (if it makes it before the release).

@knutwannheden
Copy link
Contributor

From what I understand from https://quarkus.io/blog/path-resolution-in-quarkus/ the configured paths will be relative to some context-dependent path if they don't start with a slash. Whenever they start with a slash they will be absolute (i.e. not even relative to the HTTP root). I think that would probably also work well for the paths configured with quarkus.http.auth.permission.

@sberyozkin
Copy link
Member Author

As I commented at #25155, these security related paths are URL path components, I see no point in introducing a notion of absolute paths in the context of dealing with these paths.
I'm fine with auto-prepending http.root-path if that works for everyone, I had to revert the earlier change because it was a breaking a change, with @knutwannheden confirming it.
As proposed by @gsmet it can be done for the minor version like 2.9.0.Final or more likely 2.10.0.Final

@sberyozkin
Copy link
Member Author

Absolute vs relative to http.root-path non-application /q paths situation is different - in that context it is about the endpoint access. With security paths it is about configuring the URI path values that have to be compared against the current request's path component - hence absolute vs relative does not work here.

For example, if a user configures quarkus.http-root=/root

then quarkus.http.auth.permission.permit1.paths=/public

has to work with GET /root/public, even though /public starts with /

@sberyozkin
Copy link
Member Author

Erin @ebullient has educated me more about this idea of absolute vs relative paths, and I think I'm getting it.

So if it is /a is there - nothing is added to it, if it is a - we prepend http.root-path - it is actually so simple :-) and not even a problem and not a breaking change :-).

@knutwannheden
Copy link
Contributor

Yes. Apparently I did a poor job of explaining myself 😅

@mschorsch
Copy link
Contributor

mschorsch commented Apr 26, 2022

Erin @ebullient has educated me more about this idea of absolute vs relative paths, and I think I'm getting it.

So if it is /a is there - nothing is added to it, if it is a - we prepend http.root-path - it is actually so simple :-) and not even a problem and not a breaking change :-).

Sounds great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants