Skip to content

[make:user] user entities implement PasswordAuthenticatedUserInterface #849

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.

Sign up for GitHub

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
Mar 30, 2021
Merged

[make:user] user entities implement PasswordAuthenticatedUserInterface #849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/Doctrine/EntityClassGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
use Symfony\Bundle\MakerBundle\Generator;
use Symfony\Bundle\MakerBundle\Str;
use Symfony\Bundle\MakerBundle\Util\ClassNameDetails;
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
* @internal
Expand Down Expand Up @@ -69,6 +71,15 @@ public function generateRepositoryClass(string $repositoryClass, string $entityC
{
$shortEntityClass = Str::getShortClassName($entityClass);
$entityAlias = strtolower($shortEntityClass[0]);

$passwordUserInterfaceName = UserInterface::class;

if (interface_exists(PasswordAuthenticatedUserInterface::class)) {
$passwordUserInterfaceName = PasswordAuthenticatedUserInterface::class;
}

$interfaceClassNameDetails = new ClassNameDetails($passwordUserInterfaceName, 'Symfony\Component\Security\Core\User');
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need ClassNameDetails here? Can't we just use the normal class strings?

You might be doing this due to the logic in the template to figure out which location the use statement should be. I'd prefer passing in a second flag to know that - e.g. is_using_password_authenticated_interface. That should simplify the template

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need the full class name for the use statement and the shortName for the type hint in upgradePassword().

We currently already have a $with_password_upgrade var available in the templates - we could add a $is_using_password_authenticated_interface var as well, but i think that would make things even messier in the template when we need to determine the sort order of the use statements. Ultimately what we really need is a use statement sorter to solve the messy part - but I'm open to ideas.

Also to consider, #824 introduces the concept of a TemplateClassDetails object the returns formatted strings for the particular use case. e.g.

$userDetails = new TemplateClassDetails(User::class, true);    // second arg === $isTyped

$userDetails->getUseStatement();
// use App\Entity\User;

$userDetails->getConstructorArg();
// User $user

$userDetails->getMethodArg();  // similar to the above but spacing is slightly different
// User $user

$userDetails->getVariable();
// $user

$userDetails->getProperty();
// $this->user

// All of the above add appropriate spacing, casing, trailing commas, new lines, etc..

While 824 is not ready yet, ideally if we merge that in with, a lot of stuff in this PR will be refactored to leverage the new helper and cleanup the templates.

Copy link
Member

Choose a reason for hiding this comment

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

Ok then!

Ultimately what we really need is a use statement sorter to solve the messy part

👍


$this->generator->generateClass(
$repositoryClass,
'doctrine/Repository.tpl.php',
Expand All @@ -77,6 +88,7 @@ public function generateRepositoryClass(string $repositoryClass, string $entityC
'entity_class_name' => $shortEntityClass,
'entity_alias' => $entityAlias,
'with_password_upgrade' => $withPasswordUpgrade,
'password_upgrade_user_interface' => $interfaceClassNameDetails,
'doctrine_registry_class' => $this->managerRegistryClassName,
'include_example_comments' => $includeExampleComments,
]
Expand Down
6 changes: 2 additions & 4 deletions src/Maker/MakeUser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,8 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
true
);
$manipulator->setIo($io);
$this->userClassBuilder->addUserInterfaceImplementation(
$manipulator,
$userClassConfiguration
);

$this->userClassBuilder->addUserInterfaceImplementation($manipulator, $userClassConfiguration);

$generator->dumpFile($classPath, $manipulator->getSourceCode());

Expand Down
5 changes: 3 additions & 2 deletions src/Resources/skeleton/doctrine/Repository.tpl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use <?= $doctrine_registry_class; ?>;
<?= $with_password_upgrade ? "use Symfony\Component\Security\Core\Exception\UnsupportedUserException;\n" : '' ?>
<?= ($with_password_upgrade && str_contains($password_upgrade_user_interface->getFullName(), 'Password')) ? sprintf("use %s;\n", $password_upgrade_user_interface->getFullName()) : null ?>
<?= $with_password_upgrade ? "use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;\n" : '' ?>
<?= $with_password_upgrade ? "use Symfony\Component\Security\Core\User\UserInterface;\n" : '' ?>
<?= ($with_password_upgrade && str_contains($password_upgrade_user_interface->getFullName(), '\UserInterface')) ? sprintf("use %s;\n", $password_upgrade_user_interface->getFullName()) : null ?>

/**
* @method <?= $entity_class_name; ?>|null find($id, $lockMode = null, $lockVersion = null)
Expand All @@ -28,7 +29,7 @@ public function __construct(ManagerRegistry $registry)
/**
* Used to upgrade (rehash) the user's password automatically over time.
*/
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
public function upgradePassword(<?= sprintf('%s ', $password_upgrade_user_interface->getShortName()); ?>$user, string $newEncodedPassword): void
{
if (!$user instanceof <?= $entity_class_name ?>) {
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
Expand Down
51 changes: 41 additions & 10 deletions src/Security/UserClassBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

use PhpParser\Node;
use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
Expand All @@ -22,21 +24,49 @@
*/
final class UserClassBuilder
{
public function addUserInterfaceImplementation(ClassSourceManipulator $manipulator, UserClassConfiguration $userClassConfig)
public function addUserInterfaceImplementation(ClassSourceManipulator $manipulator, UserClassConfiguration $userClassConfig): void
{
$manipulator->addInterface(UserInterface::class);

$this->addGetUsername($manipulator, $userClassConfig);

$this->addGetRoles($manipulator, $userClassConfig);

$this->addGetPassword($manipulator, $userClassConfig);

$this->addGetSalt($manipulator, $userClassConfig);
$this->addPasswordImplementation($manipulator, $userClassConfig);

$this->addEraseCredentials($manipulator, $userClassConfig);
}

private function addPasswordImplementation(ClassSourceManipulator $manipulator, UserClassConfiguration $userClassConfig): void
{
if (60000 > Kernel::VERSION_ID) {
// Add methods required to fulfill the UserInterface contract
$this->addGetPassword($manipulator, $userClassConfig);
$this->addGetSalt($manipulator, $userClassConfig);

// Symfony >=5.3 uses "@see PasswordAuthenticatedInterface" for getPassword()
if (interface_exists(PasswordAuthenticatedUserInterface::class)) {
$manipulator->addUseStatementIfNecessary(PasswordAuthenticatedUserInterface::class);
}

// Future proof the entity for >= Symfony 6 && the entity will check passwords
if ($userClassConfig->hasPassword() && interface_exists(PasswordAuthenticatedUserInterface::class)) {
$manipulator->addInterface(PasswordAuthenticatedUserInterface::class);
}

return;
}

// Future proof >= Symfony 6.0
if (!$userClassConfig->hasPassword()) {
return;
}

$manipulator->addInterface(PasswordAuthenticatedUserInterface::class);

$this->addGetPassword($manipulator, $userClassConfig);
}

private function addGetUsername(ClassSourceManipulator $manipulator, UserClassConfiguration $userClassConfig)
{
if ($userClassConfig->isEntity()) {
Expand Down Expand Up @@ -161,16 +191,18 @@ private function addGetRoles(ClassSourceManipulator $manipulator, UserClassConfi

private function addGetPassword(ClassSourceManipulator $manipulator, UserClassConfiguration $userClassConfig)
{
$seeInterface = interface_exists(PasswordAuthenticatedUserInterface::class) ? '@see PasswordAuthenticatedUserInterface' : '@see UserInterface';

if (!$userClassConfig->hasPassword()) {
// add an empty method only
$builder = $manipulator->createMethodBuilder(
'getPassword',
'string',
true,
[
'This method is not needed for apps that do not check user passwords.',
'This method can be removed in Symfony 6.0 - is not needed for apps that do not check user passwords.',
'',
'@see UserInterface',
$seeInterface,
]
);

Expand Down Expand Up @@ -221,9 +253,8 @@ private function addGetPassword(ClassSourceManipulator $manipulator, UserClassCo
'string',
false,
[
'@see UserInterface',
],
true
$seeInterface,
]
);
}

Expand All @@ -236,7 +267,7 @@ private function addGetSalt(ClassSourceManipulator $manipulator, UserClassConfig
];
} else {
$methodDescription = [
'This method is not needed for apps that do not check user passwords.',
'This method can be removed in Symfony 6.0 - is not needed for apps that do not check user passwords.',
];
}

Expand Down
16 changes: 15 additions & 1 deletion tests/Maker/MakeUserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function getTestDetails()
'y', // entity
'email', // identity property
'y', // with password
'y', // argon
'y', // argon @TODO This should only be done in <5.0
])
->addExtraDependencies('doctrine')
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeUserEntityPassword')
Expand All @@ -37,6 +37,20 @@ public function getTestDetails()
->updateSchemaAfterCommand(),
];

yield 'user_security_entity_with_password_authenticated_user_interface' => [MakerTestDetails::createTest(
$this->getMakerInstance(MakeUser::class),
[
// user class name
'User',
'y', // entity
'email', // identity property
'y', // with password
])
->addRequiredPackageVersion('symfony/security-bundle', '>=5.3')
->addExtraDependencies('doctrine')
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeUserEntityPasswordAuthenticatedUserInterface'),
];

yield 'user_security_model_no_password' => [MakerTestDetails::createTest(
$this->getMakerInstance(MakeUser::class),
[
Expand Down
11 changes: 10 additions & 1 deletion tests/Security/UserClassBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Bundle\MakerBundle\Security\UserClassBuilder;
use Symfony\Bundle\MakerBundle\Security\UserClassConfiguration;
use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;

class UserClassBuilderTest extends TestCase
{
Expand All @@ -33,7 +34,15 @@ public function testAddUserInterfaceImplementation(UserClassConfiguration $userC
$classBuilder = new UserClassBuilder();
$classBuilder->addUserInterfaceImplementation($manipulator, $userClassConfig);

$expectedPath = __DIR__.'/fixtures/expected/'.$expectedFilename;
$expectedPath = __DIR__.'/fixtures/expected';

// Can be removed when < Symfony 6 support is dropped.
if (!interface_exists(PasswordAuthenticatedUserInterface::class)) {
$expectedPath = sprintf('%s/legacy', $expectedPath);
}

$expectedPath = sprintf('%s/%s', $expectedPath, $expectedFilename);

if (!file_exists($expectedPath)) {
throw new \Exception(sprintf('Expected file missing: "%s"', $expectedPath));
}
Expand Down
7 changes: 4 additions & 3 deletions tests/Security/fixtures/expected/UserEntityEmailWithPassword.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
* @ORM\Entity()
*/
class User implements UserInterface
class User implements UserInterface, PasswordAuthenticatedUserInterface
{
/**
* @ORM\Id
Expand Down Expand Up @@ -80,11 +81,11 @@ public function setRoles(array $roles): self
}

/**
* @see UserInterface
* @see PasswordAuthenticatedUserInterface
*/
public function getPassword(): string
{
return (string) $this->password;
return $this->password;
}

public function setPassword(string $password): self
Expand Down
7 changes: 4 additions & 3 deletions tests/Security/fixtures/expected/UserEntityUser_nameWithPassword.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
* @ORM\Entity()
*/
class User implements UserInterface
class User implements UserInterface, PasswordAuthenticatedUserInterface
{
/**
* @ORM\Id
Expand Down Expand Up @@ -75,11 +76,11 @@ public function setRoles(array $roles): self
}

/**
* @see UserInterface
* @see PasswordAuthenticatedUserInterface
*/
public function getPassword(): string
{
return (string) $this->password;
return $this->password;
}

public function setPassword(string $password): self
Expand Down
7 changes: 4 additions & 3 deletions tests/Security/fixtures/expected/UserEntityUsernameNoPassword.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
Expand Down Expand Up @@ -69,17 +70,17 @@ public function setRoles(array $roles): self
}

/**
* This method is not needed for apps that do not check user passwords.
* This method can be removed in Symfony 6.0 - is not needed for apps that do not check user passwords.
*
* @see UserInterface
* @see PasswordAuthenticatedUserInterface
*/
public function getPassword(): ?string
{
return null;
}

/**
* This method is not needed for apps that do not check user passwords.
* This method can be removed in Symfony 6.0 - is not needed for apps that do not check user passwords.
*
* @see UserInterface
*/
Expand Down
7 changes: 4 additions & 3 deletions tests/Security/fixtures/expected/UserEntityUsernameWithPassword.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
* @ORM\Entity()
*/
class User implements UserInterface
class User implements UserInterface, PasswordAuthenticatedUserInterface
{
/**
* @ORM\Id
Expand Down Expand Up @@ -75,11 +76,11 @@ public function setRoles(array $roles): self
}

/**
* @see UserInterface
* @see PasswordAuthenticatedUserInterface
*/
public function getPassword(): string
{
return (string) $this->password;
return $this->password;
}

public function setPassword(string $password): self
Expand Down
7 changes: 4 additions & 3 deletions tests/Security/fixtures/expected/UserModelEmailWithPassword.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

namespace App\Security;

use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

class User implements UserInterface
class User implements UserInterface, PasswordAuthenticatedUserInterface
{
private $email;

Expand Down Expand Up @@ -57,11 +58,11 @@ public function setRoles(array $roles): self
}

/**
* @see UserInterface
* @see PasswordAuthenticatedUserInterface
*/
public function getPassword(): string
{
return (string) $this->password;
return $this->password;
}

public function setPassword(string $password): self
Expand Down
7 changes: 4 additions & 3 deletions tests/Security/fixtures/expected/UserModelUsernameNoPassword.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace App\Security;

use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

class User implements UserInterface
Expand Down Expand Up @@ -47,17 +48,17 @@ public function setRoles(array $roles): self
}

/**
* This method is not needed for apps that do not check user passwords.
* This method can be removed in Symfony 6.0 - is not needed for apps that do not check user passwords.
*
* @see UserInterface
* @see PasswordAuthenticatedUserInterface
*/
public function getPassword(): ?string
{
return null;
}

/**
* This method is not needed for apps that do not check user passwords.
* This method can be removed in Symfony 6.0 - is not needed for apps that do not check user passwords.
*
* @see UserInterface
*/
Expand Down
Loading