Do not block third-party cookies from reaching auth handlers#793
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Otherwise, third-party origins cannot log the user out.
| } | ||
| next() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
The blocking code used to be before the auth handlers, so third-party auth requests would never reach those handlers.
| req.session.save = done => done() | ||
| } | ||
| next() | ||
| }) |
There was a problem hiding this comment.
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 left a comment
kidehen left a comment There was a problem hiding this comment.
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 commented Sep 28, 2018
Thanks! |
RubenVerborgh commented Sep 28, 2018
Upgraded https://solid.community/ and https://solidtest.space/. |
kidehen commented Sep 28, 2018
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 commented Sep 28, 2018
Thanks @kidehen for your help, patience, and persistence! |
There was a problem hiding this comment.
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)){
Regression compared to 4.1.4 or compared to your fix? So then there must be more cases still that need handling. Can you create an issue with a request that fails? |
This might actually be intended behavior, depending on the details. See #526. |
kidehen commented Oct 2, 2018
Current setup, following downgrade: OIDC Relying Party: OIDC Provider |
kjetilk commented Oct 2, 2018
BTW, if we are targetting it for NSS 5, it should be based on the |
@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 commented Oct 2, 2018
Yeah, it is fine with me, I'll transfer it to the other project. |
kjetilk commented Oct 2, 2018
Oh, reading the backlog in more detail, please create another issue on this, since this one is an already merged PR. |
This might fix logout from third-party origins.