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.

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

Merged
merged 12 commits into from
Dec 20, 2021

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Dec 16, 2021

👋 

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.
@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
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.

@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.

Co-authored-by: Robin Chalas <[email protected]>
dunglas and others added 7 commits December 16, 2021 15:38
Co-authored-by: Saif Eddin Gmati <[email protected]>
Co-authored-by: Saif Eddin Gmati <[email protected]>
Co-authored-by: Saif Eddin Gmati <[email protected]>
Co-authored-by: Saif Eddin Gmati <[email protected]>
Co-authored-by: Saif Eddin Gmati <[email protected]>
Co-authored-by: Saif Eddin Gmati <[email protected]>
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
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants