8000 Update mercure.rst regarding JWT token secret by tchapi · Pull Request #16151 · symfony/symfony-docs · GitHub
[go: up one dir, main page]

Skip to content

Update mercure.rst regarding JWT token secret #16151

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

Closed
wants to merge 2 commits into from
Closed

Conversation

tchapi
Copy link
Contributor
@tchapi tchapi commented Nov 24, 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.

👋 

The documentation for the configuration of mercure seems quite wrong: it has 
8000
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.
@javiereguiluz
Copy link
Member

Thanks for this contribution.

However, before merging we'll need some review from a Mercure expert (@dunglas maybe?) Thanks!

@javiereguiluz javiereguiluz added the help wanted Issues and PRs which are looking for volunteers to complete them. label Nov 24, 2021
@dunglas
Copy link
Member
dunglas commented Nov 24, 2021

Indeed this is outdated but it's still possible to use a "static" JWT as described in this section by setting the mercure.jwt.value key, so I suggest moving these (still useful for advanced cases) instructions in a new section explaining this.

javiereguiluz added a commit that referenced this pull request Dec 20, 2021
…various improvements (tchapi, dunglas)

This PR was merged into the 5.3 branch.

Discussion
----------

[Mercure] Compatibility with the Docker integration and various improvements

Includes #16151. Closes dunglas/symfony-docker#200.

Commits
-------

ad0cbe5 nitpicking
adf49ef Update mercure.rst
daaa3f1 Update mercure.rst
ec87085 Update mercure.rst
b4fb9fb Update mercure.rst
3ca1aa1 Update mercure.rst
81c1387 Update mercure.rst
e35f1cc Update mercure.rst
7496149 review
142a6f9 [mercure] Compatibility with the Docker integration and various improvements
123ad73 Remove unneeded JWT reference
f948ab2 Update mercure.rst regarding JWT token secret
@javiereguiluz
Copy link
Member

I'm closing this one in favor of #16293, which was merged recently and hopefully fixed this and other issues.

In any case, thanks a lot @tchapi for your proposal to fix this issue!

@dunglas
Copy link
Member
dunglas commented Dec 27, 2021

By the way, I included your commits in #16293 @tchapi, so you have been credited on GitHub.

@tchapi tchapi deleted the patch-1 branch January 1, 2022 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues and PRs which are looking for volunteers to complete them. Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0