Skip to content

feat: use JWT bundle's command to create keys #1448

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 1 commit into from
Oct 20, 2021

Conversation

wiese
Copy link
Contributor

@wiese wiese commented Oct 20, 2021

Creation of directory and files is now covered by
lexik/jwt-authentication-bundle's command.

This somewhat causes the directory/file location somewhat pop up out of
nowhere in the subsequent permission change (setfacl). That could be
avoided by extracting JWT_SECRET_KEY and JWT_PUBLIC_KEY from api's .env
file (section maintained by the bundle) but it adds bloat without apparent
benefit.

The previously documented behaviour was closer to using the --overwrite
option on the command, but I doubt it is in the user's best interest in
a starter guide. Using --skip-if-exists would make some sense, but
bailing feels like the sane option – the user obviously touched this
area before and should be made aware.

Resolves #1446

@wiese
Copy link
Contributor Author

wiese commented Oct 20, 2021

If this makes sense maybe the demo app's docker-entrypoint could benefit from it, too (with the --skip-if-exists option).

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

LGTM. And good idea to do it in the demo too.

Creation of directory and files is now covered by
lexik/jwt-authentication-bundle's command.

This somewhat causes the directory/file location to pop up out of
nowhere in the subsequent permission change (`setfacl`). That could be
avoided by extracting JWT_SECRET_KEY and JWT_PUBLIC_KEY from api's
`.env` file (section maintained by the bundle) but it adds bloat without
apparent benefit.

The previously documented behaviour was closer to using the
`--overwrite` option on the command, but I doubt it is in the user's
best interest in a starter guide. Using `--skip-if-exists` would make
some sense, but bailing feels like the sane option – the user obviously
touched this area before and should be made aware.

Resolves api-platform#1446

Co-authored-by: Kévin Dunglas <kevin@dunglas.fr>
Copy link
Contributor

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

👍

@dunglas dunglas merged commit 4cd9a1a into api-platform:2.6 Oct 20, 2021
@dunglas
Copy link
Member

dunglas commented Oct 20, 2021

Thanks!

@wiese wiese deleted the patch-2 branch October 20, 2021 13:51
vincentchalamon pushed a commit to api-platform/demo that referenced this pull request Nov 3, 2021
The idea was proposed in api-platform/docs#1446
and added to the docs in api-platform/docs#1448.

The command was added to the bundle in v2.11.

Composer install was moved first to ensure that the bundle is installed.

This now attempts the permission modification (`setfacl`) irrespective of
the previous existence of the key files. While the cost of this operation
are certainly not zero, it ensures the permissions are as intended no
matter how the key files came to be and leaves the configuration of the
key file name entirely with the bundle/.env file.
billybrona pushed a commit to billybrona/symfony-demo that referenced this pull request Nov 26, 2023
The idea was proposed in api-platform/docs#1446
and added to the docs in api-platform/docs#1448.

The command was added to the bundle in v2.11.

Composer install was moved first to ensure that the bundle is installed.

This now attempts the permission modification (`setfacl`) irrespective of
the previous existence of the key files. While the cost of this operation
are certainly not zero, it ensures the permissions are as intended no
matter how the key files came to be and leaves the configuration of the
key file name entirely with the bundle/.env file.
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.

[Suggestion] JWT Authentication key pair generation via (lexik) bundle command
4 participants