Skip to content

[Mercure] Compatibility with the Docker integration and various improvements #16293

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Dec 20, 2021
Merged

[Mercure] Compatibility with the Docker integration and various improvements #16293

merged 12 commits into from
Dec 20, 2021
+112 −83

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Dec 16, 2021 •
edited
Loading

@tchapi @dunglas
Update mercure.rst regarding JWT token secret

Verified

This commit was signed with the committer’s verified signature.
dunglas Kévin Dunglas
GPG key ID: 4D04EBEF06AAF3A6
Verified
Learn about vigilant mode
f948ab2 
👋 

The documentation for the configuration of mercure seems quite wrong: it has a big "caution" block stating that the `MERCURE_JWT_SECRET` should contain an _actual JWT_ ... instead of a secret, which is weird (and definitely not how it works).

I propose to remove these (maybe out of date?) parts to prevent further confusion (_I spent a full day examining the actual source to understand what was really needed in the env var_).

Also, added a tip on how setting the cookies twice would not work.

PS: I created this PR against the 5.3 (current) branch since the 4.4 branch does not have the same paragraphs. Hope it's good.
@tchapi @dunglas
Remove unneeded JWT reference

Verified

This commit was signed with the committer’s verified signature.
dunglas Kévin Dunglas
GPG key ID: 4D04EBEF06AAF3A6
Verified
Learn about vigilant mode
123ad73
@carsonbot carsonbot added this to the 5.3 milestone Dec 16, 2021
@carsonbot carsonbot changed the title [mercure] Compatibility with the Docker integration and various improvements [Mercure] Compatibility with the Docker integration and various improvements Dec 16, 2021
@dunglas
[mercure] Compatibility with the Docker integration and various impro…

Verified

This commit was signed with the committer’s verified signature.
dunglas Kévin Dunglas
GPG key ID: 4D04EBEF06AAF3A6
Verified
Learn about vigilant mode
142a6f9 
…vements
azjezz
azjezz reviewed Dec 16, 2021
View reviewed changes
Copy link
Contributor

@azjezz azjezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the JWT section could be extended to mention the jwt.publish, jwt.publish, jwt.algorithm, and using custom JWT provider/factory.

chalasr
chalasr reviewed Dec 16, 2021
View reviewed changes
@dunglas
review

Verified

This commit was signed with the committer’s verified signature.
dunglas Kévin Dunglas
GPG key ID: 4D04EBEF06AAF3A6
Verified
Learn about vigilant mode
7496149
@dunglas
Copy link
Member Author

dunglas commented Dec 16, 2021

@azjezz

I think the JWT section could be extended to mention the jwt.publish, jwt.publish, jwt.algorithm, and using custom JWT provider/factory.

Indeed, done.

@dunglas @chalasr
Update mercure.rst

Verified

This commit was signed with the committer’s verified signature.
dunglas Kévin Dunglas
GPG key ID: 4D04EBEF06AAF3A6
Verified
Learn about vigilant mode
e35f1cc 
Co-authored-by: Robin Chalas <[email protected]>
azjezz
azjezz reviewed Dec 16, 2021
View reviewed changes
azjezz
azjezz reviewed Dec 16, 2021
View reviewed changes
azjezz
azjezz reviewed Dec 16, 2021
View reviewed changes
azjezz
azjezz reviewed Dec 16, 2021
View reviewed changes
azjezz
azjezz reviewed Dec 16, 2021
View reviewed changes
azjezz
azjezz reviewed Dec 16, 2021
View reviewed changes
azjezz
azjezz reviewed Dec 16, 2021
View reviewed changes
dunglas and others added 7 commits December 16, 2021 15:38
@dunglas @azjezz
Update mercure.rst

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
81c1387 
Co-authored-by: Saif Eddin Gmati <[email protected]>
@dunglas @azjezz
Update mercure.rst

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
3ca1aa1 
Co-authored-by: Saif Eddin Gmati <[email protected]>
@dunglas @azjezz
Update mercure.rst

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
b4fb9fb 
Co-authored-by: Saif Eddin Gmati <[email protected]>
@dunglas @azjezz
Update mercure.rst

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
ec87085 
Co-authored-by: Saif Eddin Gmati <[email protected]>
@dunglas @azjezz
Update mercure.rst

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
daaa3f1 
Co-authored-by: Saif Eddin Gmati <[email protected]>
@dunglas @azjezz
Update mercure.rst

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
adf49ef 
Co-authored-by: Saif Eddin Gmati <[email protected]>
@dunglas
nitpicking

Verified

This commit was signed with the committer’s verified signature.
dunglas Kévin Dunglas
GPG key ID: 4D04EBEF06AAF3A6
Verified
Learn about vigilant mode
ad0cbe5
azjezz
azjezz approved these changes Dec 16, 2021
View reviewed changes
Copy link
Contributor

@azjezz azjezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@javiereguiluz
Copy link
Member

Thanks a lot Kévin

@javiereguiluz javiereguiluz merged commit 764d67c into symfony:5.3 Dec 20, 2021
@javiereguiluz javiereguiluz mentioned this pull request Dec 27, 2021
Closed
@dunglas dunglas deleted the fix/mercure-docker branch December 27, 2021 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalasr chalasr

@azjezz azjezz

Assignees
No one assigned
Labels
Mercure Status: Reviewed
Projects
None yet
Milestone
5.3
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@dunglas @javiereguiluz @chalasr @azjezz @carsonbot @tchapi