Support for post_logout_redirect_uri

[thomasleplus]

I need the ability control the final page where the End-User’s User
Agent gets redirected to by the OP during an RP-Initiated Logout.

As per OpenID Connect Session Management 1.0 draft spec, the optional
URL parameter post_logout_redirect_uri is intended for this:

https://openid.net/specs/openid-connect-session-1_0.html#RPLogout

So I’ve added support for this parameter. Note that the current version
of Ping Federate uses a different URL parameter (TargetResource).

[thomasleplus]

Sorry I didn't add a test to the pull request because I have very limited knowledge in LUA so I was not sure how the test framework actually works.

[bodewig]

I wonder whether we should also add a parameter to control the query parameter's name (as Ping needs a different name it is likely I need something other than post_logout_redirect_uri in the case where redirect_after_logout_uri is set (as it may use any kind of schema of its own).

Also I don't really like the duplication building the table, but this is something we can tackle after merging the request so we avoid merge conflicts. Same for additional tests :-)

One thing we also need to do is improve the documentation of both redirect_after_logout_uri and post_logout_redirect_uri so they don't get confused with each other. See also #169

[thomasleplus]

Making the query parameter's name configurable would provide more flexibility. On the other hand I am afraid it would make the options even more confusing. I was initially confused thinking that redirect_after_logout_uri was the option that I was looking for until I realized it wasn't.

By the way Ping also supports a parameter InErrorResource to redirect the user in case of a logout failure: https://documentation.pingidentity.com/pingfederate/pf90/index.shtml#concept_idpEndpoints.html#concept_idpStartSloPing

I considered implementing this too but didn't because it's not part of any OIDC spec as far as I can tell and it would have complicated things even more.

I've added a few comments to the README.md which hopefully clarify the options.

Regarding the amount of duplication of code, I completely agree so I've read a bit more about the LUA syntax and revised my pull request (sorry it took me a couple of pushes).

[bodewig]

Looks good to me. I'll apply it locally, add a few tests and merge it later today or tomorrow.

[bodewig]

Many thanks!

[jgoux]

Sorry for commenting an old PR but is it possible to pass the post_logout_redirect_uri dynamically from the caller instead of setting it as an option?

For example : http://my-custom/backend/logout?post_logout_redirect_uri=http://some-other-place

[bodewig]

That would allow open redirects - see https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html - unless we required the client provided post_logout_redirect to be one of a bunch of pre-registered URLs.

Without pre-registration I'd be opposed. With a pre-configured set I'd be fine. That would still be a separate issue, though :-)

[jgoux]

@bodewig Thanks for the answer! As I'm a keycloak user, it uses pre-registered valid uri 😄
And to answer my own question, here is how I did it :
post_logout_redirect_uri = ngx.var.arg_redirect_uri