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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Google phasing out third-party cookies #213

Closed
2 tasks done
nicob-29 opened this issue Oct 13, 2023 · 20 comments 路 Fixed by #221
Closed
2 tasks done

Google phasing out third-party cookies #213

nicob-29 opened this issue Oct 13, 2023 · 20 comments 路 Fixed by #221

Comments

@nicob-29
Copy link
Contributor

nicob-29 commented Oct 13, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

馃殌 Feature Proposal

Hi
Google plans soon to activate phasing out third-party cookies
See Third-Party Cookies doc: https://privacysandbox.com/open-web/#the-privacy-sandbox-timeline

Doc about phasing out third-party cookies: https://developer.chrome.com/en/docs/privacy-sandbox/third-party-cookie-phase-out/#partitioned-cookies

This feature request requires the possibility to add "Partitioned" to the cookie like Secure.

Motivation

Soon activation from Google
Google plans soon to activate phasing out third-party cookies (Search Third-Party Cookies doc: https://privacysandbox.com/open-web/#the-privacy-sandbox-timeline)

Example

Extend interface CookieOptions to allow the possibility to set Partioned.

I think that request some dependency on https://github.com/jshttp/cookie

@gurgunday
Copy link
Member

Currently unimplemented in upstream: jshttp/cookie#147

@mcollina
Copy link
Member

We should fork upstream and add this feature.

@Uzlopak
Copy link
Contributor

Uzlopak commented Oct 13, 2023

Am I banned by jshttp or can you also not comment and stuff?

image

@mcollina
Copy link
Member

I could, but really it's better if we fork.

@gurgunday
Copy link
Member

Am I banned by jshttp

Weird, I can post comments

I could, but really it's better if we fork.

I'm down to forking for various reasons, just letting you know that a PR for Partitioned is up:

jshttp/cookie#153

@Uzlopak
Copy link
Contributor

Uzlopak commented Oct 13, 2023

We had cookie integrated in fastify-cookie and we reverted back to cookie.

@dougwilson
Did you ban me from jshttp?

@dougwilson
Copy link

Why would I ban you? Not really familiar with your name.

@jsumners
Copy link
Member

That's just a bad GitHub error. It happens to me occasionally on various repos, including Fastify ones. You have to wait and try again later.

@mcollina
Copy link
Member

mcollina commented Nov 6, 2023

@nicob-29 I've opened up fastify/fastify-cookie#260 to move back to use our fork. Would you like to add that feature to fastify-cookie directly?

@nicob-29
Copy link
Contributor Author

nicob-29 commented Nov 7, 2023

Thanks @mcollina
After some effort, jshttp/cookie#153 was merged.
So as you like if you prefer use your fork or not.

@gurgunday
Copy link
Member

Maybe we can pull that change as well while keeping the fork if there are performance improvements we can do

I have鈥檛 gone through its source code yet

@nicob-29
Copy link
Contributor Author

Hi
Any progress ?

@gurgunday
Copy link
Member

It鈥檚 fixed

Make sure to pass partitioned:true to cookie options

@nicob-29
Copy link
Contributor Author

nicob-29 commented Nov 20, 2023

We use https://www.npmjs.com/package/@fastify/session#cookie to set the cookie information.
interface: fastifySession.CookieOptions
Is it possible to upgrade fastify/session to support partitioned too ?

@gurgunday
Copy link
Member

I see, yeah will do

@nicob-29
Copy link
Contributor Author

@gurgunday

When do you plan to release this version about partitioned cookie ?

@gurgunday
Copy link
Member

Just released 10.6.0

@nicob-29
Copy link
Contributor Author

nicob-29 commented Nov 27, 2023

@gurgunday
We have a breaking-change about property secure.
import { SerializeOptions } from "@fastify/cookie"
SerializeOptions doesn't support auto

Previously on @10.0.2
/** The boolean value of the Secure attribute. Set this option to false when communicating over an unencrypted (HTTP) connection. Value can be set to auto; in this case the Secure attribute will be set to false for HTTP request, in case of HTTPS it will be set to true. Defaults to true. */
secure?: boolean | 'auto';

The fix is not only impact the interface, the class cookie https://github.com/fastify/session/blob/master/lib/cookie.js should be modified too to support partitioned

Something like that
`module.exports = class Cookie {
constructor (cookie, request) {
const originalMaxAge = cookie.originalMaxAge || cookie.maxAge || null
this.path = cookie.path || '/'
this.secure = cookie.secure ?? null
this.sameSite = cookie.sameSite || null
this.domain = cookie.domain || null
this.httpOnly = cookie.httpOnly !== undefined ? cookie.httpOnly : true
this._expires = null
this.partitioned = cookie.partitioned ?? null

if (originalMaxAge) {
  this.maxAge = originalMaxAge
} else if (cookie.expires) {
  this.expires = new Date(cookie.expires)
  this.originalMaxAge = null
} else {
  this.originalMaxAge = originalMaxAge
}

if (this.secure === 'auto') {
  if (isConnectionSecure(request)) {
    this.secure = true
  } else {
    this.sameSite = 'Lax'
    this.secure = false
  }
}

}

set expires (date) {
this._expires = date
}

get expires () {
return this._expires
}

set maxAge (ms) {
this.expires = new Date(Date.now() + ms)
// we force the same originalMaxAge to match old behavior
this.originalMaxAge = ms
}

get maxAge () {
if (this.expires instanceof Date) {
return this.expires.valueOf() - Date.now()
} else {
return null
}
}

toJSON () {
return {
expires: this._expires,
originalMaxAge: this.originalMaxAge,
sameSite: this.sameSite,
secure: this.secure,
path: this.path,
httpOnly: this.httpOnly,
domain: this.domain,
partitioned: this.partitioned
}
}
}
`

@gurgunday
Copy link
Member

You're right - seems like I needed to import CookieSerializeOptions and export it back while omitting its signed property

Or we can just extend SerializeOptions again and add secure

Super busy right now, so a PR from users would be very welcome

@nicob-29
Copy link
Contributor Author

nicob-29 commented Dec 4, 2023

@gurgunday
Please take a look to #225 Ignore this one

I created a new one #226 (I fixed merge issue)

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 a pull request may close this issue.

7 participants