Solid Community server - post_logout_redirect_url not working

[cblakeley]

@RubenVerborgh /cc @kidehen @smalinin

Please could you confirm whether or not the Solid Community server is up-to-date wrt https://github.com/solid/oidc-op. (Likewise with https://solid.space.) Support for post_logout_redirect_url isn't working, but has been implemented in https://github.com/solid/oidc-op/blob/master/src/handlers/RPInitiatedLogoutRequest.js

When OSDB logs out of Solid, the user is redirected to Solid's goodbye page, not the post_logout_redirect_uri supplied by OSDB:
https://github.com/solid/oidc-op/blob/e1d7ad249e6119cf5eb3b0a047123bec4433f111/src/handlers/RPInitiatedLogoutRequest.js#L203

My Chrome console shows:

Headers - General:
Request URL: https://solid.community/logout?post_logout_redirect_uri=https%3A%2F%2Fosdb.openlinksw.com%2Fosdb
Request Method: GET
Status Code: 302 Found
...
Response Headers:
Location: /goodbye
...
Request Headers:
Referer: https://osdb.openlinksw.com/osdb

[kidehen]

@RubenVerborgh ,

Our server installation shows:

npm list @solid/oidc-op
solid-server@4.1.3 /home/smalinin/node-solid-server-oidc
├─┬ @solid/oidc-auth-manager@0.16.3 (git+https://github.com/OpenLinkSoftware/oidc-auth-manager.git#37d392cabacb8fc104d234ffac4cc5def87fb1d3)
│ └── @solid/oidc-op@0.4.0  deduped
└── @solid/oidc-op@0.4.0 

Does that align with what's installed on https://solid.community, https://solidtest.space, and https://drive.verborgh.org ?

/cc @cblakeley @smalinin @justinwb @kjetilk

[RubenVerborgh]

https://solid.community, https://solidtest.space, and https://drive.verborgh.org are at 4.1.4 with an npm ci, so they exactly follow the versions in package-lock.json.

package-lock.json demands @solid/oidc-op@0.3.2: https://github.com/solid/node-solid-server/blob/v4.1.4/package-lock.json#L211

This is because package.json demands @solid/oidc-auth-manager@^0.16.3 https://github.com/solid/node-solid-server/blob/v4.1.4/package.json#L36, which in turn demands @solid/oidc-op@0.3.2: https://github.com/solid/oidc-auth-manager/blob/v0.16.3/package.json#L41

In other words, if you want @solid/oidc-op@0.4.0, we need to change that in oidc-auth-manager.

Since you are using a fork of @solid/oidc-auth-manager, you already get that new version. Are there any other differences in your fork besides that oidc-op version?

[kidehen]

@RubenVerborgh,

Nothing else, so we need to sort out @solid/oidc-auth-manager release then. I assume you can do this quickly?

/cc @justinwb @kjetilk @cblakeley @smalinin

[RubenVerborgh]

PR available in nodeSolidServer/node-solid-server#795, please test.

[kidehen]

@RubenVerborgh ,

Okay, will have it tested and then applied to our instance as per usual.

/cc @cblakeley @smalinin

[kidehen]

@RubenVerborgh ,

Following your merge and release (re nodeSolidServer/node-solid-server#793 (comment) and nodeSolidServer/node-solid-server#795 (comment)), the issue still remains when I test https://solid.community functioning as an OIDC (OP / Idp) for https://osdb.openlinksw.com#this (functioning as Relying Party).

https://solid.community setup:

npm ls | grep oidc
├─┬ @solid/oidc-auth-manager@0.16.4
│ ├── @solid/oidc-op@0.4.0 deduped
│ ├─┬ @solid/oidc-rs@0.3.3
│ │ ├── @solid/oidc-rp@0.8.0 deduped
├─┬ @solid/oidc-op@0.4.0
├─┬ @solid/solid-auth-oidc@0.3.0
│ └─┬ @solid/oidc-rp@0.8.0
├─┬ oidc-op-express@0.0.3
│ ├── @solid/oidc-rp@0.8.0 deduped

Our NSS setup:

npm ls | grep oidc
solid-server@4.1.4 /home/smalinin/node-solid-server-oidc
├─┬ @solid/oidc-auth-manager@0.16.4 (git+https://github.com/OpenLinkSoftware/oidc-auth-manager.git#045a256fb75040d8206bf0bb7ad034b725260dad)
│ ├── @solid/oidc-op@0.4.0 deduped
│ ├─┬ @solid/oidc-rs@0.3.3
│ │ ├── @solid/oidc-rp@0.8.0 deduped
├─┬ @solid/oidc-op@0.4.0
├─┬ @solid/solid-auth-oidc@0.3.0
│ └─┬ @solid/oidc-rp@0.8.0
├─┬ oidc-op-express@0.0.3
│ ├── @solid/oidc-rp@0.8.0 deduped

So we need to revisit https://github.com/OpenLinkSoftware/oidc-auth-manager.git#045a256fb75040d8206bf0bb7ad034b725260dad , I guess?

/cc @cblakeley @smalinin

[RubenVerborgh]

First question would be how your oidc-auth-manager is different?

[cblakeley]

@RubenVerborgh /cc @kidehen @smalinin

The two versions of oidc-auth-manager differ at oidc-auth-manager/src/handlers/logout-request.js:40

Solid master:
solid/oidc-auth-manager@0.16.4

static parseReturnUrl (req) {
    let query = req.query || {}
    return query.returnToUrl // [1]
  }

OpenLink's fork:
https://github.com/OpenLinkSoftware/oidc-auth-manager.git#045a256

  static parseReturnUrl (req) {
    let query = req.query || {}
    return query.post_logout_redirect_uri ? query.post_logout_redirect_uri :  query.returnToUrl // [2]
  }

This change was submitted as commit a2b6b8b, as part of PR#25: OpenLink changes relating to Solid logout and switching accounts. However this PR was closed and the PR branch never merged, because it was superceded by PR solid/oidc-op#7 opened by @dmitrizagidulin (see comment 5 Jul).

Reading the code, ISTM there are two logout() routines:

logout v1)
oidc-auth-manager --> host_api.js#logout --> LogoutRequest#handle --> LogoutRequest#fromParams --> LogoutRequest#parseReturnUrl.
The last call executes [1] or [2] above.

logout v2)
oidc-op --> Provider#logout --> RPInitiatedLogoutRequest#handle

logout v2 should invoke the new post_logout_redirect_uri handling in oidc-op's RPInitiatedLogoutRequest.js, introduced as part of PR solid/oidc-op#7 (e.g. commits Refactor redirectToGoodbye(): fda1dec and
Refactor/expand RPInitiatedLogout: 2dd6332).

If I've understood correctly, it looks like logout v2 is intended to replace logout v1. If so, OpenLink's change (commit a2b6b8b, in PR#25) should be redundant. But from local testing, logout v1 is still being used, not logout v2. I've not figured out why yet.

[RubenVerborgh]

But from local testing, logout v1 is still being used, not logout v2. I've not figured out why yet.

Thanks, keep us posted if you find this out.

[cblakeley]

@RubenVerborgh /cc @kidehen @smalinin

The binding of Solid's route handlers is the cause of the new logout v2 code not being invoked.
middleware() binds /logout directly to logout v1 (LogoutRequest#handle), not to logout v2 (RPInitiatedLogoutRequest#handle).
https://github.com/solid/node-solid-server/blob/b53cb851aa43a9e01c7e0da203114e67241f210c/lib/api/authn/webid-oidc.js#L83

Can nodeSolidServer/oidc-auth-manager#31 be merged while this is sorted out?
(The Travis build failure with PR#31 with looks to be due to an error in istanbul-reports, not due to the patch?)

[RubenVerborgh]

Ooh cool. The team will look into this.