-
Notifications
You must be signed in to change notification settings - Fork 26
Add support to cookie partition #88
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
Add support to cookie partition #88
Conversation
kanongil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this very nice PR.
The only minor implementation issue, is that I would prefer the options validation to fail, if isPartitioned: true without isSecure: true.
However, I'm not sure this is a suitable feature to implement, given the current state (expired) of the spec. Currently it is a Google-only feature, where support could potentially be dropped.
The main problem with not adding it, is that there is no way for users to add this property manually. As such, a better solution might be to add support for custom names and name/value pairs.
|
Additional properties could be specified using an object: {
…,
set: {
Partitioned: true,
OtherName: 'TheValue'
}
} |
Hi @kanongil, As for the spec being expired, I don't know if this expiration date is being taken seriously as Google is rolling out this new policy already: https://developers.google.com/privacy-sandbox/3pcd I like the additional properties suggestion, but I wonder if it's necessary now as I'm not aware of other properties being proposed. |
|
@kanongil , so what are the next steps to move this PR forward? |
|
Hi @kanongil Thank you |
|
I'm also interested in having support for the |
|
What is left to merge this? Hapijs is not capable delivering api's towards chrome users anymore |
kanongil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, other than my inline comment.
lib/index.js
Outdated
| segment = `${segment}; SameSite=${definition.isSameSite}`; | ||
| } | ||
|
|
||
| if (definition.isPartitioned && definition.isSecure && definition.isSameSite === 'None') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silently ignoring an option is not how hapi usually works. I would much rather see it throw Boom.badImplementation() if it is not a valid combination.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kanongil , thank you for reviewing.
I've applied your suggestion and updated tests.
I wasn't able to run tests though (I'm unable to run tests on master either).
I'll need your help to run tests before merging.
statehood feature/cookie-partition % npm run test
> @hapi/[email protected] test
> lab -a @hapi/code -t 100 -L
/Users/leandro/Projects/statehood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2156
const formattedValue = JSON.stringify(error.data);
^
TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Object'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the CI pipeline failed.
Not being able to run my tests locally is blocking me.
What I'm doing:
node -v
# v20.11.0
npm -v
# 10.2.4
git checkout master
rm -rf node_modules && rm package-lock.json
npm i
npm run testAny thoughts on what I'm doing wrong?
dd521f2 to
fb986ce
Compare
|
@leandrohlsilva tests failed |
Thank you for letting me know. |
|
@leandrohlsilva If you're on the slack, ping one of the channels and I'll help you out |
fb986ce to
8678e8e
Compare
|
Don't worry about the tests. It seems to be an issue with the v23 node release line, which is tested for, but have no official support. Merging is best handled by someone from @hapijs/tsc, who can also publish a new release. |
|
Thanks! It's released as 8.2, I've prepared a matching hapi PR to enforce that update for everyone. |
## Summary Since direct option for partitioned cookie configuration has been merged as part of hapijs/statehood#88. Cleaning up old code. __Closes: https://github.com/elastic/kibana/issues/188720__
## Summary Since direct option for partitioned cookie configuration has been merged as part of hapijs/statehood#88. Cleaning up old code. __Closes: https://github.com/elastic/kibana/issues/188720__ (cherry picked from commit da5981e)
## Summary Since direct option for partitioned cookie configuration has been merged as part of hapijs/statehood#88. Cleaning up old code. __Closes: https://github.com/elastic/kibana/issues/188720__ (cherry picked from commit da5981e)
) # Backport This will backport the following commits from `main` to `9.1`: - [[chore]: refactored CHIPS support configuration (#231472)](#231472) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Elena Shostak","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-08-13T11:00:38Z","message":"[chore]: refactored CHIPS support configuration (#231472)\n\n## Summary\n\nSince direct option for partitioned cookie configuration has been merged\nas part of hapijs/statehood#88. Cleaning up old\ncode.\n\n__Closes: https://github.com/elastic/kibana/issues/188720__","sha":"da5981ebb938a9094fe3f8a5ef58b6114b348925","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["chore","Team:Security","release_note:skip","Feature:Security/Session Management","backport:current-major","v9.2.0"],"title":"[chore]: refactored CHIPS support configuration","number":231472,"url":"https://github.com/elastic/kibana/pull/231472","mergeCommit":{"message":"[chore]: refactored CHIPS support configuration (#231472)\n\n## Summary\n\nSince direct option for partitioned cookie configuration has been merged\nas part of hapijs/statehood#88. Cleaning up old\ncode.\n\n__Closes: https://github.com/elastic/kibana/issues/188720__","sha":"da5981ebb938a9094fe3f8a5ef58b6114b348925"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/231472","number":231472,"mergeCommit":{"message":"[chore]: refactored CHIPS support configuration (#231472)\n\n## Summary\n\nSince direct option for partitioned cookie configuration has been merged\nas part of hapijs/statehood#88. Cleaning up old\ncode.\n\n__Closes: https://github.com/elastic/kibana/issues/188720__","sha":"da5981ebb938a9094fe3f8a5ef58b6114b348925"}}]}] BACKPORT--> Co-authored-by: Elena Shostak <[email protected]>
) # Backport This will backport the following commits from `main` to `9.0`: - [[chore]: refactored CHIPS support configuration (#231472)](#231472) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Elena Shostak","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-08-13T11:00:38Z","message":"[chore]: refactored CHIPS support configuration (#231472)\n\n## Summary\n\nSince direct option for partitioned cookie configuration has been merged\nas part of hapijs/statehood#88. Cleaning up old\ncode.\n\n__Closes: https://github.com/elastic/kibana/issues/188720__","sha":"da5981ebb938a9094fe3f8a5ef58b6114b348925","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["chore","Team:Security","release_note:skip","Feature:Security/Session Management","backport:current-major","v9.2.0"],"title":"[chore]: refactored CHIPS support configuration","number":231472,"url":"https://github.com/elastic/kibana/pull/231472","mergeCommit":{"message":"[chore]: refactored CHIPS support configuration (#231472)\n\n## Summary\n\nSince direct option for partitioned cookie configuration has been merged\nas part of hapijs/statehood#88. Cleaning up old\ncode.\n\n__Closes: https://github.com/elastic/kibana/issues/188720__","sha":"da5981ebb938a9094fe3f8a5ef58b6114b348925"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/231472","number":231472,"mergeCommit":{"message":"[chore]: refactored CHIPS support configuration (#231472)\n\n## Summary\n\nSince direct option for partitioned cookie configuration has been merged\nas part of hapijs/statehood#88. Cleaning up old\ncode.\n\n__Closes: https://github.com/elastic/kibana/issues/188720__","sha":"da5981ebb938a9094fe3f8a5ef58b6114b348925"}}]}] BACKPORT--> Co-authored-by: Elena Shostak <[email protected]>
## Summary Since direct option for partitioned cookie configuration has been merged as part of hapijs/statehood#88. Cleaning up old code. __Closes: https://github.com/elastic/kibana/issues/188720__
## Summary Since direct option for partitioned cookie configuration has been merged as part of hapijs/statehood#88. Cleaning up old code. __Closes: https://github.com/elastic/kibana/issues/188720__
## Summary Since direct option for partitioned cookie configuration has been merged as part of hapijs/statehood#88. Cleaning up old code. __Closes: https://github.com/elastic/kibana/issues/188720__
## Summary Since direct option for partitioned cookie configuration has been merged as part of hapijs/statehood#88. Cleaning up old code. __Closes: https://github.com/elastic/kibana/issues/188720__
Why is this PR necessary?
To support Chrome's CHIPS initiative
https://developers.google.com/privacy-sandbox/3pcd/chips
Tests