[2.7][SecurityBundle] Added a command to encode a password #12818

saro0h · 2014-12-03T00:30:15Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#11206
License	MIT
Doc PR	~

Give some love to the UI (need your feedbacks)
Write tests
Write documentation

The encoder got depend directly from the configuration of the user class in the security.yml.
So the user choose the user type, and, using the method getEncoder($user) of the security.encoder_factory service, I get the right encoder configured.

Here is the output for security:encode-password:

jderusse · 2014-12-03T07:12:46Z

src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php

+        $table = new Table($output);
+        $table
+            ->setHeaders(array('Key', 'Value'))
+            ->addRow(array('Raw password', $input->getOption('show-password') ? $password : str_repeat('*', mb_strlen($password))))


By showing the length, you're partially leaking some informations about the password

I know. What's the problem about that?

Why showing something when the use ask for not "--show-password"?

To have some consistency between the 2 outputs.

stloyd · 2014-12-03T22:18:03Z

src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php

+    {
+        $saltQuestion = new Question('<question>Provide a salt (if none, a salt will be generated):</question> ');
+
+        $that = $this;


Why not saving container instead of whole class?

saro0h · 2014-12-17T00:03:48Z

Ping @javiereguiluz : Is that what you expected?

javiereguiluz · 2014-12-17T08:04:47Z

@saro0h sorry for the late response. This is indeed what I expected, but I think we can simplify the user experience of the command. I find it confusing having to read so many text and it's also confusing to me the order of the questions.

The ida of this command is to provide a very simple and lightweight utility to encode plain text passwords using the appropriate encoding. The way passwords are encoded makes it almost "impossible" to encode the password by hand. This is necessary when you want to encode passwords for the "in_memory" provider and when you want to do checks of encoded passwords in the database.

This is how I imagined this command could work:

 Symfony Password Encoder Utility
 ================================

 > Select the encoder for which you want to encode the password:
   o Symfony\Component\Security\Core\User\User
   o ...

 > Type in your password (it won't be shown):
   ***********

 > (Optional) Type in the random salt (<Enter>):
   <Enter>

 [OK] The encoded password is:
 238ae5152rd77b787f78a7e7c723909fcd09090ea09908122332f23c32d23e43456a

In addition, if you only define one encoder, the command shouldn't ask you to pick one.

saro0h · 2014-12-21T14:08:23Z

@javiereguiluz Unfortunatly, I can't access the security.encoders parameter in the container… The user has to type the user class.

saro0h · 2014-12-21T15:12:28Z

ping @fabpot : to close this issue #11206

mickaelandrieu · 2014-12-23T12:52:46Z

I like the idea, but why should we do a totaly "Symfony-coupled" implementation instead of console application ? This can be useful for a lot of PHP projects (and developpers).

Maybe can you check the minimal dependencies to provide this kind of stuff ? I you don't, I will :)

ogizanagi · 2014-12-25T12:46:54Z

@saro0h : I provided a similar command (but much less worked) in a separate bundle, and I faced the same issue about the security.encoders, which I cannot access from the container.
I didn't go further, as my own needs where limited, but can't you simply use the Yaml Parser in order to retrieve them from the security.yml ? (And provide an option to specify the location of the yaml file where they are described, if not the default location)

hhamon · 2014-12-25T13:55:44Z

@mickaelandrieu which PHP Open-Source projects? Maybe Silex eventually. I don't think decoupling this command really worths it for now. It mostly serves Symfony users first. Maybe if there is a real need to get it extracted somewhere else, we will be able to do it.

@ogizanagi instead of parsing the YAML files, the easiest way is to rely on a global configuration parameter in the container. Remember the console is executed at "runtime" whereas configuration files like security.yml are processed and validated at "compile" (very first request) time. So, you're doing the same thing twice without validating the file if you do it in your command. Also, relying on the container allows to rely on an environment. Console commands are "environment aware" thanks to the global --env option.

ogizanagi · 2014-12-25T15:27:37Z

@hhamon : Indeed, I completely ignored the environments in my suggestion. This was a bad idea...

instead of parsing the YAML files, the easiest way is to rely on a global configuration parameter in the container.

Should the SecurityExtension expose the $config['encoders'] as a global parameter in the container, then ? (Or should the EncoderFactory provide access to the currently private $encoders property through a dedicated method ?)

saro0h · 2014-12-25T15:49:42Z

Nope it doesn't and I think it's a bad idea to expose it just for this command.

saro0h · 2015-03-15T08:20:32Z

Ping @fabpot

fabpot · 2015-03-15T19:54:01Z

src/Symfony/Bundle/SecurityBundle/composer.json

@@ -18,12 +18,13 @@
    "require": {
        "php": ">=5.3.9",
        "symfony/security": "~2.7|~3.0.0",
-        "symfony/http-kernel": "~2.2|~3.0.0"
+        "symfony/http-kernel": "~2.2|~3.0.0",
+        "ircmaxell/password-compat": "dev-master"


Do we really need to depend on dev-master, that makes me nervous :)

Also, is it really a hard requirement? It's only useful on older versions of PHP.

I don't remember why I added it :/ Let me remove it.

fabpot · 2015-03-16T16:40:26Z

src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php

+The command allows you to provide your own <comment>salt</comment>. If you don't provide any,
+the command will take about that for you.
+
+You can also use the non interactive


Looks unfinished to me. While you are at it, can you rework the documentation to describe the non-interactive way first?

lyrixx · 2015-03-16T17:13:13Z

👍 for the feature.

saro0h · 2015-03-16T17:28:39Z

ping @fabpot Done

fabpot · 2015-03-17T12:49:17Z

src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php

+      with the <comment>bcrypt</comment> encoder.
+
+The command allows you to provide your own <comment>salt</comment>. If you don't provide any,
+the command will take about that for you.


saro0h · 2015-03-17T16:50:57Z

@fabpot Now I remember why I added the package ircmaxell/password-compat => https://travis-ci.org/symfony/symfony/jobs/54716343#L3250

fabpot · 2015-03-17T17:16:23Z

src/Symfony/Bundle/SecurityBundle/composer.json

@@ -37,7 +37,8 @@
        "symfony/yaml": "~2.0,>=2.0.5|~3.0.0",
        "symfony/expression-language": "~2.6|~3.0.0",
        "doctrine/doctrine-bundle": "~1.2",
-        "twig/twig": "~1.12"
+        "twig/twig": "~1.12",
+        "ircmaxell/password-compat": "1.0.4"


Do you really need a specific version here? Can't we just use ~1 or ~1.0?

saro0h · 2015-03-18T05:22:23Z

Ping @fabpot modification done.

fabpot · 2015-03-19T18:21:11Z

Thank you @saro0h.

…word (saro0h) This PR was merged into the 2.7 branch. Discussion ---------- [2.7][SecurityBundle] Added a command to encode a password | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #11206 | License | MIT | Doc PR | ~ - [x] Give some love to the UI (need your feedbacks) - [x] Write tests - [x] Write documentation ------------------------------------------------------- The encoder got depend directly from the configuration of the user class in the security.yml. So the user choose the user type, and, using the method `getEncoder($user)` of the `security.encoder_factory` service, I get the right encoder configured. *Here is the output for `security:encode-password`*: ![capture d ecran 2015-01-01 a 19 48 07](https://cloud.githubusercontent.com/assets/667519/5593008/3af45686-91ef-11e4-8024-e66a2e000fbe.png) Commits ------- a7bd0fc Added a command to encode a password

saro0h · 2015-03-19T22:32:10Z

\o/

…guments order. (ogizanagi) This PR was merged into the 2.7 branch. Discussion ---------- [SecurityBundle] UserPasswordEncoderCommand: fix help arguments order. | Q | A | ------------- | --- | Fixed tickets | ✘ | License | MIT A very small fix about the newly introduced command to encode a user password (#12818). The help message states the command looks like: ``` security:encode-password [password] [salt] [user-class] ``` but is the following instead: ``` security:encode-password [password] [user-class] [salt] ``` Commits ------- 0a5b1b9 [SecurityBundle] UserPasswordEncoderCommand: fix help arguments order.

saro0h force-pushed the command-encode-password branch from 68b50d1 to 75a9926 Compare December 3, 2014 00:30

jderusse reviewed Dec 3, 2014
View reviewed changes

saro0h force-pushed the command-encode-password branch from 75a9926 to 728f231 Compare December 3, 2014 20:16

saro0h changed the title ~~[WIP] Added a command to encode a password~~ [SecurityBundle][WIP] Added a command to encode a password Dec 3, 2014

stloyd reviewed Dec 3, 2014
View reviewed changes

saro0h force-pushed the command-encode-password branch from 728f231 to 50e0662 Compare December 5, 2014 00:28

fabpot added the Security label Dec 7, 2014

saro0h force-pushed the command-encode-password branch 3 times, most recently from 13b8d53 to 8e227b1 Compare December 13, 2014 19:26

saro0h force-pushed the command-encode-password branch 2 times, most recently from dc3816d to 35ad91f Compare December 17, 2014 00:02

saro0h changed the title ~~[SecurityBundle][WIP] Added a command to encode a password~~ [SecurityBundle Added a command to encode a password Dec 17, 2014

saro0h changed the title ~~[SecurityBundle Added a command to encode a password~~ [SecurityBundle] Added a command to encode a password Dec 17, 2014

saro0h force-pushed the command-encode-password branch from 35ad91f to 1b877b9 Compare December 19, 2014 22:44

saro0h force-pushed the command-encode-password branch from 1b877b9 to addbfb9 Compare December 21, 2014 15:10

saro0h force-pushed the command-encode-password branch 7 times, most recently from 0ca0f46 to f71109f Compare March 11, 2015 20:16

fabpot reviewed Mar 15, 2015
View reviewed changes

saro0h force-pushed the command-encode-password branch from f71109f to 6561f11 Compare March 15, 2015 22:41

fabpot reviewed Mar 16, 2015
View reviewed changes

saro0h force-pushed the command-encode-password branch from 6561f11 to 033236c Compare March 16, 2015 17:11

fabpot reviewed Mar 17, 2015
View reviewed changes

saro0h force-pushed the command-encode-password branch 2 times, most recently from 964c4fb to 0e8aedf Compare March 17, 2015 13:15

saro0h force-pushed the command-encode-password branch from 0e8aedf to 3c6acc4 Compare March 17, 2015 16:54

fabpot reviewed Mar 17, 2015
View reviewed changes

Added a command to encode a password

a7bd0fc

saro0h force-pushed the command-encode-password branch from 3c6acc4 to a7bd0fc Compare March 17, 2015 22:11

fabpot merged commit a7bd0fc into symfony:2.7 Mar 19, 2015

This was referenced Mar 19, 2015

[SecurityBundle] UserPasswordEncoderCommand: fix help arguments order. #13985

Merged

[SecurityBundle] UserPasswordEncoderCommand concerns #13988

Closed

Uh oh!

[2.7][SecurityBundle] Added a command to encode a password #12818

[2.7][SecurityBundle] Added a command to encode a password #12818

Uh oh!

Conversation

saro0h commented Dec 3, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saro0h commented Dec 17, 2014

Uh oh!

javiereguiluz commented Dec 17, 2014

Uh oh!

saro0h commented Dec 21, 2014

Uh oh!

saro0h commented Dec 21, 2014

Uh oh!

mickaelandrieu commented Dec 23, 2014

Uh oh!

ogizanagi commented Dec 25, 2014

Uh oh!

hhamon commented Dec 25, 2014

Uh oh!

ogizanagi commented Dec 25, 2014

Uh oh!

saro0h commented Dec 25, 2014

Uh oh!

saro0h commented Mar 15, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lyrixx commented Mar 16, 2015

Uh oh!

saro0h commented Mar 16, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saro0h commented Mar 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saro0h commented Mar 18, 2015

Uh oh!

fabpot commented Mar 19, 2015

Uh oh!

saro0h commented Mar 19, 2015

Uh oh!

Uh oh!