Send full returnToUrl to oidc-auth-manager#648
Uh oh!
There was an error while loading. Please reload this page.
Conversation
| </div> | ||
| <divclass="container"> | ||
| <formmethod="post"action="/api/auth/select-provider"> | ||
| <formmethod="post"> |
There was a problem hiding this comment.
Always POST to self, so we don't lose query string.
| <labelfor="webid">WebID or server URL:</label> | ||
| <inputtype="text"class="form-control"name="webid"id="webid" | ||
| value="{{serverUri}}" /> | ||
| <inputtype="hidden"name="returnToUrl"id="returnToUrl"value="" /> |
There was a problem hiding this comment.
This one was always empty, so rather pointless. Instead, the query string should be used.
| '/api/auth/select-provider?returnToUrl='+currentUrl | ||
| constlocals=req.app.locals | ||
| constcurrentUrl=util.fullUrlForReq(req) | ||
| constloginUrl=`${locals.host.serverUri}/api/auth/select-provider?returnToUrl=${encodeURIComponent(currentUrl)}` |
There was a problem hiding this comment.
Escaping was definitely important here (for query strings and such).
| <meta charset="UTF-8"> | ||
| <script> | ||
| window.location.href = "${url}" + window.location.hash | ||
| window.location.href = "${url}" + encodeURIComponent(window.location.hash) |
There was a problem hiding this comment.
So it is the client who appends the hash (and only the client can do that).
| res.header('Content-Type','text/html') | ||
| letcurrentUrl=util.fullUrlForReq(req) | ||
| req.session.returnToUrl=currentUrl |
There was a problem hiding this comment.
We can't rely on session, because the server cannot see the hash.
There was a problem hiding this comment.
This fixed mechanism (not relying on session) will still work on direct requests by the browser (like, requesting an image or a pdf file or whatever), not mediated by a client?
There was a problem hiding this comment.
If we end up in this code, we are in the process of rendering an HTML error page. So if that's the behavior we exhibit in response to a browser directly requesting an image (I hope not), it's broken for another reason already 😄
There was a problem hiding this comment.
Ahh, I see. Makes sense.
Addresses the node-solid-server part of #571.
RubenVerborghforce-pushed the fix/redirect-hash branch from 8e3c6d0 to 299f5d6CompareRubenVerborghforce-pushed the fix/redirect-hash branch from b349038 to e7be5ddCompare
RubenVerborgh commented May 4, 2018
RubenVerborghforce-pushed the fix/redirect-hash branch from e7be5dd to 3da620eCompare
timbl left a comment
timbl left a comment There was a problem hiding this comment.
Works for me on timbl.com
Looks reasonable
An important fix!!
Addresses the node-solid-server part of #571.
oidc-auth-manager still loses the URL we send, so fixes on that side are needed to fully solve #571. Tracking that in nodeSolidServer/oidc-auth-manager#17.