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

Blocking CORS checks on DevUI - 403 #34097

Closed
federico-s opened this issue Jun 16, 2023 · 13 comments
Closed

Blocking CORS checks on DevUI - 403 #34097

federico-s opened this issue Jun 16, 2023 · 13 comments
Labels
area/dev-ui kind/bug Something isn't working

Comments

@federico-s
Copy link

Describe the bug

Hi everyone,
I have a problem with CORS and DevUI. If an external IP tries to open DevUI on the application that I host, client gets a blue screen with Quarkus logo and a "403 CORS Rejected - Invalid origin" response.
In the server log I see this:
Only localhost origin is allowed, but Origin header value is: http://xxx.xxx.xxx.xxx

This behaviour has been introduced since version 3.1.1 and I think this is the PR that causes this behaviour: #33659 .

I tried disabling CORS check in dev mode using this guide but it's not working.

Can you please have a look? I had to downgrade to 3.1.0...

Thanks!

Expected behavior

As 3.1.0

Actual behavior

403 CORS Rejected - Invalid origin

How to Reproduce?

Steps to reproduce the behaviour:

  1. Run a Qaurkus 3.1.1 (or above) application with quarkus.http.host=0.0.0.0
  2. Let an external client access to http://xxx.xxx.xxx.xxx:yyyy/q/dev-ui/
  3. Client gets a "403 CORS Rejected - Invalid origin"
  4. Try to add this configuration
  5. Client still gets a "403 CORS Rejected - Invalid origin"
  6. Downgrade Quarkus to 3.1.0
  7. Cilent can now access to DevUI

Output of uname -a or ver

Darwin MacBook-Pro-2.local 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar 6 21:00:17 PST 2023; root:xnu-8796.101.5~3/RELEASE_X86_64 x86_64

Output of java -version

openjdk-19.0.2

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.1.1 or above

Build tool (ie. output of mvnw --version or gradlew --version)

maven 3.9.2

Additional information

No response

@federico-s federico-s added the kind/bug Something isn't working label Jun 16, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 16, 2023

/cc @cescoffier (devui), @phillip-kruger (devui)

@sberyozkin
Copy link
Member

@federico-s Allowing unrestricted to from Dev UI from non localhost is a security risk and we've had to deal with such reported risks before.
Perhaps we should allow users to configure additional origins ? @phillip-kruger What do you think ?

@federico-s
Copy link
Author

@federico-s Allowing unrestricted to from Dev UI from non localhost is a security risk and we've had to deal with such reported risks before. Perhaps we should allow users to configure additional origins ? @phillip-kruger What do you think ?

I agree with you, but I think that should be kept the possibilty to do that in dev mode and the configuration mentioned in the guide doesn't work.

@sberyozkin
Copy link
Member

@federico-s That configuration is about accessing application endpoints in devmode, Dev UI CORS filter is about blocking the access to devconsole endpoints such as configuration editor for example.

For this latter case, Dev UI CORS filter should allow extra origins if configured by the user, so if quarkus.http.cors.origins is enabled, it should be recognized since Dev UI CORS filter is delegating to the base CORS filter.

Having said that, I'm not sure external client should be allowed to access DevUI meant for the local devmode, since Quarkus has:
https://quarkus.io/guides/maven-tooling#remote-development-mode

CC @stuartwdouglas as well

@edeandrea
Copy link
Contributor

Isn't this what quarkus.dev-ui.cors.enabled=false solves? Maybe some configuration options should be introduced to allow further customization of it?

@cescoffier
Copy link
Member

We should add a way to add origins. If we reuse the one from the CORS filter, we will have to document them using %dev otherwise we would change the prod ones.

@edeandrea
Copy link
Contributor

edeandrea commented Dec 10, 2023

I would think using %dev would not be a great idea since that is for the app itself? I think it would be better to treat the dev UI separately. They could reuse their apps cors settings if they wanted to...

quarkus.devui.cors.origins=${quarkus.http.cors.origins}

Or maybe we even default the devui settings to the app settings, but I feel like they should be treated separately since they are 2 separate things.

@cescoffier
Copy link
Member

I agree they are 2 separate things, but I'm not sure how it's implemented. If it's the same cors filter, having 2 properties can be tricky as ordering becomes important. @phillip-kruger do you know?

@cescoffier
Copy link
Member

ah ah, thanks! Ok, then, separate property it is!

@phillip-kruger
Copy link
Member

Sorry only catching up now. So are we saying we want a new config for Dev UI that does the same as the normal CORS Filter but for the Dev UI one ?

@cescoffier
Copy link
Member

We do not need everything, just the allowed origins.

@maxandersen
Copy link
Member

closing this and replace with more foxused specific issue #40477

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dev-ui kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants