Skip to content
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

Add a lexik:jwt:generate-token command #485

Merged
merged 1 commit into from
Jun 20, 2018

Conversation

sroze
Copy link
Contributor

@sroze sroze commented Apr 11, 2018

For debugging purposes (and more depending on how the tokens are used - i.e. as API keys for example) it's very useful to be able to generate tokens... therefore let's ship a command that does that 👍


$output->writeln([
'',
' <info>'.$token.'</info>',
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you put a space before the <info> tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it looks nice :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

adding extra chars would prevent redirecting the output (e.g. to clipboard), I would be very raw here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Meh, fair point.

protected function execute(InputInterface $input, OutputInterface $output)
{
$userClass = $input->getOption('user-class');
$token = $this->tokenManager->create(new $userClass($input->getArgument('username'), null));
Copy link
Contributor

Choose a reason for hiding this comment

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

my User entity does not accept any arguments in its constructor, so this will most likely fail with my specific user-class

FYI, when I make the same command in my projects, this is how I retrieve the token:

        $user = $this->userProvider->loadUserByUsername($input->getArgument('username'));
        $token = $this->tokenManager->create($user);

where userProvider is a UserProviderInterface which I map to my user provider service like this:

services:
    _defaults:
        autowire: true
    # ...
    Symfony\Component\Security\Core\User\UserProviderInterface: '@security.user.provider.concrete.entity_provider'

# note: "entity_provider" is the name of my provider in "security.yaml"

So, maybe this would be safer to have a user-provider-class option instead of user-class (or even configuring this to the lexik_jwt_authentication.yaml file)
, since we can check that the given class implements the UserProviderInterface

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that the current solution is not ideal, but I'm not fond of user-provider-class either as it implies to have a service named accordingly, whereas you could have several providers based on the same class (e.g. an entity provider for two different entities).

What about giving all configured user providers to the command (as a service locator, indexed by their short name) and change the option name to user-provider?
In case no --user-provider is passed, if there is only one configured provider then use it, fail otherwise (saying that the option must be passed since multiple providers exist).
Providers can be retrieved from the context listener definition, see https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php#L180.
I can help if you like it

@chalasr
Copy link
Collaborator

chalasr commented Apr 24, 2018

Definitely 👍 for adding this.

@sroze
Copy link
Contributor Author

sroze commented May 1, 2018

I've updated the PR to use user providers. Though, I can't see how we can easily get their short names so I've kept the service ID as name. I think that's OK as we have a default one used when only one provider is registered.

@sroze sroze force-pushed the add-generate-jwt-command branch 3 times, most recently from 26261c1 to 9bfd605 Compare May 1, 2018 12:42
Copy link
Contributor

@Oliboy50 Oliboy50 left a comment

Choose a reason for hiding this comment

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

thanks, seems nice 👍

@@ -9,6 +9,17 @@
<argument type="service" id="lexik_jwt_authentication.key_loader" />
<tag name="console.command" command="lexik:jwt:check-config" />
</service>

<service id="lexik_jwt_authentication.user_provider_locator">
Copy link
Contributor

Choose a reason for hiding this comment

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

should be explicitly public="false" to not lose DI performance in older symfony versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can yep. Changed 👍

@chalasr chalasr force-pushed the add-generate-jwt-command branch 4 times, most recently from 711ca4d to fb5da72 Compare June 18, 2018 13:26
protected function execute(InputInterface $input, OutputInterface $output)
{
if ($this->userProviders instanceof \Countable && 0 === \count($this->userProviders)) {
throw new \RuntimeException('You must have at least 1 configured user provider for generating a token.');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You must have at least one configured user provider to generate a token
?

@chalasr
Copy link
Collaborator

chalasr commented Jun 20, 2018

Thank you @sroze.

@chalasr chalasr merged commit c687029 into lexik:master Jun 20, 2018
chalasr added a commit that referenced this pull request Jun 20, 2018
This PR was merged into the 2.x-dev branch.

Discussion
----------

Add a `lexik:jwt:generate-token` command

For debugging purposes (and more depending on how the tokens are used - i.e. as API keys for example) it's very useful to be able to generate tokens... therefore let's ship a command that does that 👍

Commits
-------

c687029 Add a `lexik:jwt:generate-token` command
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.

3 participants