From 5709ecb36809fb009446a11a758232bbe8f264e4 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Tue, 30 May 2017 07:56:01 +0200 Subject: [PATCH] Re-use `NewUserType` to validate registration MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- .../Controller/UserRestController.php | 119 ++++++++++++------ src/Wallabag/UserBundle/Entity/User.php | 25 +++- .../Controller/UserRestControllerTest.php | 98 +++++++++++++++ .../ApiBundle/WallabagApiTestCase.php | 2 +- 4 files changed, 200 insertions(+), 44 deletions(-) create mode 100644 tests/Wallabag/ApiBundle/Controller/UserRestControllerTest.php diff --git a/src/Wallabag/ApiBundle/Controller/UserRestController.php b/src/Wallabag/ApiBundle/Controller/UserRestController.php index c5ffbdf1..a1b78e3f 100644 --- a/src/Wallabag/ApiBundle/Controller/UserRestController.php +++ b/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']); - $json = $this->get('serializer')->serialize($this->getUser(), 'json', $serializationContext); - - return (new JsonResponse())->setJson($json); + return $this->sendUser($this->getUser()); } /** - * Register an user + * 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"="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 */ - // TODO : Make this method (or the whole API) accessible only through https - public function putUserAction($username, $password, $email) + 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); } - if ($password === '') { // TODO : might be a good idea to enforce restrictions here - $json = $this->get('serializer')->serialize(['error' => 'Password is blank'], 'json'); - return (new JsonResponse())->setJson($json)->setStatusCode(400); - } + $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, + ]); - // 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); - } + // 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 ($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); - } + if ($form->isSubmitted() && false === $form->isValid()) { + $view = $this->view($form, 400); + $view->setFormat('json'); - $em = $this->get('doctrine.orm.entity_manager'); + // handle errors in a more beautiful way than the default view + $data = json_decode($this->handleView($view)->getContent(), true)['children']; + $errors = []; - $userManager = $this->get('fos_user.user_manager'); - $user = $userManager->createUser(); + if (isset($data['username']['errors'])) { + $errors['username'] = $this->translateErrors($data['username']['errors']); + } - $user->setUsername($username); + if (isset($data['email']['errors'])) { + $errors['email'] = $this->translateErrors($data['email']['errors']); + } - $user->setPlainPassword($password); + if (isset($data['plainPassword']['children']['first']['errors'])) { + $errors['password'] = $this->translateErrors($data['plainPassword']['children']['first']['errors']); + } - $user->setEmail($email); + $json = $this->get('serializer')->serialize(['error' => $errors], 'json'); - $user->setEnabled(true); - $user->addRole('ROLE_USER'); + return (new JsonResponse())->setJson($json)->setStatusCode(400); + } - $em->persist($user); + $userManager->updateUser($user); // dispatch a created event so the associated config will be created - $event = new UserEvent($user); + $event = new UserEvent($user, $request); $this->get('event_dispatcher')->dispatch(FOSUserEvents::USER_CREATED, $event); - $serializationContext = SerializationContext::create()->setGroups(['user_api']); - $json = $this->get('serializer')->serialize($user, 'json', $serializationContext); + return $this->sendUser($user); + } - return (new JsonResponse())->setJson($json); + /** + * 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); } + /** + * Translate errors message. + * + * @param array $errors + * + * @return array + */ + private function translateErrors($errors) + { + $translatedErrors = []; + foreach ($errors as $error) { + $translatedErrors[] = $this->get('translator')->trans($error); + } + + return $translatedErrors; + } } diff --git a/src/Wallabag/UserBundle/Entity/User.php b/src/Wallabag/UserBundle/Entity/User.php index 1863c966..1ff3046a 100644 --- a/src/Wallabag/UserBundle/Entity/User.php +++ b/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; diff --git a/tests/Wallabag/ApiBundle/Controller/UserRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/UserRestControllerTest.php new file mode 100644 index 00000000..21d59a16 --- /dev/null +++ b/tests/Wallabag/ApiBundle/Controller/UserRestControllerTest.php @@ -0,0 +1,98 @@ +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')); + } +} diff --git a/tests/Wallabag/ApiBundle/WallabagApiTestCase.php b/tests/Wallabag/ApiBundle/WallabagApiTestCase.php index cf9b3347..a67655c8 100644 --- a/tests/Wallabag/ApiBundle/WallabagApiTestCase.php +++ b/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())); -- 2.41.0