UWNetworksLab / uProxy-p2p

Internet without borders
https://www.uproxy.org/
Apache License 2.0
866 stars 183 forks source link

Grant permission to users, not devices #1014

Closed bemasc closed 9 years ago

bemasc commented 9 years ago

When Alice grants permission to Bob to access the internet through her current device, that permission grant should not be limited to only one of Bob's devices, and Alice should not be asked to choose which of his device(s) it applies to. (Bob can trivially copy or swap his own device identifiers, so as a security matter, granting permission to a single device is impossible.)

To be specific, each permission grant allows a specified user to access a specified device. Thus, the "Share Access" tab will show a list of users with whom one may share access through this device, or not, and the "Get Access" tab will show a list of devices (grouped by user) through which one has permission to access the internet, or not.

Assigning to @zahoriea for further consideration. (Did you already create mocks for this?)

dborkan commented 9 years ago

I don't object to this as I think it will be nicer for the majority of users who only use 1 tab, but I do think this will make both our code and UI more complex, as we will still need to keep the controls on the GET ACCESS tab per device. So if we do this, going forward I think we would have two different looking rosters (GET which shows all controls per device, and SHARE which only shows users), and I think likewise our data structures in the uProxy core will need to be split so that we have some permissions on the RemoteInstance (whether we can get from them) and some on the User (whether we are willing to share with them)

bemasc commented 9 years ago

I do think this will make both our code and UI more complex

It certainly breaks the symmetry between Get and Share, so I agree that it probably requires more code, including UI code. However, it also significantly reduces the amount of information and the number of actions/options displayed in the UI, so I'm not sure I agree that the UI would be "more complex"

our data structures in the uProxy core will need to be split so that we have some permissions on the RemoteInstance (whether we can get from them) and some on the User (whether we are willing to share with them)

I think this is a key point. If we don't make this change before launch, then we will have users with stored permissions that are incommensurable with the new model, leading to a major problem. That's why I think it's important that we fix this before launch.

ryscheng commented 9 years ago

This seems related to the chat we had in the fall about multi-device management, cryptographic device keys and pairing devices. Are you suggesting we try to support this issue without solving the crypto first?

On Fri, Feb 27, 2015 at 11:40 AM, Benjamin M. Schwartz < notifications@github.com> wrote:

I do think this will make both our code and UI more complex

It certainly breaks the symmetry between Get and Share, so I agree that it probably requires more code, including UI code. However, it also significantly reduces the amount of information and the number of actions/options displayed in the UI, so I'm not sure I agree that the UI would be "more complex"

our data structures in the uProxy core will need to be split so that we have some permissions on the RemoteInstance (whether we can get from them) and some on the User (whether we are willing to share with them)

I think this is a key point. If we don't make this change before launch, then we will have users with stored permissions that are incommensurable with the new model, leading to a major problem. That's why I think it's important that we fix this before launch.

— Reply to this email directly or view it on GitHub https://github.com/uProxy/uproxy/issues/1014#issuecomment-76425416.

iislucas commented 9 years ago

Outlining all the options here: https://docs.google.com/document/d/1p16jeodYUYF9tp2mh9nDVIHlKagzHZLYVHRKZLZkspw/edit#

I think crypto for user-authentication authogonal (you can either use a social network which is trusted to auth users, or you do crypto for sending messages); but either way you can do consent in an asymmetric user (getter) to instance (sharer) way.

bemasc commented 9 years ago

@ryscheng yes, that is issue #708. I've split the issue to track changing the user-visible behavior separately from inventing a cryptographic protocol for it.

ryscheng commented 9 years ago

Makes sense. Thanks for the clarification

zahoriea commented 9 years ago

Hi all, here's a first stab at this .. let me know if I'm capturing the intent here: userlevelpermissions_01

There are a couple of other new design elements pertaining to other bugs as well. You'll notice iconography in expanded and collapsed states for "pending" and "access granted" in issue https://github.com/uProxy/uproxy/issues/875 and a location for upload/download rates and time elapsed (or whatever data we decide to show) in issue https://github.com/uProxy/uproxy/issues/855 and https://github.com/uProxy/uproxy/issues/16

Feedback welcome! I was also reply to the other bugs individually.

zahoriea commented 9 years ago

PS, these may be easier to view/comment on in folio https://folio.googleplex.com/izzie/uProxy/022715%20-%20User%20level%20permissions#%2FUserLevelPermissions_01.png%3Fz=half

bemasc commented 9 years ago

Wow, beautiful!

I think all the mocks are correct, but there might be a misunderstanding in the annotations. Under "USER-LEVEL PERMISSIONS", the text says "she grants access across all of her devices and you can choose which to proxy from". I think this is not quite right. If this said, "Roothu clicks 'Offer Access' on each of her devices so you can choose which to proxy from", I would agree. However, as I noted above, the intention for this change is that each permission grant allows a specified user to access a specified device.

As a use case example, consider someone who has uProxy at home and at work, and wants to allow friends access to the internet from home, but not to the corporate network.

zahoriea commented 9 years ago

Thanks! :+1: Final question, and then I think I'm clear on this model (and apologies if I'm getting in the weeds)

Suppose I have uProxy installed on a Work Laptop and a Personal Computer. I am currently online with my Work Laptop and my Personal Computer is asleep at home. Can I grant access to a device remotely? What about if that device is offline?

If the answer to these is "yes" then I will need to make a couple small changes to the "Share access" mocks.

bemasc commented 9 years ago

I am currently online with my Work Laptop and my Personal Computer is asleep at home. Can I grant access to a device remotely?

I think we would love to be able to do this one day, but it is very far away, and requires considerable concept-design work on which we have not even started. For the foreseeable future, each device can only control access to itself.

What about if that device is offline?

I hadn't even thought about this. This is an interesting idea but it's definitely even further away, out in the dreamy mists.

zahoriea commented 9 years ago

Cool. Thanks!

iislucas commented 9 years ago

If a user currently wants to control a remote device, I think our best strategy is point them to remote desktop tools (like Chrome Remote Desktop). As Ben says, this isn't planned for v1.

dborkan commented 9 years ago

I think that we can pretty easily make a change so that the SHARE ACCESS tab only shows users, and when I grant access to Bob from my laptop, I am granting access to all of Bob's devices to my laptop only (and would have to permission him separately from my desktop).

I think the proposed changes to the GET ACCESS tab will be much more work and I'm not convinced it will be a better experience. Specifically:

I can think of a few more annoying questions if you want regarding this.

Any objection to just making the proposed changes to SHARE ACCESS but leaving the GET ACCESS tab alone?

dborkan commented 9 years ago

Just discussed this with Lucas and I think we have a better plan:

iislucas commented 9 years ago

OK, I think all the thinking is converging nicely... putting it all in one place (same link as above): https://docs.google.com/document/d/1p16jeodYUYF9tp2mh9nDVIHlKagzHZLYVHRKZLZkspw/edit#

zahoriea commented 9 years ago

@bemasc @dborkan @lucyhe and myself talked about this at the UI sync on Thursday. The outlined proposal SGTM and I think this bug is mostly "Ready for Work" (with the exception of the 'Name your device' dialog for first-time sharers ... I am happy to provide a mock for that)

zahoriea commented 9 years ago

Attached is a first stab at a first-time-sharing-on-any-given-device "Name device" flow. Feedback welcome!

firsttime_namedevice

It is a bit reliant on cleaning up the "Share access" icon tooltip in #1076 .. otherwise users may feel stuck in a bit of pop-up mayhem: the name device modal, success modal, and then share access icon popup

iislucas commented 9 years ago

Great! Two small ideas to try and set expectations a little more:

zahoriea commented 9 years ago

On the first idea, SGTM!

On the second, right now the Success modal reads "If you ever want to change it, you can do so within settings." Also made a version that points straight to the Settings hamburger instead:

success

Thoughts on wording/styling in either?

dborkan commented 9 years ago

I just created https://github.com/uProxy/uproxy/issues/1077 for naming the device when a user shares for the first time. We can do that work completely independently of permission changes, so let's keep this issue 1014 just for the permission change parts