sandstorm-io / sandstorm

Sandstorm is a self-hostable web productivity suite. It's implemented as a security-hardened web app package manager.
https://sandstorm.io
Other
6.75k stars 706 forks source link

What should happen when ctrl+clicking on an intra-grain link? #1848

Open mitar opened 8 years ago

mitar commented 8 years ago

I see in the Rocket.Chat app that I can open internal frame directly in the browser. Shouldn't the app redirect to Sandstorm URL in this case? So always require the frame to be around? Is this problem of Rocket.Chat?

kentonv commented 8 years ago

We don't consider this a problem. Opening the grain host in a window without the Sandstorm chrome doesn't allow the app to do anything bad. Mostly it hurts the app because if the grain tab in the sandstorm UI is closed then the app server will shut down and any external windows will stop working.

mitar commented 8 years ago

@kentonv: Hm, it seems there is no way really for grain to know what is the URL of the grain in Sandstorm tab? So there is no way to redirect back to Sandstorm?

For me this is a question of usability. Users might open a link by mistake in a new tab. It should get Sandstorm UI automatically.

kpreid commented 8 years ago

By mistake? How about users opening links on purpose in a new tab? That not working in the straightforward way (Sandstorm UI always present) is a problem. Can we reopen this, please?

elimisteve commented 8 years ago

@mitar Why do you need the full URL? Can't use relative URLs and just specify the path, (e.g., /app/users/whatever)?

mitar commented 8 years ago

Because sandstrom sessions run in their own origin. You have to know how to redirect back to Sandstorm with full URL.

mitar commented 8 years ago

Hm, it seems I can get full URL from window.parent.location. So I should just associate this from the client for a particular iframe origin. And then a page is opened with window.parent for the same iframe origin, I redirect to window.parent.location.

But it would be much better if there would be a HTTP header for this send to the app.

paulproteus commented 8 years ago

Hi Mitar.

I generally like your idea. One other thought is that Sandstorm could expose a URL within the grain like

.well-known/sandstorm/grain-redirect

and then:

if (! window.parent) { winddow.location = "/.well-known/sandstorm/grain-redirect"; } you could do a redirect to that, and then Sandstorm would do the redirect to the grain URL.

One difficulty is that I don't know how to handle sharing links in this case, since sharing links are not stored server-side.

mitar commented 8 years ago

BTW, redirect should be done to the path of an grain as well, not just to the top URL of the grain.

zarvox commented 8 years ago

@mitar I don't think you can actually get the full URL from window.parent.location - are you running that in the context of the grain iframe? It should be sandboxed.

I believe @kentonv had defensible privacy/anti-worm reasons for not wanting grains to be able to obtain references to themselves without explicit user interaction. I'm not sure if the .well-known/sandstorm/grain-redirect proposal preserves these security properties, but if it does, then I'm +1 on that idea. I think it does if we enable it only for web-session (but not api-session) because it can't be used outside the context of the current user's session.

mitar commented 8 years ago

@mitar I don't think you can actually get the full URL from window.parent.location - are you running that in the context of the grain iframe? It should be sandboxed.

I tried on Oasis in Firefox in one grain through web console, switched to iframe context and wrote window.parent.location and it prints it out. But you are right. I can access the object, but I cannot really do anything with it. So the console can print it. But if I do "" + window.parent.location to convert it to string, it fails.

mitar commented 8 years ago

Can then this ticket be reopened and let's do this redirect? That would be great to have. In some way it would be inverse of my code here. Instead of posting to parent the current location, the app would redirect to /.well-known/sandstorm/grain-redirect/<path> with current path.

kentonv commented 8 years ago

This proposal still seems problematic to me. The popup window will load a whole new copy of Sandstorm with tabs and everything -- is that really what you want?

Maybe what we should really have here is a new postMessage API for "Please open a copy of this grain in a popup window"? That way the popup can share a Javascript context and can perhaps be rendered in a lightweight version of the shell.

Though there's a problem that the browser's popup blocker will block popups created in response to a postMessage, since it will lose association with the user click.

I wonder why you want a popup window at all. Popups are pretty awful UX most of the time. I am almost inclined to ban apps from making popups (which we could enforce with iframe sandboxing), except that it would break HackerSlides' presentation mode...

mitar commented 8 years ago

I was not thinking about popups. I am thinking about a simple grain like: https://alpha.sandstorm.io/grain/kB7bofATTL2jGuKSFwmqdA/GithubPRCommands

Click with command-click or whatever you have in your browser to open a link in a tab, on any link in the sidebar of MediaWiki. It opens the page in new tab. Now, that tab is not in Sandstorm anymore. It should be. The question is how to get it to be in Sandstorm.

Redirect is one option. But it would be great if it could be the same session (so same origin). Not another Sandstorm instance.

kentonv commented 8 years ago

OK, I changed the bug title to reflect this.

You could argue that ctrl+click on an intra-grain link should open a sidebar tab, although we currently expect to have only one sidebar tab for each grain.

This question might also relate to a separate feature request (not sure if there's an open issue): Being able to "pop out" a grain into a separate window.

mitar commented 8 years ago

You could argue that ctrl+click on an intra-grain link should open a sidebar tab

This interferes with user's browsing habits and user's use of my browser UI, screen-estate and so on. On the mobile phone user might want a link to open in a new background window, not in a in-window sidebar.

But it is one option. The best would be if we could support native browsing somehow.

kpreid commented 8 years ago

It is, in my opinion, very important that Sandstorm not harm the things that make web apps web apps rather than just apps. If open-in-new-tab is either nonstandard (Sandstorm sidebar) or deficient (missing UI) then this is bad.