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

RESOLVED INVALID

Status

()

RESOLVED INVALID
3 years ago
3 years ago

People

(Reporter: huseby, Assigned: huseby)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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.
(Assignee)

Updated

3 years ago
Assignee: nobody → huseby
(Assignee)

Updated

3 years ago
Blocks: 1197283
(Assignee)

Comment 1

3 years ago
Created attachment 8679571 [details] [diff] [review]
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.
Flags: needinfo?(sworkman)
Attachment #8679571 - Flags: review?(bobbyholley)
(Assignee)

Comment 2

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

whoops, there's more to this patch...coming soon.
Attachment #8679571 - Flags: review?(bobbyholley)
(Assignee)

Comment 3

3 years ago
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.
Flags: needinfo?(sworkman)

Comment 4

3 years ago
(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.
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
about:permissions has been removed in bug 933917.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.