Closed Bug 961116 Opened 8 years ago Closed 8 years ago

Add a WebIDL annotation for the permission to check before exposing a method/attribute/interface

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 952486

People

(Reporter: ehsan.akhgari, Unassigned)

References

(Blocks 1 open bug)

Details

Building up on the same idea as in bug 958667, we need to be able to declare a permission as a prerequisite for a method/attribute/interface to be exposed.  Strawman proposal: [Permission="foobar"].

Currently we do this by using a [Func] attribute to point to some custom C++ code implementing the permission check, in a manner similar to what Navigator::CheckPermission does.  We sometimes duplicate this kind of logic in other places too, such as <http://mxr.mozilla.org/mozilla-central/source/dom/camera/DOMCameraManager.cpp#75>.

Again, the codegen'ed check here should do a logical and with other similar attributes which affect the name visibility, such as [Pref], [Func] and [AvailableIn].

One complication is how this would interact with [NavigatorProperty].  We can either codegen something that Navigator::DoNewResolve would call into, or fail the codegen in that case and solve this in a follow-up.  As far as I can see, we only use permission checks for navigator.mozSettings and navigator.mozDownloadManager through the resolve hook.

Once we have this available, we should rewrite the existing permission checks on top of this.  I would be happy to help out with that.
> One complication is how this would interact with [NavigatorProperty].

Just fine. The resolve hook already checks the enabled func, so it would just work with this new setup.

Doing this on top of bug 958667 for NavigatorProperty and interface objects is easy.  For methods/properties it's a bigger problem, unless we want to disallow some combinations of Func/AvailableIn/Permission or unless we add more stuff to Prefable.  :(
Oh, a second complication, actually: we need to think a bit about how to find the right window to use for the permissions check...
But generally, I expect we can crib what navigator bits do.
(In reply to Boris Zbarsky [:bz] from comment #1)
> > One complication is how this would interact with [NavigatorProperty].
> 
> Just fine. The resolve hook already checks the enabled func, so it would
> just work with this new setup.

Great!

> Doing this on top of bug 958667 for NavigatorProperty and interface objects
> is easy.  For methods/properties it's a bigger problem, unless we want to
> disallow some combinations of Func/AvailableIn/Permission or unless we add
> more stuff to Prefable.  :(

I don't know enough about the codegen code to tell which option is better, so please pick the easiest/best option! :-)
(In reply to Boris Zbarsky [:bz] from comment #2)
> Oh, a second complication, actually: we need to think a bit about how to
> find the right window to use for the permissions check...

I think we always get it from the global passed to the codegen'ed [Func].  (Except of course for workers.)
> so please pick the easiest/best option

Easiest is only allowing two of them.  Best is to allow all three.  ;)
(In reply to comment #6)
> > so please pick the easiest/best option
> 
> Easiest is only allowing two of them.  Best is to allow all three.  ;)

If I had to pick, I would say leave out [Func] for now.  I would expect that for most cases, [AvailableIn] and [Permission] should make it unnecessary to write a custom function.
Oh, I can support any two, on a per-property basis, with the stuff in bug 958667.  So you'd be able to do Func and Permission or Func and AvailableIn or Permission and AvailableIn.  Just not all three at once.  ;)
(In reply to comment #8)
> Oh, I can support any two, on a per-property basis, with the stuff in bug
> 958667.  So you'd be able to do Func and Permission or Func and AvailableIn or
> Permission and AvailableIn.  Just not all three at once.  ;)

OK I see, thanks for the explanation.  That sounds good to me.
Is this not a dupe of bug 952486?
That bug seems to be filed for interfaces and not other things, but please feel free to dupe in either direction if needed.
So have we decided what we want here, exactly?  Permission="single string here" for a start, listing just one permission name?
The patch I have been working on (for interfaces, but not too hard to support properties) supports a list of permissions and the checks for them are or'ed, which seems to support most (all?) cases that we have.
Depends on: 952486
(In reply to comment #13)
> The patch I have been working on (for interfaces, but not too hard to support
> properties) supports a list of permissions and the checks for them are or'ed,
> which seems to support most (all?) cases that we have.

That sounds good to me.
I think bug 952486 did everything we need here.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 952486
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.