Path probing fails in presence of redirects

drigz commented 2 months ago

What happened?

We're hosting the dashboard behind an auth proxy on a subpath. However, we find that unless we create special ingress rules to work around this, the dashboard doesn't work in this environment:

Open Chrome inspector.
Navigate to https://example.com/kubernetes-dashboard/
Observe a request to /assets/config/config.json (from this probing code)
The application assumes this is served by the single-page app and serves a redirect to the login page.
It then serves status code 200 and HTML on the login page.

The tab remains blank.

What did you expect to happen?

Probing requests that return 3xx status codes or HTML content are ignored, and the probing continues until it reaches the "real" config.json.

How can we reproduce it (as minimally and precisely as possible)?

I don't have a repro as it'd take some work to put one together with kind or similar rather than with our cloud infra - let me know if that would be decisive for whether this is addressed and/or whether a PR is accepted.

Anything else we need to know?

I was able to work around this by creating a new Ingress resource to serve errors on /assets/config/config.json for the probed, incorrect paths. Creating a new subdomain to expose the dashboard would have also worked.

What browsers are you seeing the problem on?

No response

Kubernetes Dashboard version

7.4.0

Kubernetes version

1.29.4

Dev environment

No response

floreks commented 2 months ago

We can extend the logic that tries to dynamically configure base href. It's quite annoying to configure an environment locally that will allow testing more complex scenarios like yours. If you've got an environment where you could test those changes, feel free to open a PR.

Palollo commented 1 month ago

I think I am in the same case... The problem is here:

      const probePath = 'assets/config/config.json';
      const origin = document.location.origin;
      const pathSegments = document.location.pathname.split('/');

      let basePath = '/';
      let configFound = false;

      for (let i = 0; i < pathSegments.length; i++) {
        const segment = pathSegments[i];
        if (segment.length > 0) {
          basePath = basePath + segment + '/';
        }

        const fullPath = origin + basePath + probePath;
        configFound = configExists(fullPath);
        if (configFound) {
          break;
        }
      }

As @drigz, I have the kubernetes dashboard in the path https://example.com/kubernetes-dashboard/, so the value of pathSegments is ["","kubernetes-dashboard",""].
The "for" loop is looking for config files in all the base paths.
When it tries with the first one "",
basePath = "/" + "" + "assets/config/config.json",
then configExist("https://example.com/assets/config/config.json") returns true because I have another application in that path which has another different "config.json" file! And the browser network log says it is corrupted (NS_ERROR_CORRUPTED_CONTENT), because it is not the expected config file.

Why it is looking for a config file in base paths?? It should look for the config file in the complete path: https://example.com/kubernetes-dashboard/assets/config/config.json
and not in the previous paths like this one: https://example.com/assets/config/config.json

It works when I change the previous code to that (taking the configExist() out of the loop, to try only the final complete path):

      const probePath = 'assets/config/config.json';
      const origin = document.location.origin;
      const pathSegments = document.location.pathname.split('/');

      let basePath = '/';
      let configFound = false;

      for (let i = 0; i < pathSegments.length; i++) {
        const segment = pathSegments[i];
        if (segment.length > 0) {
          basePath = basePath + segment + '/';
        }
      }
      const fullPath = origin + basePath + probePath;
      configFound = configExists(fullPath);

floreks commented 1 month ago

The reason it starts from the root and goes up is that if you i.e. use a direct link to a subview in Dashboard, it has to be able to find a base path it can use in combination with file subpath to get the config. It also doesn't know if it is being served on a root path or on a sub path thus starting from the root.

Consider these examples:

https://example.com - access main view
https://example.com/pods/xyz - access some pod details view

https://example.com/dashboard - access main view on a subpath
https://example.com/dashboard/pods/xyz - access some pod details view on a subpath

It could start from the full path and subtract parts of it, but unfortunately, it will be suboptimal. Long URLs will take longer to load initially as it will have to traverse through more paths to find the correct one.

Palollo commented 1 month ago

Thanks for your quick response @floreks, I don't know if I have different version than yours or different configuration, but in my case, the direct links are like this:

https://example.com/dashboard/ - access main view on a subpath - pathSegments is ["","dashboard",""]
https://example.com/dashboard/#/pod/mynamespace/xyz - access some pod details view on a subpath - pathSegments is the same ["","dashboard",""]

The path within the kubernetes-dashboard app is specified after the hash char, i.e. in the fragment part, not in the path part. So I think the solution you propose is good! Even if the URLs were like you say, the looking for the config file would be only in the initial access. And not so long, probably 2-3 levels, not more, I think. In my case 0 levels ;)

Palollo commented 1 month ago

It is better to have a solution working for all the cases, although suboptimal in some specific cases, than a solution optimal for some cases and not working on some other cases.

drigz commented 1 month ago

I have another application in that path which has another different "config.json" file!

Does your application serve valid JSON from /assets/config/config.json? What does the JSON look like, compared to the config from the application itself?

I think the "root cause" here is the false positive, and while reversing the search order trades speed against false-positive risk, another way to address it would be to make the check less likely to trigger falsely. I'd previously thought it would be enough to check for valid JSON that looks like dashboard config, but I'm now questioning whether there should be a purpose-specific /assets/sentinel.json that is unmistakeable in its content. (that said, I haven't had/taken the time to set up a dev environment and try this, so feel free to ignore my ideas in favor of those with time to work on this)

Palollo commented 4 days ago

Yes, that would be another solution in case of the search is required: start from the root path but check for a unique field value in the config file. As I said, in our case the search is not required and it's perfectly working.

This is the code we currently have:

      let url = document.location
      const configUrl = url + 'assets/config/config.json';
      configFound = configExists(configUrl);

      document.write("<base href='" + (configFound ? url : '') + "' />");

Instead of this:

      const probePath = 'assets/config/config.json';
      const origin = document.location.origin;
      const pathSegments = document.location.pathname.split('/');

      let basePath = '/';
      let configFound = false;

      for (let i = 0; i < pathSegments.length; i++) {
        const segment = pathSegments[i];
        if (segment.length > 0) {
          basePath = basePath + segment + '/';
        }

        const fullPath = origin + basePath + probePath;
        configFound = configExists(fullPath);
        if (configFound) {
          break;
        }
      }

      document.write("<base href='" + (configFound ? basePath : '') + "' />");

kubernetes / dashboard