Secure create-user command #123 (#127)

* ask the user for the password #123
* added integration test #123

.gitignore

@@ -7,7 +7,7 @@
 !bin/console
 
 /var/data/kimai.sqlite
-
+/var/coverage/
 /var/cache/*
 !var/cache/.gitkeep
 

README.md

@@ -115,10 +115,10 @@ bin/console doctrine:schema:create
 bin/console cache:warmup --env=prod
 ```
 
-Create your first user:
+Create your first user with the following command. You will be asked to enter a password afterwards.
 
 ```bash
-bin/console kimai:create-user username password admin@example.com ROLE_SUPER_ADMIN
+bin/console kimai:create-user username admin@example.com ROLE_SUPER_ADMIN
 ```
 
 For available roles, please refer to the [user documentation](var/docs/users.md).

composer.json

@@ -8,7 +8,6 @@
         "ext-pdo_sqlite": "*",
         "avanzu/admin-theme-bundle": "dev-kevinpapst",
         "beberlei/DoctrineExtensions": "^1.0",
-        "dama/doctrine-test-bundle": "^4.0",
         "erusev/parsedown": "^1.6",
         "sensio/framework-extra-bundle": "^5.1",
         "symfony/asset": "^4.0",
@@ -34,6 +33,7 @@
         "white-october/pagerfanta-bundle": "^1.1"
     },
     "require-dev": {
+        "dama/doctrine-test-bundle": "^5.0",
         "doctrine/doctrine-fixtures-bundle": "^3.0",
         "friendsofphp/php-cs-fixer": "^2.10",
         "phpunit/phpunit": "^6.5",

config/bundles.php

@@ -17,4 +17,5 @@ return [
     WhiteOctober\PagerfantaBundle\WhiteOctoberPagerfantaBundle::class => ['all' => true],
     Symfony\Bundle\MakerBundle\MakerBundle::class => ['dev' => true],
     Symfony\Bundle\DebugBundle\DebugBundle::class => ['dev' => true, 'test' => true],
+    DAMA\DoctrineTestBundle\DAMADoctrineTestBundle::class => ['test' => true],
 ];

phpunit.xml.dist

@@ -13,6 +13,7 @@
         <env name="APP_ENV" value="test"/>
         <env name="APP_DEBUG" value="1"/>
         <env name="APP_SECRET" value="5a79a1c866efef9ca1800f971d689f3e"/>
+        <env name="DATABASE_PREFIX" value="kimai2_"/>
         <!-- define your env variables for the test env here -->
 
         <!-- ###+ doctrine/doctrine-bundle ### -->
@@ -25,14 +26,26 @@
     </php>
 
     <testsuites>
-        <testsuite name="Project Test Suite">
+        <testsuite name="Kimai">
             <directory>tests/</directory>
         </testsuite>
     </testsuites>
 
+    <filter>
+        <whitelist processUncoveredFilesFromWhitelist="true">
+            <directory suffix=".php">src/</directory>
+            <directory suffix=".php">templates/</directory>
+            <exclude>
+                <directory suffix=".php">vendor/</directory>
+                <directory suffix=".php">tests/</directory>
+            </exclude>
+        </whitelist>
+    </filter>
+
     <listeners>
         <!-- it begins a database transaction before every testcase and rolls it back after
              the test finished, so tests can manipulate the database without affecting other tests -->
-        <listener class="\DAMA\DoctrineTestBundle\PHPUnit\PHPUnitListener" />
+        <listener class="DAMA\DoctrineTestBundle\PHPUnit\PHPUnitListener" />
+        <listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener" />
     </listeners>
 </phpunit>

src/Command/CreateUserCommand.php

@@ -13,9 +13,11 @@ use App\Entity\User;
 use Doctrine\Bundle\DoctrineBundle\Registry;
 use Symfony\Bridge\Doctrine\RegistryInterface;
 use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Helper\QuestionHelper;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Question\Question;
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\Security\Core\Encoder\UserPasswordEncoder;
 use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface;
@@ -65,10 +67,10 @@ class CreateUserCommand extends Command
             ->setName('kimai:create-user')
             ->setDescription('Create a new user')
             ->setHelp('This command allows you to create a new user.')
-            ->addArgument('username', InputArgument::REQUIRED, 'New username (must be unique)')
-            ->addArgument('password', InputArgument::REQUIRED, 'Users password')
-            ->addArgument('email', InputArgument::REQUIRED, 'Users email address (must be unique)')
-            ->addArgument('role', InputArgument::OPTIONAL, 'Users role (comma separated list)', User::DEFAULT_ROLE)
+            ->addArgument('username', InputArgument::REQUIRED, 'The username of the user to be created (must be unique)')
+            ->addArgument('email', InputArgument::REQUIRED, 'Email address of the user to be created (must be unique)')
+            ->addArgument('role', InputArgument::OPTIONAL, 'A comma separated list of roles to assign. Examples: "ROLE_USER,ROLE_SUPER_ADMIN"', User::DEFAULT_ROLE)
+            ->addArgument('password', InputArgument::OPTIONAL, 'Password for the user to be created')
         ;
     }
 
@@ -81,9 +83,14 @@ class CreateUserCommand extends Command
 
         $username = $input->getArgument('username');
         $email = $input->getArgument('email');
-        $password = $input->getArgument('password');
         $role = $input->getArgument('role');
 
+        if ($input->getArgument('password') !== null) {
+            $password = $input->getArgument('password');
+        } else {
+            $password = $this->askForPassword($input, $output);
+        }
+
         $role = $role ?: User::DEFAULT_ROLE;
 
         $user = new User();
@@ -103,7 +110,7 @@ class CreateUserCommand extends Command
                 $value = $error->getInvalidValue();
                 $io->error(
                     $error->getPropertyPath()
-                    . " (" . (is_array($value) ? implode(',', $value) : $value) .")"
+                    . ' (' . (is_array($value) ? implode(',', $value) : $value) . ')'
                     . "\n    "
                     . $error->getMessage()
                 );
@@ -121,4 +128,31 @@ class CreateUserCommand extends Command
             $io->error('Reason: ' . $ex->getMessage());
         }
     }
+
+    /**
+     * @param InputInterface $input
+     * @param OutputInterface $output
+     *
+     * @return string
+     */
+    protected function askForPassword(InputInterface $input, OutputInterface $output): string
+    {
+        /* @var QuestionHelper $helper */
+        $helper = $this->getHelper('question');
+
+        $passwordQuestion = new Question('Please enter the password');
+        $passwordQuestion->setHidden(true);
+        $passwordQuestion->setHiddenFallback(false);
+        $passwordQuestion->setValidator(function (?string $value) {
+            $password = trim($value);
+            if (empty($password) || strlen($password) < 6) {
+                throw new \Exception('The password is too short, must be at least 6 character');
+            }
+
+            return $value;
+        });
+        $passwordQuestion->setMaxAttempts(3);
+
+        return $helper->ask($input, $output, $passwordQuestion);
+    }
 }

symfony.lock

@@ -416,6 +416,9 @@
             "ref": "85834af1496735f28d831489d12ab1921a875e0d"
         }
     },
+    "symfony/security-csrf": {
+        "version": "v4.0.4"
+    },
     "symfony/stopwatch": {
         "version": "v4.0.3"
     },

tests/Command/CreateUserCommandTest.php

@@ -0,0 +1,86 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Twig;
+
+use App\Command\CreateUserCommand;
+use App\Entity\User;
+use Symfony\Bundle\FrameworkBundle\Console\Application;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Symfony\Component\Console\Tester\CommandTester;
+use Symfony\Component\Validator\ConstraintViolationListInterface;
+use Symfony\Component\Validator\Validator\ValidatorInterface;
+
+/**
+ * @coversDefaultClass \App\Command\CreateUserCommand
+ * @group integration
+ */
+class CreateUserCommandTest extends KernelTestCase
+{
+    /**
+     * @var Application
+     */
+    protected $application;
+
+    protected function setUp()
+    {
+        $kernel = self::bootKernel();
+        $this->application = new Application($kernel);
+        $container = self::$kernel->getContainer();
+
+        $passwordEncoder = $container->get('security.password_encoder');
+
+        $validationResult = $this->getMockBuilder(ConstraintViolationListInterface::class)->getMock();
+        $validationResult->method('count')->willReturn(0);
+        $validator = $this->getMockBuilder(ValidatorInterface::class)->getMock();
+        $validator->method('validate')->willReturn($validationResult);
+
+        $this->application->add(new CreateUserCommand(
+            $passwordEncoder,
+            $container->get('doctrine'),
+            $validator
+        ));
+    }
+
+    public function testCreateUser()
+    {
+        $commandTester = $this->createUser('MyTestUser', 'user@example.com', 'ROLE_USER', 'foobar');
+
+        $output = $commandTester->getDisplay();
+        $this->assertContains('[OK] Success! Created user: MyTestUser', $output);
+
+        $container = self::$kernel->getContainer();
+        $user = $container->get('doctrine')->getRepository(User::class)->loadUserByUsername('MyTestUser');
+        $this->assertNotNull($user);
+    }
+
+    protected function createUser($username, $email, $role, $password)
+    {
+        $command = $this->application->find('kimai:create-user');
+        $commandTester = new CommandTester($command);
+        $commandTester->execute([
+            'command'  => $command->getName(),
+            'username' => $username,
+            'email' => $email,
+            'role' => $role,
+            'password' => $password
+        ]);
+
+        return $commandTester;
+    }
+
+    public function testUserAlreadyExisting()
+    {
+        $commandTester = $this->createUser('MyTestUser', 'user@example.com', 'ROLE_USER', 'foobar');
+        $commandTester = $this->createUser('MyTestUser', 'user@example.com', 'ROLE_USER', 'foobar');
+
+        $output = $commandTester->getDisplay();
+        $this->assertContains('[ERROR] Failed to create user: MyTestUser', $output);
+    }
+}

tests/Controller/HelpControllerTest.php

@@ -10,6 +10,7 @@
 namespace App\Tests\Controller;
 
 /**
+ * @coversDefaultClass \App\Controller\HelpController
  * @group integration
  */
 class HelpControllerTest extends ControllerBaseTest
@@ -26,7 +27,7 @@ class HelpControllerTest extends ControllerBaseTest
         $this->request($client, '/help/');
         $this->assertTrue($client->getResponse()->isSuccessful());
         $this->assertContains('<h1>Kimai documentation</h1>', $client->getResponse()->getContent());
-        $this->assertNotContains('<a href="/en/help/README">', $client->getResponse()->getContent());
+        $this->assertNotContains('<a href="/en/help/">', $client->getResponse()->getContent());
     }
 
     public function testUsersPage()
@@ -34,7 +35,7 @@ class HelpControllerTest extends ControllerBaseTest
         $client = $this->getClientForAuthenticatedUser();
         $this->request($client, '/help/users');
         $this->assertTrue($client->getResponse()->isSuccessful());
-        $this->assertContains('<a href="/en/help/README">', $client->getResponse()->getContent());
+        $this->assertContains('<a href="/en/help/">', $client->getResponse()->getContent());
     }
 
     public function testValidateRouteDoesNotAllowSpecialChars()