search

SolidOS / solidos

The operating system for Solid
https://solidos.solidcommunity.net/
MIT License
127 stars 19 forks source link

Solved : Auth-upgrade - handle HTML Links with care #61

Closed jeff-zucker closed 3 years ago

jeff-zucker commented 3 years ago

There are several places in SolidOS where we use HTML links rather than gotoSubject() links. The HTML links open new web pages and thereby destroy any existing login information. The root index.html does this with "gotto X's files". Creating a new meeting offers you a "open newly created meeting" button which likewise jumps to a new page. There may be more places. [EDIT : my thinking on this has changed, please see https://github.com/solid/solidos/issues/61#issuecomment-936242619 for my current thinking]

timea-solid commented 3 years ago

@jeff-zucker with html links do you mean code like <a href? or windows.location or...?

jeff-zucker commented 3 years ago

We need to test both of the href and windows.location because it is unclear if they will work in the new auth. This issue is specifically about links (a href) so perhaps raise a separate one about window.location - I am not aware of that being a problem but you may have found someplace that it is.

The "goto Xs files" that occurs when a user is not logged in and visits a pod root is the problem since it is a href link. In Data Kitchen, clicking on it destroys the Data Kitchen context and opens a non-data-kitchen page . Rather than an HTML link to the public folder, In the data-browser as stand-alone web app, it also will not behave as expected since it uses a relative link which can only work in the server-side databrowser.

I think the way to solve this is to have a more sophisticated link generator - so that it uses an absolute URL and wraps it in the proper address of the app. So instead of a lnk to "/public/" it should be a link to https://my-data-kitchen-host;3000/?uri="https://some-other-host/public/".

jeff-zucker commented 3 years ago

I have tested using the above mentioned solution (generate a full link) and it works fine. I am marking this solved but leaving it open until a PR is ready (not sure which branch to submit it to).

jeff-zucker commented 3 years ago

I've submitted the PR to fix this at https://github.com/solid/solid-panes/pull/306

timbl commented 3 years ago

This is quite a big change. A big new rule not to link to things directly is quite a big constraint on how things operate. It is the constraint of a data bowser to keep within itself, like social network sites tend to try to keep the user on the same site. So long as these links are made by just a few bits of code we may be able to do it generically but where they are lots of individual bits of code this could be a long task.

One thing I am a bit concerned with is the problem that is we replace the links to gotoSubject(x), if x is in another domain, then the current web page location may be here doesn't have the rights to read non-public data on another person't pod that I do have access to from being logged in. We'll see whether that becomes a noticeable problem I guess.

jeff-zucker commented 3 years ago

@timbl - as it turned out, I did not eliminate this link, I modified it to respect its contexts. In other words, originally it assumed it was on a server and so "/public/" was a valid address because mashlib is being served by the server so that equates to https://user.solidcommunity.net/public/. But when used in a web-app or the kitchen, we need something more like "https://dk-localhost/?uri="https://user.solidcommunity.net/public/" to keep the context of the kitchen. In the code it's just window.location.href + "/public/" so not very complicated codewise. So this part of the problem is not "eliminte HTML links" but rather, if you link to something within the same pod, make sure that you use the location.href to build the actual URL rather than assuming you are on the server you want to visit.

So, if we are creating an HTML link to a pod address, in order to avoid jumping out of the kitchen or webapp context we must always use an absolute URL that includes the search params.

If we are creating an HTML link to a non-pod resource, we can use an HTML link, but it should open in a new window or tab letting our current pod/kitchen context remain.

Those two things should solve the "jumping out of the kitchen" problem. The other problem with HTML links is the "losing the login" problem caused by the new auth. This is a problem on the old index.html pages because those attempt to login on the index page and then have an HTML pointer to a mashlib-served page. That flow won't work, we need to login on the mashlib-served page itself. (see https://github.com/solid/node-solid-server/issues/1619). That can be solved by eliminating the login button on the index.html page and have the login occur on the mashlib-served page.

So the remaining possible HTML problem is a link from being logged in to podA and then using an HTML link to podB. I believe this will lose the login context but because we are having issues with the test server, I have not been able to fully test it.

timea-solid commented 3 years ago

This issue is merged into: https://github.com/solid/node-solid-server/issues/1619

jeff-zucker commented 3 years ago

No, there are two separate issues. 1) NSS issue of being able to login via the server-root and 2) SolidOS issue of what happens on HTML links such as the "show profile" and the goto button. These do cause problems when logged in from the server-root but they also cause problems when logged in normally. For example, try them with ESS or CSS.

jeff-zucker commented 3 years ago

Issue #1 above is handled at https://github.com/solid/node-solid-server/issues/1619. And the part of issue #2 that deals with the profile link at https://github.com/solid/mashlib/issues/129. I am opening a new issue for the goto links at https://github.com/solid/solidos/issues/70.

So this issue can be closed because it is handled in the others.