Support for the SameSite cookie option

[stissot]

Q	A
Documentation	no
Bugfix	yes
BC Break	no
New Feature	yes
RFC	yes
QA	no

Description

Add support for the SameSite SetCookie parameter, described in RFC6265bis and supported by the session.cookie_samesite PHP ini option from PHP 7.3+. The parameter can have the Lax or Strict or Null values, any other value will be ignored.

[PowerKiKi]

SameSite::fromString() already provides some validation. Maybe we don't need to duplicate validation in our code ? only check for empty values ?

[michalbundyra]

@stissot Thanks for you contribution. It looks good for me, we just need some unit tests to cover these changes. Would you be able to provide some? Thanks!

[stissot]

@stissot Thanks for you contribution. It looks good for me, we just need some unit tests to cover these changes. Would you be able to provide some? Thanks!

I added unit tests, but we have a dependency problem: support for SameSite cookie was added in dflydev/fig-cookies 2.0, but this version requires PHP >7.1, so either we drop the support for PHP 7.1, or shall we skip some tests when running with PHP 7.1 what do you suggest?

[PowerKiKi]

IMHO, we should migrate to require only dflydev/fig-cookies ^2.0 and PHP ^7.2, because it was also done in mezzio/mezzio-swoole@18fe812 for exactly the same reason. So it makes sense to follow the same trend in this ecosystem.

[stissot]

I agree with @PowerKiKi
I have upgraded the PR to require PHP ^7.2* and FigCookies ^2.0.1, upgraded to phpUnit 8.5 and added test suites with PHP 7.4.

* We still require PHP 7.3 to set SameSite in the SetCookie header, because the INI setting session.cookie_samesite was added in that version.

[michalbundyra]

@PowerKiKi

IMHO, we should migrate to require only dflydev/fig-cookies ^2.0 and PHP ^7.2

I agree, but 2.0.1 and PHP 7.2 ;)
There is no other way we can provide SameSite support when our external dependencies do not support it on PHP < 7.2.

[PowerKiKi]

LGTM

[michalbundyra]

@PowerKiKi @stissot As the feature requires PHP 7.3 anyway and PHP 7.2 is in security fixes only phase I think we should bump PHP version to 7.3 in this library for the next minor release.

@stissot Would you be able to update it, please? BTW Thanks for the PR :)

[PowerKiKi]

I'm usually in favor of dropping unsupported PHP version. But we still have 8 months to go for PHP 7.2. I feel it's a bit early.

Bugs affecting users that runs PHP 7.2 could be fixed in that period, forcing them to upgrade PHP or leave open bugs. And it also is slightly out of scope of this PR.

I'd prefer to have another dedicated PR to drop PHP 7.2 in November 2020.

[michalbundyra]

@PowerKiKi We will provide security updates for the previous minor until PHP 7.2 EOL so in general it should not be a problem.

[PowerKiKi]

Oh, I was under the impression, that you only released "higher" version and didn't have security update branches for older versions. If that's the case, then dropping PHP 7.2 is fine with me.

[michalbundyra]

@PowerKiKi Yes, that's the plan - have security updates synchronised with PHP release cycle :)

[stissot]

Thanks for your feedback, I agree to require PHP 7.3 in the next release and the PR has been updated accordingly.

[stissot]

Could you make a release? I need this feature for our project.

[michalbundyra]

Thanks, @stissot!
Thanks, @PowerKiKi!