-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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`. | ||
|
@@ -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) | ||
* @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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a note about that in the unchanged text
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, we are storing the plain password ? This must be fixed There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
*/ | ||
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) | ||
*/ | ||
|
@@ -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() | ||
} | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
---------------------------- | ||
|
||
Next, create the form for the ``User`` entity:: | ||
|
||
|
@@ -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; | ||
|
@@ -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); | ||
|
@@ -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 | ||
|
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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.