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.72k stars 705 forks source link

Remove/hide the webkey option and disable grain renaming if it’s not useful #863

Open neynah opened 9 years ago

neynah commented 9 years ago

E.g. for Let’s Chat, the user cannot see contents of the iframe and therefore cannot do anything until they’ve signed in. If Let's Chat is the only one that requires sign-in then maybe this issue can be dismissed.

paulproteus commented 9 years ago

FWIW Sandstorm apps don't currently have a programmatic way of telling Sandstorm that they insist on the user being signed in. Arguably they should have a way to say that.

But given that, we don't have a programmatic way (that I can think of) of knowing that the webkey & rename options aren't useful yet.

zarvox commented 9 years ago

I'd say we should deprecate and remove the webkey UI in favor of offer templates, but we need to still provide a way to enumerate and revoke "forSharing: false" tokens. Perhaps that could be folded into the "share access" or "who has access" view?

I'm curious which apps still expect the user to use the webkey UI and if they'd be better-served by offer templates. I think TinyTinyRSS does still; do you know of any others?

ocdtrekkie commented 9 years ago

@zarvox Definitely actively using webkeys with TinyTinyRSS. I thought the Git apps (Gitlab/Gitweb) also depended on webkeys, but since Gitlab seems to just present me with a key now, I assume that uses offer templates?

zarvox commented 9 years ago

Re: Gitlab/Gitweb: yeah, that's the offer template workflow in action. I think it is almost always superior for desktop uses, since you can easily copy-paste the key in whatever context it's supposed to be used, and basically nobody is going to have a sense for how webkeys are supposed to work. :)

For mobile, it's more of a mixed bag, since text is harder to copy there. For connecting a mobile app to a Sandstorm server with an API token, though, you will generally need to enter:

We could potentially do that with a second offer template that renders the templated string as a QR code, allowing you to scan it with the camera rather than typing in a text string. Then that QR code could simply encode the whole URL, complete with webkey, and it's a single step to connect the mobile app to that grain. (Of course, the app should probably also support the text-string version as well, if you're setting everything up from the phone itself.)

paulproteus commented 8 years ago

Questions here (I think @zarvox might know more than I; @dwrensha might also)

This came up in a conversation earlier today but I figure this ticket is the best place to discuss.

If we think that we need to support the WebKey icon for older apps, then I suggest that we replace the :key: icon in the topbar with the word "More..." and show a dropdown that includes webkey underneath that.

I guess I just really want that webkey icon to go away. But I am curious to have the current situation documented better, even if I can't make it go away quite yet, so I can read this and remind myself why things are they way they are.

zarvox commented 8 years ago

AFAIK, only Tiny Tiny RSS requires the webkey button in the top bar for creating webkeys. A secondary use of the webkey button is a hack to "share all the permissions, even those which the app does not declare a roleAssignment for", which has been useful in the past when you have an app which does not include a particular permission with any of the declared roles, like Piwik.

However.

As noted in the previous comments in this thread: we can't just get rid of the UI entirely, or there's no place to go to to audit/revoke tokens generated with forSharing: false - including those created via offer templates. This is used for Davros, and Tiny Tiny RSS, Gitweb, GitLab, and basically anything that exports something on the API endpoint.

So any solution that would remove webkeys and the webkey dropdown as a whole would need to replace that functionality with something comparable. I hope that we can replace the webkey dialog with something which details at least the exported capabilities, which today includes webkeys, and in the future will contain other capabilities offered by grains. This would be, in essence, the powerbox audit view.

paulproteus commented 8 years ago

The auditing, right.​ Thanks for spelling that out; I hadn't really understood that from the previous comments. (Partly because earlier you wrote forSharing: true, but here you wrote forSharing: false - one of these is probably a quick typo.)

FWIW, I had zero idea that clicking the webkey icon would result in me being able to see API token-type things. I expected to see those things in the "Who has access" area. So I think moving it there is a pretty OK idea.

To get a sense of what user flow I expected for revoking these keys, I visited a gogs grain, and followed its instructions for cloning a repo. Revocation didn't really occur to me. If I wanted to revoke it, I'd expect to see some UI in the app. If we want to keep the forSharing: false token info within the webkey icon, then maybe we can wiggle the webkey icon, or show a drop-down from it, saying, "Click here to revoke" or "Click here for more information." Or maybe we can show a notification bell when the token is first used.

I'll add to this comment in the future; for now, I'll let this serves to me as notes to self about what questions remain. No specific action requested by anyone at this point.

zarvox commented 8 years ago

Whoops, yeah, the first comment is the one that's wrong. Edited for consistency.

It seems to me we have several things that we will need to display, audit, and potentially revoke:

...all of which are ApiTokens in implementation, and which may or may not be worth grouping conceptually as well.

Perhaps all of these could coalesce under a single "review access" (there's probably a better name) header?

ndarilek commented 8 years ago

I can't follow all of the subtleties of this discussion because I don't know how sharing works under the hood, but here's the idea I expressed in the group last Friday:

Seems to me that webkeys are just a different kind of sharing. I can share by link, which gets posted on a site or sent via email, and lets someone else access my grain by clicking the link. I can share via email, which sends a link to a given email address and is essentially a shortcut for a common workflow for the previous method. Or I can share with another app operating on my behalf with the permissions I expose to it.

I'd almost entirely forgotten about webkeys. Had they been a third tab in the sharing screen, rather than an entirely separate thing, I wouldn't have wondered how to revoke access to a Git repository. I'd also likely have shared access to Jenkins by clicking Share, the Apps tab, then using the credentials generated therein. Webkeys are confusing to anyone who hasn't encountered them, but sharing with apps is something anyone who has used an oauth workflow is familiar with (whether they realize they're using oauth not withstanding.)

So maybe get rid of the webkey button and add a third tab on the sharing screen?

kentonv commented 8 years ago

@ndarilek You are absolutely correct. In fact, if you generate a webkey, copy the secret token part, and then use it to replace the secret part of a /shared/ URL, you'll find it actually works there! (There is a subtle behind-the-scenes difference that a webkey can be used to perform actions on behalf of you, whereas a share URL normally cannot. Thus share URLs cannot be converted to webkeys -- or at least, not ones that can act on your behalf.)

We in fact do plan to merge the webkey audit interface into the "who has access" dialog. But the webkey creation interface we'll probably get rid of entirely, or move into an obscure location, because it is increasingly not needed due to offer templates and powerbox.