1218803 - change createCodebasePrincipal calls in about:permissions to use user context id

Status: RESOLVED INVALID

(Firefox :: Security, defect)

Description

[Dave Huseby [:huseby]]

as part of the audit in Bug 1197283 the calls to createCodebasePrincipal in the about:permissions dialog might need to be fixed.  by adding code to pass along the userContextId we're going to isolate the permissions based on context.  this changes user-facing behavior and we should discuss this.

Comment 1

[Dave Huseby [:huseby]]

Attachment: Bug_1218803.patch

This *should* result in the about:permissions page doing separation on the user context ID.  The user will see one set of sites when they type in "about:permissions" on the "Banking" context tab.  Then if they type in "about:permissions" on the "Personal" context tab, the set of sites will be completely different.

Do we want to do this?  I'm in favor of it, but it will change user facing behavior so I'm sure this should spark wider review and consensus building.

Comment 2

[Dave Huseby [:huseby]]

Comment on attachment 8679571 [details] [diff] [review]
Bug_1218803.patch

whoops, there's more to this patch...coming soon.

Comment 3

[Dave Huseby [:huseby]]

So I just confirmed that this *will* isolation on userContextId.  Here's how:

1) nsPermission::Matches is used to compare principals when looking up permissions for a site.
2) nsPermission::Matches calls nsIPrincipal::Equals to compare it's principal with the one that we're looking up.
3) nsIPrincipal::Equals calls nsIPrincipal::Subsumes calls nsIPrincipal::SubsumesInternal calls OriginAttributes::operator== which compares userContextId as well as the other origin attributes.

If we propagate the userContextId into the Site records in the permissions database, then the end result is that site permissions will be isolated on userContextId and the about:Permissions page will show only the sites/permissions set in the current context.

BTW, there is a redundant comparison of origin attributes in nsPermission::Matches.  There's no need to compare them directly since they get compared when comparing principals.

Comment 4

[Tanvi Vyas[:tanvi]]

(In reply to Dave Huseby [:huseby] from comment #3)
> 
> If we propagate the userContextId into the Site records in the permissions
> database, then the end result is that site permissions will be isolated on
> userContextId and the about:Permissions page will show only the
> sites/permissions set in the current context.
> 
I'm not sure we want this for v1.  It might add more confusion, when a user thinks they've removed a permission, only to see it added back in a different context.  We should have a toggle in about:permissions to choose the context.  Do you want to change permissions for;
* all context
* default context
* banking context
* personal context ... etc

We can work with UX to design this for a future version of Contextual Identity.  For now, we should load about:permissions with all site preferences regardless of context.  If the user gives permission to a site in one context, that should propagate to all contexts.

Comment 5

[Dão Gottwald [:dao]]

about:permissions has been removed in bug 933917.