feat: check custom header for dev tools connection verification #18827
Conversation
Allows to specify a custom header name to get the client address that is verfied to allow access to dev tools. In addition, validates all hops in the X-Forwaded-For chain.
vaadin-spring/src/main/java/com/vaadin/flow/spring/VaadinConfigurationProperties.java
Show resolved
Hide resolved
| return false; | ||
| } | ||
| // Always allow localhost | ||
| try { | ||
| InetAddress inetAddress = InetAddress.getByName(remoteAddress); | ||
| if (inetAddress.isLoopbackAddress()) { | ||
| if (inetAddress.isLoopbackAddress() && allowLocal) { | ||
| return true; |
There was a problem hiding this comment.
If it IS a local address and local is NOT allowed, should this not immediately return false, i.e. if it is a loopback address, always return allowLocal
| // No remote address available so we cannot check... | ||
| String hostsAllowed, boolean allowLocal) { | ||
| if (remoteAddress == null || remoteAddress.isBlank() | ||
| || (hostsAllowed == null && !allowLocal)) { |
There was a problem hiding this comment.
this interprets hostsAllowed == "" different from null while the code below interprets them as the same
There was a problem hiding this comment.
Is it ok to have the same behavior for null, empty and blank?
If so, I would directly trim to null the value read from the configuration
There was a problem hiding this comment.
I would say it is better to have the same behavior and fewer cases
| String forwardedFor = request.getHeader("X-Forwarded-For"); | ||
| if (forwardedFor != null) { | ||
| String remoteHeaderIp = configuration.getStringProperty( | ||
| SERVLET_PARAMETER_DEVMODE_REMOTE_ADDRESS_HEADER, null); |
There was a problem hiding this comment.
Are we assuming this is a single header with a single address? Should it be in the docs?
There was a problem hiding this comment.
Yes, the assumption is a single header with a single address, as this is supposed to be safely handled by the proxy server.
I'll update the docs
|
|
This ticket/PR has been released with Vaadin 24.4.0. |
Allows to specify a custom header name to get the client address that is verified to allow access to dev tools.
In addition, validates all hops in the X-Forwaded-For chain.