-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Add security:oidc-token:generate command #60660
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
base: 7.4
Are you sure you want to change the base?
Conversation
4fba1cd
to
d5b0868
Compare
Good call 👍I would make "token" or "jwt" part of the command name though as I find "oidc:generate" rather confusing (yup, no better suggestion yet 🙃). |
Sure! Not more inspired than you, I renamed it to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a nice idea!
No remarks for this PR and OK with the name oidc-token:generate
.
Maybe |
@@ -79,6 +80,24 @@ public function create(ContainerBuilder $container, string $id, array|string $co | |||
] | |||
); | |||
} | |||
|
|||
// Generate command | |||
if (!$container->hasDefinition('security.access_token_handler.oidc.command.generate')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to check whether the command exists, as SecurityBundle 7.4 could be installed with security-http version 7.3
$this | ||
->addArgument('user-identifier', InputArgument::REQUIRED, 'User identifier') | ||
->addOption('firewall', null, InputOption::VALUE_REQUIRED, 'Firewall') | ||
->addOption('algorithm', null, InputOption::VALUE_REQUIRED, 'Algorithm name to use to sign') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should provide completion for the list of supported algorithms
{ | ||
$this | ||
->addArgument('user-identifier', InputArgument::REQUIRED, 'User identifier') | ||
->addOption('firewall', null, InputOption::VALUE_REQUIRED, 'Firewall') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should provide completion for the list of firewall names
) { | ||
} | ||
|
||
public function generate(string $userIdentifier, ?string $algorithmAlias = null, ?string $issuer = null, ?int $ttl = null, ?int $notBefore = null): string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
making not-before an int is not good UX. Humans will generally not provide a unix timestamp as the value of the option.
That class should take ?\DateTimeImmutable
for this argument, and the command should convert the option when provided, allowing to use date strings (like the ISO format for instance, but probably supporting anything supported by the DateTimeImmutable constructor makes sense)
I like the |
The
OidcTokenHandler
, introduced in Symfony 6.3, allows to decode a JWT token, validate it and retrieve the user info from it.This pull request introduces a new Symfony command,
bin/console security:oidc-token:generate
, designed to facilitate the generation of JWTs. It can be useful for generating a token for testing purpose.Argument and options
Usage
Tip
When there is only one value, both "firewall", "algorithm" and "issuer" are not required..