-
Notifications
You must be signed in to change notification settings - Fork 35
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
Will returned cookies expose their metadata (expiry, path etc)? #11
Comments
FWIW Cookie: has the same restriction. Path exposure can perhaps be polyfilled using hidden IFRAMES to multiple semi-cooperating super-pathed pages. Unfortunately replaceState does not change the cookie jar view. Domain exposure is trickier since document.domain is not taken into account when serializing document.cookie. I actually think much more valuable information to expose would be Secure, which cannot be polyfilled AFAIK. |
True, but is that reason enough to follow suit? Seems weird to have an API where set data becomes partially ungettable. |
Agreed, but if we fix it for JS we should at least consider extending that fix to HTTP with an additional request header |
This issue is still unresolved, but I think to be truly useful it needs to include a proposal for exposing the same information in HTTP requests. So far I haven't found any proposals for doing that, but it's likely I'm not looking hard enough. FWIW the missing metadata could be represented very compactly:
Depending on the serialization format chosen, all the 0's above could perhaps be omitted entirely. In addition to the already-serialized name, only |
I think it'd make sense to expose metadata in JS, even if it's not exposed anywhere else (in HTTP). Asides from #12, exposing the metadata is necessary to make the API testing-friendly. (Apps should be able to write automated tests for their code that uses the API.) |
It's still not clear what are the benefits of exposing cookie metadata to the user. @pwnall mentioned testing, do you have an example of which kind of testing would benefit from this? Maybe since it's a browser API it's good to expose everything earlier because you can't extend later without causing breaking changes. In this case, would add a getter later really to be considered a breaking change? |
@FagnerMartinsBrack An example is an automated test where I read back a cookie that I've set, and I assert that the domain / path / expiration date match some expected values. (this is aside from the benefit of being able to pass cookie data to To your second point - Avoiding breaking changes would suggest not exposing the properties until later. Exposing the properties would be a non-breaking change, whereas removing them would be a breaking change. That being said, I think the cookie metadata is useful and should be in the API, assuming that the security concerns will be mitigated. |
What's the benefit from the POV of a developer of having to assert that a cookie has been set with domain/ path / expiration by the browser? You'll be essentially testing the browser API which is assumed to already have been working and tested by the browser vendor. One can easily test that the standard cookie API has been called correctly by a domain-specific wrapper and then test the wrapper which is owned by the developer, not the platform (pseudo-code below): MySystemClientStorageAPIDomainSpecificWrapper = ()=>{
let storage;
return {
setStorage: (_storage) => { storage = _storage }
set: (key, value) => { storage.set(key, value) }
};
};
test(() =>{
storageAPI = MySystemClientStorageAPIDomainSpecificWrapper();
storageAPI.setStorage(cookieAPI);
storageAPI.set('key', 'value')
assertThat(cookieAPI).hasBeenCalledWith('key', 'value')
}); That would allow the developer to test that the cookie was set with the given expected value. If the intent is to test the integration between the developer code and the platform (let's say, to make sure the application won't fail in production because of some browser bug), then that integration can be done by testing the behavior of the browser cookie API, otherwise there's no real coverage going on. Why the behavior? Well, just because the browser cookie API is returning the same value that was set, that doesn't mean the cookie API is working. It just means the browser is returning that value: The test is the "first client" of an API. One would expect the API being used in the test (in this case, the getter) will also have a use case for the "second client" and onwards: the code outside the test, the "production code". In this case, what's the use case for a developer to use the cookie getters in production code other than in testing?
That's what I thought. My only concern is that the addition of new features for web APIs are known to have caused problems in the wild even when authors believe they would not have caused a breaking change. That's one of the reasons why no property in If the methods are enumerable, for example, would it be reasonable to think the developer's code ("the client") can use the length of the methods in a way that changing them (adding the getters later) would cause websites to break? We've seen similar things like this happening before. |
The whole point I'm trying to make is:
Despite anything, there probably should be some consultation with early |
@FagnerMartinsBrack Sorry, I had trouble parsing the question / points out of your earlier comments. Here are responses based on a brief chat with @bsittler Cookie testing changesFirst, I agree that you can mock / stub APIs, but that can both testing and production code more complex. Second, there is a bit of value in being able to use the real Cookies API rather than a mock / stub. Writing to / reading from the real cookie store gives you some assurance that you used the correct APIs and that the values you passed were accepted by the browser. Breaking the Web with API changesThis API is in incubation, so it shouldn't be used anywhere in the wild. So I don't think that the concern of breaking things applies here. JustificationAsides from testing, #12 explains that the current API proposal has a rough edge where passing the result of a |
It doesn't make it more complex, it forces your internal API to be more decoupled as long as you can keep cohesion. That's the whole purpose of designing to improve testability, which a great think for maintainability purposes. With that in mind, you can avoid mocks if you want to: test(() =>{
storageAPI = MySystemClientStorageAPIDomainSpecificWrapper();
storageAPI.setStorage(inMemoryAPI);
storageAPI.set('key', 'value')
assertThat(inMemoryAPI.get('key')).to.equal('value')
});
@pwnall In the 7 years of jquery-cookie/js-cookie existence, there was never a bug or use case where having the getter APIs in the browser would make things easier. It does give you some assurance that the browser is working fine, but testing the browser is a theoretical benefit unless it's measurable. It may not be worth the tradeoff of cluttering the browser API unless one can come up with code examples where that would be useful.
I'm not on top of that issue. If the idea is to allow the dev to remove cookies despite the subdomain they are, it might be worth investigating the potential security implications. |
@FagnerMartinsBrack Thank you for your feedback! I don't currently see any security issues with giving site authors the information that they've set in cookies, and I see some upside as mentioned above. I currently plan to expose cookie metadata (as shown in the current explainer), but will definitely keep looking at developer feedback as the API goes through Chrome's origin trials. |
We don't know what we don't know. The problem is not legit site authors, but reading the cookies from cross-origin. We're all aware that cookies are used to store sensitive information like the session, so just need to be extra careful this is not opening a whole new can of worms. Just a head's up anyway. |
I agree that it's important to not regress the security story here. For the kind of vulnerabilities that you've described, I think the principles at https://wicg.github.io/cookie-store/explainer.html#the-scope-path--domain are a helpful way to frame the problem. If what you're concerned with can be done with |
@pwnall @FagnerMartinsBrack I hope it's not too late to jump in with a developer perspective on these points:
We are developing a sandboxing proxy at Surfly. One of the required tasks is quite literally "transfer existing cookies from one domain to another". When our solution kicks in, the integrating application has already set some cookies, and we need to carefully transfer those values to a sandboxed scope. Currently we have a quite a complicated implementation involving all those hacks mentioned above and in #10 by @jakearchibald. Non-httpOnly cookies are transferred using our JS API library, and for httpOnly cookies we ask our clients to implement a server-side integration. The main challenge in this is to determine and transfer the complete scope of a cookie (path, domain, and flags), otherwise there will be inconsistencies in sandbox behaviour. With the current From the security point of view, I suppose the question is whether cookie metadata is considered sensitive, because as far as I understand, this debate is only about those cookies whose names/values are already readable. Although I agree with @FagnerMartinsBrack that we'd better ask a security person, which I am not anymore. |
I realise that can't be polyfilled using
document.cookie
, but are we going to have the same restriction?The text was updated successfully, but these errors were encountered: