Re-use NewUserType to validate registration

The only ugly things is how we handle error by generating the view and then parse the content to retrieve all errors…

Fix exposition fields in User entity

src/Wallabag/ApiBundle/Controller/UserRestController.php

@@ -6,12 +6,14 @@ use FOS\UserBundle\Event\UserEvent;
 use FOS\UserBundle\FOSUserEvents;
 use JMS\Serializer\SerializationContext;
 use Nelmio\ApiDocBundle\Annotation\ApiDoc;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\JsonResponse;
+use Wallabag\UserBundle\Entity\User;
 
 class UserRestController extends WallabagRestController
 {
     /**
-     * Retrieve user informations
+     * Retrieve current logged in user informations.
      *
      * @ApiDoc()
      *
@@ -21,78 +23,117 @@ class UserRestController extends WallabagRestController
     {
         $this->validateAuthentication();
 
-        $serializationContext = SerializationContext::create()->setGroups(['user_api']);
+        return $this->sendUser($this->getUser());
-        $json = $this->get('serializer')->serialize($this->getUser(), 'json', $serializationContext);
+    }
+
+    /**
+     * Register an user.
+     *
+     * @ApiDoc(
+     *      requirements={
+     *          {"name"="username", "dataType"="string", "required"=true, "description"="The user's username"},
+     *          {"name"="password", "dataType"="string", "required"=true, "description"="The user's password"},
+     *          {"name"="email", "dataType"="string", "required"=true, "description"="The user's email"}
+     *      }
+     * )
+     *
+     * @todo Make this method (or the whole API) accessible only through https
+     *
+     * @return JsonResponse
+     */
+    public function putUserAction(Request $request)
+    {
+        if (!$this->container->getParameter('fosuser_registration')) {
+            $json = $this->get('serializer')->serialize(['error' => "Server doesn't allow registrations"], 'json');
+
+            return (new JsonResponse())->setJson($json)->setStatusCode(403);
+        }
+
+        $userManager = $this->get('fos_user.user_manager');
+        $user = $userManager->createUser();
+        // enable created user by default
+        $user->setEnabled(true);
+
+        $form = $this->createForm('Wallabag\UserBundle\Form\NewUserType', $user, [
+            'csrf_protection' => false,
+        ]);
+
+        // simulate form submission
+        $form->submit([
+            'username' => $request->request->get('username'),
+            'plainPassword' => [
+                'first' => $request->request->get('password'),
+                'second' => $request->request->get('password'),
+            ],
+            'email' => $request->request->get('email'),
+        ]);
+
+        if ($form->isSubmitted() && false === $form->isValid()) {
+            $view = $this->view($form, 400);
+            $view->setFormat('json');
+
+            // handle errors in a more beautiful way than the default view
+            $data = json_decode($this->handleView($view)->getContent(), true)['children'];
+            $errors = [];
+
+            if (isset($data['username']['errors'])) {
+                $errors['username'] = $this->translateErrors($data['username']['errors']);
+            }
+
+            if (isset($data['email']['errors'])) {
+                $errors['email'] = $this->translateErrors($data['email']['errors']);
+            }
+
+            if (isset($data['plainPassword']['children']['first']['errors'])) {
+                $errors['password'] = $this->translateErrors($data['plainPassword']['children']['first']['errors']);
+            }
+
+            $json = $this->get('serializer')->serialize(['error' => $errors], 'json');
+
+            return (new JsonResponse())->setJson($json)->setStatusCode(400);
+        }
+
+        $userManager->updateUser($user);
+
+        // dispatch a created event so the associated config will be created
+        $event = new UserEvent($user, $request);
+        $this->get('event_dispatcher')->dispatch(FOSUserEvents::USER_CREATED, $event);
+
+        return $this->sendUser($user);
+    }
+
+    /**
+     * Send user response.
+     *
+     * @param User $user
+     *
+     * @return JsonResponse
+     */
+    private function sendUser(User $user)
+    {
+        $json = $this->get('serializer')->serialize(
+            $user,
+            'json',
+            SerializationContext::create()->setGroups(['user_api'])
+        );
 
         return (new JsonResponse())->setJson($json);
     }
 
     /**
-     * Register an user
+     * Translate errors message.
      *
-     * @ApiDoc(
+     * @param array $errors
-     *      requirements={
+     *
-     *          {"name"="username", "dataType"="string", "required"=true, "description"="The user's username"},
+     * @return array
-     *          {"name"="password", "dataType"="string", "required"=true, "description"="The user's password"}
-     *          {"name"="email", "dataType"="string", "required"=true, "description"="The user's email"}
-     *      }
-     * )
-     * @return JsonResponse
      */
-    // TODO : Make this method (or the whole API) accessible only through https
+    private function translateErrors($errors)
-    public function putUserAction($username, $password, $email)
     {
-        if (!$this->container->getParameter('fosuser_registration')) {
+        $translatedErrors = [];
-            $json = $this->get('serializer')->serialize(['error' => "Server doesn't allow registrations"], 'json');
+        foreach ($errors as $error) {
-            return (new JsonResponse())->setJson($json)->setStatusCode(403);
+            $translatedErrors[] = $this->get('translator')->trans($error);
         }
 
-        if ($password === '') { // TODO : might be a good idea to enforce restrictions here
+        return $translatedErrors;
-            $json = $this->get('serializer')->serialize(['error' => 'Password is blank'], 'json');
-            return (new JsonResponse())->setJson($json)->setStatusCode(400);
-        }
-
-
-        // TODO : Make only one call to database by using a custom repository method
-        if ($this->getDoctrine()
-            ->getRepository('WallabagUserBundle:User')
-            ->findOneByUserName($username)) {
-            $json = $this->get('serializer')->serialize(['error' => 'Username is already taken'], 'json');
-            return (new JsonResponse())->setJson($json)->setStatusCode(409);
-        }
-
-        if ($this->getDoctrine()
-            ->getRepository('WallabagUserBundle:User')
-            ->findOneByEmail($email)) {
-            $json = $this->get('serializer')->serialize(['error' => 'An account with this email already exists'], 'json');
-            return (new JsonResponse())->setJson($json)->setStatusCode(409);
-        }
-
-        $em = $this->get('doctrine.orm.entity_manager');
-
-        $userManager = $this->get('fos_user.user_manager');
-        $user = $userManager->createUser();
-
-        $user->setUsername($username);
-
-        $user->setPlainPassword($password);
-
-        $user->setEmail($email);
-
-        $user->setEnabled(true);
-        $user->addRole('ROLE_USER');
-
-        $em->persist($user);
-
-        // dispatch a created event so the associated config will be created
-        $event = new UserEvent($user);
-        $this->get('event_dispatcher')->dispatch(FOSUserEvents::USER_CREATED, $event);
-
-        $serializationContext = SerializationContext::create()->setGroups(['user_api']);
-        $json = $this->get('serializer')->serialize($user, 'json', $serializationContext);
-
-        return (new JsonResponse())->setJson($json);
-
     }
-
 }

src/Wallabag/UserBundle/Entity/User.php

@@ -5,11 +5,10 @@ namespace Wallabag\UserBundle\Entity;
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\ORM\Mapping as ORM;
 use JMS\Serializer\Annotation\Groups;
+use JMS\Serializer\Annotation\XmlRoot;
 use Scheb\TwoFactorBundle\Model\Email\TwoFactorInterface;
 use Scheb\TwoFactorBundle\Model\TrustedComputerInterface;
 use FOS\UserBundle\Model\User as BaseUser;
-use JMS\Serializer\Annotation\ExclusionPolicy;
-use JMS\Serializer\Annotation\Expose;
 use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Wallabag\ApiBundle\Entity\Client;
@@ -19,23 +18,24 @@ use Wallabag\CoreBundle\Entity\Entry;
 /**
  * User.
  *
+ * @XmlRoot("user")
  * @ORM\Entity(repositoryClass="Wallabag\UserBundle\Repository\UserRepository")
  * @ORM\Table(name="`user`")
  * @ORM\HasLifecycleCallbacks()
- * @ExclusionPolicy("all")
  *
  * @UniqueEntity("email")
  * @UniqueEntity("username")
  */
 class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterface
 {
+    /** @Serializer\XmlAttribute */
     /**
      * @var int
      *
-     * @Expose
      * @ORM\Column(name="id", type="integer")
      * @ORM\Id
      * @ORM\GeneratedValue(strategy="AUTO")
+     *
      * @Groups({"user_api"})
      */
     protected $id;
@@ -44,14 +44,30 @@ class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterf
      * @var string
      *
      * @ORM\Column(name="name", type="text", nullable=true)
+     *
      * @Groups({"user_api"})
      */
     protected $name;
 
+    /**
+     * @var string
+     *
+     * @Groups({"user_api"})
+     */
+    protected $username;
+
+    /**
+     * @var string
+     *
+     * @Groups({"user_api"})
+     */
+    protected $email;
+
     /**
      * @var date
      *
      * @ORM\Column(name="created_at", type="datetime")
+     *
      * @Groups({"user_api"})
      */
     protected $createdAt;
@@ -60,6 +76,7 @@ class User extends BaseUser implements TwoFactorInterface, TrustedComputerInterf
      * @var date
      *
      * @ORM\Column(name="updated_at", type="datetime")
+     *
      * @Groups({"user_api"})
      */
     protected $updatedAt;

tests/Wallabag/ApiBundle/Controller/UserRestControllerTest.php

@@ -0,0 +1,98 @@
+<?php
+
+namespace Tests\Wallabag\ApiBundle\Controller;
+
+use Tests\Wallabag\ApiBundle\WallabagApiTestCase;
+
+class UserRestControllerTest extends WallabagApiTestCase
+{
+    public function testGetUser()
+    {
+        $this->client->request('GET', '/api/user.json');
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertArrayHasKey('id', $content);
+        $this->assertArrayHasKey('email', $content);
+        $this->assertArrayHasKey('name', $content);
+        $this->assertArrayHasKey('username', $content);
+        $this->assertArrayHasKey('created_at', $content);
+        $this->assertArrayHasKey('updated_at', $content);
+
+        $this->assertEquals('bigboss@wallabag.org', $content['email']);
+        $this->assertEquals('Big boss', $content['name']);
+        $this->assertEquals('admin', $content['username']);
+
+        $this->assertEquals('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+    }
+
+    public function testCreateNewUser()
+    {
+        $this->client->request('PUT', '/api/user.json', [
+            'username' => 'google',
+            'password' => 'googlegoogle',
+            'email' => 'wallabag@google.com',
+        ]);
+
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertArrayHasKey('id', $content);
+        $this->assertArrayHasKey('email', $content);
+        $this->assertArrayHasKey('username', $content);
+        $this->assertArrayHasKey('created_at', $content);
+        $this->assertArrayHasKey('updated_at', $content);
+
+        $this->assertEquals('wallabag@google.com', $content['email']);
+        $this->assertEquals('google', $content['username']);
+
+        $this->assertEquals('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+    }
+
+    public function testCreateNewUserWithExistingEmail()
+    {
+        $this->client->request('PUT', '/api/user.json', [
+            'username' => 'google',
+            'password' => 'googlegoogle',
+            'email' => 'bigboss@wallabag.org',
+        ]);
+
+        $this->assertEquals(400, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertArrayHasKey('error', $content);
+        $this->assertArrayHasKey('username', $content['error']);
+        $this->assertArrayHasKey('email', $content['error']);
+
+        // $this->assertEquals('fos_user.username.already_used', $content['error']['username'][0]);
+        // $this->assertEquals('fos_user.email.already_used', $content['error']['email'][0]);
+        // This shouldn't be translated ...
+        $this->assertEquals('This value is already used.', $content['error']['username'][0]);
+        $this->assertEquals('This value is already used.', $content['error']['email'][0]);
+
+        $this->assertEquals('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+    }
+
+    public function testCreateNewUserWithTooShortPassword()
+    {
+        $this->client->request('PUT', '/api/user.json', [
+            'username' => 'facebook',
+            'password' => 'face',
+            'email' => 'facebook@wallabag.org',
+        ]);
+
+        $this->assertEquals(400, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertArrayHasKey('error', $content);
+        $this->assertArrayHasKey('password', $content['error']);
+
+        $this->assertEquals('validator.password_too_short', $content['error']['password'][0]);
+
+        $this->assertEquals('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+    }
+}

tests/Wallabag/ApiBundle/WallabagApiTestCase.php

@@ -37,7 +37,7 @@ abstract class WallabagApiTestCase extends WebTestCase
         $firewallName = $container->getParameter('fos_user.firewall_name');
 
         $this->user = $userManager->findUserBy(['username' => 'admin']);
-        $loginManager->loginUser($firewallName, $this->user);
+        $loginManager->logInUser($firewallName, $this->user);
 
         // save the login token into the session and put it in a cookie
         $container->get('session')->set('_security_'.$firewallName, serialize($container->get('security.token_storage')->getToken()));