My-Little-Forum / mylittleforum

A simple PHP and MySQL based internet forum that displays the messages in classical threaded view (tree structure)
GNU General Public License v3.0
124 stars 48 forks source link

Adding attribute SameSite to cookies #696

Closed auge8472 closed 8 months ago

auge8472 commented 8 months ago

When looking into the developer tools of my browser and requesting my testing forum I encountered warnings about lacking the cookie attribute SameSite.

Das Cookie "mlf2_auto_login" verfügt über keinen gültigen Wert für das "SameSite"-Attribut. Bald werden Cookies ohne das "SameSite"-Attribut oder mit einem ungültigen Wert dafür als "Lax" behandelt. Dadurch wird das Cookie nicht länger an Kontexte gesendet, die zu einem Drittanbieter gehören. Falls Ihre Anwendung das Cookie in diesen Kontexten benötigt, fügen Sie bitte das Attribut "SameSite=None" zu ihm hinzu. ...

translated with deepl.com:

The cookie "mlf2_auto_login" does not have a valid value for the "SameSite" attribute. Soon cookies without the "SameSite" attribute or with an invalid value for it will be treated as "Lax". This means that the cookie will no longer be sent to contexts that belong to a third-party provider. If your application requires the cookie in these contexts, please add the "SameSite=None" attribute to it. ...

Additionally there is a link to the MDN-manual-page for cookies, section SameSite.

Because our cookies are internal and not intended to be sent to any third-party providers, we do not need to use the proposed value None. For our purposes the value Strict seems to be the correct one.

Important to mention: I do not see the warning, when opening a page in the project forum on mylittleforum.net.

loesler commented 8 months ago

Do we need to adapt the session function itself

session_set_cookie_params(['samesite' => 'strict']);
session_start();

or the setcookie function affected, too?

auge8472 commented 8 months ago

Do we need to adapt the session function itself

session_set_cookie_params(['samesite' => 'strict']);
session_start();

or the setcookie function affected, too?

As far as I can see in the console of the development tools of my browser (Firefox), only the autologin cookie is affected.

Cookie “mlf2424_auto_login” does not have a proper “SameSite” attribute value. …

I would expect to get a warning message for all affected cookies at once. So I must assume, that it's only the mentioned one.

loesler commented 8 months ago

Well, afaik the auto_login cookie is set via setcookie, i.e. login.inc.php#L65. This is not the session cookie, isn't it?

auge8472 commented 8 months ago

This is not the session cookie, isn't it?

No, it's not. It's the cookie, that should prevent one from login every single browser session.

loesler commented 8 months ago

So, we have to add the SameSite attribute via calling setcookie, too

loesler commented 8 months ago

I added the attribute to the session cookie as well as to the user-setting cookie. I hope I haven't missed any setcookie call.