FriendsOfSymfony / FOSUserBundle

Provides user management for your Symfony project. Compatible with Doctrine ORM & ODM, and custom storages.
https://symfony.com/doc/master/bundles/FOSUserBundle/index.html
MIT License
3.24k stars 1.57k forks source link

Disabled user gets access by resetting password #2298

Open markussc opened 8 years ago

markussc commented 8 years ago

For a disabled user (enabled = false, locked = false) it is possible to retrieve the password reset link which allows him to easily access the system. After password is reset, the enabled flag is set to true. Is this expected behaviour?

SimonHeimberg commented 8 years ago

maybe related: #1869

XWB commented 7 years ago

It's not related to #1869, we explicitly set enabled to true in https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/EventListener/ResettingListener.php#L77

ethernal commented 7 years ago

This is still a ug as it allows user to enable the account that should not be enabled. Maybe adding a distinction between Enabled and (B)Locked account would be better?

stof commented 7 years ago

well, if you want to lock accounts in your app, using isAccountNotLocked is the way to go (there used to be a locked field previously to handle this, but it is removed in 2.0-beta1 as we were not providing any management for it, and so it is better for apps needing it to add it in the way fitting their needs rather than forcing all apps to have this field)

ethernal commented 7 years ago

OK that makes sense and it's better to leave that to the user.

PS. any idea when 2.0 stable will be released? There is nothing on the 2.0 roadmap to do. Are we all just waiting few weeks and assume it's all ok or are there open issues?

stof commented 7 years ago

the only thing left is updating translations as many locales are not in sync with the English messages due to changes done in 2.x (removing some placeholder in some places). @XWB told me he would create an issue about it to follow the progress, but it is not done yet. I may do it myself if I find time for it before him. And translations will not be a blocker for the release (I will comment outdated translations so that they get the English message rather than a broken one though), but the more locales are complete, the better.

I had another thing on my list before the release: updating my own project to beta1 to check it in real world before releasing it, but this is done now.

vlitovka commented 7 years ago

Well, I can review whole Ukrainian translation in this case(and probably russian if needed).

Seb33300 commented 6 years ago

I think $enabled field should be renamed because it is very confusing name and everybody (I did the same mistake) seems to think we can rely on this field to enable / disable users in our application.

julbrs commented 5 years ago

Hello,

I am running an app with FOSUserBundle 2.x and fall exactly into this issue, I was using the $enabled field as an administrative ban tool.

I found an easy workaround in my case by updating my User entity:

    /**
     * {@inheritdoc}
     */
    public function isAccountNonLocked()
    {
        return $this->enabled;
    }

This simple trick will use the $enabled field and prevent using the reset password function if user is explicitly set to $enabled = false.

Give a try :)