SymfonyCasts / reset-password-bundle

Need a killer reset password feature for your Symfony? Us too!
https://symfonycasts.com
MIT License
473 stars 67 forks source link

Multiple entities #110

Open seb-jean opened 4 years ago

seb-jean commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

seb-jean commented 4 years ago

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 commented 4 years ago

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

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;
        }
    }
}

}

seb-jean commented 4 years ago

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

Clorr commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

cristobal85 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

ytilotti commented 2 years ago

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 commented 1 year ago

@savvasal it's awesome. Thank you so much

seb-jean commented 6 months ago

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

Hanmac commented 5 months ago

i recently got the Idea of using FirewallConfiguration https://github.com/symfony/symfony/issues/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