Post Logout Redirect URIs "+" behavior is inconsistent with other usages (i.e. Web Origins)

[jsorah]

Before reporting an issue

• I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

oidc

Describe the bug

Post Logout URIs can be defined in the Admin Console. For "legacy" Keycloak installations when they are migrated, the default value is "+".

As a consumer using a legacy installation that was migrated, I saw "+" in the Post Logout Redirect URI list and assumed it worked much like the Web Origins, where I could add additional values and the "+" list would be expanded and include my additional values. My natural feeling then was to just add my "post logout" URI and keep the "+".

However, in this case, the "+" is treated specially - it must be the first in the list and it does not expand the list of redirect URIs to include any additional added. To my surprise, "+" means that regardless of any other value in the Post Logout Redirect URI list, it will ONLY return the values in the Redirect URIs for the client. This is unexpected behavior compared to other existing features using the "+" nomenclature.

Version

latest 23.x branch

Expected behavior

"+" in Post Logout Redirect URIs is treated similarly to other areas (e.g. Web Origins) where it is expanded and includes any new, distinct entities and that it doesn't matter where in the list of Post Logout Redirect URIs where the "+" is. Additional definitions of "+" should likely be ignored.

For example:

Scenario 1:

  Redirect URIs:
    https://foo.example.com
    https://foo1.example.com

  Post logout redirect URIs:
    https://someother.foo.example.com
    +

  Net allowed Post Logout Redirect URIs:
    https://foo.example.com
    https://foo1.example.com
    https://someother.foo.example.com
    
  Logout allowed for all URIs.

Scenario 2:
  Redirect URIs:
    https://foo.example.com
    https://foo1.example.com

  Post logout redirect URIs:
    +
    https://someother.foo.example.com
    +

  Net allowed Post Logout Redirect URIs:
    https://foo.example.com
    https://foo1.example.com
    https://someother.foo.example.com

  Logout allowed for all URIs.

Actual behavior

Scenario 1:
Receive Invalid redirect uri page when using the additional post logout redirect URI in an OIDC logout request.
e.g. http://localhost:8080/realms/master/protocol/openid-connect/logout?post_logout_redirect_uri=https://someother.foo.example.com&client_id=my-client

Scenario 2:
Receive Invalid redirect uri page when using the additional post logout redirect URI in an OIDC logout request.
e.g. http://localhost:8080/realms/master/protocol/openid-connect/logout?post_logout_redirect_uri=https://someother.foo.example.com&client_id=my-client

How to Reproduce?

1. Start Keycloak
2. Create a standard flow client in Keycloak e.g. my-client in a realm (e.g. master), you could just use http://localhost:8080 as the initial redirect URI - actual value doesn't matter.
3. Add + as a post logout redirect URI to the client
4. Add https://www.google.com as a post logout redirect URI to the client
5. Visit the logout endpoint http://${HOST_AND_PORT}/realms/master/protocol/openid-connect/logout?post_logout_redirect_uri=https://www.google.com&client_id=my-client
6. Receive "We are sorry..." page with "Invalid redirect uri"

Anything else?

There is likely some history as part of #14884 around the particular decisions in this endpoint, but I don't see a particular reason why "+" could not be expanded and act more similarly to the "Web Origins" behavior.

The "-" behavior would be somewhat ambiguous - I'm not sure if there would be expectation that - could be added anywhere in the list to negate any additions, and maybe that should take precedence ?

[sschu]

The reason for this is that people should actually not use the '+' to just use the 'redirect_uri' also for post logout (this was the old behaviour when 'post_logout_redirect_uris' could not be specified). The intention is that people should rather specify their post logout uri's explicitely. The '+' was mostly added to stay compatible using a migration. I could imagine that the '+' goes away at some point. So I am not sure this change is a good idea as it basically means the '+' will stay.

[mposolda]

The reason for this is that people should actually not use the '+' to just use the 'redirect_uri' also for post logout (this was the old behaviour when 'post_logout_redirect_uris' could not be specified). The intention is that people should rather specify their post logout uri's explicitely. The '+' was mostly added to stay compatible using a migration. I could imagine that the '+' goes away at some point. So I am not sure this change is a good idea as it basically means the '+' will stay.

@sschu I understand your point. But TBH I don't think that + will go away as I can imagine lots of people already uses it and it can be a pain for the backwards compatibility to properly handle all the cases where people use + ... I would rather like to see the compatibility switches on OIDC protocol ( legacy-logout-redirect-uri , suppress-logout-confirmation-screen) to go away, but I guess this is also problematic as lots of people still rely on it - including you if I remember correctly? :-)

I can imagine we can improve with client policies to allow more strict validations for redirect uris and post logout redirect uris (Like support for disallow using any wildcards or + stuff in the URIs etc).

So for me, the changes in the PR looks acceptable as of now?

[sschu]

@mposolda Also fine for me. I will also try to find out whether we still need these legacy switches.

[mposolda]

@sschu Cool, Thanks!