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

feat: add support for partitioned cookies #1167

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

EmilePerron
Copy link
Contributor

This PR adds support for partitioned third-party cookies in a backwards compatible way, as described in #1166.

I have updated the documentation's configuration examples, but have not added any tests regarding this new option as the existing tests did not seem to cover similar options. I'll gladly add some if you think it's worth it.

Let me know if you have any comments/feedback!


closes #1166

@EmilePerron
Copy link
Contributor Author

EmilePerron commented Nov 13, 2023

The CI pipeline errors seem to be unrelated to this PR's changes, and should probably be dealt with in a different PR. I checked other recent PRs and they are all getting this same error, which seems related to an updated dependency.

--- Expected
+++ Actual
@@ @@
         "title": "LexikJWTAuthenticationBundle",
         "version": "1.0.0"
     },
-    "openapi": "3.0.0",
+    "openapi": "3.1.0",
     "paths": {
         "/login_check": {
             "parameters": [],

/home/runner/work/LexikJWTAuthenticationBundle/LexikJWTAuthenticationBundle/Tests/Functional/Command/ApiPlatformOpenApiExportCommandTest.php:28

FAILURES!
Tests: 166, Assertions: 483, Failures: 1, Skipped: 22.

@EmilePerron
Copy link
Contributor Author

EmilePerron commented Nov 22, 2023

Hello! Would it be possible to know when we can expect this to be reviewed and/or merged?
Or if there is anything prevent this from moving forward :)

@EmilePerron
Copy link
Contributor Author

Freshly rebased to get the Symfony 7 tests and the OpenAPI 3.1 update that fixes the CI pipeline errors :)

@chalasr
Copy link
Collaborator

chalasr commented Nov 29, 2023

@EmilePerron Awesome. One thing: given the feature can only be used on Symfony 6.4 and higher, should we throw an exception if ones tries to use it with an lower Symfony version? If it makes sense, I'd suggest throwing both from the DI Extension (when setting the option) and the CookieProvider constructor (for potential direct instantiation).

@EmilePerron
Copy link
Contributor Author

EmilePerron commented Nov 29, 2023

One thing: given the feature can only be used on Symfony 6.4 and higher, should we throw an exception if ones tries to use it with an lower Symfony version? If it makes sense, I'd suggest throwing both from the DI Extension (when setting the option) and the CookieProvider constructor (for potential direct instantiation).

@chalasr Makes sense to me! I just added this in a677b38 :)

@chalasr
Copy link
Collaborator

chalasr commented Nov 30, 2023

Thank you @EmilePerron.

@chalasr chalasr merged commit 5c9bcac into lexik:2.x Nov 30, 2023
8 checks passed
@EmilePerron EmilePerron deleted the feat-partitioned-cookies branch November 30, 2023 15:29
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.

Support for partitioned authentication cookies
3 participants