Add CORS route to DevConsole

[sberyozkin]

No description provided.

[quarkus-bot]

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

• title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

[sberyozkin]

Hi @gsmet Not sure about the bot message, it starts with Add ...

[sberyozkin]

@michalvavrik Hi Michal, looks like io.quarkus.resteasy.test.security.HttpPolicyAuthFailureExceptionMapperTest.testAuthFailedExceptionMapper failure on Win might suggest there is some race condition there, please investigate when you'll have some time

[michalvavrik]

@michalvavrik Hi Michal, looks like io.quarkus.resteasy.test.security.HttpPolicyAuthFailureExceptionMapperTest.testAuthFailedExceptionMapper failure on Win might suggest there is some race condition there, please investigate when you'll have some time

I'll check today and get back to you.

[michalvavrik]

@michalvavrik Hi Michal, looks like io.quarkus.resteasy.test.security.HttpPolicyAuthFailureExceptionMapperTest.testAuthFailedExceptionMapper failure on Win might suggest there is some race condition there, please investigate when you'll have some time

I'll check today and get back to you.

I run your PR on Windows Server 2022 and could not reproduce it, I'll try investigate how could it theoretically happen, but please, if you ever see this failure again in another PR/CI, please let me know, thank you.

[michalvavrik]

I run your PR on Windows Server 2022 and could not reproduce it, I'll try investigate how could it theoretically happen, but please, if you ever see this failure again in another PR/CI, please let me know, thank you.

Ah, I can explain it (with little guessing), will provide PR later today.

[sberyozkin]

@stuartwdouglas I've added a dedicated Cors configuration group, for now it only has a single enabled property, but we can use this group to let users tune it, example, allow a dns lookup, etc

[pmlopes]

@sberyozkin @stuartwdouglas @cescoffier: be aware that Chrome behavior for localhost may not be what you expect:

https://bugs.chromium.org/p/chromium/issues/detail?id=67743

[sberyozkin]

Hi @pmlopes

be aware that Chrome behavior for localhost may not be what you expect:
https://bugs.chromium.org/p/chromium/issues/detail?id=67743

Yeah, thanks, we touched upon some of the issues like that one; here though the DevConsole filter requires concrete Origin values, wildcard will not be returned

[rsvoboda]

I think this PR needs some changes.

HTTPS topic was covered in my previous comments, config update using DevUI on https is not working.

There are other issues I found:

127.0.0.1 use-case
Changing value using http://127.0.0.1:8080/api/q/dev/io.quarkus.quarkus-vertx-http/config triggers 2022-11-23 08:58:39,800 ERROR [io.qua.ver.htt.run.dev.DevConsoleCORSFilter] (vert.x-eventloop-thread-1) Only localhost origin is allowed, but Origin header value is: http://127.0.0.1:8080

hostname use-case
Changing value using http://rsvoboda-mac:8080/api/q/dev/io.quarkus.quarkus-vertx-http/config triggers 2022-11-23 09:45:31,871 ERROR [io.qua.ver.htt.run.dev.DevConsoleCORSFilter] (vert.x-eventloop-thread-0) Only localhost origin is allowed, but Origin header value is: http://rsvoboda-mac:8080

traceroute rsvoboda-mac                                                                                                                                               
traceroute to localhost (127.0.0.1), 64 hops max, 52 byte packets
 1  localhost (127.0.0.1)  0.611 ms  0.050 ms  0.040 ms

I think at least 127.0.0.1 use-case should be covered.
Hostname use-case could be handled later

[sberyozkin]

@rsvoboda

I think at least 127.0.0.1 use-case should be covered.

Sure, fixed it.

Hostname use-case could be handled later

I agree, I'm not sure a DNS related check will have no side-effects on the overall DevUI experience. Local DevUI is meant to be used locally, and typing a fully qualified host name during the local work seems unnecessary.

But we have a new DevUI specific CORS group - so we can add a property there, allow-dns-reverse-lookup, which, if set to true will allow us check if the current DNS name resolves to localhost, we should probably do it later if you agree

[rsvoboda]

@sberyozkin agree with the plan. I will try some experiments with this PR and provide feedback tmr morning

[rsvoboda]

No further concerns with this PR

Checked items:

• http vs. https

• localhost use-case vs. 127.0.0.1 use-case vs. hostname use-case

• /q/dev vs. customized /api/q/dev

• OS: macOS, Windows

• Browsers: Firefox, Vivaldi, Brave, Chrome, Edge, Safari

[sberyozkin]

@rsvoboda

Browsers: Firefox, Vivaldi, Brave, Chrome, Edge, Safari

This is a proper testing :-), thanks

[cescoffier]

Browsers: Firefox, Vivaldi, Brave, Chrome, Edge, Safari

And no Netscape?

[maxandersen]

I'm missing Opera :)

[rsvoboda]

No worries, I tried Opera too, just didn't list it :) Netscape ... hm, I would need to time travel back to Windows 98 times
Fun experiment was with iOS based Safari check using Web Inspector when I had my iOS device connected to Safari on my MBP machine.

[rsvoboda]

Note to https check - the overall DevUI experience is affected by #29431, application.properties gets updated but the UI stays grey for the edited entry, the page needs to be reloaded.

[quarkus-bot]

/cc @mjurc, @rsvoboda