Validation Groups: Conditional Validation

My workaround or fix was to grab the original password ( $origpw = $user->getPassword(); ) then check if form data's 'plainPassword' was empty, and if so, set form->setPassword($origpw); Hopefully, that is the right way to do it!

Hey John,

Yes, the right way to check the $plainPassword: if $plainPassword is empty - then user does not change his pass, so skip it. But if the $plainPassword is set - encode it and store in $password field. BTW, it's a bit incorrect behavior for registration form, because user have to fill $password during registration. Check out the Symfony Form Validation Groups, it should helps in this case, i.e. require password on registration page and make it optional on edit page.

Cheers!

Hey Terry Caliendo!

That's an interesting feature you want to implement. Can the validation fail for any other reasons, like invalid email ?
if it's only for the password, then you can do something like this:

if($form->submitted()){
    if($form->isValid(){
         // on valid submit logic
    }
    else {
        // invalid logic
        // logout user
    } 
}

The easiest way I can think of how to logout an user is by redirecting them to your logout path, and you may want to set a flash message with the reason of why he is now logged out (if you want to do it manually, I'm not sure how, but this might help you http://stackoverflow.com/a/6474975 )

Now, if you don't want to be logged out inmediately, you will need some sort of a flag that informs the system he must be logout in his next request (maybe in the session), the easiest but not cleanest way would be a listener to "kernel.request" (http://symfony.com/doc/current/reference/events.html#kernel-request)

Have a nice day!

Hi MolloKhan
Thanks for your response. Unfortunately looking back I think I was pretty unclear as to what I am fully looking to accomplish. Or maybe I'm not understanding your response.

Here's what's happening...
I'm working on the Edit profile page. So the user is logged in and editing their profile. They can update their name, address, password, etc. If the user leaves the password blank, its not updated (as indicated in the video by using validation groups and I just now went back and added the @assert to my code above). However, if the user enters a password, it needs to be checked to verify that it at least has some minimum security (so the user doesn't use say a 1-letter password).

I am validating other fields and if one of those fields fails validation the form is returned with those error messages as well.

I DON'T want the user to be logged out if the user puts in a password that doesn't pass the regular expression, but this is what is happening.

Say the user tries to change his/her password to "dog" which is too short. The user submits the password and gets the error page back that says "Password must be at least 5 chars in length with 1 letter and 1 number". So that's good. But then when the user goes to submit a retry of say "dog55", which meets the criteria, the user is redirected to the login page because of this logout issue I'm having.

Is that more clear? (I'm sorry if I didn't understand your answer if that's the question you were answering).

Thanks again!
Terry

Ohhh I got you know, I totally misunderstood you.

Updating any other field than password works fine ?
Also, try returning a RedirectResponse Object when submit is valid, I think the problem is that symfony need's to reload the user token.

I hope it helps you :)

Thanks again for the response. Yes, updating other fields that don't have to do with authentication do not cause this subsequent logout issue (address, phone, etc) regardless if they are valid or invalid.

Unfortunately returning a RedirectResponse isn't helpful because it never calls the controller. The user is redirected to the login page before the controller for the re-submission is even called (to prove it, I put a dump/die statement as the first line to prove its not called).

I think I've found the issue, but don't know the remediation...

Here is my controller:

    /**
     * @Route("/profile", name="admin_profile")
     */
    public function profileAction(Request $request){
        
        $form = $this->createForm(UserProfile::class, $this->getUser());

        // only handles data on POST request
        $form->handleRequest($request);

        if($form->isValid())
        {
            $user = $form->getData();
            $em = $this->getDoctrine()->getManager();
            $em->persist($user);
            $em->flush();

            $this->addFlash('BodySuccessMessage', "User Profile Successfully Updated");

            return $this->redirectToRoute('admin_profile');
        }
        return $this->render('admin/user.profile.html.twig', array(
            'ProfileForm' => $form->createView(),
        ));
    }

I did some testing and it appears as though any change to the current User object like the following causes the same issue I'm experiencing:
<br />$user = $this->getUser();<br />$user->setPlainPassword('password'); // updated but not persisted<br />

So making a change to the current $user object must automatically update the security even though its not persisted. So does guard have a listener on updates to the User object? I figured the listener would be on the persistence to the database.

In my controller code above, I have the $form and subsequently its method handleRequest operating on the current User object.
<br />$form = $this->createForm(UserProfile::class, $this->getUser());<br /> // only handles data on POST request<br /> $form->handleRequest($request);<br />

Since handleRequest therefor updates the current User object to the incorrect values (as it would any other field) to be sent back to the displayed form with the processed errors, the security must be getting updated when the current User object is updated (per my testing). That would explain why the user is subsequently logged out. But I don't have a deep enough understanding of Symfony to stop that from happening.

Maybe I shouldn't pass $this->getUser() to the create form:
<br />$form = $this->createForm(UserProfile::class, $this->getUser());<br />

But instead I should then pass a new User object:
<br />$form = $this->createForm(UserProfile::class, new User());<br />

But then how do I persist that new User object in place of the old User object? It seems ridiculous to have to copy each of the values when the form isValid() returns true:

        if($form->isValid())
        {
            $NewUser = $form->getData();
            $User->setUsername($NewUser->getUsername);
            $User->setAddress($NewUser->getAddress);
            // ... All the other values inbetween
           $User->setPassword($NewUser->getPassword);


            $em = $this->getDoctrine()->getManager();
            $em->persist($user);
            $em->flush();

            $this->addFlash('BodySuccessMessage', "User Profile Successfully Updated");

            return $this->redirectToRoute('admin_profile');
        }

Hey Terry Caliendo!

That's why FOSUserBundle shines, it gives you all that functionality for free ;)
But let's figure it out.

I think you need to split your Form, one for only updating the password and another one for all the other fields, when you process the PasswordForm (and is valid), you will have to do two things:
- Encode and update the new password
- Set to null "plainPassword" field, and persist User object (3 things)

Then return a RedirectResponse, I hope it would do the trick
Maybe you should give a look into FOSUserBundle guts, it might help you a bit

Cheers!

Thanks again for the response Diego.

I just figured out the issue! There isn't a listener on the User object as I suspected, but the user object gets serialized into the session variables and (I think) verified against the user object upon the next session.

So I started dumping the session variables at the beginning and end of my controller. They were the same. It took me a while to realize that there isn't a listener on the User object but that the session variable for the user gets updated after my controller ends, which is why I don't see the change.

Anyway, the fix was to save the original password and put it back into the User object so that when it gets serialized, it doesn't cause issues with Guard. I was also having the same logout issue on username change validation, so its "reset" back to the original as well.

Here is my updated code:

    /**
     * @Route("/profile", name="admin_profile")
     */
    public function profileAction(Request $request){

        $originalPass = $this->getUser()->getPassword();
        $originalUser = $this->getUser()->getUsername();

        $form = $this->createForm(UserProfile::class, $this->getUser());


        // only handles data on POST request
        $form->handleRequest($request);

        if($form->isValid())
        {
            $user = $form->getData();
            $em = $this->getDoctrine()->getManager();
            $em->persist($user);
            $em->flush();

            $this->addFlash('BodySuccessMessage', "User Profile Successfully Updated");

            return $this->redirectToRoute('admin_profile');
        }

        // Failed Validation
        //   - put the username and password back
        //   - if user is not trying to change either; this effectively does nothing, which is fine.
        $this->getUser()->setUsername($originalUser);
        $this->getUser()->setPlainPassword(null);  // tested to not be needed; I do not know why. It must not be checked upon deserialize
        $this->getUser()->setPassword($originalPass);

        return $this->render('admin/user.profile.html.twig', array(
            'ProfileForm' => $form->createView(),
        ));
    }

I'm glad you managed to fix your problem, that's great!

FYI, we have a dedicated tutorial for Guard, it might help you to understand in a deeper way how Guard works https://knpuniversity.com/s... It's not finished yet, but hey, it's free! :)

Cheers!

Thanks. I've seen it but figured it was old because you have to install Guard. Isn't Guard part of Symfony by default now?

Yo Terry Caliendo!

Yep, Guard is now part of Symfony (woohoo!). So you're right, that tutorial needs an update (it's super on my list!). A better tutorial to point to is.. well... this one - or the new FOSUserBundle tutorial (https://knpuniversity.com/screencast/fosuserbundle).

Anyways, the issue you hit is a weird spot in Symfony's security, and you understand what's going on well. At the beginning of each request, the User object is deserialized from the session and then "refreshed" (i.e. we re-query the database by using the id of the serializer User). Then, yep, they are compared to see if they are "equal". This reason this is done is for security - e.g. if I change my password on one machine, I would be logged out on a different machine (important if you imagine your account gets hacked). But, it can cause some quirky behavior.

Ultimately, if you're having issues, the "official" solution is to add an extra interface to your User class: EquatableInterface (https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Core/User/EquatableInterface.php). With this, Symfony will now call the isEqualTo method at the beginning of the request to see if the user has "changed" (instead of trying its own logic). Btw, you can see all of this in action here: https://github.com/symfony/symfony/blob/1bbc35a34a5ecdf5e96d780e709925983c0333fb/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php#L244-L287

Phew! Well, I hope that brightens what's going on even a bit more. Nice job digging in - I'm impressed how well you were able to see what was going on in the core :).

Cheers!

Thanks for popping in weaverryan and for the extra notes on the "official" solution! And the references/links are super helpful to understand what's going on.

I can't wait to get the chance to dig into the new FOSUserBundle tutorial!

And thanks for the compliment :)
tc

Hey Alexis!

Weeeeelcome!!! Posting questions here is just fine - we typically recommend to find the "closest" page on the site about your topic :).

So... yes! Really, I think you have 2 options:

1) The Callback constraint. The advantage is that it's very simple, and because the method lives right inside the entity class itself, you have access to all the data you need. You can also assign the errors to whatever fields you want. Your logic seems *just* simple enough, that I think this is a good option. If it were any more complex, you would want option (2) for sure.

2) A custom constraint (as you guessed) is also a good option. How do you get all the values in the validate() method? The key is to set your validation annotation on the CLASS, not on an individual property. When you add your validation annotation above a property, then yep, you're only passed the value for that one field. But if you put it on the class, you are passed the entire object, and then you have all the data you need.

I hope this helps! And welcome! We love excited people and great questions ;).

Cheers!

Hello again,
I reply to myself, I think a got a viable solution. I discovered it's possible to add a constraint on a public function starting with : is, has or get. So I created a public function inside my class that does all the test and return a true/false. I used the assert "isTrue".

The drawback is that I dont get an explicit error about what is wrong.

I'm open to any new, better, ideas :)

Here an example :

class MyEntity {

    // ... my properties 

    /**
     * @Assert\IsTrue(message = "SSL properties are not valid")
     */
    public function isSslStuffValid()
    {
        // First case, no SSL at all
        if(!$this->isSslAvailable){
            return true;
        }
        // Second case, SSL is available so we expect at least a CRT and a KEY
        if(!file_exists($this->sslKeyFile) || !file_exists($this->sslCrtFile)){
            return false;
        }
        $key = file_get_contents($this->sslKeyFile);
        $crt = file_get_contents($this->sslCrtFile);

        if (!openssl_x509_parse($crt)) {
            return false;
        }
        if (!openssl_x509_check_private_key($crt, $key)) {
            return false;
        }

        // Last case, sometime we have a cabundle
        if(file_exists($this->sslCaBundleFile)){
            $ca = file_get_contents($this->sslCaBundleFile);
            if(!openssl_x509_parse($ca)){
                return false;
            }
        }

        return true;
    }
}

Hey Alexis,

Try a custom validation constraint, where you will be able to set different messages in different cases. We talk about it here: https://knpuniversity.com/s... . Or check the docs: https://symfony.com/doc/cur...

Cheers!

Hey Peter K.

First, you are doing it great by creating your submit buttons by hand, actually that is a best practice: https://symfony.com/doc/current/best_practices/forms.html#form-button-configuration

And second (your real question): Using validation groups fits in your case, and there are two ways (maybe more?) to disable Form validations. You can use the Resolver to set validation_groups option to false, like this:

// inside your FormType

public function configureOptions(OptionsResolver $resolver)
{
    $resolver->setDefaults(array(
        'validation_groups' => false,
    ));
}

Or you can use the POST_SUBMIT form event to disable validations at all.
You can get more information about how to do it in the docs: https://symfony.com/doc/current/form/disabling_validation.html

Cheers!

Hi Giorgio Pagnoni!

For this, there are two main pieces

1) Create a new form class - like EditUserForm. This will allow you to have different fields than your registration form. If many of the fields are shared (between EditUserForm and the RegistrationForm), then, if you want, you can create a common base class and add those common fields in the buildForm method there (then override that method and call parent::buildForm() form the child classes)

2) Then yes, *if* you have some validation differences, handle those with groups. But like we did here, I don't automatically add everything to groups - I identify the specific constraints that are conditional, and then handle those with groups. For something like a Captcha, it depends on what library you use, but its validation *may* be built right into the form field itself (i.e. http://knpuniversity.com/sc.... In that case, simply *not* having the field will remove its built-in validation.

Cheers!

Ah, thank you! I've fixed that (https://github.com/knpunive... and published it!

Cheers!

Hey Mike,

Wow, you do cool things, well done! :)

Probably, the best practice is to use a separate bundle which provide this feature, but since this is not a big deal, that's OK to do it by yourself ;)

Well, more common practice is after exceeding the limit of failures you need to reset those attempts in a some time, so user can try again, e.g. reset attempts in an hour. And for security reasons better do not say user that he exceeded the limit, just do not log them in and show the default message "Invalid credentials". Finally, user will reset his password by itself after some count of failure attempts, but saying that they exceeded the limit will give bad guys a tip :) And I'd rather check for exceeded attempts in checkCredentials() than in User repository - I think that's a more proper place for such things.

And thanks for sharing it with our users!

Cheers!

Hey Mike,

A few minor notices: use "$this->getUser()" shortcut in controllers to get the currently logged in user and avoid using "#" comments - use "//" instead. Also, you're passing $user as a second parameter to createForm(), so you can avoid useless overwriting with "$user = $form->getData();" - "$form->handleRequest()" will automatically bind form's data to the $user. And you don't need to call "$em->persist($user);" on users which already in DB, use persist() only when creating a new User with "$user = new User()", i.e. for those entities which Doctrine do not know yet.

So, except my minor comments you're on the right way ;)

Cheers!

I have found a better way of doing this.
Ive removed the $oldPassword in User.php Entity and instead added this into the FormType:

        $builder

            ->add('oldPassword', PasswordType::class, array(

                'mapped' => false, // extra field in the form that is not mapped to the underlying object

                'constraints' => new UserPassword()

            ))

Hey Mike P.

What you mean with "old password", are you tracking password changes, and then ask for them ?
or you mean user's current password ? If this is the case, you could use UserPassword constraint (it's very handy for situations like this)
More info here: https://symfony.com/doc/cur...

Cheers!

Hey @Mike!
​
​When you try to retrieve an User from the TokenStorage, it will give you the logged in user object (or a string if he is anonymous), BTW, if your controller extends from Symfony's base controller, you can use $this->getUser() method, it will return you an User Object or null
​
​So you could end up with something like this:
​`

​// inside an action method

$user = $this->​getUser();
​if (!$user) {
​ // User is not logged in
​}
​// ... more code
​`

​Cheers!

I found in the next video someone mentioned the same problem and fixed it via refreshing:
http://disq.us/p/1hsbk6e

Anyway I think performance wise the above mentioned solution is the better one.

UPDATE1: Iam using the refresh method now as well because it fixes several errors.

Good find Mike P.! The refresh() is the correct way to handle this unfortunate behavior :).

Hey Mike,

Well, that's not a security hole, but kind of a problem ;) Actually, this transaction was failed and those data even wasn't saved into the DB, so you won't loss this data partially - you will lose them entirely. But well done! And yes, it's a good idea to add a length constraint to the all string fields, where you specify a maximum valid length of the fields, probably not always 255, because it depends on length property in ORM\Column() annotation.

Cheers!

One more question:
I see that json_array is a longtext field in MySQL.
Does it make sense due to security reasons to give that field a length() constraint as well? (To prevent the error from question 1 above)

Hey Mike,

Hm, a good question! Maybe, but it's not for security reasons - probably just for preventing fatal errors. And you probably need to customize default error message to explain why the form is invalid. Anyway, I really don't see any reasons do not add this constraint for json_array fields.

Cheers!

Hey Dan,

Thanks for noticing it! Fixed in https://github.com/knpunive...

Cheers!