Flag of Ukraine
SymfonyCasts stands united with the people of Ukraine

Validation Groups: Conditional Validation

Keep on Learning!

If you liked what you've learned so far, dive in!
Subscribe to get access to this tutorial plus
video, code and script downloads.

Start your All-Access Pass
Buy just this tutorial for $12.00

Ready for the problem? Right now, we need the plainPassword to be required. But later when we create an edit profile page, we don't want to make plainPassword required. Remember, this is not saved to the database. So if the user leaves it blank on the edit form, it just means they don't want to change their password. And that should be allowed.

So, we need this annotation to only work on the registration form.

Validation Groups to the Rescue!

Here's how you do it: take advantage of something called validation groups. On the NotBlank constraint, add groups={"Registration"}:

... lines 1 - 15
class User implements UserInterface
{
... lines 18 - 38
/**
... line 40
* @Assert\NotBlank(groups={"Registration"})
... lines 42 - 43
*/
private $plainPassword;
... lines 46 - 116
}

This "Registration" is a string I just invented: there's no significance to it.

Without doing anything else, go back, hit register, and check it out! The error went away. Here's what's happening: by default, all constraints live in a group called Default. And when your form is validated, it validates all constraints in this Default group. So now that we've put this into a different group called Registration, when the form validates, it doesn't validate using this constraint.

To use this annotation only on the registration form, we need to make that form validate everything in the Default group and the Registration group. Open up UserRegistrationForm and add a second option to setDefaults(): validation_groups set to Default - with a capital D and then Registration:

... lines 1 - 12
class UserRegistrationForm extends AbstractType
{
... lines 15 - 23
public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults([
... line 27
'validation_groups' => ['Default', 'Registration']
]);
}
}

That should do it. Refresh: validation is back.

Ok team: one final mission: automatically authenticate the user after registration. Because really, that's what our users want.

Leave a comment!

42
Login or Register to join the conversation
Default user avatar

I'm not sure if I have a problem, but on my edit user page, I have the same fields as the new user form, so the user can choose to edit the password field or not. Yet when I leave the password fields blank, it resets the password to blank in the database. Is that the correct behavior of the edit form, or am I doing something wrong? Please help :)

Reply
Default user avatar

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!

Reply

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!

Reply
Default user avatar
Default user avatar Terry Caliendo | posted 5 years ago | edited

I'm trying to validate the plainPassword with a regular expression. If the user passes validation, the password gets updated and can continue on logged-in for the session. However, if the validation fails, the error is shown on a returned page, but upon the next submission (or clicking any protected link), the user is suddenly logged out and brought to the login page. Any ideas on how to solve the logout issue when the password authentication fails?


    /**
     * @Assert\NotBlank(groups={"Registration"})
     * @Assert\Regex(
     *     pattern="/^(?=.*[A-Za-z])(?=.*\d)[[A-Za-z\d$@$!%*#?&]{5,}$/",
     *     message="Password must be at least 5 chars in length with 1 letter and 1 number"
     * )
     */
    private $plainPassword;

Reply

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!

Reply
Default user avatar
Default user avatar Terry Caliendo | MolloKhan | posted 5 years ago | edited

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

Reply

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 :)

Reply
Default user avatar
Default user avatar Terry Caliendo | MolloKhan | posted 5 years ago | edited

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');
        }
Reply

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!

Reply
Default user avatar
Default user avatar Terry Caliendo | MolloKhan | posted 5 years ago | edited

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(),
        ));
    }

Reply

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!

Reply
Default user avatar
Default user avatar Terry Caliendo | MolloKhan | posted 5 years ago

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?

Reply

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!

Reply
Default user avatar
Default user avatar Terry Caliendo | weaverryan | posted 5 years ago | edited

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

Reply
Default user avatar

Hello,
I'm new Here, I'm not sure where to post so correct me if I'm wrong.
I have a question about validation, right now I have a pretty complicated case : the validation of one field depends on the value of other fields.
Here an example, in my entity class, I have a "isSSL", "key", "crt" and "cabundle" properties and you guessed it, it's related to SSL/HTTPS stuff !

The problem is that the value of the "crt" property depends on the "key" property (and vice-versa). I have to check that the cryptographic stuff is correct and in order to do that, I must use an "openssl_x509_check_private_key" function with the crt and key value. Here a sample of non-object oriented code to give an example : https://pastebin.com/raw/9j...

I probably need a custom constraint (not a problem) but how can I get all the values of multiple fields/properties when the validate() method is called ?

With some research I discovered the "Expression validation" but I'm not sure if it's useful in this case. Maybe it's possible to do something with a callback too but I dont see how.

Any ideas ?

Thanks in advance and thanks for all the good tutorials, I'm addict to this website now, it helped me a lot.

Reply

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!

1 Reply
Default user avatar
Default user avatar Alexis | Alexis | posted 5 years ago | edited

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

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!

2 Reply
Peter-K Avatar
Peter-K Avatar Peter-K | posted 5 years ago

I have kind of complicated scenario but I will try to explain what I am doing.

I have an entity with 100+ fields.

Entity contains also stage.

In order to make it more user friendly I will ask user to complete this entity in couple of stages.

Stage 1 complete 20 fields.
Stage 2 complete another 20 fields etc.

Now each stage got 2 buttons (Save) and (Save & Continue)

When user will click on save I do not want to throw any validation errors. Blanks are allowed in this case.

I want to throw errors only when user will click on Save & Continue.

Issue here is that I can not put assert on all the fields because when I create entity and user click on save then for example blanks are allowed.

I would sort it by adding validation group I think this would work for me.

But I can not figure it out how to attach this validation group only to saveAndContinue button.

my code is :

if ($form->isSubmitted() && $form->isValid()) {
....
if( $form->get('saveAndContinue')->isClicked()){
....
....
********* SOMEWHERE HERE I WOULD LIKE TO ATTACH VALIDATION GROUP *************
********* BECAUSE I KNOW SAVE AND CONTINUE HAS BEEN CLICKED SO *************
********* I WANT TO VALIDATE IF ALL FIELDS FOR THAT STAGE ARE PUPULATED ******
// but I do not know how to do it. Can you point me to right direction please

$em->persist($foo);
$em->flush();
.....
}else{
$em->persist($foo);
$em->flush();
}
}

User must be able to be able to save entity at any stage.
Each Stage will create different form with different fields to be saved so I can have different validation group for each stage.

One more thing to add I am creating these 2 submit buttons on my own. like <button type="submit" .....="">

According to symfony doc I should do it this way:
->add('save', SubmitType::class, array(
'validation_groups' => false,
))

but I still like to generate my buttons on my own instead of form_row...
because I can put a font awesome icon inside of the button without creating/customizing form theme

Hope it makes sense.

Reply

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!

Reply
Default user avatar
Default user avatar Giorgio Pagnoni | posted 5 years ago

Awesome tutorials, as usual. How would you go about creating an edit form? A form which is almost exactly like our registration form, but where, say, some fields (like email) are read-only or some missing (like a capctha, that might be useful when signing up but not when editing)? Would you, say, create a basic edit form, and then extend it for registration, changing the validation groups?

Reply

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!

Reply
Default user avatar
Default user avatar Dominik | posted 5 years ago

Script typo:
validation_groups
set to Defaults - with a capital D and then

should be: Default (without S)

Reply

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

Cheers!

Reply
Mike P. Avatar
Mike P. Avatar Mike P. | posted 5 years ago | edited

Awesome, Iam feeling so dangerous that I've implemented a login constraint of max. 10 failed login attempts.
I want to share my code for others and ask you: Is this the easiest/best practice way of doing it?

User.php


...
    /**
     * @Assert\NotBlank()
     * @ORM\Column(type="integer")
     */
    private $loginAttempts = 0;
...

LoginFormAuthenticator.php


use AppBundle\Entity\User;
...
use Symfony\Component\Security\Core\Exception\CustomUserMessageAuthenticationException;
...

    public function getUser($credentials, UserProviderInterface $userProvider)
    {
        $username = $credentials['_username'];

        $user = $this->em->getRepository('AppBundle:User')
            ->findOneBy(['email' => $username]);

        // Max. 10 login_attempts
        if ($user instanceof User && $user->getLoginAttempts() >= 10) {
            throw new CustomUserMessageAuthenticationException(
                'Max. 10 login attempts. Please reset your password.'
            );
        }
        return $user;
    }
...

    public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey)
    {
        // Reset login_attempts to 0
        $user = $token->getUser();

        // If User found reset
        if ($user instanceof User) {
            $user->setLoginAttempts(0);
            $this->em->persist($user);
            $this->em->flush();
        }

        // if the user hits a secure page and start() was called, this was
        // the URL they were on, and probably where you want to redirect to
        $targetPath = $this->getTargetPath($request->getSession(), $providerKey);
        if (!$targetPath) {
            $targetPath = $this->router->generate('homepage');
        }

        return new RedirectResponse($targetPath);
    }

    public function onAuthenticationFailure(Request $request, AuthenticationException $exception)
    {
        // Increase failed login attempt
        if ($request->request->get('login_form')['_username']){
            $username = $request->request->get('login_form')['_username'];
            $user = $this->em->getRepository('AppBundle:User')
                ->findOneBy(['email' => $username]);

            // If Username found +1 failed login attempt
            if ($user instanceof User) {
                $user->setLoginAttempts($user->getLoginAttempts()+1);
                $this->em->persist($user);
                $this->em->flush();
            }
        }
        return parent::onAuthenticationFailure($request, $exception); // TODO: Change the autogenerated stub
    }
Reply

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!

Reply
Mike P. Avatar
Mike P. Avatar Mike P. | posted 5 years ago | edited

Im programming a "edit account" page. Of course a user can only edit his/her own account data.
Is this code sufficient for that?


public function accountAction(Request $request)
    {

        // Get user for form creation
        $token = $this->get('security.token_storage')
            ->getToken();

        $user = $token->getUser();

        $form = $this->createForm(EditUserFormType::class, $user);

        # only handles data on POST
        $form->handleRequest($request);
        # If it is a post request and form passed all validation
        if ($form->isSubmitted() && $form->isValid()){
            #dump($form->getData()); die;

            # Save in DB
            $user = $form->getData();

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

            # Redirect always after submit
            // Encurage users, gets saved in session
            $this->addFlash('success', 'User updated - You are amazing!');
            return $this->redirectToRoute('admin_user_list');
        }

        return $this->render('admin/user/edit.html.twig', [
            'userForm' => $form->createView()
        ]);
    }

Iam wondering how Symfony knows that it updates ONLY the userID/userdata set of the currently logged in user and not one from another user (if a hacker wants to change the source code of the form by example and edit the user id, symfony should deny this)

Does Symfony knows the userID due to the token which is submitted with the form? Because the source code doesn't reveal a (hidden) field with the userID inside of it.

Iam just curious if its really secure and that only the currently logged in user can edit his/her own dataset.
Symfony seems to do a lot of magic behind the scenes.

Reply

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!

Reply
Mike P. Avatar
Mike P. Avatar Mike P. | posted 5 years ago | edited

For the mentioned edit password action, I want the user to enter his old password.
I found this solution, is it the right way of doing it?

Extending the User.php Entity


    /**

     * @SecurityAssert\UserPassword(

     *     message = "Wrong value for your current password"

     * )

     */

    protected $oldPassword;

...
    public function getOldPassword()

    {

        return $this->oldPassword;

    }

Reply
Mike P. Avatar
Mike P. Avatar Mike P. | Mike P. | posted 5 years ago | edited

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()

            ))

Reply

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!

Reply
Mike P. Avatar
Mike P. Avatar Mike P. | posted 5 years ago | edited

Im programming a "edit account" page. Of course a user can only edit his/her own account data.

Is this code sufficient for that?



public function accountAction(Request $request)

    {

        // Get user for form creation

        $token = $this->get('security.token_storage')

            ->getToken();

        $user = $token->getUser();

        $form = $this->createForm(EditUserFormType::class, $user);

        # only handles data on POST

        $form->handleRequest($request);

        # If it is a post request and form passed all validation

        if ($form->isSubmitted() && $form->isValid()){

            #dump($form->getData()); die;

            # Save in DB

            $user = $form->getData();

            $em = $this->getDoctrine()->getManager();

            $em->persist($user);

            $em->flush();

            # Redirect always after submit

            // Encurage users, gets saved in session

            $this->addFlash('success', 'User updated - You are amazing!');

            return $this->redirectToRoute('admin_user_list');

        }

        return $this->render('admin/user/edit.html.twig', [

            'userForm' => $form->createView()

        ]);

    }

Iam wondering how Symfony knows that it updates ONLY the userID/userdata set of the currently logged in user and not one from another user (if a hacker wants to change the source code of the form by example and edit the user id, symfony should deny this)

Does Symfony knows the userID due to the token which is submitted with the form? Because the source code doesn't reveal a (hidden) field with the userID inside of it.

Iam just curious if its really secure and that only the currently logged in user can edit his/her own dataset.

Symfony seems to do a lot of magic behind the scenes.

Reply

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!

1 Reply
Mike P. Avatar
Mike P. Avatar Mike P. | posted 5 years ago | edited

After I've now completed the "edit" profile page you're talking about I've one strange behavior.
If the user wants to change his email and the email is already taken, it shows the correct error message for email field.
But one "request afterwards" (whatever route is choosen), the user gets automatically logged out.

How can we prevent the user from being logged out if he chooses to change his user email to an email that is already taken?

UPDATE1:
Ive found out that on the "request afterwards"Symfony throws the following error (which cause to be logged out):
"An AuthenticationException was thrown; redirecting to authentication entry point."

UPDATE2:
Ive found out, that "$form->handleRequest($request);" is causing a email property change inside the user.php entity.
It happens even if the error message is thrown.

Symfony checks on the next request in the serialize() event if
"$this->email = $user->getEmail"
and returns false, because "$this->email" is the email that caused the not unique error and $user->getEmail is the right email.
Due to this symfony is logging out the user automatically (as stated in the Docs).

The first fix I've found is to implement "EquatableInterface" and exclude the email check in this method.
Afterwards I've found another, I think better solution:

    /**
     * @Route("/account", name="settings_account")
     */
    public function accountAction(Request $request)
    {

        // Get user for form creation
        $token = $this->get('security.token_storage')
            ->getToken();

        $user = $token->getUser();
        $oldEmail = $user->getUsername();

        $form = $this->createForm(AccountSettingsFormType::class, $user);

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

        # If it is a post request and form passed all validation
        if ($form->isSubmitted()) {
            if ( $form->isValid()) {

                //dump($form->getData()); die;

                # Save in DB
                $user = $form->getData();

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

                # Redirect always after submit
                // Encurage users, gets saved in session
                $this->addFlash('success', 'Account updated - You are amazing!');
                //print_r($user);die;

            } else {
                // Reset Email in User Entity, if form is invalid (to prevent auto-logout if email is already taken)
                $token->setAttribute('username', $oldEmail);
            }
        }

        return $this->render('settings/account/edit.html.twig', [
            'userForm' => $form->createView()
        ]);
    }```

Reply
Mike P. Avatar

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.

Reply

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

Reply
Mike P. Avatar
Mike P. Avatar Mike P. | posted 5 years ago

It seems that iam so dangerous right now and have found a security hole in the code? :)
If we go to the register page and enter an Email address over 100 chars, we get an exception:

SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'email' at row 1

Do I see it right that every field with type="string" should habe the length constraint with 255?

Reply

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!

1 Reply
Mike P. Avatar

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)

Reply

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!

1 Reply
Patrick Avatar
Patrick Avatar Patrick | posted 5 years ago

Little typo in the first paragraph, plainPasword should be plainPassword :)

Reply

Hey Dan,

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

Cheers!

Reply
Cat in space

"Houston: no signs of life"
Start the conversation!

What PHP libraries does this tutorial use?

// composer.json
{
    "require": {
        "php": ">=5.5.9",
        "symfony/symfony": "3.1.*", // v3.1.4
        "doctrine/orm": "^2.5", // v2.7.2
        "doctrine/doctrine-bundle": "^1.6", // 1.6.4
        "doctrine/doctrine-cache-bundle": "^1.2", // 1.3.0
        "symfony/swiftmailer-bundle": "^2.3", // v2.3.11
        "symfony/monolog-bundle": "^2.8", // 2.11.1
        "symfony/polyfill-apcu": "^1.0", // v1.2.0
        "sensio/distribution-bundle": "^5.0", // v5.0.22
        "sensio/framework-extra-bundle": "^3.0.2", // v3.0.16
        "incenteev/composer-parameter-handler": "^2.0", // v2.1.2
        "composer/package-versions-deprecated": "^1.11", // 1.11.99
        "knplabs/knp-markdown-bundle": "^1.4", // 1.4.2
        "doctrine/doctrine-migrations-bundle": "^1.1" // 1.1.1
    },
    "require-dev": {
        "sensio/generator-bundle": "^3.0", // v3.0.7
        "symfony/phpunit-bridge": "^3.0", // v3.1.3
        "nelmio/alice": "^2.1", // 2.1.4
        "doctrine/doctrine-fixtures-bundle": "^2.3" // 2.3.0
    }
}
userVoice