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

Why does CacheStorage have [SecureContext] but not Cache? #941

Closed
inexorabletash opened this issue Aug 4, 2016 · 10 comments
Closed

Why does CacheStorage have [SecureContext] but not Cache? #941

inexorabletash opened this issue Aug 4, 2016 · 10 comments

Comments

@inexorabletash
Copy link
Member

inexorabletash commented Aug 4, 2016

CC: @mikewest

The global caches attributes have [SecureContext] (link) which makes sense - hide the entry points except in secure contexts.

But the interfaces are inconsistent. CacheStorage has [SecureContext] but Cache does not. This seems inconsistent.

Is this just an oversight in applying a fix for #687 ?

@mkruisselbrink
Copy link
Collaborator

That does look a bit inconsistent indeed. I wonder what would be preferred though with regard to SecureContext on interfaces. For security it doesn't really matter if these interfaces have [SecureContext] or not, as the only way to get access to instances that do things is via methods that are SecureContext only. But maybe for consistency and/or feature detection it might make sense to hide all the interfaces as well? Not sure. Similarly ServiceWorker and ServiceWorkerRegistration currently don't have [SecureContext] but ServiceWorkerContainer does...

@inexorabletash inexorabletash changed the title Why does CacheStorage have [SecureContext] but not Cache Why does CacheStorage have [SecureContext] but not Cache? Aug 4, 2016
@inexorabletash
Copy link
Member Author

inexorabletash commented Aug 4, 2016

I can see wanting [SecureContext] on the interface if there's a constructor and therefore we should be consistent for types without constructors. Although not a technical requirement (so maybe not in http://heycam.github.io/webidl/#SecureContext) we should probably be consistent in the platform and document it somewhere. Hence pestering @mikewest :)

So yeah, what @mkruisselbrink said.

@mikewest
Copy link
Member

mikewest commented Aug 5, 2016

If the interface doesn't do anything at all without being inside a secure context, then yeah, mark it up with SecureContext. If there's value to your feature to being exposed, then do it that way. We don't really have rigid rules yet; we'll have to learn together what makes sense.

@mikewest
Copy link
Member

mikewest commented Aug 5, 2016

I'd personally suggest erring on the side of putting the attribute on too many things; we can always remove it later if it turned out to be overzealous. :)

@jakearchibald
Copy link
Contributor

Yeah this is just an oversight I think

@jungkees
Copy link
Collaborator

jungkees commented Aug 5, 2016

I can see wanting [SecureContext] on the interface if there's a constructor and therefore we should be consistent for types without constructors.

The reason was exactly that. I.e., the Cache, ServiceWorker and ServiceWorkerRegistration don't have a constructor and can be gotten only from the methods exposed with [SecureContext].

We don't really have rigid rules yet; we'll have to learn together what makes sense.

Are there any other such cases yet? I can do either way but would like to have some good rationale.

@annevk
Copy link
Member

annevk commented Aug 5, 2016

You can also get them from the global. Since that doesn't enable anything useful, you should prevent exposure there with [SecureContext].

@jungkees
Copy link
Collaborator

jungkees commented Aug 5, 2016

They can't be gotten from a non-secure context's global, either. But it seems all agreed on putting it. I'll address it in that way.

@annevk
Copy link
Member

annevk commented Aug 5, 2016

They can, e.g., window.Cache. You need [SecureContext] to prevent that.

@jungkees
Copy link
Collaborator

jungkees commented Aug 5, 2016

Ah.. didn't know it's exposed even without a constructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants