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

GetPartitionKey updates #712

Open
wants to merge 60 commits into
base: main
Choose a base branch
from
Open

Conversation

aselya
Copy link
Contributor

@aselya aselya commented Oct 25, 2024

When implementing the getPartitionKey api, there were two issues that I felt should be updated in the proposal.

The first is removing the ability to allow for just a frameId to be used. Allowing this requires the browser to make an assumption on the intended tab which is not explicitly stated in the proposal and could lead to inconsistent behavior between browsers.

In chromium there are cases where the CookiePartitionKey of a cookie can be unserializable (if it's got a nonce or the topLevelSite is opaque). The API should return an error in this case to ensure that we are only returning valid keys with this method key.

aselya and others added 30 commits March 28, 2024 11:31
Add initial proposal
Updates for clarity
Update formatting of url
Updates in phrasing and document organization.
Additional content and formatting
Update pr to incorporate reviewer feedback.

Co-authored-by: Rob Wu <[email protected]>
Update to reflect reviewer suggestion.

Co-authored-by: Rob Wu <[email protected]>
Update to remove reference to Chrome

Co-authored-by: Rob Wu <[email protected]>
Added background information on x-site ancestor chain bit
Update language for cookies.remove()
Add new use case and add clarification to table
Add language describing logic for determining hasCrossSiteAncestor value when not provided. Update method descriptions to incorporate new language and indicate when errors will be thrown.
aselya and others added 29 commits July 26, 2024 13:19
Update the description of cookies.remove()
Add language that explicitly allows for get(), getAll() and remove() to have values that would not be valid for set(). To allow for migration of cookies that are no longer valid at runtime.
Add language to privacy to point out the pre-existing risks associated with editing partition keys
Add description for getPartitionKey() api.
Updating language for getAll() to align it with the other APIs, where a partitionKey with no topLevelSite and a value for hasCrossSiteAncestor returns an error.
Update language to specify that `{hasCrossSiteAncestor: false}` and `{hasCrossSiteAncestor:true}` are invalid keys.
Add table containing valid partitionKeys. Update language in the background section to be more consistent.
Clarify language surrounding the empty partitionKey
Co-authored-by: bvandersloot-mozilla <[email protected]>
Update language to specify the domain of the cookie's url
Return {} to the table of valid partition keys
Add table describing inputs for getPartitionKey()
describe error conditions of new api
Updates to the getPartitionKey() Api:
Remove ability to just pass a frameId without a tabId.

Update language to describe that an error should be returned when a partitionKey is not serializable.
Update language to include reference to nonce
@@ -115,6 +114,7 @@ A Promise that will be fulfilled with a `Cookie.partitionKey` object that matche
##### Error conditions
- If host permissions are not granted for the document whose partitionkey is getting queried an error will be returned.
- When the parameters passed do not correspond to an existing frame, an error will be returned.
- If the partitionKey that would be associated with the frame can not be serialized an error will be returned. This can happen when the origin associated with the topLevelSite is opaque or if the underlying key associated with the frame has a nonce.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this equivalent to getting an error when document.cookie is read/set from the web page? If not, then I would still expect a way to manage the cookies for that document.

If it is, it may be worth calling out that there cookies cannot exist in such documents.

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

Successfully merging this pull request may close these issues.

2 participants