fixup createCodebasePrincipalFromOrigin to use origin attributes in translation permission handling

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
3 years ago
3 years ago

People

(Reporter: huseby, Assigned: timhuang)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [userContextId][OA])

Attachments

(1 obsolete attachment)

(Reporter)

Description

3 years ago
in the file browsers/components/preferences/translation.js there are two callsites that use the createCodebasePrincipalFromOrigin:

> 213   onSiteDeleted: function() {
> 214     let removedSites = this._siteTree.getSelectedItems();
> 215     for (let origin of removedSites) {
> 216       let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(origin);
> 217       Services.perms.removeFromPrincipal(principal, kPermissionType);
> 218     }
> 219   },

and

> 221   onAllSitesDeleted: function() {
> 222     if (this._siteTree.isEmpty)
> 223       return;
> 224
> 225     let removedSites = this._sites.splice(0, this._sites.length);
> 226     this._siteTree.boxObject.rowCountChanged(0, -removedSites.length);
> 227
> 228     for (let origin of removedSites) {
> 229       let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(origin);
> 230       Services.perms.removeFromPrincipal(principal, kPermissionType);
> 231     }
> 232
> 233     this.onSiteSelected();
> 234   },

this removes the translate permission from the principal.  this creates a principal from an origin, then internally the origin is pulled back out of the principal when creating the premission object that needs to be removed.  since we are not isolating on user context id for permissions, the fix is, create an origin attributes from the origin (e.g. populateFromOrigin), then create a GlobalContextOriginAttributes from that to force user context id to 0, then call createCodebasePrincipal with the origin and origin attributes.
(Assignee)

Updated

3 years ago
Assignee: huseby → tihuang
(Assignee)

Comment 1

3 years ago
Created attachment 8734222 [details] [diff] [review]
Fixup createCodebasePrincipalFromOrigin to use origin attributes in translation permission handling.

Dave, Could you take a look on this patch? And, who should I ask for reviewing this bug?
Attachment #8734222 - Flags: feedback?(huseby)
(Reporter)

Comment 2

3 years ago
I don't think this patch is necessary anymore.  Consider the code:

> Services.perms.removeFromPrincipal(principal, kPermissionType);

That calls the function nsPermissionManager::RemoveFromPrincipal (http://mzl.la/1UL9rRO) which calls nsPermissionManager::AddInternal() (http://mzl.la/1PtZobU) That function uses an instance of the PermissionKey class (http://mzl.la/1PtZwrM) to get the hash string used to look up the permission.  The hash string is based solely on the origin string and *NOT* the origin attributes.

Therefore, it makes no sense to worry about putting the correct origin attributes into the principal that gets passed to the .removeFromPrincipal() function call.  The origin attributes aren't used in there at all.

This bug should be flagged as RESOLVED, WON'T FIX.
(Reporter)

Comment 3

3 years ago
Comment on attachment 8734222 [details] [diff] [review]
Fixup createCodebasePrincipalFromOrigin to use origin attributes in translation permission handling.

Review of attachment 8734222 [details] [diff] [review]:
-----------------------------------------------------------------

This patch isn't needed IMO.
Attachment #8734222 - Flags: feedback?(huseby) → feedback-
(Assignee)

Comment 4

3 years ago
Comment on attachment 8734222 [details] [diff] [review]
Fixup createCodebasePrincipalFromOrigin to use origin attributes in translation permission handling.

According comment 3, mark as obsolete.
Attachment #8734222 - Attachment is obsolete: true
(Assignee)

Comment 5

3 years ago
According to comment 3, change the flag into RESOLVED, WON'T FIX.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
(Reporter)

Updated

3 years ago
Whiteboard: [userContextId] → [userContextId][OA]
You need to log in before you can comment on or make changes to this bug.