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

Multiple entities #110

Open
seb-jean opened this issue May 9, 2020 · 22 comments · May be fixed by #318
Open

Multiple entities #110

seb-jean opened this issue May 9, 2020 · 22 comments · May be fixed by #318
Labels
enhancement New feature or request

Comments

@seb-jean
Copy link

seb-jean commented May 9, 2020

Hi,
Is it possible to use this bundle with multiple entities? A User entity and an Admin entity for example.
If not, it would be a good idea of feature :)
Thank you :)

@jrushlow jrushlow added the enhancement New feature or request label May 9, 2020
@jrushlow
Copy link
Collaborator

jrushlow commented May 9, 2020

Good morning,

The short answer is kind of - the bundle it self consumes a user at runtime, and then persists a request object that references that user. However, make:reset-password created a ResetPasswordRequest entity that has a ManyToOne relation to a User entity. So the long answer:

The bundle can handle multiple user entities, maker's make:reset-password can not. With a bit of modification to the generated maker files, this would be possible.

I'll add this to the board for possible future features - perhaps we could better support multiple user entity objects.

@seb-jean
Copy link
Author

seb-jean commented May 9, 2020

Thank you very much for your answer ! What could the config/packages/reset_password.yaml file look like as there are mutiple repositories? Thanks

@jrushlow
Copy link
Collaborator

jrushlow commented May 9, 2020

If I get a second tonight, I'll try coding it out to confirm. But off the top of my head, the reset_password.yaml file wouldn't change if you wanted to use multiple user entities. It just needs to know what repository to use for the ResetPasswordRequest entity. Which there should only be 1 of those.

What would change would be the Entity\ResetPasswordRequest::user Doctrine Annotation. make:reset-password does something like @ORM\ManyToOne(targetEntity="App\Entity\User") for that property.

If you have multiple entities, you couldn't use a ManyToOne relation. Knee jerk thinking would be the request entity would need a new property(ies) to track which user "type" (entity) and the id of the user. Then in the request repository, you would use the new properties to fetch and set Entity\ResetPasswordRequest::user with the correct user entity before returning the Entity\ResetPasswordRequest::class to the consumer.

@ziedelifa
Copy link

Hi,
if you want use multiple repository you can following these steps:
create a file in config/packages/reset_password.php

$repoclass='App\Repository\UserType1ResetPasswordRequestRepository';
if( put your logic here ){
$repoclass = 'App\Repository\UserType2ResetPasswordRequestRepository';
}
$container->loadFromExtension('symfonycasts_reset_password', [
'request_password_repository' => $repoclass,
]);

That's all !

@seb-jean
Copy link
Author

Hi @jrushlow
I understand your answer :) but could you see that in code please?
Thanks for reply :)

@seb-jean
Copy link
Author

If I get a second tonight, I'll try coding it out to confirm. But off the top of my head, the reset_password.yaml file wouldn't change if you wanted to use multiple user entities. It just needs to know what repository to use for the ResetPasswordRequest entity. Which there should only be 1 of those.

What would change would be the Entity\ResetPasswordRequest::user Doctrine Annotation. make:reset-password does something like @ORM\ManyToOne(targetEntity="App\Entity\User") for that property.

If you have multiple entities, you couldn't use a ManyToOne relation. Knee jerk thinking would be the request entity would need a new property(ies) to track which user "type" (entity) and the id of the user. Then in the request repository, you would use the new properties to fetch and set Entity\ResetPasswordRequest::user with the correct user entity before returning the Entity\ResetPasswordRequest::class to the consumer.

@jrushlow Could you make the change to make this bundle work with multiple entities please? Thanks :)

@cristobal85
Copy link

cristobal85 commented Jul 29, 2020

I had the same problem and my fastly (it's not the best) solution was these:

  • App\Entity\ResetPasswordRequest
class ResetPasswordRequest implements ResetPasswordRequestInterface
{
    use ResetPasswordRequestTrait;

    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\ManyToOne(targetEntity="App\Entity\Category1\User1")
     */
    private $user1;
    
    /**
     * @ORM\ManyToOne(targetEntity="App\Entity\Category2\User2")
     */
    private $user2;

    public function __construct(object $user, \DateTimeInterface $expiresAt, string $selector, string $hashedToken)
    {
        switch (get_class($user)){
            case User1::class:
                $this->user1 = $user;
                break;
            case User2::class:
                $this->user2 = $user;
                break;
        }
        $this->initialize($expiresAt, $selector, $hashedToken);
    }

    public function getUser(): object
    {
        if ($this->user1 != null) {
            return $this->user1;
        }
        if ($this->user2 != null) {
            return $this->user2;
        }
    }
}

  • App\Repository\ResetPasswordRequestRepository
class ResetPasswordRequestRepository extends ServiceEntityRepository implements ResetPasswordRequestRepositoryInterface {

    use ResetPasswordRequestRepositoryTrait;

    public function __construct(ManagerRegistry $registry) {
        parent::__construct($registry, ResetPasswordRequest::class);
    }

    public function createResetPasswordRequest(
            object $user,
            \DateTimeInterface $expiresAt,
            string $selector,
            string $hashedToken
    ): ResetPasswordRequestInterface {
        return new ResetPasswordRequest(
                $user,
                $expiresAt,
                $selector,
                $hashedToken
        );
    }
    
    public function getMostRecentNonExpiredRequestDate(object $user): ?\DateTimeInterface
    {
        // Normally there is only 1 max request per use, but written to be flexible
        /** @var ResetPasswordRequestInterface $resetPasswordRequest */
        $resetPasswordRequest = $this->createQueryBuilder('t')
            ->where('t.user1 = :user OR t.user2 = :user')
            ->setParameter('user', $user)
            ->orderBy('t.requestedAt', 'DESC')
            ->setMaxResults(1)
            ->getQuery()
            ->getOneorNullResult()
        ;

        if (null !== $resetPasswordRequest && !$resetPasswordRequest->isExpired()) {
            return $resetPasswordRequest->getRequestedAt();
        }

        return null;
    }
    
    public function removeResetPasswordRequest(ResetPasswordRequestInterface $resetPasswordRequest): void
    {
        $this->createQueryBuilder('t')
            ->delete()
            ->where('t.user1 = :user OR t.user2 = :user')
            ->setParameter('user', $resetPasswordRequest->getUser())
            ->getQuery()
            ->execute()
        ;
    }

}

@seb-jean
Copy link
Author

But as a result, you have 2 user fields. This is obviously not ideal.

@Clorr
Copy link

Clorr commented Sep 7, 2020

Answer of @cristobal85 is not ideal for sure, but it helped me do the job. Note that you also have to modify ResetPasswordController.php. In processSendingPasswordResetEmail() you have to duplicate the query for the user :

        $user = $this->getDoctrine()->getRepository(User1::class)->findOneBy([
            'email' => $emailFormData,
        ]);

        if (empty($user)) $user = $this->getDoctrine()->getRepository(User2::class)->findOneBy([
            'email' => $emailFormData,
        ]);

and in reset(), you can redirect according to the user type :

            if ($user instanceof User1) return $this->redirectToRoute('user1_home');
            if ($user instanceof User2) return $this->redirectToRoute('user2_home');

@cristobal85
Copy link

cristobal85 commented Sep 7, 2020

Answer of @cristobal85 is not ideal for sure, but it helped me do the job. Note that you also have to modify ResetPasswordController.php. In processSendingPasswordResetEmail() you have to duplicate the query for the user :

        $user = $this->getDoctrine()->getRepository(User1::class)->findOneBy([
            'email' => $emailFormData,
        ]);

        if (empty($user)) $user = $this->getDoctrine()->getRepository(User2::class)->findOneBy([
            'email' => $emailFormData,
        ]);

and in reset(), you can redirect according to the user type :

            if ($user instanceof User1) return $this->redirectToRoute('user1_home');
            if ($user instanceof User2) return $this->redirectToRoute('user2_home');

Sorry, I forgot to explain that I have two controllers, one for each entity "App\Entity\{Category1}\{User1}" and "App\Entity\{Category2}\{User2}" and the controllers are on "App\Controller\{Category1}\ResetPasswordController" and "App\Controller\{Category2}\ResetPasswordController"

@Clorr
Copy link

Clorr commented Sep 8, 2020

I think your solution is better than my ifs ;-) (Even if I think it's too much of a duplicated code...)

@cristobal85
Copy link

I think your solution is better than my ifs ;-) (Even if I think it's too much of a duplicated code...)

Yes, the truth is that it is not the best solution.

@Clorr
Copy link

Clorr commented Sep 8, 2020

Has anyone considered using inheritance mapping? It looks like it could solve the problem in most cases since password reset would apply on the parent instance. I can't try right now, but maybe it is a hint to a better resolution of this problem...

@ghost
Copy link

ghost commented Jan 8, 2021

Hello everyone. I also faced this problem and searching for the best approach. @jrushlow could you manage to work on an example of your approach?

I also thought about inheritance mapping as one approach but for me this is a bit too difficult. Maybe one the more experienced guys here can provide an example with multiple tragetEntities for $user property of ResetPasswordRequest?

Thanks in advance.

@SviatlanaViarbitskaya
Copy link

Hi,
if you want use multiple repository you can following these steps:
create a file in config/packages/reset_password.php

$repoclass='App\Repository\UserType1ResetPasswordRequestRepository';
if( put your logic here ){
$repoclass = 'App\Repository\UserType2ResetPasswordRequestRepository';
}
$container->loadFromExtension('symfonycasts_reset_password', [
'request_password_repository' => $repoclass,
]);

That's all !

Could you possibly suggest how to put my logic in here? I want to differential between two cases based on URL of the request. How to get here the request object?

@fd6130
Copy link

fd6130 commented Apr 12, 2021

I think the current best way is create your own controller to handle your request. You may reuse the token component from this bundle to achieve your goal there Or use signed url bundle https://github.com/coopTilleuls/UrlSignerBundle it is almost the same way to reset password except that the token is not persisted to database.

@savvasal
Copy link

savvasal commented Jul 2, 2021

It can be done, thanks to the Symfony DI !

For the default user in the reset_password.yaml

symfonycasts_reset_password:
    request_password_repository: App\Repository\UserRepository
    lifetime: 3600
    throttle_limit: 3600
    enable_garbage_collection: true

This will instantiate a helper with the alias symfonycasts.reset_password.helper

Then for the custom helper, in the services.yaml instantiate a new Helper directly using the existing services:

    admin.reset_password_helper:
        class: SymfonyCasts\Bundle\ResetPassword\ResetPasswordHelper
        arguments:
            $generator: '@symfonycasts.reset_password.token_generator'
            $cleaner: '@symfonycasts.reset_password.cleaner'
            $repository: '@App\Repository\AdminRepository'
            $resetRequestLifetime: 3600
            $requestThrottleTime: 3600

So if you have a UserService that is handling the reset password

namespace App;
class UserService
{
    public function __construct(
            private ResetPasswordHelperInterface $userResetPasswordHelper,
            private ResetPasswordHelperInterface $adminResetPasswordHelper
   ) {}

  public function resetAdminPassword()
  {
        // ...
  }
  
 public function resetUserPassword()
 {
     // ...
 } 
}

In the services.yaml

 App\UserService:
    arguments:
        $userResetPasswordHelper: '@symfonycasts.reset_password.helper'
        $adminResetPasswordHelper: '@admin.reset_password_helper'

@seb-jean
Copy link
Author

@savvasal It's still a hack but I would have preferred a feature in the heart of the bundle :)

@ytilotti
Copy link

ytilotti commented Dec 6, 2021

I do something like this:

#reset_password.yaml

symfonycasts_reset_password:
    request_password_repository: App\Repository\UsersResetPasswordRequestRepository
#services.yaml

client.reset_password.cleaner:
    class: SymfonyCasts\Bundle\ResetPassword\Util\ResetPasswordCleaner
    arguments:
        $repository: '@App\Repository\ClientsResetPasswordRequestRepository'

client.reset_password_helper:
    class: SymfonyCasts\Bundle\ResetPassword\ResetPasswordHelper
    arguments:
        $generator: '@symfonycasts.reset_password.token_generator'
        $cleaner: '@client.reset_password.cleaner'
        $repository: '@App\Repository\ClientsResetPasswordRequestRepository'
        $resetRequestLifetime: 3600
        $requestThrottleTime: 3600

App\Controller\ClientController:
    arguments:
        $resetPasswordHelper: '@client.reset_password_helper'

I have 2 differents entities Clients & Users.
Just warning to the Entity of ResetPasswordRequest, need to have exactly $user for the function getMostRecentNonExpiredRequestDate() and removeResetPasswordRequest() of the trait ResetPasswordRequestRepositoryTrait:

/**
 * @ORM\Entity(repositoryClass=ClientsResetPasswordRequestRepository::class)
 */
class ClientsResetPasswordRequest implements ResetPasswordRequestInterface
{
    use ResetPasswordRequestTrait;

    /**
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\ManyToOne(targetEntity=Clients::class)
     * @ORM\JoinColumn(name="client_id", nullable=false)
     */
    private $user;
}

@tiennguyennfq
Copy link

@savvasal it's awesome. Thank you so much

@seb-jean
Copy link
Author

Hi,
Should I close the issue even though it is not resolved?

@Hanmac
Copy link

Hanmac commented May 5, 2024

i recently got the Idea of using FirewallConfiguration symfony/symfony#54713 to get the UserProvider for the Current Firewall (or Chain)

then an extended UserProvider can be used to fetch the right ResetPasswordRequestRepository (or make one Repository work for Multiple Users, separated by their Firewall?)

i need try to make a demo Project to show my idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.