From 92cbd64430b07db94bd8025cf8b0184511bcc619 Mon Sep 17 00:00:00 2001 From: Tomasz Kuter Date: Tue, 7 Aug 2018 20:19:45 +0200 Subject: [PATCH 1/2] - Refactoring --- src/Aurora/Domain/Article/ArticleService.php | 31 +++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/Aurora/Domain/Article/ArticleService.php b/src/Aurora/Domain/Article/ArticleService.php index c28fbd0..c6b46d3 100644 --- a/src/Aurora/Domain/Article/ArticleService.php +++ b/src/Aurora/Domain/Article/ArticleService.php @@ -5,6 +5,7 @@ use App\Aurora\App\Support\FractalService; use App\Aurora\Domain\Article\Entity\Article; use App\Aurora\Domain\User\Entity\User; +use App\Aurora\Infrastructure\Article\ArticleRepository; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityNotFoundException; @@ -28,10 +29,17 @@ class ArticleService * @var ArticleTransformer */ private $articleTransformer; + /** * @var RouterInterface */ private $router; + + /** + * @var ArticleRepository + */ + private $articleRepository; + /** * @var FractalService */ @@ -43,10 +51,10 @@ public function __construct( FractalService $fractalService ) { - $this->entityManager = $entityManager; $this->articleTransformer = $articleTransformer; $this->fractalService = $fractalService; + $this->articleRepository = $this->entityManager->getRepository(Article::class); } /** @@ -59,9 +67,7 @@ public function getArticles(Request $request, RouterInterface $router) $page = NULL !== $request->get('page') ? (int) $request->get('page') : 1; $perPage = NULL !== $request->get('per_page') ? (int) $request->get('per_page') : 10; - $articles = $this->entityManager->getRepository(Article::class); - - $doctrineAdapter = new DoctrineORMAdapter($articles->getArticles()); + $doctrineAdapter = new DoctrineORMAdapter($this->articleRepository->getArticles()); $paginator = new Pagerfanta($doctrineAdapter); $paginator->setCurrentPage($page); $paginator->setMaxPerPage($perPage); @@ -73,11 +79,13 @@ public function getArticles(Request $request, RouterInterface $router) $inputParams = $request->attributes->get('_route_params'); $newParams = array_merge($inputParams, $request->query->all()); $newParams['page'] = $page; + return $router->generate($route, $newParams, 0); }); - $resource = new Collection($filteredResults,$this->articleTransformer, 'article'); + $resource = new Collection($filteredResults, $this->articleTransformer, 'article'); $resource->setPaginator($paginatorAdapter); + return $resource; } @@ -88,10 +96,11 @@ public function getArticles(Request $request, RouterInterface $router) */ public function getArticleById($id) { - $article = $this->entityManager->getRepository(Article::class)->find($id); + $article = $this->articleRepository->find($id); - if ($article) + if ($article) { return new Item($article, $this->articleTransformer, 'article'); + } throw new EntityNotFoundException("Article not found"); } @@ -112,21 +121,21 @@ public function addArticle(Request $request) $article->setAuthor($user); $article->setContributors(new ArrayCollection([$user])); - //set tags - if(is_array($request->request->get('tags'))){ + // set tags + if (is_array($request->request->get('tags'))) { foreach ($request->request->get('tags') as $tag) { $article->addTagFromName($tag); } } - $this->entityManager->getRepository(Article::class)->save($article); + $this->articleRepository->save($article); } public function searchArticle(Request $request) { - $resource = $this->entityManager->getRepository(Article::class)->searchArticle($request); + $resource = $this->articleRepository->searchArticle($request); $collection = new Collection($resource,$this->articleTransformer,'article'); From 6c2127d642bc3d92c44d3ce7fd94904418105e52 Mon Sep 17 00:00:00 2001 From: Tomasz Kuter Date: Tue, 7 Aug 2018 20:23:00 +0200 Subject: [PATCH 2/2] - Changed password encrypting: bcrypt is now used instead of sha1 - Handled 'password' grant type --- config/packages/fos_oauth_server.yaml | 8 +- config/packages/security.yaml | 11 +- src/Aurora/Domain/User/Entity/User.php | 120 +++++++++++++++++- .../mapping/Article.Entity.Article.orm.yml | 1 - .../doctrine/mapping/User.Entity.User.orm.yml | 11 +- src/DataFixtures/UserFixtures.php | 30 ++++- src/Security/Domain/User/UserProvider.php | 64 ++++++++++ symfony.lock | 12 ++ 8 files changed, 245 insertions(+), 12 deletions(-) create mode 100644 src/Security/Domain/User/UserProvider.php diff --git a/config/packages/fos_oauth_server.yaml b/config/packages/fos_oauth_server.yaml index 7c86b23..784b7be 100644 --- a/config/packages/fos_oauth_server.yaml +++ b/config/packages/fos_oauth_server.yaml @@ -4,4 +4,10 @@ fos_oauth_server: client_class: App\Authorization\Entity\Oauth2\Client access_token_class: App\Authorization\Entity\Oauth2\AccessToken refresh_token_class: App\Authorization\Entity\Oauth2\RefreshToken - auth_code_class: App\Authorization\Entity\Oauth2\AuthCode \ No newline at end of file + auth_code_class: App\Authorization\Entity\Oauth2\AuthCode + + service: + user_provider: security.user.provider.concrete.my_database_provider + options: + supported_scopes: user + access_token_lifetime: 3600 \ No newline at end of file diff --git a/config/packages/security.yaml b/config/packages/security.yaml index e31f148..2f73b09 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -1,7 +1,16 @@ security: + encoders: + App\Aurora\Domain\User\Entity\User: + algorithm: bcrypt + cost: 12 + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers providers: - in_memory: { memory: ~ } + my_database_provider: + entity: + class: App\Aurora\Domain\User\Entity\User + property: username + firewalls: dev: pattern: ^/(_(profiler|wdt)|css|images|js)/ diff --git a/src/Aurora/Domain/User/Entity/User.php b/src/Aurora/Domain/User/Entity/User.php index 332052e..c66e90e 100644 --- a/src/Aurora/Domain/User/Entity/User.php +++ b/src/Aurora/Domain/User/Entity/User.php @@ -2,7 +2,11 @@ namespace App\Aurora\Domain\User\Entity; -class User { +use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Core\User\EquatableInterface; + +class User implements UserInterface, EquatableInterface, \Serializable +{ /** * @var integer @@ -24,10 +28,20 @@ class User { */ protected $password; + /** + * @var string + */ + protected $salt; + + /** + * @var array + */ + private $roles = []; + /** * @return int */ - public function getId() + public function getId(): int { return $this->id; } @@ -36,7 +50,7 @@ public function getId() /** * @return string */ - public function getUsername() + public function getUsername(): string { return $this->username; } @@ -44,7 +58,7 @@ public function getUsername() /** * @param string $username */ - public function setUsername($username) + public function setUsername(string $username): void { $this->username = $username; } @@ -60,7 +74,7 @@ public function getEmail(): string /** * @param string $email */ - public function setEmail(string $email) + public function setEmail(string $email): void { $this->email = $email; } @@ -76,10 +90,104 @@ public function getPassword(): string /** * @param string $password */ - public function setPassword(string $password) + public function setPassword(string $password): void { $this->password = $password; } + /** + * Returns the salt that was originally used to encode the password. + * + * {@inheritdoc} + */ + public function getSalt(): ?string + { + // See "Do you need to use a Salt?" at https://symfony.com/doc/current/cookbook/security/entity_provider.html + // we're using bcrypt in security.yml to encode the password, so + // the salt value is built-in and you don't have to generate one + + return null; + } + + /** + * @param string $salt + */ + public function setSalt(string $salt): void + { + $this->salt = $salt; + } + /** + * {@inheritdoc} + */ + public function getRoles(): array + { + $roles = $this->roles; + + // guarantees that a user always has at least one role for security + if (empty($roles)) { + $roles[] = 'ROLE_USER'; + } + + return array_unique($roles); + } + + public function setRoles(array $roles): void + { + $this->roles = $roles; + } + + /** + * Removes sensitive data from the user. + * + * {@inheritdoc} + */ + public function eraseCredentials(): void + { + // if you had a plainPassword property, you'd nullify it here + // $this->plainPassword = null; + } + + /** + * {@inheritdoc} + */ + public function serialize(): string + { + // add $this->salt too if you don't use Bcrypt or Argon2i + return serialize([$this->id, $this->username, $this->password]); + } + + /** + * {@inheritdoc} + */ + public function unserialize($serialized): void + { + // add $this->salt too if you don't use Bcrypt or Argon2i + [$this->id, $this->username, $this->password] = unserialize($serialized, ['allowed_classes' => false]); + } + + /** + * @param UserInterface $user + * @return bool + */ + public function isEqualTo(UserInterface $user): bool + { + if (!$user instanceof User) { + return false; + } + + if ($this->password !== $user->getPassword()) { + return false; + } + + if ($this->salt !== $user->getSalt()) { + return false; + } + + if ($this->username !== $user->getUsername()) { + return false; + } + + return true; + } } \ No newline at end of file diff --git a/src/Aurora/Resources/doctrine/mapping/Article.Entity.Article.orm.yml b/src/Aurora/Resources/doctrine/mapping/Article.Entity.Article.orm.yml index 3f68ebe..0890f19 100644 --- a/src/Aurora/Resources/doctrine/mapping/Article.Entity.Article.orm.yml +++ b/src/Aurora/Resources/doctrine/mapping/Article.Entity.Article.orm.yml @@ -35,7 +35,6 @@ App\Aurora\Domain\Article\Entity\Article: joinTable: name: article_contributors - manyToOne: author: targetEntity: App\Aurora\Domain\User\Entity\User diff --git a/src/Aurora/Resources/doctrine/mapping/User.Entity.User.orm.yml b/src/Aurora/Resources/doctrine/mapping/User.Entity.User.orm.yml index c16b466..5c6e6c7 100644 --- a/src/Aurora/Resources/doctrine/mapping/User.Entity.User.orm.yml +++ b/src/Aurora/Resources/doctrine/mapping/User.Entity.User.orm.yml @@ -9,8 +9,15 @@ App\Aurora\Domain\User\Entity\User: fields: username: type: string - length: 100 + length: 31 email: type: string + length: 127 password: - type: string \ No newline at end of file + type: string + salt: + type: string + length: 32 + nullable: true + roles: + type: json \ No newline at end of file diff --git a/src/DataFixtures/UserFixtures.php b/src/DataFixtures/UserFixtures.php index f1a158a..e382aef 100644 --- a/src/DataFixtures/UserFixtures.php +++ b/src/DataFixtures/UserFixtures.php @@ -6,18 +6,40 @@ use Doctrine\Bundle\FixturesBundle\Fixture; use Doctrine\Common\DataFixtures\OrderedFixtureInterface; use Doctrine\Common\Persistence\ObjectManager; +use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface; class UserFixtures extends Fixture implements OrderedFixtureInterface { const TEST_USER = 'test-user'; + + /** + * @var UserPasswordEncoderInterface + */ + private $passwordEncoder; + + /** + * @param UserPasswordEncoderInterface $encoder + */ + public function __construct(UserPasswordEncoderInterface $encoder) + { + $this->passwordEncoder = $encoder; + } + public function load(ObjectManager $manager) { $faker = \Faker\Factory::create(); + $salt = $this->generateSalt(); $user = new User(); $user->setUsername($faker->userName); $user->setEmail("user@example.com"); - $user->setPassword(sha1('secret')); + $user->setRoles(['ROLE_USER']); + + $plainPassword = 'secret'; + + // See https://symfony.com/doc/current/book/security.html#security-encoding-password + $encodedPassword = $this->passwordEncoder->encodePassword($user, $plainPassword); + $user->setPassword($encodedPassword); $manager->persist($user); $manager->flush(); @@ -35,4 +57,10 @@ public function getOrder() return 1; } + private function generateSalt(): string + { + $bytes = random_bytes(32); + + return base_convert(bin2hex($bytes), 16, 36); + } } diff --git a/src/Security/Domain/User/UserProvider.php b/src/Security/Domain/User/UserProvider.php new file mode 100644 index 0000000..e640910 --- /dev/null +++ b/src/Security/Domain/User/UserProvider.php @@ -0,0 +1,64 @@ +userRepository = $userRepository; + } + + public function loadUserByUsername($username) + { + return $this->fetchUser($username); + } + + public function refreshUser(UserInterface $user) + { + if (!$user instanceof User) { + throw new UnsupportedUserException( + sprintf('Instances of "%s" are not supported.', get_class($user)) + ); + } + + $username = $user->getUsername(); + + return $this->fetchUser($username); + } + + public function supportsClass($class) + { + return User::class === $class; + } + + private function fetchUser($username) + { + // make a call to your webservice here + $user = $this->userRepository->findOneBy(['username' => $username]); + // pretend it returns an array on success, false if there is no user + + if ($user) { + return $user; + } + + throw new UsernameNotFoundException( + sprintf('Username "%s" does not exist.', $username) + ); + } +} \ No newline at end of file diff --git a/symfony.lock b/symfony.lock index fd2ce16..3c66eed 100644 --- a/symfony.lock +++ b/symfony.lock @@ -47,6 +47,9 @@ "ref": "2ea6070ecf365f9a801ccaed4b31d4a3b7af5693" } }, + "doctrine/event-manager": { + "version": "v1.0.0" + }, "doctrine/inflector": { "version": "v1.3.0" }, @@ -59,6 +62,12 @@ "doctrine/orm": { "version": "v2.6.1" }, + "doctrine/persistence": { + "version": "v1.0.0" + }, + "doctrine/reflection": { + "version": "v1.0.0" + }, "exsyst/swagger": { "version": "v0.4.0" }, @@ -239,6 +248,9 @@ "ref": "41416370e732d1c8e7a0953a5ce815b641cfba0f" } }, + "symfony/polyfill-ctype": { + "version": "v1.9.0" + }, "symfony/polyfill-intl-icu": { "version": "v1.7.0" },