Skip to content

[Cookbook] Make registration_form follow best practices #4773

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

Merged
merged 1 commit into from
Feb 7, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 64 additions & 13 deletions cookbook/doctrine/registration_form.rst
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
.. index::
single: Doctrine; Simple Registration Form
single: Form; Simple Registration Form
single: Security; Simple Registration Form

How to Implement a simple Registration Form
How to Implement a Simple Registration Form
===========================================

Creating a registration form is pretty easy - it *really* means just creating
a form that will update some ``User`` model object (a Doctrine entity in this example)
and then save it.
a form that will update some ``User`` model object (a Doctrine entity in this
example) and then save it.

.. tip::

The popular `FOSUserBundle`_ provides a registration form, reset password form
and other user management functionality.
The popular `FOSUserBundle`_ provides a registration form, reset password
form and other user management functionality.

If you don't already have a ``User`` entity and a working login system,
first start with :doc:`/cookbook/security/entity_provider`.
Expand Down Expand Up @@ -61,27 +62,27 @@ With some validation added, your class may look something like this::
private $id;

/**
* @ORM\Column(type="string", length=255)
* @ORM\Column(type="string", length=255, unique=true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to also add a UniqueEntity constraint then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, it's already part of the folded code above.

* @Assert\NotBlank()
* @Assert\Email()
*/
private $email;

/**
* @ORM\Column(type="string", length=255)
* @ORM\Column(type="string", length=255, unique=true)
* @Assert\NotBlank()
*/
private $username;

/**
* @Assert\NotBlank()
* @Assert\Length(max = 4096)
* @Assert\Length(max=4096)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you think it's time to remove this constraint? Thinking about a password longer than 4,000 chars is ridiculous and the Security component already has this constraint hardcoded: https://github.com/symfony/symfony/blob/eee117285ae84b9aa859814d6d0cef9fd7f05baa/src/Symfony/Component/Security/Core/Encoder/BasePasswordEncoder.php#L23

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a note about that in the unchanged text

Why the 4096 Password Limit?

Notice that the plainPassword field has a max length of 4096 characters. For security purposes (CVE-2013-5750), Symfony limits the plain password length to 4096 characters when encoding it. Adding this constraint makes sure that your form will give a validation error if anyone tries a super-long password.

I agree that a password longer than 4000 chars is ridiculous, but for me, it sounds like the constraint is still reasonable to provide a user friendly validation error. As I write this... a friendly user would never enter 4000 chars and a malicious user don't need a user friendly error message.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xelaris to avoid getting an exception when trying to encode it (a malicious user should still receive a 400 status code rather than a 500)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof I agree, responding with 500 in this case would be neat. But if I am not wrong, without further changes the form with validation error will be shown along with status code 200. Anyway I agree the 4096 constraint makes sense to avoid 500.

While testing this, I saw an other issue with the example. Although the note mentions that the limit is relevant when it comes to encoding the password, in the current version, where the password is stored in plain text, the user gets a 500 response due to a PDOException, when entering a password longer than the table field but shorter than 4096.

What about expanding this chapter to store an encoded password? Alternatively either the column length should be set to match the constraint (e.g. 4096) or vice versa (e.g. 255). What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, we are storing the plain password ? This must be fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-read the preamble of this entry. Thereby I become unsure about what the actual topic of this entry is. On the one hand it resides in the doctrine category and is about "extra fields whose values don't need to be stored in the database", on the other hand it's about a registration form and thus related to security.
The more I think about it... maybe this entry should be splitted into two entries. One to fulfill the preamble of the current entry and one to implement a comprehensive registration form like the headline promises. Therefore it requires another example/usecase for the doctrine/form related entry. The new entry about a registration form wouldn't need the embedded form, because the "terms accepted" can be ignored there. Assuming that the registration from would operate with the security system (e.g. an entity that implements the UserInterface of the Security component and using the recommended bcrypt encoder via security.yml) it would not be as "simple" as the current registration form. I think it would be nice to complement http://symfony.com/doc/current/cookbook/security/form_login_setup.html and http://symfony.com/doc/current/cookbook/security/entity_provider.html with the new entry about a registration from since the latter one already describes the required User implementing the UserInterface and the security.yml.
What are your opinions?

*/
private $plainPassword;

/**
* The below length depends on the "algorithm" you use for encoding
* the password, but this works well with bcrypt
* the password, but this works well with bcrypt.
*
* @ORM\Column(type="string", length=64)
*/
Expand Down Expand Up @@ -124,6 +125,13 @@ With some validation added, your class may look something like this::
$this->password = $password;
}

public function getSalt()
{
// The bcrypt algorithm don't require a separate salt.
// You *may* need a real salt if you choose a different encoder.
return null;
}

// other methods, including security methods like getRoles()
}

Expand All @@ -146,8 +154,10 @@ example, see the :ref:`Entity Provider <security-crete-user-entity>` article.
only place where you don't need to worry about this is your login form,
since Symfony's Security component handles this for you.

Create a Form for the Model
---------------------------
.. _create-a-form-for-the-model:

Create a Form for the Entity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

----------------------------

Next, create the form for the ``User`` entity::

Expand Down Expand Up @@ -196,8 +206,9 @@ There are just three fields: ``email``, ``username`` and ``plainPassword``
Handling the Form Submission
----------------------------

Next, you need a controller to handle the form. Start by creating a simple
controller for displaying the registration form::
Next, you need a controller to handle the form rendering and submission. If the
form is submitted, the controller performs the validation and saves the data
into the database::

// src/AppBundle/Controller/RegistrationController.php
namespace AppBundle\Controller;
Expand All @@ -223,6 +234,7 @@ controller for displaying the registration form::
// 2) handle the submit (will only happen on POST)
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {

// 3) Encode the password (you could also do this via Doctrine listener)
$encoder = $this->get('security.encoder_factory')
->getEncoder($user);
Expand All @@ -249,6 +261,45 @@ controller for displaying the registration form::
}
}

To define the algorithm used to encode the password in step 3 configure the
encoder in the security configuration:

.. configuration-block::

.. code-block:: yaml
# app/config/security.yml
security:
encoders:
AppBundle\Entity\User: bcrypt
.. code-block:: xml
<!-- app/config/security.xml -->
<?xml version="1.0" charset="UTF-8" ?>
<srv:container xmlns="http://symfony.com/schema/dic/security"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:srv="http://symfony.com/schema/dic/services"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
<config>
<encoder class="AppBundle\Entity\User">bcrypt</encoder>
</config>
</srv:container>
.. code-block:: php
// app/config/security.php
$container->loadFromExtension('security', array(
'encoders' => array(
'AppBundle\Entity\User' => 'bcrypt',
),
));
In this case the recommended ``bcrypt`` algorithm is used. To learn more
about how to encode the users password have a look into the
:ref:`security chapter <book-security-encoding-user-password>`.

.. note::

If you decide to NOT use annotation routing (shown above), then you'll
Expand Down