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.

Sign up for GitHub

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

Jump to bottom

Make sure dev ui only loads on localhost #40979

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Make sure dev ui only loads on localhost #40979

merged 1 commit into from
Jun 7, 2024

Conversation

phillip-kruger
Copy link
Member

Fix #40828

@phillip-kruger phillip-kruger requested a review from cescoffier June 5, 2024 09:01
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@phillip-kruger phillip-kruger force-pushed the remote_dev_ui branch 2 times, most recently from 2809dd9 to 5c8f1f0 Compare June 6, 2024 07:19
@phillip-kruger phillip-kruger requested a review from cescoffier June 6, 2024 07:19
cescoffier
cescoffier approved these changes Jun 6, 2024
View reviewed changes
extensions/vertx-http/runtime/src/main/java/io/quarkus/devui/runtime/LocalHostOnlyFilter.java Show resolved Hide resolved
extensions/vertx-http/runtime/src/main/java/io/quarkus/devui/runtime/LocalHostOnlyFilter.java
private static final Logger LOG = Logger.getLogger(LocalHostOnlyFilter.class);

private static final String LOCAL_HOST = "localhost";
private static final String LOCAL_HOST_IP = "127.0.0.1";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to support IPv6 (::1)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't help feel like we are somehow circumventing or not utilizing the right cors setup if we have to do these tricks? how come this is not caught by normal cors?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not really CORS. It's allowed hosts. In the case if fails the origin header is null, so CORS can not work

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@phillip-kruger phillip-kruger requested a review from cescoffier June 6, 2024 17:39
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Jun 7, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 16ad783.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@phillip-kruger phillip-kruger merged commit d6b9759 into quarkusio:main Jun 7, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone Jun 7, 2024
@slallemand
Copy link

Hello @phillip-kruger

Thanks for this improvement !
Is there a way to allow a domain with these settings ?
I want to use the dev-ui in DevSpaces so I know my domain but not the FQDN of my Quarkus service.

I tried with *.domain , %.domain with no success.

Thanks !

@cescoffier
Copy link
Member

@phillip-kruger, can you look at @slallemand's request?

@phillip-kruger
Copy link
Member Author

@slallemand - so you want to be able to pass in a regex and if it match we should allow ? Will that work ?

@slallemand
Copy link

@phillip-kruger exactly !

@phillip-kruger phillip-kruger mentioned this pull request Aug 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxandersen maxandersen maxandersen left review comments

@cescoffier cescoffier Awaiting requested review from cescoffier

Assignees
No one assigned
Labels
area/vertx kind/bugfix triage/flaky-test
Projects
None yet
Milestone
3.12.0.CR1
Development

Successfully merging this pull request may close these issues.

dynamic default index page has bad behavior when access from non localhost
4 participants
@phillip-kruger @slallemand @cescoffier @maxandersen