Relative path in ui_url or default_browser_return_url cause runtime crash

[zhming0]

Describe the bug

When selfservice.default_browser_return_url or selfservice.flows.XXX.ui_url is set to be relative path (e.g. /foo/bar), Kratos will crash whenever it access those setting.

Reproducing the bug

Steps to reproduce the behavior:

1. Setup the demo kratos server using demo config.
2. Change selfservice.flows.login.ui_url to /login
3. Try to access login page
4. See Kratos crashes with following log

Server configuration

Just like the demo config:

serve:
  public:
    base_url: http://127.0.0.1:3001/kratos/
  admin:
    base_url: http://kratos:4434/

selfservice:
  default_browser_return_url: http://127.0.0.1:3001/
  whitelisted_return_urls:
    - /

  methods:
    password:
      enabled: true

  flows:
    login:
      ui_url: /login

Expected behavior

It works just like v0.5 or it throws a proper error instead of crashing

Environment

• Version: v0.6.3
• Environment: Docker

Additional context

• It appears this new behavior was introduced in v0.6.
• The same problem applies to the default_browser_return_url setting

[zepatrik]

I am not sure that ever worked, are you sure that it worked in v0.5 to have a relative URL for selfservice.flows.login.ui_url? The history shows it always used the same parse function: https://github.com/ory/kratos/blame/2e6f87ba7f3d9b4df8851e204554464ecc199c15/driver/config/config.go#L565
Same goes for default_browser_return_url: https://github.com/ory/x/blame/e598287a046d0178b4648b6175ed732ea065e20f/configx/provider.go#L461
However, at least the error should be handled correctly.

[aeneasr]

We should probably change this in the schema config and only allow URL-URIs to be defined

[zhming0]

@zepatrik I am sure it worked in v0.5. I have been running it on prod using such configuration. I thought the serve.public.base_url was designed to support this feature?

For default_browser_return_url, the doc mentions this (/dashboard) as an example.

[zhming0]

@aeneasr @zepatrik I did some quick digging I believe this is where the absolute paths were rejected.

And this is the original PR that introduces such check.

I do believe it's a bug, I can raise a PR to remove the scheme validation if you guys agree.

[aeneasr]

I believe I added this to ensure that the login, registration, ... redirects are deterministic and not implicitly set via the e.g. public base URL. This ensures that all redirects end up at the page we configured, and are not dependent on implicit configuration.

I think the fix here would be to disallow setting relative URLs in the config schema so that we do not get a runtime error but instead a config validation error.

Is there a particular reason why you want to have relative URLs here? Given that it would be computed as such

<serve.public.base_url><selfservice.flows.login.ui>

I see no functional difference to making this explicit.

[zhming0]

Is there a particular reason why you want to have relative URLs here?

The same reason why HTML would allow anchor to support /foo ⚓
It reduces possibility of making mistake, promotes separation of concern: allow single source of truth for the base URL. Easier to use for most use cases.

But you are right, for machine, there is no functional difference. It's a just matter of human taste and use case. I thought it was a bug only because the doc mention it was supported and it was supported in the past 😄 .

I agree the bottom line is to ensure no runtime error + fix the doc.

[chlasch]

I believe I added this to ensure that the login, registration, ... redirects are deterministic and not implicitly set via the e.g. public base URL. This ensures that all redirects end up at the page we configured, and are not dependent on implicit configuration.

I think the fix here would be to disallow setting relative URLs in the config schema so that we do not get a runtime error but instead a config validation error.

Is there a particular reason why you want to have relative URLs here? Given that it would be computed as such

<serve.public.base_url><selfservice.flows.login.ui>

I see no functional difference to making this explicit.

Relative URLs worked in v0.5. This was an important feature for us, for an on-premise application that does not know a reliable hostname when it is deployed, it can be behind a load-balancer for instance with a different DNS name that originally configured, or might be just be IP.

Is there a security concern with relative URLs that I'm missing?