Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POWER: IDL syntax? #262

Closed
annevk opened this issue Apr 16, 2015 · 24 comments
Closed

POWER: IDL syntax? #262

annevk opened this issue Apr 16, 2015 · 24 comments
Labels

Comments

@annevk
Copy link
Member

annevk commented Apr 16, 2015

It would be kind of nice to have IDL syntax for restricting a feature to secure^H^H^H^H^Hprivileged contexts. Makes API review even easier.

@mikewest
Copy link
Member

Sounds good to me. [RequiresPrivilegedContext]?

Should I poke @heycam about adding something to WebIDL, or define something in this document for the moment?

@mikewest
Copy link
Member

(Relatedly, I'm wondering if adding an attribute to the global object would be a reasonable thing to do...)

@annevk
Copy link
Member Author

annevk commented Apr 16, 2015

@heycam or @bzbarsky I guess. Specifying a strawman seems fine. I would prefer a slightly shorter name if we can get away with it. Perhaps it can be part of the [Exposed] thing somehow.

File a separate issue for the global thing?

@mikewest
Copy link
Member

I'm happy to spell the concept however folks who know things about WebIDL want me to spell it.

@annevk
Copy link
Member Author

annevk commented Apr 16, 2015

I guess we need to decide on the semantics first. When added, would the API simply not be present in non-privileged contexts? Or would it be a method annotation of sorts that makes the method throw or reject the promise the method returns?

@mikewest
Copy link
Member

I'd suggest the latter. That's what Chrome and Firefox are doing now for things like ServiceWorker.

@annevk
Copy link
Member Author

annevk commented Apr 16, 2015

Okay, in that case I suggest we invent a [Privileged] annotation.

@bzbarsky
Copy link

I would rather not overload [Exposed] here, because we'll get combinatorial explosion (need a privileged version of every global type or something, and it gets worse if we ever have more than two privilege levels).

I agree with @annevk that we need to decide on semantics. Gecko certainly has ways to mark things privileged which cause them to not appear at all in what we consider non-privileged contexts. We obviously also have ways of making things just throw. Which one is desired depends on whether we want non-privileged contexts to be able detect that the functionality is supported in a privileged context, right? Are there any other reasons to expose in a non-privileged context?

In general, I'm leery of exposing always-throwing API, esp. of the property getter variety...

@annevk
Copy link
Member Author

annevk commented Apr 17, 2015

Chrome always exposes Crypto and SW. Gecko always exposes SW. We might be stuck with throwing. However, I was thinking of trying to limit this to methods. Those that request permission grants or get the thing rolling. I agree that throwing on property getters would be ugly.

@bzbarsky
Copy link

Gecko always exposes SW.

Not in anything we've actually shipped we don't. We could totally change the nightly behavior here if that's desired.

@mikewest
Copy link
Member

We should revisit this; Chrome continues to expose the feature, and that makes sense to me. The feature is there, it just doesn't work. I can see reasonable arguments for the alternative: the feature doesn't work, you see.

I think I vaguely prefer leaving the API in place, and allowing it to do something reasonable based on its processing model (e.g. Geolocation would call the error callback, a Promise-based system would reject, etc).

WDYT?

@bzbarsky
Copy link

@mikewest If the behavior would have to be defined in prose anyway (which is what you're suggesting), what is the value of the IDL annotation, exactly?

@mikewest
Copy link
Member

Sorry, I should have been more clear: if we continue exposing the feature, I don't think there's value in the IDL annotation. The annotation is only interesting if we stop exposing the feature entirely, as that seems reasonably definable in IDL, as opposed to prose in every effected specification.

@annevk
Copy link
Member Author

annevk commented Sep 11, 2015

Why is it not reasonable for IDL to check if the environment is a secure context and throw (which results in a rejected promise if the thing returns a promise) if that is not the case?

@mikewest
Copy link
Member

I guess I was thinking of "legacy" APIs like Geolocation that use callbacks.

If we agree that we should throw or reject, then defining that in IDL for new APIs might be reasonable. shrug Do we agree on that? :)

@annevk
Copy link
Member Author

annevk commented Sep 11, 2015

I think so. The couple of legacy APIs we have would indeed need to hook into the primitives themselves. But new APIs not needing any complicated language would greatly simplify adoption.

@bzbarsky
Copy link

Again, we've had significant pushback from web developers in the past when an API throws-when-touched. This is especially critical for attribute getters (due to libraries that enumerate all attributes on objects you pass in to them), a bit less so for methods.

So whether we agree on the throwing really depends on what the APIs involved look like. One viable option if we do want the throwing behavior would be to have the extended attribute only apply to methods, not to attributes. That leaves inconsistencies between methods and attributes, if there are any attributes we don't want available in insecure contexts. So that's the first question: are there any such attributes, and do we foresee any appearing in the future?

@mikewest
Copy link
Member

Chrome isn't currently blocking any attributes based on secure contexts.

@bifurcation
Copy link

If I'm reading the thread correctly, it sounds like:

  • The options for semantics are (1) hide or (2) throw
  • State of the art right now is mostly (2) ...
  • ... but @bzbarsky says there's been significant pushback on that

It seems to me like having this notation could be valuable as we look at restricting more of the web API to secure contexts. And it seems like checking for the presence of an API is already an established pattern for backward compatibility reasons. Do folks think that it would be reasonable to move the currently restricted stuff (e.g., WebCrypto) from throwing to being hidden?

@bzbarsky
Copy link

... but @bzbarsky says there's been significant pushback on that

Specifically for throwing attribute getters. Note that WebCrypto doesn't throw at all, by the way, since you can get window.crypto.subtle no matter what as far as I can tell, and all the methods on it are promise-returning, hence reject, not throw....

@bifurcation
Copy link

Yeah, I was being lazy and conflating reject and throw. It seems like "throw when called" and "return a rejected promise" are pretty much equivalent, when it comes to methods?

@bzbarsky
Copy link

They are, yes.

@mikewest
Copy link
Member

I took a stab at this in whatwg/webidl#65. I am not a WebIDL expert, so help with the patch's language would be very appreciated. :)

heycam pushed a commit to whatwg/webidl that referenced this issue Mar 22, 2016
As discussed in w3c/webappsec-secure-contexts#8 (and w3c/webappsec#262),
this patch defines a [SecureContext] extended attribute for interfaces,
interface members, and dictionaries with constructors, whose presence
prevents the construct from being exposed in ECMAScript global
environments that are not secure contexts.
@annevk
Copy link
Member Author

annevk commented Dec 10, 2018

This was fixed (and it turned out more than kind of nice, yay).

@annevk annevk closed this as completed Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants
@mikewest @bifurcation @bzbarsky @annevk and others