[SDK] Permission system #606

stevenvergenz · 2020-06-05T19:11:19Z

No description provided.

eanders-ms · 2020-06-05T20:19:32Z

For the first implementation, I'd recommend starting with acquisition method 3: passing the requested permissions at initialization time, although I disagree with the third sub-bullet regarding whether the app should proceed or not. It shouldn't be an all-or-nothing decision at startup. See comments lower down for how I think this should work.

Now would be a good time to think about how the permissions UI should work. I'd steer clear of popup dialogs. How about a new button on the control bar (near the host panel) that would open an SDK App Control Panel. The button would show a notification when one or more apps is requesting permissions.

I'd include these additional permissions:

"Allow this app to know about me" (this maps to user-join)
"Allow this app to play audio"
"Allow this app to play video"

Consider making permissions tri-state: "Disallow", "Allow this time", "Allow always".

On the control panel, the user should also be able to:

Set an app's global volume (this was a feature request from another team).
Disable an app entirely.

Thinking about how this works from the app's perspective: The app won't get its requested permissions right away, different permissions may come in at different times, and the user could revoke a permission at any time. I think this warrants a new callback on Context: onPermissionsChanged(user: User), and have a permissions: Permission[] getter on User. This way, the app starts out with no permissions, and permission changes come in over time as they're granted or revoked.

App permissions should be persisted on the backend (so they follow the user from device to device).

Permissions must be enforced on the client, not in the SDK. Otherwise anyone could modify the SDK to circumvent it. #Resolved

tombuMS · 2020-06-08T20:31:04Z

I agree with mostly what Eric has said. The third acquiring option seems in line with the most ideal v1 solution and shouldn't necessarily be an all or nothing gate. Also the additional permissions proposed for "me", audio, and video also make a lot of sense. The three states of permission also seems in line with many other permissioning models out there.

I think that the UI conversation is something that needs to be had at the Altspace level. Currently we have not enforced a specific UI or interaction model for MRE management, so I think this should be vetted with Altspace as the first shipped customer. As a client runtime for an SDK, I think that the specific UI/UX experience for managing MRE's is something that should exist outside of the SDK. We can certainly recommend ways to handle it and implement them in the Test Bed and potentially Altspace, but I want to encourage that we focus on how we present the collection of permissions from the client API surface as a clear deliverable for this feature, rather than on enforcing how this is handled in a host apps UI. #Resolved

tombuMS · 2020-06-08T20:35:37Z

Though I do like the idea of a popup MRE panel like the host tools in Altspace for the specific UI case in Altspace. #Resolved

stevenvergenz · 2020-06-08T21:03:16Z

@eanders-ms @tombuMS The "me" permission can already be implemented host-side since there are no MRE requirement that clients have to join as a user. Ditto the "video" permission since it goes through a non-default factory. I don't think I've seen any other cases of explicit permission being needed to play sound, though I agree that a master volume control host-side is desirable.

I'd like to keep the discussion of UI implementations out of this review if possible, given that different hosts can implement it differently. Let's decide what the API surface should look like, then design a UI around that.

As an app developer, IMO the more external async dependencies your app has, the harder it becomes to reason about. I don't like the idea of different permissions being granted at different times for the same user, seems like it would add a lot of complexity for using those gated features. My preference leans towards acquisition 3, failure mode 2: request all permissions during setup, and users grant permissions as a condition of joining. #Resolved

eanders-ms · 2020-06-09T06:25:46Z

I actually think that will work just fine. The user-joined message would only be sent after permissions are granted. And if permissions are ever revoked, the host sends a user-left. That is simple and works within the async callbacks we have today. #Resolved

sorenhan · 2020-06-09T06:35:46Z

Great work Steven, and great comments. I agree with most, but not all, of Eric's and Tom's comments.

I'd like to see clear separation of what's in MRE API, node-side, in runtime, and in host app, and the api surfaces between them.
Additional permission: Connect to MRE (if a user doesn't want to run an MRE, that's their choice). While this would shut off a whole MRE, I think it would be cleaner to add that as one of the different permission flags, and help message to host app implementers that we recommend letting users choose which MREs to connect to.
Additional permission: Stealing Input ("simple controller input" etc.) - that's another hypothetical future feature
(I don't think it's super important to nail the exact list of permissions we want - "connect to mre", "me", "movement", "attachments" are important, but the rest we should be able to just add/remove as we go along)
For Steven's note on the suggestion of "Me": rather than make it not call user-joined, what about making it a non-persistent user? i.e., if there is no "me" permission, user connects with a random user ID and name at each time a user enters a space? I'd much rather have there always be an anonymous user than deal with no user, to make it more consistent for developers.
To Steven's comment the comment that "me" can already be controlled host-side: Yes it can, and so can "Connect to MRE". However, I think it is cleaner to have all of these permissions run in the runtime system rather than have some hostapp-side and some runtime-side. The more logic that can be done runtime-side instead of hostapp-side, the easier it is for new hostapp integrators.
Agree with Steven that we shouldn't worry about persisting app permissions, and general UI in this discussion. IMO it's more important to focus on the capabilities of the MRE SDK itself than to have the full bells-and-whistles UI in Altspace.
I like that permission is generally requested during initialization (method 3): It could even come before the connection handshake, as part of a downloaded manifest (It seems a manifest would be helpful in other areas, in the future, right? MRE SDK version, description, configuration options, link to thumbnail etc).
As for the comment about "if not all permissions are given, app can't run" - maybe manifest could contain a list of required and optional permissions - don't waste time joining unless all required permissions are given.
I agree with Eric that the host needs to call something when permissions change. However, I strongly disagree with Eric that permissions should come in while you are connected, and the MRE should react to the changes. IMO any change to permissions should just brute force a disconnect and rejoin, rather than gracefully try to update the permissions.
Two reasons:
As steven says, it's complex for MRE devs to deal with changing permissions. This way it's just not a concern for them.
Runtime implementation gets pretty complex - every single permission would be a custom implementation. For example, if there was no audio or video permission, and you started the app and all of a sudden gave permission, do we have to manually keep track of all the audio and video that is playing, and manually calculate how far along the playback is - we did it in the server, but it is troublesome.
Because it would be not commonly used, it is not easily tested, and bugs can hang around
I think realistically users will not change permissions that often as the app is running, so I would rather go for our more reliable, most-walked code path (join-in-progress), and then we spend our energy getting this solid and optimized.
Agree with Eric: All code that performs permissions checking is 100% in the unity runtime, but the server should know which permissions each user has (so MREs can ask about the granted permissions).

#Resolved

sorenhan

(see feedback above)

tombuMS · 2020-06-17T15:40:40Z

design/permissions.md

+	* `string appOrigin` - The origin of the MRE requesting permission, as described above.
+	* `string appDisplayName` - A human-readable identifier for the MRE server provided from the manifest.
+	* `Permissions permissionsNeeded` - A bitfield of the permissions required to run the app.
+	* `Permissions permissionsWanted` - A bitfield of permissions that the app can use, but are not needed to run.


Since this is something being passed to a host app, we may want to consider passing this in a more friendly unpacked format, so that the host app can easily digest this for their UI. Otherwise the parsing of the bitfield in to actionable items and repacking has to happen as code written by each host app. If we send this as something already parsed and receive in the same unpacked manner, we can abstract away the need to deal with separating out actionable items from a bitfield. Maybe an array of Permissions enum value that can easily be iterated over to add prompts to an overall UI? #Closed

In fact based on this proposal, the json will already contain an array of permission strings that could be passed along as is to this call to the host app. You could just wait until the permissions task has completed before you pack this in to a bitfield of approved permissions.

In reply to: 441643214 [](ancestors = 441643214)

That's a good point: since we're sending the perms down as arrays, there's no point in repacking them. I like bitfields because of O(1) lookups, but for <32 items I suppose it doesn't matter.

In reply to: 441644095 [](ancestors = 441644095,441643214)

I mean still packing for lookups is fine, I was just suggesting doing that packing after the host app has gathered the approved permissions. #Closed

I'm pretty sure C# lets you do foreach loops on bitfields of enums. It's cool like that. Might not matter, even for enumeration. I'll play with this come implementation time.

In reply to: 441733983 [](ancestors = 441733983)

It's a bit convoluted how you have to iterate over the values of the enum, but there is a way. It is still easier at this point since the message is carrying as an array of values to just pass that array in to the dialog and receive one back.

In reply to: 441766267 [](ancestors = 441766267,441733983)

tombuMS · 2020-06-17T15:48:23Z

design/permissions.md

+The default implementation of the permissions manager will also include the following virtual methods:
+
+* `void ReadFromStorage()` - Populate the in-memory permission database from offline storage.
+* `void WriteToStorage()` - Write the "remembered" portions of the in-memory permission database to offline storage.


Is this storage we would be providing and interfacing with in our runtime code, since this refers to a default permission manager? Does this mean that no permission manager is required to be supplied by the host app? #Closed

The interface does not require any kind of persistent storage. But since most hosts will probably want it, the MRE namespace will also have an abstract base implementation that provides some stubs. Haven't worked out all the details here yet.

In reply to: 441648519 [](ancestors = 441648519)

Ok. So the stubs are not for a default implementation of local storage then. They are just for optional hooks to the permission manager supplied by the host app? #Closed

Correct.

In reply to: 441735090 [](ancestors = 441735090)

sorenhan · 2020-06-17T21:39:31Z

design/permissions.md

+requests `user-interaction`, that permission is associated with `ws://mres.altvr.com`, and all user decisions apply
+equally to new connections to the same server. These user decisions may or may not be persisted indefinitely by the host app.
+
+


I think the host app should decide the scope here.
I'd also recommend rights to be managed by AppId over URL, if AppId is available

Could you explain your reasoning for hosts making the scope determination? I can see the case for AppId over URL origin, because AppIds can uniquely identify an app and not just a server. I disagree, and think server trust is all that's required since there won't be origins with apps from multiple authors, and it's the author that the user is ultimately trusting.

In reply to: 441849523 [](ancestors = 441849523)

I should have worded that differently. I wanted to say that I don't think we should assume that scope goes beyond the single app, and the host app should ask the user if they would want to permit all apps, all apps by same developer, or a single app.

I, as a user, wouldn't want the assumption made about other apps for me. Just because I want to grant one app a specific permission, doesn't mean that all other apps that are deployed to the same root url should have the same permissions.

So how do you define a single app, the full URL? Does it include query arguments? The more specific we get, the more nag screens the users are gonna be presented with. I think origin strikes a good balance.

Yeah, a unique app to me is the full URL but excluding query arguments.
I think the best analogy is phone apps: If I install an app on my phone and I grant a permission, it shouldn't automatically grant permission to all other apps by that publisher. But if I launch the app with a different deep link, the permissions given to the app before still apply.

Agree that there will be more nag screens. To me that's up to the host app for how to manage. I imagine "yes/no once", "yes/no for this MRE always", "yes/no for all MREs always" would be required. It could be optional to add "yes/no for all MREs on this domain" (or "by this publisher" once MRE App registry is done), but I see that as lower priority

sorenhan

tombuMS

🕐

tombuMS · 2020-06-22T17:15:53Z

packages/sdk/src/user/permissions.ts

+ */
+export enum Permissions {
+	/**
+	 * Grants access to a persistent user identity across sessions. Needed for things like high scores lists or


[](start = 99, length = 1)

comma

tombuMS

stevenvergenz · 2020-06-24T16:06:11Z

design/permissions.md

+This window is paginated, one page per MRE that requires permissions. The name of the app requesting permission, the
+server name it's hosted on, and a secure protocol indicator are all featured prominently. Each permission has a
+checkbox next to it: required permissions are checked and disabled, and optional permissions are checked or not based
+on app origin decision history and settings.


Suggestion: indicate the number of pending requests in the tab label, like the friends requests UI does

revoking review

tombuMS · 2020-06-29T16:12:37Z

packages/sdk/src/util/manifestSchema.json

+			"description": "An SPDX short license ID",
+			"type": "string"
+		},
+		"repositoryUri": {


Uri [](start = 13, length = 3)

Url, not Uri

tombuMS · 2020-06-29T20:47:54Z

packages/functional-tests/public/manifest.json

+	"description": "A set of samples that exercise various features of the Mixed Reality Extensions SDK",
+	"author": "Microsoft",
+	"license": "MIT",
+	"repositoryUri": "https://github.com/microsoft/mixed-reality-extension-sdk",


Uri [](start = 12, length = 3)

Url #Resolved

tombuMS

Add first-draft permissions design

8dfe581

stevenvergenz requested review from sorenhan, eanders-ms and tombuMS June 5, 2020 19:11

This was referenced Jun 5, 2020

Feature Request: Permission system #299

Open

[AltspaceVR] Add an MRE volume control. #534

Open

sorenhan suggested changes Jun 9, 2020

View reviewed changes

stevenvergenz added 3 commits June 15, 2020 17:12

Start fleshing out APIs

4739294

Finish permissions design second draft

b978b79

Add another proposed permission

45491ce

tombuMS reviewed Jun 17, 2020

View reviewed changes

stevenvergenz added 2 commits June 17, 2020 11:53

Add some Altspace UI mockups

9e2188f

More detail on the requests mockup

a86aa98

sorenhan reviewed Jun 17, 2020

View reviewed changes

sorenhan approved these changes Jun 17, 2020

View reviewed changes

stevenvergenz added 4 commits June 17, 2020 15:09

Add the other tabs

8b4e46d

Add permissions infrastructure

73bac1c

Merge remote-tracking branch 'origin/red' into stvergen/permissions

8749135

Add perms to webhost

a23def6

tombuMS previously requested changes Jun 22, 2020

View reviewed changes

tombuMS reviewed Jun 22, 2020

View reviewed changes

Change bitfields to enumerables

40a7d05

tombuMS previously approved these changes Jun 22, 2020

View reviewed changes

stevenvergenz commented Jun 24, 2020

View reviewed changes

stevenvergenz added 2 commits June 24, 2020 15:25

Validate manifest, warn when permissions not requested

8ce1080

Change manifest

25d2d82

stevenvergenz mentioned this pull request Jun 26, 2020

[Unity] Permission system microsoft/mixed-reality-extension-unity#203

Merged

stevenvergenz changed the title ~~Permissions system design~~ [SDK] Permission system Jun 26, 2020

Update design to match implementation

90f1c61

stevenvergenz requested review from sorenhan and tombuMS June 26, 2020 19:57

tombuMS reviewed Jun 29, 2020

View reviewed changes

repositoryUri => repositoryUrl

45bc86b

tombuMS reviewed Jun 29, 2020

View reviewed changes

Fix other manifest

a4bed58

tombuMS approved these changes Jun 30, 2020

View reviewed changes

stevenvergenz merged commit d69927c into red Jun 30, 2020

stevenvergenz deleted the stvergen/permissions branch June 30, 2020 16:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SDK] Permission system #606

[SDK] Permission system #606

stevenvergenz commented Jun 5, 2020

eanders-ms commented Jun 5, 2020 •

edited by stevenvergenz

Loading

tombuMS commented Jun 8, 2020 •

edited by stevenvergenz

Loading

tombuMS commented Jun 8, 2020 •

edited by stevenvergenz

Loading

stevenvergenz commented Jun 8, 2020 •

edited

Loading

eanders-ms commented Jun 9, 2020 •

edited by stevenvergenz

Loading

sorenhan commented Jun 9, 2020 •

edited by stevenvergenz

Loading

sorenhan left a comment

tombuMS Jun 17, 2020 •

edited

Loading

tombuMS Jun 17, 2020 •

edited

Loading

stevenvergenz Jun 17, 2020

tombuMS Jun 17, 2020 •

edited

Loading

stevenvergenz Jun 17, 2020

tombuMS Jun 22, 2020

tombuMS Jun 17, 2020 •

edited

Loading

stevenvergenz Jun 17, 2020

tombuMS Jun 17, 2020 •

edited

Loading

stevenvergenz Jun 17, 2020

sorenhan Jun 17, 2020

stevenvergenz Jun 18, 2020

sorenhan Jun 18, 2020

stevenvergenz Jun 19, 2020

sorenhan Jun 19, 2020

sorenhan left a comment

tombuMS left a comment

tombuMS Jun 22, 2020

tombuMS left a comment

stevenvergenz Jun 24, 2020

tombuMS Jun 29, 2020

tombuMS Jun 29, 2020 •

edited

Loading

tombuMS left a comment

		requests `user-interaction`, that permission is associated with `ws://mres.altvr.com`, and all user decisions apply
		equally to new connections to the same server. These user decisions may or may not be persisted indefinitely by the host app.

[SDK] Permission system #606

[SDK] Permission system #606

Conversation

stevenvergenz commented Jun 5, 2020

eanders-ms commented Jun 5, 2020 • edited by stevenvergenz Loading

tombuMS commented Jun 8, 2020 • edited by stevenvergenz Loading

tombuMS commented Jun 8, 2020 • edited by stevenvergenz Loading

stevenvergenz commented Jun 8, 2020 • edited Loading

eanders-ms commented Jun 9, 2020 • edited by stevenvergenz Loading

sorenhan commented Jun 9, 2020 • edited by stevenvergenz Loading

sorenhan left a comment

Choose a reason for hiding this comment

tombuMS Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

tombuMS Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuMS Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuMS Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuMS Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorenhan left a comment

Choose a reason for hiding this comment

tombuMS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuMS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuMS Jun 29, 2020 • edited Loading

Choose a reason for hiding this comment

tombuMS left a comment

Choose a reason for hiding this comment

eanders-ms commented Jun 5, 2020 •

edited by stevenvergenz

Loading

tombuMS commented Jun 8, 2020 •

edited by stevenvergenz

Loading

tombuMS commented Jun 8, 2020 •

edited by stevenvergenz

Loading

stevenvergenz commented Jun 8, 2020 •

edited

Loading

eanders-ms commented Jun 9, 2020 •

edited by stevenvergenz

Loading

sorenhan commented Jun 9, 2020 •

edited by stevenvergenz

Loading

tombuMS Jun 17, 2020 •

edited

Loading

tombuMS Jun 17, 2020 •

edited

Loading

tombuMS Jun 17, 2020 •

edited

Loading

tombuMS Jun 17, 2020 •

edited

Loading

tombuMS Jun 17, 2020 •

edited

Loading

tombuMS Jun 29, 2020 •

edited

Loading