quarkus.http.root-path and paths in application.properties

[Sergey Beryozkin]

Hi

A couple of issues have been opened recently against Quarkus Security where `quarkus.http.root-path` is not taken into consideration, for example,

GET /example/a

will not be checked by Quarkus Security with

quarkus.http.root-path=/example

quarkus.http.auth.permission.authenticated.paths=/a

So I fixed it with https://github.com/quarkusio/quarkus/pull/24794

by prepending `quarkus.http.root-path` to various `quarkus.http.auth.permission.*` paths.

Another issue has been opened recently against quarkus-oidc, with CodeAuthenticationMechanism's `logout-path` not taking `quarkus.http.root-path` into consideration either. CodeAuthenticationMechanism has probably at least 4 more configurable paths like redirect path, backchannel logout, error path, etc. quarkus-oidc if the current request path matches one of these configured paths and then acts accordingly.

And I've started doubting the fix I did at https://github.com/quarkusio/quarkus/pull/24794 (related to `quarkus.http.auth.permission.*` paths was correct).

I appreciate that when JAX-RS endpoints/Vertx routes and non application paths under `/q/*` are set up, `quarkus.http.root-path` is prepended under the hood.

However, we can have many extensions with some extension specific paths, so I wonder should it really be a user's responsibility to have

1) `${quarkus.http.root-path:""}/some-extension-path` in `application.properties` instead of 2) every extension prepending ${quarkus.http.root-path} to `some-extension-path` when checking if the current request matches this `some-extension-path`.

I think  1) `${quarkus.http.root-path:""}/some-extension-path` in `application.properties` is reasonable and I'd actually like to revert https://github.com/quarkusio/quarkus/pull/24794/files#diff-16046b5f1984bb865609bb722770c2ea36e4f47c71aa030a1157eafba48c982d before it really got into the product(s).

But I guess what can tilt it in favour of 2) (extensions prepending it themselves) is that `quarkus.http.root-path` is a build time property while many of these extension specific paths are runtime properties.

What do you think ?

Thanks, Sergey

[David Lloyd]

On Mon, Apr 25, 2022 at 9:15 AM Sergey Beryozkin <sbia...@redhat.com> wrote:
> However, we can have many extensions with some extension specific paths, so I wonder should it really be a user's responsibility to have
>
> 1) `${quarkus.http.root-path:""}/some-extension-path` in `application.properties` instead of 2) every extension prepending ${quarkus.http.root-path} to `some-extension-path` when checking if the current request matches this `some-extension-path`.

Do we really require users to do this manually? Doesn't each extension
already provide this kind of info as default configuration values when
that extension is enabled?

--
- DML • he/him

[Sergey Beryozkin]

Do you mean, that for example, when a user configures `quarkus.oidc.logout.logout-path=/logout` and `quarkus.http.root-path=/root`

then what the extension (in this case, quarkus-oidc) sees, when checking `quarkus.oidc.logout.logout-path`, is `/example/logout` ?

It is not the case at the moment as far as I know. So I started with updating some of Quarkus Security code (as per my earlier message) but I'm not sure if it is the right way to continue;

I think I see your point though, expecting users to prepend `${quarkus.http.root-path:""}/some-extension-path` is not convenient for the users...

Cheers, Sergey

 

--
- DML • he/him

[Sergey Beryozkin]

On Mon, Apr 25, 2022 at 3:40 PM Sergey Beryozkin <sbia...@redhat.com> wrote:

On Mon, Apr 25, 2022 at 3:27 PM David Lloyd <david...@redhat.com> wrote:

On Mon, Apr 25, 2022 at 9:15 AM Sergey Beryozkin <sbia...@redhat.com> wrote:
> However, we can have many extensions with some extension specific paths, so I wonder should it really be a user's responsibility to have
>
> 1) `${quarkus.http.root-path:""}/some-extension-path` in `application.properties` instead of 2) every extension prepending ${quarkus.http.root-path} to `some-extension-path` when checking if the current request matches this `some-extension-path`.

Do we really require users to do this manually? Doesn't each extension
already provide this kind of info as default configuration values when
that extension is enabled?

Do you mean, that for example, when a user configures `quarkus.oidc.logout.logout-path=/logout` and `quarkus.http.root-path=/root`

then what the extension (in this case, quarkus-oidc) sees, when checking `quarkus.oidc.logout.logout-path`, is `/example/logout` ?

Sorry, in this case I meant `/root/logout`...

[David Lloyd]

On Mon, Apr 25, 2022 at 9:40 AM Sergey Beryozkin <sbia...@redhat.com> wrote:
> On Mon, Apr 25, 2022 at 3:27 PM David Lloyd <david...@redhat.com> wrote:
>> On Mon, Apr 25, 2022 at 9:15 AM Sergey Beryozkin <sbia...@redhat.com> wrote:
>> > However, we can have many extensions with some extension specific paths, so I wonder should it really be a user's responsibility to have
>> >
>> > 1) `${quarkus.http.root-path:""}/some-extension-path` in `application.properties` instead of 2) every extension prepending ${quarkus.http.root-path} to `some-extension-path` when checking if the current request matches this `some-extension-path`.
>>
>> Do we really require users to do this manually? Doesn't each extension
>> already provide this kind of info as default configuration values when
>> that extension is enabled?
>>
> Do you mean, that for example, when a user configures `quarkus.oidc.logout.logout-path=/logout` and `quarkus.http.root-path=/root`
>

> then what the extension (in this case, quarkus-oidc) sees, when checking `quarkus.oidc.logout.logout-path`, is `/root/logout` ?

Not exactly, I was thinking more that maybe (in this particular case
at least) the extension could have a default logout path already which
might or might not be enabled separately from the configuration
property that specifies the path.

> It is not the case at the moment as far as I know. So I started with updating some of Quarkus Security code (as per my earlier message) but I'm not sure if it is the right way to continue;
>
> I think I see your point though, expecting users to prepend `${quarkus.http.root-path:""}/some-extension-path` is not convenient for the users...

Yeah, but at the same time it's hard to think of another solution that
doesn't introduce an inconsistency or problem somewhere else. Having
the prepended property is the most flexible because then the user can
put the path somewhere outside of the `root-path`.

That said, maybe another indirection would be better, e.g.:

quarkus.oidc.root-path=${quarkus.http.root-path:"/"} # this would
be the default so the user doesn't have to specify
quarkus.oidc.logout.logout-path=logout # `quarkus.oidc.root-path`
is prepended to this

Then if the user wants to escape the `http.root-path` or use a
sub-path of that path only for OIDC, they can do so, and they also
don't have to add a property expression. The downside is that it is
another property to configure, slightly more complexity, etc.
--
- DML • he/him

[Sergey Beryozkin]

Hi David

On Mon, Apr 25, 2022 at 4:03 PM David Lloyd <david...@redhat.com> wrote:

On Mon, Apr 25, 2022 at 9:40 AM Sergey Beryozkin <sbia...@redhat.com> wrote:
> On Mon, Apr 25, 2022 at 3:27 PM David Lloyd <david...@redhat.com> wrote:
>> On Mon, Apr 25, 2022 at 9:15 AM Sergey Beryozkin <sbia...@redhat.com> wrote:
>> > However, we can have many extensions with some extension specific paths, so I wonder should it really be a user's responsibility to have
>> >
>> > 1) `${quarkus.http.root-path:""}/some-extension-path` in `application.properties` instead of 2) every extension prepending ${quarkus.http.root-path} to `some-extension-path` when checking if the current request matches this `some-extension-path`.
>>
>> Do we really require users to do this manually? Doesn't each extension
>> already provide this kind of info as default configuration values when
>> that extension is enabled?
>>
> Do you mean, that for example, when a user configures `quarkus.oidc.logout.logout-path=/logout` and `quarkus.http.root-path=/root`
>
> then what the extension (in this case, quarkus-oidc) sees, when checking `quarkus.oidc.logout.logout-path`, is `/root/logout` ?

Not exactly, I was thinking more that maybe (in this particular case
at least) the extension could have a default logout path already which
might or might not be enabled separately from the configuration
property that specifies the path.

OK

> It is not the case at the moment as far as I know. So I started with updating some of Quarkus Security code (as per my earlier message) but I'm not sure if it is the right way to continue;
>
> I think I see your point though, expecting users to prepend `${quarkus.http.root-path:""}/some-extension-path` is not convenient for the users...

Yeah, but at the same time it's hard to think of another solution that
doesn't introduce an inconsistency or problem somewhere else.

This is why I started getting worried about my earlier fix (done at the vertx-http level) where I updated the extension for it to add `quarkus.http.root-path` itself to the configured security policy paths - in fact it can break 2.7.x users attempting to migrate to 2.8.1 or from 2.8.0 to 2.8.1 if they already prepend `${quarkus.http.root-path}` to the configured security policy paths themselves. I'll revert that fix.

 

Having
the prepended property is the most flexible because then the user can
put the path somewhere outside of the `root-path`.

That said, maybe another indirection would be better, e.g.:

   quarkus.oidc.root-path=${quarkus.http.root-path:"/"} # this would
be the default so the user doesn't have to specify
   quarkus.oidc.logout.logout-path=logout # `quarkus.oidc.root-path`
is prepended to this

Yeah, interesting...

Then if the user wants to escape the `http.root-path` or use a
sub-path of that path only for OIDC, they can do so, and they also
don't have to add a property expression. The downside is that it is
another property to configure, slightly more complexity, etc.

Thanks for the idea David, but indeed, I think I'd rather avoid adding such a new property right now - as we'd need then, for consistency purposes, do something similar in vertx-http ( for security policy paths, keycloak-authorization for its own paths, and so on for other extensions which might have such paths). Worth keeping your idea in mind though :-)

Cheers, Sergey

--
- DML • he/him

[Erin Schnabel]

I had a long conversation w/ Sergey about this, and here is what I'll say: 

From a user perspective, being consistent in how we specify paths is really important (because we otherwise get in a tangle).

- Path resolution: https://quarkus.io/blog/path-resolution-in-quarkus/

For simplicity's sake, this is what you should be thinking about when creating something that would allow a user to configure an endpoint path: 

- If possible, use the build items to resolve config: 

- io.quarkus.vertx.http.deployment.HttpRootPathBuildItem;

    - io.quarkus.vertx.http.deployment.NonApplicationRootPathBuildItem;

- If you can't use build items (because the paths can be changed at runtime)

    - Allow specification of relative paths (resolved against quarkus.root.path)

    - Allow specification of absolute paths (interpreted as-is/literally)

I suppose the vertx http runtime component could provide a Converter that is aware of the root path and would handle this absolute/relative stuff? Would that be a useful thing?



 

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAMsYBfXPW6HpEwpNJd%3Db4971G-55LEG7%2B%2BtWOKJgrJOsFa8mFw%40mail.gmail.com.

[Sergey Beryozkin]

Hi Erin

Thanks, it makes sense, and what really works here is that it would not be a breaking change when it comes to the way various security policy path configurations are done and documented, they all start from `/`. So, if a user would like to have `quarkus.http.root-path` prepended then `/` needs to be dropped from a configured path value.

I was quite confused initially by an `absolute path` terminology, since these policy paths are representing relative URL path component values to be compared against HTTP request paths, absolute URLs are not compared. However `relative to quarkus.http.root-path` point made it easy to grasp the idea...

Cheers, Sergey

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAJwjk9yGVdA%2Bue9W-GzXw1hQ0EY0k14YzUByHOKvRaU5kjp5rw%40mail.gmail.com.