From 93e8b6d3b5c5fdcb32ba30d7591e7f8a502c68bb Mon Sep 17 00:00:00 2001 From: Julio Montoya Date: Thu, 24 Jun 2021 12:06:06 +0200 Subject: [PATCH] Remove deprecated LoginFormAuthenticator.php, fix phpunit tests Fix User.php --- src/CoreBundle/Entity/User.php | 12 +- .../Authorization/Voter/ResourceNodeVoter.php | 4 +- .../Security/LoginFormAuthenticator.php | 214 ------------------ .../Node/PersonalFileRepositoryTest.php | 2 +- .../Repository/CDocumentRepositoryTest.php | 11 +- 5 files changed, 16 insertions(+), 227 deletions(-) delete mode 100644 src/CoreBundle/Security/LoginFormAuthenticator.php diff --git a/src/CoreBundle/Entity/User.php b/src/CoreBundle/Entity/User.php index de5ae430c2..647b884be6 100644 --- a/src/CoreBundle/Entity/User.php +++ b/src/CoreBundle/Entity/User.php @@ -66,8 +66,8 @@ use UserManager; 'lastname' => 'partial', ])] #[ApiFilter(BooleanFilter::class, properties: ['isActive'])] - -class User implements UserInterface, EquatableInterface, ResourceInterface, ResourceIllustrationInterface, PasswordAuthenticatedUserInterface +//EquatableInterface, +class User implements UserInterface, ResourceInterface, ResourceIllustrationInterface, PasswordAuthenticatedUserInterface { use TimestampableEntity; use UserCreatorTrait; @@ -254,7 +254,7 @@ class User implements UserInterface, EquatableInterface, ResourceInterface, Reso * @Groups({"user:read"}) * @ORM\Column(name="last_login", type="datetime", nullable=true) */ - protected ?DateTime $lastLogin; + protected ?DateTime $lastLogin = null; /** * Random string sent to the user email address in order to verify it. @@ -649,7 +649,7 @@ class User implements UserInterface, EquatableInterface, ResourceInterface, Reso /** * @ORM\Column(name="expiration_date", type="datetime", nullable=true, unique=false) */ - protected ?DateTime $expirationDate; + protected ?DateTime $expirationDate = null; /** * @ORM\Column(name="active", type="boolean") @@ -1659,7 +1659,7 @@ class User implements UserInterface, EquatableInterface, ResourceInterface, Reso return $this; } - public function isEqualTo(UserInterface $user): bool + /*public function isEqualTo(UserInterface $user): bool { if ($this->password !== $user->getPassword()) { return false; @@ -1674,7 +1674,7 @@ class User implements UserInterface, EquatableInterface, ResourceInterface, Reso } return true; - } + }*/ public function getSentMessages(): Collection { diff --git a/src/CoreBundle/Security/Authorization/Voter/ResourceNodeVoter.php b/src/CoreBundle/Security/Authorization/Voter/ResourceNodeVoter.php index a73b8cbbd2..a54fdc053b 100644 --- a/src/CoreBundle/Security/Authorization/Voter/ResourceNodeVoter.php +++ b/src/CoreBundle/Security/Authorization/Voter/ResourceNodeVoter.php @@ -18,6 +18,7 @@ use Laminas\Permissions\Acl\Role\GenericRole; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Security\Acl\Permission\MaskBuilder; use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken; +use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Voter\Voter; use Symfony\Component\Security\Core\Security; @@ -431,7 +432,8 @@ class ResourceNodeVoter extends Voter $acl->allow($admin); $acl->allow($superAdmin); - if ($token instanceof AnonymousToken) { + //if ($token instanceof AnonymousToken) { + if ($token instanceof NullToken) { return $acl->isAllowed('IS_AUTHENTICATED_ANONYMOUSLY', $linkId, $askedMask); } diff --git a/src/CoreBundle/Security/LoginFormAuthenticator.php b/src/CoreBundle/Security/LoginFormAuthenticator.php deleted file mode 100644 index 471992ae55..0000000000 --- a/src/CoreBundle/Security/LoginFormAuthenticator.php +++ /dev/null @@ -1,214 +0,0 @@ -router = $router; - $this->passwordHasher = $passwordHasher; - //$this->formFactory = $formFactory; - //$this->hookFactory = $hookFactory; - $this->userRepository = $userRepository; - $this->csrfTokenManager = $csrfTokenManager; - //$this->entityManager = $entityManager; - $this->urlGenerator = $urlGenerator; - $this->serializer = $serializer; - } - - public function supports(Request $request): bool - { - return self::LOGIN_ROUTE === $request->attributes->get('_route') && $request->isMethod('POST'); - } - - public function getCredentials(Request $request): array - { - $data = null; - $token = null; - if (0 === strpos($request->headers->get('Content-Type'), 'application/json')) { - $data = json_decode($request->getContent(), true); - $username = $data['username']; - $password = $data['password']; - //$token = $data['csrf_token']; - } else { - $username = $request->request->get('username'); - $password = $request->request->get('password'); - $token = $request->request->get('csrf_token'); - } - - $credentials = [ - 'username' => $username, - 'password' => $password, - 'csrf_token' => $token, - ]; - $request->getSession()->set( - Security::LAST_USERNAME, - $credentials['username'] - ); - - return $credentials; - } - - /** - * @param array $credentials - * - * @return null|UserInterface - */ - public function getUser($credentials, UserProviderInterface $userProvider) - { - /*$token = new CsrfToken('authenticate', $credentials['csrf_token']); - if (!$this->csrfTokenManager->isTokenValid($token)) { - throw new InvalidCsrfTokenException(); - }*/ - /** @var null|User $user */ - $user = $this->userRepository->findOneBy([ - 'username' => $credentials['username'], - ]); - - if (null === $user) { - // fail authentication with a custom error - throw new CustomUserMessageAuthenticationException('username could not be found.'); - } - - return $user; - } - - public function checkCredentials($credentials, $user): bool - { - //error_log('login form'); - - return $this->passwordHasher->isPasswordValid($user, $credentials['password']); - /*$hook = $this->hookFactory->build(CheckLoginCredentialsHook::class); - - if (empty($hook)) { - return false; - } - - $hook->setEventData(['user' => $user, 'credentials' => $credentials]); - - return $hook->notifyLoginCredentials();*/ - } - - /** - * Used to upgrade (rehash) the user's password automatically over time. - */ - public function getPassword($credentials): ?string - { - return $credentials['password']; - } - - public function onAuthenticationFailure(Request $request, AuthenticationException $exception) - { - /*$request->getSession()->set(Security::AUTHENTICATION_ERROR, $exception); - - return new RedirectResponse($this->router->generate('login'));*/ - - $data = [ - // you may want to customize or obfuscate the message first - 'message' => strtr($exception->getMessageKey(), $exception->getMessageData()), - - // or to translate this message - // $this->translator->trans($exception->getMessageKey(), $exception->getMessageData()) - ]; - - return new JsonResponse($data, Response::HTTP_UNAUTHORIZED); - } - - public function start(Request $request, AuthenticationException $authException = null) - { - /*$session = $request->getSession(); - - // I am choosing to set a FlashBag message with my own custom message. - // Alternatively, you could use AuthenticationException's generic message - // by calling $authException->getMessage() - $session->getFlashBag()->add('warning', 'You must be logged in to access that page');*/ - - $data = [ - // you might translate this message - 'message' => 'Authentication Required', - ]; - - return new JsonResponse($data, Response::HTTP_UNAUTHORIZED); - } - - /** - * @param string $providerKey - */ - public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey) - { - /*if ($targetPath = $this->getTargetPath($request->getSession(), $providerKey)) { - return new RedirectResponse($targetPath); - } - - return new RedirectResponse($this->urlGenerator->generate('home'));*/ - /** @var User $user */ - $user = $token->getUser(); - if ($user) { - $userClone = clone $user; - $userClone->setPassword(''); - $data = $this->serializer->serialize($userClone, JsonEncoder::FORMAT); - - return new JsonResponse($data, Response::HTTP_OK, [], true); - } - } - - public function getLoginUrl(): string - { - return $this->urlGenerator->generate(self::LOGIN_ROUTE); - } - - public function supportsRememberMe() - { - return false; - } -} diff --git a/tests/CoreBundle/Repository/Node/PersonalFileRepositoryTest.php b/tests/CoreBundle/Repository/Node/PersonalFileRepositoryTest.php index 66f5ba8848..073d49ca58 100644 --- a/tests/CoreBundle/Repository/Node/PersonalFileRepositoryTest.php +++ b/tests/CoreBundle/Repository/Node/PersonalFileRepositoryTest.php @@ -79,7 +79,7 @@ class PersonalFileRepositoryTest extends AbstractApiTest 'GET', $url ); - $this->assertResponseStatusCodeSame(403); // forbidden + $this->assertResponseStatusCodeSame(403); // unauthorized // Access with the same user should be allowed. $client = $this->getClientWithGuiCredentials($username, $password); diff --git a/tests/CourseBundle/Repository/CDocumentRepositoryTest.php b/tests/CourseBundle/Repository/CDocumentRepositoryTest.php index a7ede40700..76d4fc6617 100644 --- a/tests/CourseBundle/Repository/CDocumentRepositoryTest.php +++ b/tests/CourseBundle/Repository/CDocumentRepositoryTest.php @@ -91,6 +91,7 @@ class CDocumentRepositoryTest extends AbstractApiTest $file = $this->getUploadedFile(); $token = $this->getUserToken([]); + // Upload file. $response = $this->createClientWithCredentials($token)->request( 'POST', @@ -169,7 +170,7 @@ class CDocumentRepositoryTest extends AbstractApiTest 'headers' => ['Content-Type' => 'application/json'], ] ); - $this->assertResponseStatusCodeSame(403); // Forbidden + $this->assertResponseStatusCodeSame(403); // Unauthorized // Test access with another user. He CAN see the file, the cid is pass as a parameter // and the course is open to the world by default. @@ -201,7 +202,7 @@ class CDocumentRepositoryTest extends AbstractApiTest ], ] ); - $this->assertResponseStatusCodeSame(403); + $this->assertResponseStatusCodeSame(401); // Update course visibility to CLOSED $courseRepo = self::getContainer()->get(CourseRepository::class); @@ -219,7 +220,7 @@ class CDocumentRepositoryTest extends AbstractApiTest ], ] ); - $this->assertResponseStatusCodeSame(403); + $this->assertResponseStatusCodeSame(401); // Update course visibility to HIDDEN $courseRepo = self::getContainer()->get(CourseRepository::class); @@ -237,7 +238,7 @@ class CDocumentRepositoryTest extends AbstractApiTest ], ] ); - $this->assertResponseStatusCodeSame(403); + $this->assertResponseStatusCodeSame(401); // Change visibility of the document to DRAFT $documentRepo = self::getContainer()->get(CDocumentRepository::class); @@ -260,7 +261,7 @@ class CDocumentRepositoryTest extends AbstractApiTest ], ] ); - $this->assertResponseStatusCodeSame(403); + $this->assertResponseStatusCodeSame(401); } public function testUploadFileInSideASubFolder(): void