Do not block third-party cookies from reaching auth handlers

[RubenVerborgh]

This might fix logout from third-party origins.

[RubenVerborgh]

The blocking code used to be before the auth handlers, so third-party auth requests would never reach those handlers.

[RubenVerborgh]

By moving the code after the auth handlers (but still before the LDP handlers), we ensure that third-party origins can log you out.

[kidehen]

@RubenVerborgh ,

Confirming that this fix resolves the OIDC Identity Provider (OP) part of this "/logout" matter.
I've successfully logged in and out of:

[1] https://drive.verborgh.org -- but this only tests our server (which has the fix) functioning as the OP since your pod is a Relying Party (RP)
[2] https://solid.openlinksw.com:8444 -- our server in OP and RP modes

Thus, if you upgrade https://solid.community we will have a second OP for verifying this fix.

[RubenVerborgh]

Thanks!

[RubenVerborgh]

Upgraded https://solid.community/ and https://solidtest.space/.

[kidehen]

@RubenVerborgh ,

Confirming that both https://solid.community and https://solidtest.space now pass the login and logout tests while operating as OIDC Identity Providers (OP or Idps).

We are done, finally!

[RubenVerborgh]

Thanks @kidehen for your help, patience, and persistence!

[kidehen]

This introduces regression at the Authorization Level i.e. The server now rejects authorizations in other places :(

Tue, 02 Oct 2018 13:13:22 GMT solid:authentication Rejecting session for https://kidehen2.solid.openlinksw.com:8444/profile/card#me from https://kidehen3.solid.openlinksw.com:8444

Tue, 02 Oct 2018 13:53:47 GMT solid:authentication Rejecting session for https://smalinin.solid.openlinksw.com:8444/profile/card#me from https://kidehen.solid.openlinksw.com:8444

The issue above goes away when we roll back the fix, and repeating the Authorization tests using our server.

Code diff:

-      if (!argv.host.allowsSessionFor(userId, origin)) {
+     if (req.path !== '/logout' && req.path !== '/goodbye' && !argv.host.allowsSessionFor(userId, origin)) {

[RubenVerborgh]

This introduces regression at the Authorization Level

Regression compared to 4.1.4 or compared to your fix?
If I understand correctly, it seems to be the latter.

So then there must be more cases still that need handling.

Can you create an issue with a request that fails?

[RubenVerborgh]

The server now rejects authorizations in other places

This might actually be intended behavior, depending on the details. See #526.

[kidehen]

@RubenVerborgh ,

Current setup, following downgrade:

OIDC Relying Party:

npm list @solid/oidc-rp
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/solid-multi-rp-client@0.4.4
│   └── @solid/oidc-rp@0.8.0  deduped
├─┬ @solid/solid-auth-oidc@0.3.0
│ └── @solid/oidc-rp@0.8.0 
└─┬ solid-auth-client@2.2.6 (git+https://github.com/OpenLinkSoftware/solid-auth-client.git#a68512b8a73780c00a0b1a7dedcf041f439e4b9a)
  └── @solid/oidc-rp@0.8.0  deduped

OIDC Provider

npm list @solid/oidc-op
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-op@0.4.0}

/cc @smalinin @cblakeley

[kjetilk]

BTW, if we are targetting it for NSS 5, it should be based on the v5.0.0 branch. We can release it as a point or minor release too, and we're working on that, in which case it is OK with develop, but then it should be in the ASAP on Server project. :-)

[kidehen]

@kjetilk ,

Ideally, this has to be pre NSS 5.0. Why? Because Authorization is currently broken i.e., you can Authenticate, but Authorization doesn't reflect what's described in ACLs.

Put differently, the baseline interop across pods is currently broken.

I cannot write to @RubenVerborgh's pod despite what's in the RWWCrew acl. You can try this yourself too, then repeat using my pods (which work).

[kjetilk]

Yeah, it is fine with me, I'll transfer it to the other project.

[kjetilk]

Oh, reading the backlog in more detail, please create another issue on this, since this one is an already merged PR.